^D to credentials prompt results in "fatal: ... Success"

[Anthony Sottile]

A bit of an amusing edge case.

I'm not exactly sure the correct approach to fix this but here's my
reproduction, triage, and a few potential options I see.

Note that after the username prompt, I pressed ^D

$./prefix/bin/git --version
git version 2.18.0
$ PATH=$PWD/prefix/bin:$PATH git clone
https://github.com/asottile/this-does-not-exist-i-promise
Cloning into 'this-does-not-exist-i-promise'...
Username for 'https://github.com': fatal: could not read Username for
'https://github.com': Success

Looking at `prompt.c`, it's hitting this bit of code:

if (git_env_bool("GIT_TERMINAL_PROMPT", 1)) {
r = git_terminal_prompt(prompt, flags & PROMPT_ECHO);
err = strerror(errno);
} else {
err = "terminal prompts disabled";

}
if (!r) {
/* prompts already contain ": " at the end */
die("could not read %s%s", prompt, err);
}

>From `git_terminal_prompt` (compat/terminal.c):

r = strbuf_getline_lf(, input_fh);
if (!echo) {
putc('\n', output_fh);
fflush(output_fh);
}

restore_term();
fclose(input_fh);
fclose(output_fh);

if (r == EOF)
return NULL;
return buf.buf;


in the `EOF` case, this is returning `NULL`, but `errno = 0` at this
point causing the error string to be "Success"

I see a couple of options here:

1. special case EOF in `git_terminal_prompt` / `git_prompt` and
produce an error message such as:

fatal: could not read Username for 'https://github.com': EOF

(I tried returing `EOF` directly from `git_terminal_prompt` and was
able to get this messaging to work, however `r == EOF` is a
pointer-int comparison so this approach didn't really seem like a good
idea -- changing the signature of `git_terminal_prompt` to set a
special flag for EOF is another option, but seems a lot of work for a
case that probably doesn't happen all too often)

I also couldn't find an appropriate errno to set in the EOF case either

2. treat EOF less specially

The function this is replacing, `getpass` simply returns an empty
string on `EOF`.  This patch would implement that:

diff --git a/compat/terminal.c b/compat/terminal.c
index fa13ee672..8bd08108e 100644
--- a/compat/terminal.c
+++ b/compat/terminal.c
@@ -122,7 +122,7 @@ char *git_terminal_prompt(const char *prompt, int echo)
fputs(prompt, output_fh);
fflush(output_fh);

-   r = strbuf_getline_lf(, input_fh);
+   strbuf_getline_lf(, input_fh);
if (!echo) {
putc('\n', output_fh);
fflush(output_fh);
@@ -132,8 +132,6 @@ char *git_terminal_prompt(const char *prompt, int echo)
fclose(input_fh);
fclose(output_fh);

-   if (r == EOF)
-   return NULL;
return buf.buf;
 }


however then the output is a bit strange for ^D (note I pressed ^D twice):

$ PATH=$PWD/prefix/bin:$PATH git clone
https://github.com/asottile/this-does-not-exist-i-promise
Cloning into 'this-does-not-exist-i-promise'...
Username for 'https://github.com': Password for 'https://github.com':
remote: Repository not found.
fatal: Authentication failed for
'https://github.com/asottile/this-does-not-exist-i-promise/'

Anthony

Re: [PATCH] config.c: fix regression for core.safecrlf false

[Anthony Sottile]

On Wed, Jun 6, 2018 at 10:22 AM, Eric Sunshine  wrote:
> On Wed, Jun 6, 2018 at 1:18 PM, Anthony Sottile  wrote:
>> On Wed, Jun 6, 2018 at 10:17 AM, Eric Sunshine  
>> wrote:
>>> On Wed, Jun 6, 2018 at 1:15 PM, Eric Sunshine  
>>> wrote:
>>>> On Mon, Jun 4, 2018 at 4:17 PM, Anthony Sottile  wrote:
>>>>> +   for w in I am all CRLF; do echo $w; done | append_cr >allcrlf &&
>>>>
>>>> Simpler: printf "%s\n" I am all CRLF | append_cr >allcrlf &&
>>>
>>> Or even simpler:
>>>
>>> printf "%s\r\n" I am all CRLF >allcrlf &&
>>
>> Yeah, I just copied the line in my test from another test in this file
>> which was doing a ~similar thing. [...]
>
> Thanks for pointing that out. In that case, it's following existing
> practice, thus certainly not worth a re-roll.

Anything else for me to do here? (sorry! not super familiar with the process)

Re: [PATCH] config.c: fix regression for core.safecrlf false

[Anthony Sottile]

On Wed, Jun 6, 2018 at 10:17 AM, Eric Sunshine  wrote:
> On Wed, Jun 6, 2018 at 1:15 PM, Eric Sunshine  wrote:
>> On Mon, Jun 4, 2018 at 4:17 PM, Anthony Sottile  wrote:
>>> +   for w in I am all CRLF; do echo $w; done | append_cr >allcrlf &&
>>
>> Simpler: printf "%s\n" I am all CRLF | append_cr >allcrlf &&
>
> Or even simpler:
>
> printf "%s\r\n" I am all CRLF >allcrlf &&

Yeah, I just copied the line in my test from another test in this file
which was doing a ~similar thing.  My original bug report actually
uses `echo -en ...` to accomplish the same thing.

[PATCH] config.c: fix regression for core.safecrlf false

[Anthony Sottile]

A regression introduced in 8462ff43e42ab67cecd16fdfb59451a53cc8a945 caused
autocrlf rewrites to produce a warning message despite setting safecrlf=false.

Signed-off-by: Anthony Sottile 
---
 config.c|  2 +-
 t/t0020-crlf.sh | 10 ++
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/config.c b/config.c
index fbbf0f8..de24e90 100644
--- a/config.c
+++ b/config.c
@@ -1233,7 +1233,7 @@ static int git_default_core_config(const char *var, const 
char *value)
}
eol_rndtrp_die = git_config_bool(var, value);
global_conv_flags_eol = eol_rndtrp_die ?
-   CONV_EOL_RNDTRP_DIE : CONV_EOL_RNDTRP_WARN;
+   CONV_EOL_RNDTRP_DIE : 0;
return 0;
}
 
diff --git a/t/t0020-crlf.sh b/t/t0020-crlf.sh
index 71350e0..5f05698 100755
--- a/t/t0020-crlf.sh
+++ b/t/t0020-crlf.sh
@@ -98,6 +98,16 @@ test_expect_success 'safecrlf: git diff demotes 
safecrlf=true to warn' '
 '
 
 
