All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Thomas Gummerer <t.gummerer@gmail.com>
Cc: git@vger.kernel.org, "Jonathan Nieder" <jrnieder@gmail.com>,
	"Jens Lehmann" <Jens.Lehmann@web.de>,
	"Torsten Bögershausen" <tboegi@web.de>,
	"Eric Sunshine" <sunshine@sunshineco.com>
Subject: Re: [PATCH 1/2] diff: add test for --no-index executed outside repo
Date: Mon, 16 Dec 2013 11:42:42 -0800	[thread overview]
Message-ID: <xmqqob4goa4t.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <1387221810-32374-1-git-send-email-t.gummerer@gmail.com> (Thomas Gummerer's message of "Mon, 16 Dec 2013 20:23:29 +0100")

Thomas Gummerer <t.gummerer@gmail.com> writes:

> 470faf9 diff: move no-index detection to builtin/diff.c breaks the error
> message for "git diff --no-index", when the command is executed outside
> of a git repository and the wrong number of arguments are given. 6df5762
> diff: don't read index when --no-index is given fixes the problem.
>
> Add a test to guard against similar breakages in the future.
>
> Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
> ---
>
>>> Thanks, I've missed that one.  It only happens when run outside a git
>>> repository, but the same  comments still apply.  Will fix and send a
>>> re-roll.
>>
>> Please don't, as the last round has already been pushed on 'next'.
>
> Sorry about that, should have checked first.
>
>> An incremental change on top would also illustrate more clearly what
>> breakage needed to be fixed, which would be another good thing. It
>> could even come with a new test that makes sure that the above
>> command line is diagnosed correctly as a mistake ;-).
>
> The breakage is actually fixed with the second patch as described in
> the commit message above, so here is just a test against future
> breakages.  This test only works when the test root is outside of a
> git repository, as otherwise nongit will not be set.  Is there another
> way to write it?

Perhaps use CEILING, like this (untested)?

	mkdir -p test-outside/non/git &&
        (
                GIT_CEILING_DIRECTORIES=$TRASH_DIRECTORY/test-outside &&
		export GIT_CEILING_DIRECTORIES &&
		cd test-outside/non/git &&
		do whatever non-git thing here
	)

  parent reply	other threads:[~2013-12-16 19:42 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-12-09 12:05 [PATCH] diff: don't read index when --no-index is given Thomas Gummerer
2013-12-09 15:16 ` Jonathan Nieder
2013-12-09 17:27   ` Jens Lehmann
2013-12-09 19:06     ` Jonathan Nieder
2013-12-09 19:14   ` Thomas Gummerer
2013-12-09 19:20     ` Jonathan Nieder
2013-12-09 20:40       ` [PATCH v2] " Thomas Gummerer
2013-12-09 20:55         ` Torsten Bögershausen
2013-12-09 20:57         ` Eric Sunshine
2013-12-09 21:17           ` Thomas Gummerer
2013-12-09 20:30 ` [PATCH] " Junio C Hamano
2013-12-09 21:13   ` Thomas Gummerer
2013-12-10 17:52   ` [PATCH v3 1/2] diff: move no-index detection to builtin/diff.c Thomas Gummerer
2013-12-10 17:52     ` [PATCH v3 2/2] diff: don't read index when --no-index is given Thomas Gummerer
2013-12-10 18:18       ` Jonathan Nieder
2013-12-10 18:55         ` Thomas Gummerer
2013-12-10 18:16     ` [PATCH v3 1/2] diff: move no-index detection to builtin/diff.c Jonathan Nieder
2013-12-10 18:46       ` Thomas Gummerer
2013-12-11  9:58       ` [PATCH v4 " Thomas Gummerer
2013-12-11  9:58         ` [PATCH v4 2/2] diff: don't read index when --no-index is given Thomas Gummerer
2013-12-12 20:25           ` Junio C Hamano
2013-12-14  0:44           ` Jonathan Nieder
2013-12-14  0:43         ` [PATCH v4 1/2] diff: move no-index detection to builtin/diff.c Jonathan Nieder
2013-12-14 10:42           ` Thomas Gummerer
2013-12-16 17:48             ` Junio C Hamano
2013-12-16 19:23               ` [PATCH 1/2] diff: add test for --no-index executed outside repo Thomas Gummerer
2013-12-16 19:23                 ` [PATCH 2/2] diff: avoid some nesting Thomas Gummerer
2013-12-16 19:42                 ` Junio C Hamano [this message]
2013-12-14 13:07           ` [PATCH v5 1/2] diff: move no-index detection to builtin/diff.c Thomas Gummerer
2013-12-14 13:07             ` [PATCH v5 2/2] diff: don't read index when --no-index is given Thomas Gummerer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqob4goa4t.fsf@gitster.dls.corp.google.com \
    --to=gitster@pobox.com \
    --cc=Jens.Lehmann@web.de \
    --cc=git@vger.kernel.org \
    --cc=jrnieder@gmail.com \
    --cc=sunshine@sunshineco.com \
    --cc=t.gummerer@gmail.com \
    --cc=tboegi@web.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.