On Mon, Aug 03, 2015 at 05:21:43PM +0200, Per Cederqvist wrote: > If you run: > > git config pager.pull true > > in the hope of getting the output of "git pull" via a pager, you are > in for a surpise the next time you run "git pull --rebase" and it has > to rebase your work. It will fail with a nonsensical error message: > > > Applying: First B commit > > fatal: unrecognized input > > Repository lacks necessary blobs to fall back on 3-way merge. > > Cannot fall back to three-way merge. > > Patch failed at 0001 First B commit > > The copy of the patch that failed is found in: > > /home/cederp/badcolor/repo-b/.git/rebase-apply/patch > > > > When you have resolved this problem, run "git rebase --continue". > > If you prefer to skip this patch, run "git rebase --skip" instead. > > To check out the original branch and stop rebasing, run "git rebase > > --abort". > > Using "cat -vet" to look at the problematic patch, you can see that > there are embedded escape codes that tries to colorize the patch. > > This bug is dependent on the TERM setting. On my system (Ubuntu > 14.04) it reproduces if TERM=vt220 or TERM=rxvt-unicode, but not if > TERM=dumb. It might depend on the color.diff setting as well, but > it does reproduce with the default setting.
It looks like the use of a pager is fooling our "should we colorize the diff" check when generating the patches. Usually we check isatty(1) to see if we should use color, so "git format-patch >patches" does the right thing. But if a pager is in use, we have to override that check (since stdout goes to the pager, but the pager is going to a tty). That propagates to children via the GIT_PAGER_IN_USE environment variable. We could work around this by having pull explicitly tell rebase that it is not using a pager (by unsetting GIT_PAGER_IN_USE). Or by having rebase tell it to format-patch. But I think the best thing is probably to teach the low-level "are we going to a pager" check to only say "yes" if stdout is still a pipe, like the patch below. That lets: git format-patch --stdout >patches do the right thing; it knows that even if a pager is in use, its output is not going to it, because stdout isn't a pipe. Unfortunately this does not help: git format-patch --stdout | some_program because it cannot tell the difference between the pipe to the original pager. I wonder if we could do something even more clever, like putting the inode number in the environment. Then we could check if we have the _same_ pipe going to the pager. diff --git a/pager.c b/pager.c index 070dc11..5b3b3fd 100644 --- a/pager.c +++ b/pager.c @@ -95,9 +95,11 @@ void setup_pager(void) int pager_in_use(void) { - const char *env; - env = getenv("GIT_PAGER_IN_USE"); - return env ? git_config_bool("GIT_PAGER_IN_USE", env) : 0; + struct stat st; + + return git_env_bool("GIT_PAGER_IN_USE", 0) && + !fstat(1, &st) && + S_ISFIFO(st.st_mode); } /* -- 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