+test_expect_success 'safecrlf: no warning with safecrlf=false' '
+   git config core.autocrlf input &&
+   git config core.safecrlf false &&
+
+   for w in I am all CRLF; do echo $w; done | append_cr >allcrlf &&
+   git add allcrlf 2>err &&
+   test_must_be_empty err
+'
+
+
 test_expect_success 'switch off autocrlf, safecrlf, reset HEAD' '
git config core.autocrlf false &&
git config core.safecrlf false &&
-- 
2.7.4

Re: Regression (?) in core.safecrlf=false messaging

[Anthony Sottile]

On Mon, Jun 4, 2018 at 12:55 AM, Torsten Bögershausen  wrote:
>
> Does the following patch fix your problem ?
>
> diff --git a/config.c b/config.c
> index 6f8f1d8c11..c625aec96a 100644
> --- a/config.c
> +++ b/config.c
> @@ -1233,7 +1233,7 @@ static int git_default_core_config(const char *var, 
> const char *value)
> }
> eol_rndtrp_die = git_config_bool(var, value);
> global_conv_flags_eol = eol_rndtrp_die ?
> -   CONV_EOL_RNDTRP_DIE : CONV_EOL_RNDTRP_WARN;
> +   CONV_EOL_RNDTRP_DIE : 0;
> return 0;
> }
>


Yes!  After applying that patch:

```

$ PATH=$PWD/prefix/bin:$PATH bash test.sh
+ git --version
git version 2.18.0.rc1.dirty
+ rm -rf repo
+ git init repo
Initialized empty Git repository in /tmp/git/repo/.git/
+ cd repo
+ git config core.autocrlf input
+ git config core.safecrlf false
+ echo -en 'foo\r\nbar\r\n'
+ git add f

```

Anthony

Regression (?) in core.safecrlf=false messaging

[Anthony Sottile]

I'm a bit unclear if I was depending on undocumented behaviour or not
here but it seems the messaging has recently changed with respect to
`core.safecrlf`

My reading of the documentation
https://git-scm.com/docs/git-config#git-config-coresafecrlf (I might
be wrong here)

- core.safecrlf = true -> fail hard when converting
- core.safecrlf = warn -> produce message when converting
- core.safecrlf = false -> convert silently

(note that these are all only relevant when `core.autocrlf = true`)

I've created a small script to demonstrate:

```
set -euxo pipefail

git --version

rm -rf repo
git init repo
cd repo
git config core.autocrlf input
git config core.safecrlf false
echo -en 'foo\r\nbar\r\n' > f
git add f
```

When run against 2.16.4:

```
$ PATH=$PWD/prefix/bin:$PATH bash test.sh
+ git --version
git version 2.16.4
+ rm -rf repo
+ git init repo
Initialized empty Git repository in /tmp/git/repo/.git/
+ cd repo
+ git config core.autocrlf input
+ git config core.safecrlf false
+ echo -en 'foo\r\nbar\r\n'
+ git add f
```

(notice how there are no messages about crlf conversion while adding
-- this is what I expect given I have core.safecrlf=false)


When run against master:

```console
$ PATH=$PWD/prefix/bin:$PATH bash test.sh
+ git --version
git version 2.18.0.rc0.42.gc2c7d17
+ rm -rf repo
+ git init repo
Initialized empty Git repository in /tmp/git/repo/.git/
+ cd repo
+ git config core.autocrlf input
+ git config core.safecrlf false
+ echo -en 'foo\r\nbar\r\n'
+ git add f
warning: CRLF will be replaced by LF in f.
The file will have its original line endings in your working directory.
```

A `git bisect` shows this as the first commit which breaks this
behaviour: 8462ff43e42ab67cecd16fdfb59451a53cc8a945

https://github.com/git/git/commit/8462ff43e42ab67cecd16fdfb59451a53cc8a945

The commit appears to be a refactor (?) that shouldn't have changed behaviour?

Here's the script I was using to bisect:
https://i.fluffy.cc/2L4ZtqV3cHfzNRkKPbHgTcz43HMQJxdK.html

```
git bisect start
git bisect bad v2.17.0
git bisect good v2.16.4
git bisect run ./test.sh
```

Noticed this due to test failures here:
https://github.com/pre-commit/pre-commit/pull/762#issuecomment-394236625

Thanks,

Anthony

[PATCH v2 2/2] diff: finish removal of deprecated -q option

[Anthony Sottile]

Functionality was removed in c48f6816f0 but the cli option was not removed.

Signed-off-by: Anthony Sottile <asott...@umich.edu>
---
 builtin/diff-files.c | 2 --
 builtin/diff.c   | 2 --
 diff.h   | 4 +---
 3 files changed, 1 insertion(+), 7 deletions(-)

