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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html