diff --git a/builtin/diff-files.c b/builtin/diff-files.c
index e88493f..b0ff251 100644
--- a/builtin/diff-files.c
+++ b/builtin/diff-files.c
@@ -37,8 +37,6 @@ int cmd_diff_files(int argc, const char **argv, const char 
*prefix)
rev.max_count = 2;
else if (!strcmp(argv[1], "--theirs"))
rev.max_count = 3;
-   else if (!strcmp(argv[1], "-q"))
-   options |= DIFF_SILENT_ON_REMOVED;
else
usage(diff_files_usage);
argv++; argc--;
diff --git a/builtin/diff.c b/builtin/diff.c
index f5bbd4d..96513e8 100644
--- a/builtin/diff.c
+++ b/builtin/diff.c
@@ -227,8 +227,6 @@ static int builtin_diff_files(struct rev_info *revs, int 
argc, const char **argv
revs->max_count = 2;
else if (!strcmp(argv[1], "--theirs"))
revs->max_count = 3;
-   else if (!strcmp(argv[1], "-q"))
-   options |= DIFF_SILENT_ON_REMOVED;
else if (!strcmp(argv[1], "-h"))
usage(builtin_diff_usage);
else
diff --git a/diff.h b/diff.h
index aca150b..c9d71e1 100644
--- a/diff.h
+++ b/diff.h
@@ -65,7 +65,7 @@ typedef struct strbuf *(*diff_prefix_fn_t)(struct 
diff_options *opt, void *data)
 #define DIFF_OPT_BINARY  (1 <<  2)
 #define DIFF_OPT_TEXT(1 <<  3)
 #define DIFF_OPT_FULL_INDEX  (1 <<  4)
-#define DIFF_OPT_SILENT_ON_REMOVE(1 <<  5)
+/* (1 << 5) unused */
 #define DIFF_OPT_FIND_COPIES_HARDER  (1 <<  6)
 #define DIFF_OPT_FOLLOW_RENAMES  (1 <<  7)
 #define DIFF_OPT_RENAME_EMPTY(1 <<  8)
@@ -374,8 +374,6 @@ extern void diff_warn_rename_limit(const char *varname, int 
needed, int degraded
  */
 extern const char *diff_aligned_abbrev(const struct object_id *sha1, int);
 
-/* do not report anything on removed paths */
-#define DIFF_SILENT_ON_REMOVED 01
 /* report racily-clean paths as modified */
 #define DIFF_RACY_IS_MODIFIED 02
 extern int run_diff_files(struct rev_info *revs, unsigned int option);
-- 
2.7.4

[PATCH v2 1/2] diff: alias -q to --quiet

[Anthony Sottile]

Previously, `-q` was silently ignored:

Before:

$ git diff -q -- Documentation/; echo $?
diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index a88c767..aa6e724 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -587,6 +587,7 @@ ifndef::git-log[]
That is, it exits with 1 if there were differences and
0 means no differences.

+-q::
 --quiet::
Disable all output of the program. Implies `--exit-code`.
 endif::git-log[]
0
$

After:
$ ./git diff -q -- Documentation/; echo $?
1
$

Signed-off-by: Anthony Sottile <asott...@umich.edu>
---
 Documentation/diff-options.txt | 1 +
 diff.c | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index a88c767..aa6e724 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -587,6 +587,7 @@ ifndef::git-log[]
That is, it exits with 1 if there were differences and
0 means no differences.
 
+-q::
 --quiet::
Disable all output of the program. Implies `--exit-code`.
 endif::git-log[]
diff --git a/diff.c b/diff.c
index 69f0357..13dfc3e 100644
--- a/diff.c
+++ b/diff.c
@@ -4751,7 +4751,7 @@ int diff_opt_parse(struct diff_options *options,
}
else if (!strcmp(arg, "--exit-code"))
DIFF_OPT_SET(options, EXIT_WITH_STATUS);
-   else if (!strcmp(arg, "--quiet"))
+   else if (!strcmp(arg, "-q") || !strcmp(arg, "--quiet"))
DIFF_OPT_SET(options, QUICK);
else if (!strcmp(arg, "--ext-diff"))
DIFF_OPT_SET(options, ALLOW_EXTERNAL);
-- 
2.7.4

[PATCH] diff: alias -q to --quiet

[Anthony Sottile]

Previously, `-q` was silently ignored:

Before:

$ git diff -q -- Documentation/; echo $?
diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index a88c767..aa6e724 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -587,6 +587,7 @@ ifndef::git-log[]
That is, it exits with 1 if there were differences and
0 means no differences.

+-q::
 --quiet::
Disable all output of the program. Implies `--exit-code`.
 endif::git-log[]
0
$

After:
$ ./git diff -q -- Documentation/; echo $?
1
$

Signed-off-by: Anthony Sottile <asott...@umich.edu>
---
 Documentation/diff-options.txt | 1 +
 diff.c | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index a88c767..aa6e724 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -587,6 +587,7 @@ ifndef::git-log[]
That is, it exits with 1 if there were differences and
0 means no differences.
 
+-q::
 --quiet::
Disable all output of the program. Implies `--exit-code`.
 endif::git-log[]
diff --git a/diff.c b/diff.c
index 69f0357..13dfc3e 100644
--- a/diff.c
+++ b/diff.c
@@ -4751,7 +4751,7 @@ int diff_opt_parse(struct diff_options *options,
}
else if (!strcmp(arg, "--exit-code"))
DIFF_OPT_SET(options, EXIT_WITH_STATUS);
-   else if (!strcmp(arg, "--quiet"))
+   else if (!strcmp(arg, "-q") || !strcmp(arg, "--quiet"))
DIFF_OPT_SET(options, QUICK);
else if (!strcmp(arg, "--ext-diff"))
DIFF_OPT_SET(options, ALLOW_EXTERNAL);
-- 
2.7.4

[PATCH/RFC] git-grep: correct exit code with --quiet and -L

[Anthony Sottile]

The handling of `status_only` no longer interferes with the handling of
`unmatch_name_only`.  `--quiet` no longer affects the exit code when using
`-L`/`--files-without-match`.

Signed-off-by: Anthony Sottile <asott...@umich.edu>
---
 grep.c  | 2 +-
 t/t7810-grep.sh | 5 +
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/grep.c b/grep.c
index 2efec0e..c9e7cc7 100644
--- a/grep.c
+++ b/grep.c
@@ -1821,7 +1821,7 @@ static int grep_source_1(struct grep_opt *opt, struct 
grep_source *gs, int colle
return 0;
 
if (opt->status_only)
-   return 0;
+   return opt->unmatch_name_only;
if (opt->unmatch_name_only) {
/* We did not see any hit, so we want to show this */
show_name(opt, gs->name);
diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh
index f106387..2a6679c 100755
--- a/t/t7810-grep.sh
+++ b/t/t7810-grep.sh
@@ -374,6 +374,11 @@ test_expect_success 'grep -L -C' '
test_cmp expected actual
 '
 
+test_expect_success 'grep --files-without-match --quiet' '
+   git grep --files-without-match --quiet nonexistent_string >actual &&
+   test_cmp /dev/null actual
+'
+
 cat >expected <

Re: [PATCH/RFC] git-grep: correct exit code with --quiet and -L

[Anthony Sottile]

Sounds good, I'll wait.

I've also created a mailing list entry on the gnu grep mailing list as
I believe the current exit status for --files-without-match is
inconsistent:

http://lists.gnu.org/archive/html/bug-grep/2017-08/msg00010.html

Anthony

On Tue, Aug 15, 2017 at 3:24 PM, Junio C Hamano <gits...@pobox.com> wrote:
> Anthony Sottile <asott...@umich.edu> writes:
>
>> Ah yes, I didn't intend to include the first hunk (forgot to amend it
>> out when formatting the patch).
>>
>> I think git's exit codes for -L actually make more sense than the GNU
>> exit codes (especially when comparing with `grep` vs `grep -v`) --
>> that is, produce `0` when the search is successful (producing
>> *something* on stdout) and `1` when the search fails.
>>
>> Shall I create a new mail with the adjusted patch as suggested above?
>
> I do not mind seeing an updated patch that does not change the exit
> status (as you seem to like what we have currently that contradicts
> what GNU grep does) but makes it consistent between "--quiet" and
> "--no-quiet".  But I would not be surprised if people seeing this
> exchange from the sideline are already working on fixing the exit
> status and also making sure that the fixed code would produce the
> same corrected exit status with or without "--quiet", so an updated
> patch from you will likely conflict with their effort.
>
> So if I were you, I'd wait to see what other people would say about
> the actual exit codes we give when "git grep -L" is run without the
> "--quiet" option, and if they are also happy with the current exit
> code, then send in an updated patch.
>
> Thanks.
>
>

Re: [PATCH/RFC] git-grep: correct exit code with --quiet and -L

[Anthony Sottile]

Ah yes, I didn't intend to include the first hunk (forgot to amend it
out when formatting the patch).

I think git's exit codes for -L actually make more sense than the GNU
exit codes (especially when comparing with `grep` vs `grep -v`) --
that is, produce `0` when the search is successful (producing
*something* on stdout) and `1` when the search fails.

Shall I create a new mail with the adjusted patch as suggested above?
(I'm not familiar with the expected workflow).

Anthony

On Tue, Aug 15, 2017 at 2:33 PM, Junio C Hamano <gits...@pobox.com> wrote:
> Anthony Sottile <asott...@umich.edu> writes:
>
>> The handling of `status_only` no longer interferes with the handling of
>> `unmatch_name_only`.  `--quiet` no longer affects the exit code when using
>> `-L`/`--files-without-match`.
>
> I agree with the above statement of yours that --quiet should not
> affect what the exit status is.
>
> But I am not sure what the exit code from these commands _should_
> be:
>
> $ git grep -L qfwfq \*.h;# no file matches
> $ git grep -L \# \*.h   ;# some but not all files match
> $ git grep -L . \*.h;# all files match
>
> with or without --quiet.  I seem to get 0, 0, 1, which I am not sure
> is correct.  I do recall writing "git grep" _without_ thinking what
> the exit code should be when we added --files-without-match, so the
> exit status the current code gives out may be just a random garbage.
>
> Asking GNU grep (because --files-without-match is not a POSIX thing):
>
> $ grep -L qfwfq *.h  ;# no file matches
> $ grep -L \# *.h ;# some but not all files match
> $ grep -L . *.h  ;# all files match
>
> I seem to get 1, 0, 0.  So the exit status should reflect if there
> was _any_ hit from any file that were inspected.
>
>> @@ -1755,7 +1755,7 @@ static int grep_source_1(struct grep_opt *opt, struct 
>> grep_source *gs, int colle
>>   }
>>   if (hit) {
>>   count++;
>> - if (opt->status_only)
>> + if (!opt->unmatch_name_only && opt->status_only)
>>   return 1;
>>   if (opt->name_only) {
>>   show_name(opt, gs->name);
>
> Does the change in this hunk have any effect?
>
> Just before this hunk there is this code:
>
> /* "grep -v -e foo -e bla" should list lines
>  * that do not have either, so inversion should
>  * be done outside.
>  */
> if (opt->invert)
> hit = !hit;
> if (opt->unmatch_name_only) {
> if (hit)
> return 0;
> goto next_line;
>
> If (opt->unmatch_name_only && hit) then the function would have
> already returned and the control wouldn't have reached here.
>
> Which would mean that when the control reaches the line this hunk
> touches, either one of these must be false, and because we are
> inside "if (hit)", opt->unmatch_name_only must be false.
>
>> @@ -1820,13 +1820,14 @@ static int grep_source_1(struct grep_opt *opt, 
>> struct grep_source *gs, int colle
>>   if (collect_hits)
>>   return 0;
>>
>> - if (opt->status_only)
>> - return 0;
>>   if (opt->unmatch_name_only) {
>>   /* We did not see any hit, so we want to show this */
>> - show_name(opt, gs->name);
>> + if (!opt->status_only)
>> + show_name(opt, gs->name);
>>   return 1;
>>   }
>> + if (opt->status_only)
>> + return 0;
>
> This hunk makes sense to me (provided if the semantics we want out
> of --files-without-match is sensible, which is dubious), even though
> I would have limited the change to just a single line, i.e.
>
> if (opt->status_only)
> -   return 0;
> +   return opt->unmatch_name_only;
> if (opt->unmatch_name_only) {
> /* We did not see any hit, ... */
>
> But I suspect we want to fix the exit code not to be affected by
> the "--files-without-match" option in the first place, so all the
> code changes we see in this patch might be moot X-<.
>
>

[PATCH/RFC] git-grep: correct exit code with --quiet and -L

[Anthony Sottile]

The handling of `status_only` no longer interferes with the handling of
`unmatch_name_only`.  `--quiet` no longer affects the exit code when using
`-L`/`--files-without-match`.

Signed-off-by: Anthony Sottile <asott...@umich.edu>
---
 grep.c  | 9 +
 t/t7810-grep.sh | 5 +
 2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/grep.c b/grep.c
index 2efec0e..a893d09 100644
--- a/grep.c
+++ b/grep.c
@@ -1755,7 +1755,7 @@ static int grep_source_1(struct grep_opt *opt, struct 
grep_source *gs, int colle
}
if (hit) {
count++;
-   if (opt->status_only)
+   if (!opt->unmatch_name_only && opt->status_only)
return 1;
if (opt->name_only) {
show_name(opt, gs->name);
@@ -1820,13 +1820,14 @@ static int grep_source_1(struct grep_opt *opt, struct 
grep_source *gs, int colle
if (collect_hits)
return 0;
 
-   if (opt->status_only)
-   return 0;
if (opt->unmatch_name_only) {
/* We did not see any hit, so we want to show this */
-   show_name(opt, gs->name);
+   if (!opt->status_only)
+   show_name(opt, gs->name);
return 1;
}
+   if (opt->status_only)
+   return 0;
 
xdiff_clear_find_func();
opt->priv = NULL;
diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh
index f106387..2a6679c 100755
--- a/t/t7810-grep.sh
+++ b/t/t7810-grep.sh
@@ -374,6 +374,11 @@ test_expect_success 'grep -L -C' '
test_cmp expected actual
 '
 
+test_expect_success 'grep --files-without-match --quiet' '
+   git grep --files-without-match --quiet nonexistent_string >actual &&
+   test_cmp /dev/null actual
+'
+
 cat >expected <

Inconsistent exit code for `git grep --files-without-match` with `--quiet`

[Anthony Sottile]

Using the latest revision of git:

$ ./git --version
git version 2.14.GIT

$ ./git-grep --files-without-match nope -- blob.c; echo $?
blob.c
0

$ ./git-grep --files-without-match --quiet nope -- blob.c; echo $?
1

I expect both to exit 0

Note that blob.c does not contain "nope", here is the revision I am
using: 
https://github.com/git/git/blob/b3622a4ee94e4916cd05e6d96e41eeb36b941182/blob.c
-- this file / case isn't special I just picked a reproduction that
may be convenient for git developers.

Anthony

Re: core.autocrlf=true causes `git apply` to fail on patch generated with `git diff-index HEAD --patch`

[Anthony Sottile]

Here's where I'm hitting the problem described:
https://github.com/pre-commit/pre-commit/issues/570

Note that `git -c core.autocrlf=false` apply patch fixes this
situation, but breaks others.

Here's a testcase where `git -c core.autocrlf=false apply patch`
causes a *different* patch failure:

```
#!/bin/bash
set -ex

rm -rf foo
git init foo
cd foo

git config --local core.autocrlf true

# Commit lf into repository
python3 -c 'open("foo", "wb").write(b"1\r\n2\r\n")'
git add foo
python3 -c 'open("foo", "wb").write(b"3\n4\n")'

# Generate a patch, check it out, restore it
git diff --ignore-submodules --binary --no-color --no-ext-diff > patch
python3 -c 'print(open("patch", "rb").read())'
git checkout -- .
git -c core.autocrlf=false apply patch
```

output:

```
+ rm -rf foo
+ git init foo
Initialized empty Git repository in /tmp/foo/.git/
+ cd foo
+ git config --local core.autocrlf true
+ python3 -c 'open("foo", "wb").write(b"1\r\n2\r\n")'
+ git add foo
+ python3 -c 'open("foo", "wb").write(b"3\n4\n")'
+ git diff --ignore-submodules --binary --no-color --no-ext-diff
warning: LF will be replaced by CRLF in foo.
The file will have its original line endings in your working directory.
+ python3 -c 'print(open("patch", "rb").read())'
b'diff --git a/foo b/foo\nindex 1191247..b944734 100644\n---
a/foo\n+++ b/foo\n@@ -1,2 +1,2 @@\n-1\n-2\n+3\n+4\n'
+ git checkout -- .
+ git -c core.autocrlf=false apply patch
error: patch failed: foo:1
```

My current workaround is:
- try `git apply patch`
- try `git -c core.autocrlf=false apply patch`

which seems to work pretty well.

Anthony


On Tue, Aug 1, 2017 at 1:47 PM, Torsten Bögershausen <tbo...@web.de> wrote:
>
>
> On 08/01/2017 08:24 PM, Anthony Sottile wrote:
>>
>> Here's my minimal reproduction -- it's slightly far-fetched in that it
>> involves*committing crlf*  and
>>
>> then using `autocrlf=true` (commit lf, check out crlf).
>>
>> ```
>> #!/bin/bash
>> set -ex
>>
>> rm -rf foo
>> git init foo
>> cd foo
>>
>> # Commit crlf into repository
>> git config --local core.autocrlf false
>> python3 -c 'open("foo", "wb").write(b"1\r\n2\r\n")'
>> git add foo
>> git commit -m "Initial commit with crlf"
>>
>> # Change whitespace mode to autocrlf, "commit lf, checkout crlf"
>> git config --local core.autocrlf true
>> python3 -c 'open("foo", "wb").write(b"1\r\n2\r\n\r\n\r\n\r\n")'
>>
>> # Generate a patch, check it out, restore it
>> git diff --ignore-submodules --binary --no-color --no-ext-diff > patch
>> python3 -c 'print(open("patch", "rb").read())'
>> git checkout -- .
>> # I expect this to succeed, it fails
>> git apply patch
>> ```
>>
>> And here's the output:
>>
>> ```
>> + rm -rf foo
>> + git init foo
>> Initialized empty Git repository in/tmp/foo/.git/
>> + cd foo
>> + git config --local core.autocrlf false
>> + python3 -c 'open("foo", "wb").write(b"1\r\n2\r\n")'
>> + git add foo
>> + git commit -m 'Initial commit with crlf'
>> [master (root-commit) 02d3246] Initial commit with crlf
>>   1 file changed, 2 insertions(+)
>>   create mode 100644 foo
>> + git config --local core.autocrlf true
>> + python3 -c 'open("foo", "wb").write(b"1\r\n2\r\n\r\n\r\n\r\n")'
>> + git diff --ignore-submodules --binary --no-color --no-ext-diff
>> + python3 -c 'print(open("patch", "rb").read())'
>> b'diff --git a/foo b/foo\nindex bd956ea..fbf7936 100644\n---
>> a/foo\n+++ b/foo\n@@ -1,2 +1,5 @@\n 1\r\n 2\r\n+\r\n+\r\n+\r\n'
>> + git checkout -- .
>> + git apply patch
>> patch:8: trailing whitespace.
>>
>> patch:9: trailing whitespace.
>>
>> patch:10: trailing whitespace.
>>
>> error: patch failed: foo:1
>> error: foo: patch does not apply
>> ```
>>
>> I also tried with `git apply --ignore-whitespace`, but this causes the
>> line endings of the existing contents to be changed to*lf*  (there may
>> be two bugs here?)
>>
>> Thanks,
>>
>> Anthony
>
>
>
> I can reproduce you test case here.
>
> The line
>
> git apply patch
>
> would succeed, if you temporally (for the runtime of the apply command) set
> core.autocrlf to false:
>
> git -c core.autocrlf=false apply patch
>
> So this seems to be a bug (in a corner case ?):
>
> Typically repos which had been commited with CRLF should be normalized,
> which means that the CRLF in the repo are replaced by LF.
> So you test script is a corner case, for which Git has not been designed,
> It seems as if "git apply" gets things wrong here.
> Especially, as the '\r' is not a whitespace as a white space. but part
> of the line ending.
> So in my understanding the "--ignore-whitespace" option shouldn't affect
> the line endings at all.
>
> Fixes are possible, does anyone have a clue, why the '\r' is handled
> like this in apply ?
>
> And out of interest: is this a real life problem ?
>
>
>
>
>

core.autocrlf=true causes `git apply` to fail on patch generated with `git diff-index HEAD --patch`

[Anthony Sottile]

Here's my minimal reproduction -- it's slightly far-fetched in that it
involves *committing crlf* and
then using `autocrlf=true` (commit lf, check out crlf).

```
#!/bin/bash
set -ex

rm -rf foo
git init foo
cd foo

# Commit crlf into repository
git config --local core.autocrlf false
python3 -c 'open("foo", "wb").write(b"1\r\n2\r\n")'
git add foo
git commit -m "Initial commit with crlf"

# Change whitespace mode to autocrlf, "commit lf, checkout crlf"
git config --local core.autocrlf true
python3 -c 'open("foo", "wb").write(b"1\r\n2\r\n\r\n\r\n\r\n")'

# Generate a patch, check it out, restore it
git diff --ignore-submodules --binary --no-color --no-ext-diff > patch
python3 -c 'print(open("patch", "rb").read())'
git checkout -- .
# I expect this to succeed, it fails
git apply patch
```

And here's the output:

```
+ rm -rf foo
+ git init foo
Initialized empty Git repository in /tmp/foo/.git/
+ cd foo
+ git config --local core.autocrlf false
+ python3 -c 'open("foo", "wb").write(b"1\r\n2\r\n")'
+ git add foo
+ git commit -m 'Initial commit with crlf'
[master (root-commit) 02d3246] Initial commit with crlf
 1 file changed, 2 insertions(+)
 create mode 100644 foo
+ git config --local core.autocrlf true
+ python3 -c 'open("foo", "wb").write(b"1\r\n2\r\n\r\n\r\n\r\n")'
+ git diff --ignore-submodules --binary --no-color --no-ext-diff
+ python3 -c 'print(open("patch", "rb").read())'
b'diff --git a/foo b/foo\nindex bd956ea..fbf7936 100644\n---
a/foo\n+++ b/foo\n@@ -1,2 +1,5 @@\n 1\r\n 2\r\n+\r\n+\r\n+\r\n'
+ git checkout -- .
+ git apply patch
patch:8: trailing whitespace.

patch:9: trailing whitespace.

patch:10: trailing whitespace.

error: patch failed: foo:1
error: foo: patch does not apply
```

I also tried with `git apply --ignore-whitespace`, but this causes the
line endings of the existing contents to be changed to *lf* (there may
be two bugs here?)

Thanks,

Anthony

Re: [PATCH] Fix minor typo in git-diff docs.

[Anthony Sottile]

Thanks!

I'll keep this in mind next time I send a patch.

Anthony

On Mon, Jul 31, 2017 at 9:59 AM, Junio C Hamano <gits...@pobox.com> wrote:
> Anthony Sottile <asott...@umich.edu> writes:
>
>> diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
>> index 89cc0f4..43d18a4 100644
>> --- a/Documentation/diff-options.txt
>> +++ b/Documentation/diff-options.txt
>> @@ -392,7 +392,7 @@ endif::git-log[]
>> the diff between the preimage and `/dev/null`. The resulting patch
>> is not meant to be applied with `patch` or `git apply`; this is
>> solely for people who want to just concentrate on reviewing the
>> -   text after the change. In addition, the output obviously lack
>> +   text after the change. In addition, the output obviously lacks
>> enough information to apply such a patch in reverse, even manually,
>> hence the name of the option.
>>  +
>
> Another thing that is more severe.  You seem to have replaced all
> leading tabs with whitespaces, which makes the patch unusable.  For
> this single character patch, I can pretend as if I applied your
> patch while making the fix myself in my editor, so there is no need
> to resend, but please make sure your e-mail client does not do that
> the next time.
>
> Thanks.  Queued.

git checkout-index --all --force does not restore all files when `core.autocrlf` is set to `input`

[Anthony Sottile]

I'm not sure if this is a bug or the intended behaviour.

Here's my minimal reproduction (using python3 to write files so I can
control line endings)

```
#!/bin/bash
set -ex

rm -rf repo
git init repo
cd repo

git config --local core.autocrlf input

python3 -c 'open("foo", "wb").write(b"1\r\n2\r\n")'
git add foo
python3 -c 'open("foo", "wb").write(b"3\r\n4\r\n")'

git checkout-index --all --force
echo 'I expect this `git status` to have no modifications'
git status
```

Here's the output:

```
+ rm -rf repo
+ git init repo
Initialized empty Git repository in /tmp/foo/repo/.git/
+ cd repo
+ git config --local core.autocrlf input
+ python3 -c 'open("foo", "wb").write(b"1\r\n2\r\n")'
+ git add foo
warning: CRLF will be replaced by LF in foo.
The file will have its original line endings in your working directory.
+ python3 -c 'open("foo", "wb").write(b"3\r\n4\r\n")'
+ git checkout-index --all --force
+ echo 'I expect this `git status` to have no modifications'
I expect this `git status` to have no modifications
+ git status
On branch master

Initial commit

Changes to be committed:
  (use "git rm --cached ..." to unstage)

new file:   foo

Changes not staged for commit:
  (use "git add ..." to update what will be committed)
  (use "git checkout -- ..." to discard changes in working directory)

modified:   foo

```

In this state, `git diff` and `git diff-index` disagree as well:

```
$ git diff-index --exit-code $(git write-tree) --patch; echo $?
1
$ git diff --exit-code; echo $?
0
```

I expect the plumbing command `checkout-index -af` to exactly restore
the disk state to the index such that `git status`, and `git
diff-index` both indicate there are no changes.

Interestingly, `git checkout -- .` does exactly this, but it is a
porcelain command and not suitable for scripting.  Alternatively, I'm
looking for an equivalent to `git checkout -- .` which uses only
plumbing commands.

Thanks,

Anthony

[PATCH] Fix minor typo in git-diff docs.

[Anthony Sottile]

To be honest, I'm a bit overwhelmed by the documentation for submitting a patch!

I tried to follow as best I could, here's my attempt (please advise).

>From e88ad689a7587c11f270a10f191a3b6bc52a90d4 Mon Sep 17 00:00:00 2001
From: Anthony Sottile <asott...@umich.edu>
Date: Mon, 31 Jul 2017 06:54:14 -0700
Subject: [PATCH] Fix minor typo in git-diff docs.

Signed-off-by: Anthony Sottile <asott...@umich.edu>
---
 Documentation/diff-options.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index 89cc0f4..43d18a4 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -392,7 +392,7 @@ endif::git-log[]
the diff between the preimage and `/dev/null`. The resulting patch
is not meant to be applied with `patch` or `git apply`; this is
solely for people who want to just concentrate on reviewing the
-   text after the change. In addition, the output obviously lack
+   text after the change. In addition, the output obviously lacks
enough information to apply such a patch in reverse, even manually,
hence the name of the option.
 +
-- 
2.7.4

Re: bug: `git log --grep ... --invert-grep --author=...` negates / ignores --author

[Anthony Sottile]

I actually only expected the --grep to be inverted -- I think I'm on
the same page with what's documented.

I'd be happy to dig into the code and investigate this some more but I
am not familiar with the git codebase, any code hints on where to get
bootstrapped?

Anthony

On Thu, Jun 1, 2017 at 12:45 PM, Ævar Arnfjörð Bjarmason
 wrote:
> On Wed, May 31, 2017 at 11:40 PM, Jeff King  wrote:
>> On Wed, May 31, 2017 at 08:08:54PM +0200, Ævar Arnfjörð Bjarmason wrote:
>>
>>> $ git log --grep=bar --author=Ævar --pretty=format:%an -100 origin/pu
>>> |sort|uniq -c|sort -nr
>>> 5 Ævar Arnfjörð Bjarmason
>>>
>>> $ git log --author=Ævar --pretty=format:%an -100 origin/pu |sort|uniq
>>> -c|sort -nr
>>> 100 Ævar Arnfjörð Bjarmason
>>>
>>> $ git log --grep=bar --invert-grep --author=Ævar --pretty=format:%an
>>> -100 origin/pu |sort|uniq -c|sort -nr
>>>  78 Junio C Hamano
>>>  14 Jeff King
>>>   2 Andreas Heiduk
>>>   1 Sahil Dua
>>>   1 Rikard Falkeborn
>>>   1 Johannes Sixt
>>>   1 Johannes Schindelin
>>>   1 Ben Peart
>>>   1 Ævar Arnfjörð Bjarmason
>>>
>>> That last command should only find my commits, but instead --author is
>>> discarded.
>>
>> I would have thought that the last command wouldn't find _any_ of your
>> commits. Don't we consider --author a greppable pattern and invert it?
>
> There's two unrelated things going on here AFAICT:
>
> * Anthony expected --author to be inverted by --invert-grep, but this
> is not how it's documented, it's *only* for inverting the --grep
> pattern: "Limit the commits output to ones with log message that do
> not match the pattern specified with --grep=."
>
> * The --author filter should be applied un-inverted, but isn't, it's
> seemingly just ignored in the presence of --grep, actually mostly
> ignored, this is bizarre:
>
> $ diff -ru <(git log --grep=bar --invert-grep --pretty=format:%an -100
> origin/pu |sort|uniq -c|sort -nr) <(git log --grep=bar --invert-grep
> --pretty=format:%an -100 --author=WeMostlyIgnoreThisButNotReally
> origin/pu |sort|uniq -c|sort -nr)
> --- /dev/fd/63  2017-06-01 21:44:08.952583804 +0200
> +++ /dev/fd/62  2017-06-01 21:44:08.952583804 +0200
> @@ -1,6 +1,6 @@
> - 66 Junio C Hamano
> + 65 Junio C Hamano
>   10 Jeff King
> -  8 Stefan Beller
> +  9 Stefan Beller
>5 Lars Schneider
>2 SZEDER Gábor
>1 Tyler Brazier
>
>
>> By itself:
>>
>>   $ git log --author=Ævar --invert-grep --format=%an -100 origin/pu |
>> sort | uniq -c | sort -rn
>>79 Junio C Hamano
>> 8 Stefan Beller
>> 8 Jeff King
>> 1 Sahil Dua
>> 1 Rikard Falkeborn
>> 1 Johannes Sixt
>> 1 Johannes Schindelin
>> 1 Andreas Heiduk
>>
>> So that makes sense from the "--author is invertable" world-view.
>>
>> But I'm puzzled that you show up at all when --grep=bar is added (and
>> moreover, that we get _one_ commit from you, not the 5 found in your
>> initial command). That seems like a bug.
>
> I think that's the only thing that's not a bug, i.e. we just find 1
> match in the first 100 (note -100), sorry for the confusing example.
>
> Anyway, much of the above may be incorrect, I haven't dug deeply
> beyond just finding that something's funny going on and we definitely
> have *some* bugs here.

bug: `git log --grep ... --invert-grep --author=...` negates / ignores --author

[Anthony Sottile]

Given the following commits:

```
asottile@asottile-VirtualBox:/tmp$ git init test
Initialized empty Git repository in /tmp/test/.git/
asottile@asottile-VirtualBox:/tmp$ cd test/
asottile@asottile-VirtualBox:/tmp/test$
GIT_COMMITTER_EMAIL=f...@example.com GIT_AUTHOR_EMAIL=f...@example.com
git commit --allow-empty -m "foo"
[master (root-commit) c9df62b] foo
asottile@asottile-VirtualBox:/tmp/test$ git commit -m "blah" --allow-empty
[master 9e3ee9b] blah
asottile@asottile-VirtualBox:/tmp/test$ git log
commit 9e3ee9bc1adab2ae8eb1884a8f6237da18dfd27b
Author: Anthony Sottile <asott...@umich.edu>
Date:   Wed May 31 08:40:59 2017 -0700

blah

commit c9df62b93298a247fcfbe24ed4282ccf95448f47
Author: Anthony Sottile <f...@example.com>
Date:   Wed May 31 08:40:49 2017 -0700

foo
asottile@asottile-VirtualBox:/tmp/test$ git log --grep bar
--invert-grep --author=foo
commit 9e3ee9bc1adab2ae8eb1884a8f6237da18dfd27b
Author: Anthony Sottile <asott...@umich.edu>
Date:   Wed May 31 08:40:59 2017 -0700

blah

commit c9df62b93298a247fcfbe24ed4282ccf95448f47
Author: Anthony Sottile <f...@example.com>
Date:   Wed May 31 08:40:49 2017 -0700

foo
asottile@asottile-VirtualBox:/tmp/test$ git log --author=foocommit
c9df62b93298a247fcfbe24ed4282ccf95448f47
Author: Anthony Sottile <f...@example.com>
Date:   Wed May 31 08:40:49 2017 -0700

foo
```

I expect the same output from the last two commands, but the
`--invert-grep` one seems to match _all_ the commits.

I can try and dig into this if I have time, just trying to get a count
using this as a workaround

git log --grep ... --invert-grep --format=%ce | grep ... | wc -l

Anthony

git submodule add broken (2.11.0-rc1): Cannot open git-sh-i18n

[Anthony Sottile]

Noticed as part of my automated tests here:
https://travis-ci.org/pre-commit/pre-commit/jobs/173957051

Minimal reproduction:

rm -rf /tmp/git /tmp/foo /tmp/bar
git clone git://github.com/git/git --depth 1 /tmp/git
pushd /tmp/git
make -j 8
popd
export PATH="/tmp/git:$PATH"
git init /tmp/foo
git init /tmp/bar
cd /tmp/foo
git submodule add /tmp/bar baz

Output:

$ rm -rf /tmp/git /tmp/foo /tmp/bar
$ git clone git://github.com/git/git --depth 1 /tmp/git
Cloning into '/tmp/git'...
remote: Counting objects: 3074, done.
remote: Compressing objects: 100% (2735/2735), done.
remote: Total 3074 (delta 249), reused 1871 (delta 215), pack-reused 0
Receiving objects: 100% (3074/3074), 6.38 MiB | 905.00 KiB/s, done.
Resolving deltas: 100% (249/249), done.
Checking connectivity... done.
$ pushd /tmp/git
/tmp/git /tmp
$ make -j 8
GIT_VERSION = 2.11.0-rc0

... lots of make output ...

$ popd
/tmp
$ export PATH="/tmp/git:$PATH"
$ git init /tmp/foo
warning: templates not found /home/asottile/share/git-core/templates
Initialized empty Git repository in /tmp/foo/.git/
$ git init /tmp/bar
warning: templates not found /home/asottile/share/git-core/templates
Initialized empty Git repository in /tmp/bar/.git/
$ cd /tmp/foo
$ git submodule add /tmp/bar baz
/tmp/git/git-submodule: 46: .: Can't open
/home/asottile/libexec/git-core/git-sh-i18n
$ echo $?
2


Thanks

Anthony

Fwd: Git clone fails during pre-commit hook due to GIT_WORK_TREE=. (regression 2.5 -> 2.6)

[Anthony Sottile]

* Short description of the problem *

It seems GIT_WORK_DIR is now exported invariantly when calling git
hooks such as pre-commit.  If these hooks involve cloning repositories
they will not fail due to this exported environment variable.  This
was not the case in prior versions (such as v2.5.0).

* Simple reproduction *

```
$ cat test.sh
#!/usr/bin/env bash
set -ex

rm -rf test

# Exit non {0, 1} to abort git bisect
make -j 8 > /dev/null || exit 2

# Put our new git on the path
PATH="$(pwd):$PATH"

git init test

pushd test
mkdir -p .git/hooks
echo 'git clone git://github.com/asottile/css-explore css-explore' >
.git/hooks/pre-commit
chmod 755 .git/hooks/pre-commit

git commit -m foo --allow-empty || exit 1
```

* Under 2.6.3 *

```
$ ./test.sh

...

+ git init test
warning: templates not found /home/anthony/share/git-core/templates
Initialized empty Git repository in /home/anthony/workspace/git/test/.git/
+ pushd test
~/workspace/git/test ~/workspace/git
+ mkdir -p .git/hooks
+ echo 'git clone git://github.com/asottile/css-explore css-explore'
+ chmod 755 .git/hooks/pre-commit
+ git commit -m foo --allow-empty
fatal: working tree '.' already exists.
+ exit 1
```

* Under 2.5 *

```
$ ./test.sh

...

+ git init test
warning: templates not found /home/anthony/share/git-core/templates
Initialized empty Git repository in /home/anthony/workspace/git/test/.git/
+ pushd test
~/workspace/git/test ~/workspace/git
+ mkdir -p .git/hooks
+ echo 'git clone git://github.com/asottile/css-explore css-explore'
+ chmod 755 .git/hooks/pre-commit
+ git commit -m foo --allow-empty
Cloning into 'css-explore'...
warning: templates not found /home/anthony/share/git-core/templates
remote: Counting objects: 214, done.
remote: Total 214 (delta 0), reused 0 (delta 0), pack-reused 214
Receiving objects: 100% (214/214), 25.89 KiB | 0 bytes/s, done.
Resolving deltas: 100% (129/129), done.
Checking connectivity... done.
[master (root-commit) 5eb999d] foo
```


* Bisect *

```
$ git bisect good v2.5.0
$ git bisect bad origin/master
$ git bisect run ./test.sh

...

d95138e695d99d32dcad528a2a7974f434c51e79 is the first bad commit
commit d95138e695d99d32dcad528a2a7974f434c51e79
Author: Nguyễn Thái Ngọc Duy 
Date:   Fri Jun 26 17:37:35 2015 +0700

setup: set env $GIT_WORK_TREE when work tree is set, like $GIT_DIR

In the test case, we run setup_git_dir_gently() the first time to read
$GIT_DIR/config so that we can resolve aliases. We'll enter
setup_discovered_git_dir() and may or may not call set_git_dir() near
the end of the function, depending on whether the detected git dir is
".git" or not. This set_git_dir() will set env var $GIT_DIR.

For normal repo, git dir detected via setup_discovered_git_dir() will be
".git", and set_git_dir() is not called. If .git file is used however,
the git dir can't be ".git" and set_git_dir() is called and $GIT_DIR
set. This is the key of this problem.

If we expand an alias (or autocorrect command names), then
setup_git_dir_gently() is run the second time. If $GIT_DIR is not set in
the first run, we run the same setup_discovered_git_dir() as before.
Nothing to see. If it is, however, we'll enter setup_explicit_git_dir()
this time.

This is where the "fun" is.  If $GIT_WORK_TREE is not set but
$GIT_DIR is, you are supposed to be at the root level of the
worktree.  But if you are in a subdir "foo/bar" (real worktree's top
is "foo"), this rule bites you: your detected worktree is now
"foo/bar", even though the first run correctly detected worktree as
"foo". You get "internal error: work tree has already been set" as a
result.

Bottom line is, when $GIT_DIR is set, $GIT_WORK_TREE should be set too
unless there's no work tree. But setting $GIT_WORK_TREE inside
set_git_dir() may backfire. We don't know at that point if work tree is
already configured by the caller. So set it when work tree is
detected. It does not harm if $GIT_WORK_TREE is set while $GIT_DIR is
not.

Reported-by: Bjørnar Snoksrud 
Signed-off-by: Nguyễn Thái Ngọc Duy 
Signed-off-by: Junio C Hamano 

:100644 100644 9daa0ba4a36ced9f63541203e7bcc2ab9e1eae56
36fbba57fc83afd36d99bf5d4f3a1fc3feefba09 Menvironment.c
:04 04 1d7c4bf77e0fd49ca315271993cb69a8b055c3aa
145d85895cb6cb0810597e1854a7721ccfc8f457 Mt
bisect run success
```

Causing me a few headaches in
https://github.com/pre-commit/pre-commit/issues/300
I'm working around it in https://github.com/pre-commit/pre-commit/pull/301

Thanks,

Anthony
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html