Re: [PATCH 5/8] rebase: introduce the --recreate-merges option

[Sergey Organov]

Johannes Sixt  writes:

> Am 09.02.2018 um 07:11 schrieb Sergey Organov:
>> Johannes Schindelin  writes:
>>> Let me explain the scenario which comes up plenty of times in my work with
>>> Git for Windows. We have a thicket of some 70 branches on top of git.git's
>>> latest release. These branches often include fixup! and squash! commits
>>> and even more complicated constructs that rebase cannot handle at all at
>>> the moment, such as reorder-before! and reorder-after! (for commits that
>>> really need to go into a different branch).
>>
>> I sympathize, but a solution that breaks even in simple cases can't be
>> used reliably to solve more complex problems, sorry. Being so deep
>> into your problems, I think you maybe just aren't seeing forest for the
>> trees [1].
>
> Hold your horses! Dscho has a point here. --preserve-merges
> --first-parent works only as long as you don't tamper with the side
> branches. If you make changes in the side branches during the same
> rebase operation, this --first-parent mode would undo that change.

He has a point indeed, but it must not be used as an excuse to silently
damage user data, as if there are no other options!

Simple --first-parent won't always fit, it's obvious. I used
--first-parent patch as mere illustration of concept, it's rather
"rebase [-i] --keep-the-f*g-shape" itself that should behave. There
should be no need for actual --first-parent that only fits
no-manual-editing use-cases.

Look at it as if it's a scale where --first-parent is on one side, and
"blind re-merge" is on the other. The right answer(s) lie somewhere
in-between, but I think they are much closer to --first-parent than they
are to "blind re-merge".

> (And, yes, its result would be called an "evil merge", and that scary
> name _should_ frighten you!)

(It won't always be "evil merge", and it still doesn't frighten even if
it will, provided git stops making them more evil then they actually
deserve, and it isn't an excuse to silently distort user data anyway!)

-- Sergey

[1] The "--first-parent" here would rather keep that change from
propagation to the main-line, not undo it, and sometimes it's even the
right thing to do ("-x ours" for the original merge being one example).
Frequently though it is needed on main-line indeed, and there should be
a way to tell git to propagate the change to the main-line, but even
then automatic blind unattended re-merge is wrong answer and I'm sure
git can be made to do better than that.

Re: [PATCH 5/8] rebase: introduce the --recreate-merges option

[Sergey Organov]

Hi Johannes,

Johannes Schindelin  writes:
> Hi Sergey,
>
> On Fri, 9 Feb 2018, Sergey Organov wrote:
>
>> Johannes Schindelin  writes:
>> 
>> [...]
>> 
>> > With this patch, the goodness of the Git garden shears comes to `git
>> > rebase -i` itself. Passing the `--recreate-merges` option will generate
>> > a todo list that can be understood readily, and where it is obvious
>> > how to reorder commits. New branches can be introduced by inserting
>> > `label` commands and calling `merge -  `. And once this
>> > mode has become stable and universally accepted, we can deprecate the
>> > design mistake that was `--preserve-merges`.
>> 
>> This doesn't explain why you introduced this new --recreate-merges. Why
>> didn't you rather fix --preserve-merges to generate and use new todo
>> list format?
>
> Because that would of course break existing users of
> --preserve-merges.

How exactly? Doesn't "--recreate-merges" produce the same result as
"--preserve-merges" if run non-interactively?

> So why not --preserve-merges=v2? Because that would force me to maintain
> --preserve-merges forever. And I don't want to.
>
>> It doesn't seem likely that todo list created by one Git version is to
>> be ever used by another, right?
>
> No. But by scripts based on `git rebase -p`.
>
>> Is there some hidden reason here? Some tools outside of Git that use old
>> todo list format, maybe?
>
> Exactly.
>
> I did mention such a tool: the Git garden shears:
>
>   https://github.com/git-for-windows/build-extra/blob/master/shears.sh
>
> Have a look at it. It will inform the discussion.

I've searched for "-p" in the script, but didn't find positives for
either "-p" or "--preserve-merges". How it would break if it doesn't use
them? What am I missing?

>
>> Then, if new option indeed required, please look at the resulting manual:
>> 
>> --recreate-merges::
>>  Recreate merge commits instead of flattening the history by replaying
>>  merges. Merge conflict resolutions or manual amendments to merge
>>  commits are not preserved.
>> 
>> -p::
>> --preserve-merges::
>>  Recreate merge commits instead of flattening the history by replaying
>>  commits a merge commit introduces. Merge conflict resolutions or manual
>>  amendments to merge commits are not preserved.
>
> As I stated in the cover letter, there are more patches lined up after
> this patch series.

Good, but I thought this one should better be self-consistent anyway.
What if those that come later aren't included?

>
> Have a look at https://github.com/git/git/pull/447, especially the latest
> commit in there which is an early version of the deprecation I intend to
> bring about.

You shouldn't want a deprecation at all should you have re-used
--preserve-merges in the first place, and I still don't see why you
haven't. 

>
> Also, please refrain from saying things like... "Don't you think ..."
>
> If you don't like the wording, I wold much more appreciate it if a better
> alternative was suggested.

Sorry, but how can I suggest one if I don't understand what you are
doing here in the first place? That's why I ask you.

>
>> Don't you think more explanations are needed there in the manual on
>> why do we have 2 separate options with almost the same yet subtly
>> different description? Is this subtle difference even important? How?
>> 
>> I also have trouble making sense of "Recreate merge commits instead of
>> flattening the history by replaying merges." Is it "> commits by replaying merges> instead of " or is it
>> rather " instead of > replaying merges>?
>
> The documentation of the --recreate-merges option is not meant to explain
> the difference to --preserve-merges. It is meant to explain the difference
> to regular `git rebase -i`, which flattens the commit history into a
> single branch without merge commits (in fact, all merge commits are simply
> ignored).

Yeah, that's obvious, but the point is that resulting manual is ended
up being confusing.

> And I would rather not start to describe the difference between
> --recreate-merges and --preserve-merges because I want to deprecate the
> latter, and describing the difference as I get the sense is your wish
> would simply mean more work because it would have to be added and then
> removed again.

I suspect you actually didn't need those new option in the first place,
and that's the core reason of these troubles.

-- Sergey

Re: [PATCH 5/8] rebase: introduce the --recreate-merges option

[Sergey Organov]

Hi Johannes,

Thanks for explanations, and could you please answer this one:

[...]

>> I also have trouble making sense of "Recreate merge commits instead of
>> flattening the history by replaying merges." Is it "> commits by replaying merges> instead of " or is it
>> rather " instead of > replaying merges>?

-- Sergey

Re: [PATCH v1] worktree: set worktree environment in post-checkout hook

[Eric Sunshine]

On Fri, Feb 9, 2018 at 8:01 PM,   wrote:
> In ade546be47 (worktree: invoke post-checkout hook (unless
> --no-checkout), 2017-12-07) we taught Git to run the post-checkout hook
> in worktrees. Unfortunately, the environment of the hook was not made
> aware of the worktree. Consequently, a 'git rev-parse --show-toplevel'
> call in the post-checkout hook would return a wrong result.
>
> Fix this by setting the 'GIT_WORK_TREE' environment variable to make
> Git calls within the post-checkout hook aware of the worktree.
>
> Signed-off-by: Lars Schneider 
> ---
> I think this is a bug in Git 2.16. We noticed it because it caused a
> problem in Git LFS [1]. The modified test case fails with Git 2.16 and
> succeeds with this patch.

Thanks for reporting and diagnosing the problem.

I have some concerns about this patch's fix of setting GIT_WORK_TREE
unconditionally. In particular, such unconditional setting of
GIT_WORK_TREE might cause unforeseen problems. Although the
circumstances may not be quite the same, but the tale told by
86d26f240f (setup.c: re-fix d95138e (setup: set env $GIT_WORK_TREE
when .., 2015-12-20) makes me cautious.

More significantly, though, setting GIT_WORK_TREE seems too
specialized a solution. While it may "fix" Git commands invoked by the
hook, it does nothing for other commands ('cp', 'mv', etc.) which the
hook may employ.

As a review comment, I was going to suggest that you chdir() to the
new worktree directory instead of messing with GIT_WORK_TREE, but when
I tested it myself before making the suggestion, I discovered that the
issue is a bit more involved. The result is that I ended up posting a
patch series[1] to replace this one, with what I believe is a more
correct fix.

[1]: 
https://public-inbox.org/git/20180212031526.40039-1-sunsh...@sunshineco.com/

[PATCH 0/2] worktree: change to new worktree dir before running hook(s)

[Eric Sunshine]

This patch series replaces "worktree: set worktree environment in
post-checkout hook"[1] from Lars, which is a proposed bug fix for
ade546be47 (worktree: invoke post-checkout hook, 2017-12-07).

The problem that patch addresses is that "git worktree add" does not
provide proper context to the invoked 'post-checkout' hook, so the hook
doesn't know where the newly-created worktree is. Lars's approach was to
set GIT_WORK_TREE to point at the new worktree directory, however, doing
so has a few drawbacks:

1. GIT_WORK_TREE is normally assigned in conjunction with GIT_DIR; it is
   unusual and possibly problematic to set one but not the other.

2. Assigning GIT_WORK_TREE unconditionally may lead to unforeseen
   interactions and problems with end-user scripts and aliases or even
   within Git itself. It seems better to avoid unconditional assignment
   rather than risk problems such as those described and worked around
   by 86d26f240f (setup.c: re-fix d95138e (setup: set env $GIT_WORK_TREE
   when .., 2015-12-20)

3. Assigning GIT_WORK_TREE is too specialized a solution; it "fixes"
   only Git commands run by the hook, but does nothing for other
   commands ('mv', 'cp', etc.) that the hook might invoke.

The real problem with ade546be47 is that it neglects to change to the
directory of the newly-created worktree before running the hook, thus
the hook incorrectly runs in the directory in which "git worktree add"
was invoked. Rather than messing with GIT_WORK_TREE, this replacement
patch series fixes the problem by ensuring that the directory is changed
before the hook is invoked.

[1]: 
https://public-inbox.org/git/20180210010132.33629-1-lars.schnei...@autodesk.com/

Eric Sunshine (2):
  run-command: teach 'run_hook' about alternate worktrees
  worktree: add: change to new worktree directory before running hook

 builtin/worktree.c  | 11 ---
 run-command.c   | 23 +--
 run-command.h   |  4 
 t/t2025-worktree-add.sh | 25 ++---
 4 files changed, 55 insertions(+), 8 deletions(-)

-- 
2.16.1.291.g4437f3f132

[PATCH 1/2] run-command: teach 'run_hook' about alternate worktrees

[Eric Sunshine]

Git commands which run hooks do so at the top level of the worktree in
which the command itself was invoked. However, the 'git worktree'
command may need to run hooks within some other directory. For
instance, when "git worktree add" runs the 'post-checkout' hook, the
hook must be run within the newly-created worktree, not within the
worktree from which "git worktree add" was invoked.

To support this case, add 'run-hook' overloads which allow the
worktree directory to be specified.

Signed-off-by: Eric Sunshine 
---
 run-command.c | 23 +--
 run-command.h |  4 
 2 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/run-command.c b/run-command.c
index 31fc5ea86e..0e3995bbf9 100644
--- a/run-command.c
+++ b/run-command.c
@@ -1197,7 +1197,8 @@ const char *find_hook(const char *name)
return path.buf;
 }
 
-int run_hook_ve(const char *const *env, const char *name, va_list args)
+int run_hook_cd_ve(const char *dir, const char *const *env, const char *name,
+  va_list args)
 {
struct child_process hook = CHILD_PROCESS_INIT;
const char *p;
@@ -1206,9 +1207,10 @@ int run_hook_ve(const char *const *env, const char 
*name, va_list args)
if (!p)
return 0;
 
-   argv_array_push(, p);
+   argv_array_push(, absolute_path(p));
while ((p = va_arg(args, const char *)))
argv_array_push(, p);
+   hook.dir = dir;
hook.env = env;
hook.no_stdin = 1;
hook.stdout_to_stderr = 1;
@@ -1216,6 +1218,23 @@ int run_hook_ve(const char *const *env, const char 
*name, va_list args)
return run_command();
 }
 
+int run_hook_ve(const char *const *env, const char *name, va_list args)
+{
+   return run_hook_cd_ve(NULL, env, name, args);
+}
+
+int run_hook_cd_le(const char *dir, const char *const *env, const char *name, 
...)
+{
+   va_list args;
+   int ret;
+
+   va_start(args, name);
+   ret = run_hook_cd_ve(dir, env, name, args);
+   va_end(args);
+
+   return ret;
+}
+
 int run_hook_le(const char *const *env, const char *name, ...)
 {
va_list args;
diff --git a/run-command.h b/run-command.h
index 3932420ec8..8beddffea8 100644
--- a/run-command.h
+++ b/run-command.h
@@ -66,7 +66,11 @@ int run_command(struct child_process *);
 extern const char *find_hook(const char *name);
 LAST_ARG_MUST_BE_NULL
 extern int run_hook_le(const char *const *env, const char *name, ...);
+extern int run_hook_cd_le(const char *dir, const char *const *env,
+ const char *name, ...);
 extern int run_hook_ve(const char *const *env, const char *name, va_list args);
+extern int run_hook_cd_ve(const char *dir, const char *const *env,
+ const char *name, va_list args);
 
 #define RUN_COMMAND_NO_STDIN 1
 #define RUN_GIT_CMD 2  /*If this is to be git sub-command */
-- 
2.16.1.291.g4437f3f132

[PATCH 2/2] worktree: add: change to new worktree directory before running hook

[Eric Sunshine]

Although "git worktree add" learned to run the 'post-checkout' hook in
ade546be47 (worktree: invoke post-checkout hook, 2017-12-07), it
neglects to change to the directory of the newly-created worktree
before running the hook. Instead, the hook is run within the directory
from which the "git worktree add" command itself was invoked, which
effectively neuters the hook since it knows nothing about the new
worktree directory.

Fix this by changing to the new worktree's directory before running
the hook, and adjust the tests to verify that the hook is indeed run
within the correct directory.

While at it, also add a test to verify that the hook is run within the
correct directory even when the new worktree is created from a sibling
worktree (as opposed to the main worktree).

Reported-by: Lars Schneider 
Signed-off-by: Eric Sunshine 
---
 builtin/worktree.c  | 11 ---
 t/t2025-worktree-add.sh | 25 ++---
 2 files changed, 30 insertions(+), 6 deletions(-)

diff --git a/builtin/worktree.c b/builtin/worktree.c
index 7cef5b120b..b55c55a26c 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -345,9 +345,14 @@ static int add_worktree(const char *path, const char 
*refname,
 * Hook failure does not warrant worktree deletion, so run hook after
 * is_junk is cleared, but do return appropriate code when hook fails.
 */
-   if (!ret && opts->checkout)
-   ret = run_hook_le(NULL, "post-checkout", oid_to_hex(_oid),
- oid_to_hex(>object.oid), "1", NULL);
+   if (!ret && opts->checkout) {
+   char *p = absolute_pathdup(path);
+   ret = run_hook_cd_le(p, NULL, "post-checkout",
+oid_to_hex(_oid),
+oid_to_hex(>object.oid),
+"1", NULL);
+   free(p);
+   }
 
argv_array_clear(_env);
strbuf_release();
diff --git a/t/t2025-worktree-add.sh b/t/t2025-worktree-add.sh
index 2b95944973..cf0aaeaf88 100755
--- a/t/t2025-worktree-add.sh
+++ b/t/t2025-worktree-add.sh
@@ -454,20 +454,29 @@ post_checkout_hook () {
test_when_finished "rm -f .git/hooks/post-checkout" &&
mkdir -p .git/hooks &&
write_script .git/hooks/post-checkout <<-\EOF
-   echo $* >hook.actual
+   {
+   echo $*
+   git rev-parse --show-toplevel
+   } >../hook.actual
EOF
 }
 
 test_expect_success '"add" invokes post-checkout hook (branch)' '
post_checkout_hook &&
-   printf "%s %s 1\n" $_z40 $(git rev-parse HEAD) >hook.expect &&
+   {
+   echo $_z40 $(git rev-parse HEAD) 1 &&
+   echo $(pwd)/gumby
+   } >hook.expect &&
git worktree add gumby &&
test_cmp hook.expect hook.actual
 '
 
 test_expect_success '"add" invokes post-checkout hook (detached)' '
post_checkout_hook &&
-   printf "%s %s 1\n" $_z40 $(git rev-parse HEAD) >hook.expect &&
+   {
+   echo $_z40 $(git rev-parse HEAD) 1 &&
+   echo $(pwd)/grumpy
+   } >hook.expect &&
git worktree add --detach grumpy &&
test_cmp hook.expect hook.actual
 '
@@ -479,4 +488,14 @@ test_expect_success '"add --no-checkout" suppresses 
post-checkout hook' '
test_path_is_missing hook.actual
 '
 
+test_expect_success '"add" within worktree invokes post-checkout hook' '
+   post_checkout_hook &&
+   {
+   echo $_z40 $(git rev-parse HEAD) 1 &&
+   echo $(pwd)/guppy
+   } >hook.expect &&
+   git -C gloopy worktree add --detach ../guppy &&
+   test_cmp hook.expect hook.actual
+'
+
 test_done
-- 
2.16.1.291.g4437f3f132

Re: [PATCH v3 42/42] git-completion.bash: add GIT_COMPLETION_OPTIONS=all config

[Duy Nguyen]

On Sun, Feb 11, 2018 at 8:59 AM, Eric Sunshine  wrote:
> On Fri, Feb 9, 2018 at 6:02 AM, Nguyễn Thái Ngọc Duy  
> wrote:
>> By default, some option names (mostly --force, scripting related or for
>> internal use) are not completable for various reasons. When
>> GIT_COMPLETION_OPTIONS is set to all, all options (except hidden ones)
>> are completable.
>>
>> Signed-off-by: Nguyễn Thái Ngọc Duy 
>> ---
>> diff --git a/contrib/completion/git-completion.bash 
>> b/contrib/completion/git-completion.bash
>> @@ -36,6 +36,10 @@
>> +#
>> +#   GIT_COMPLETION_OPTIONS
>> +#
>> +# When set to "all", complete all possible options
>> @@ -303,7 +307,7 @@ __gitcomp_builtin ()
>> if [ -z "$options" ]; then
>> # leading and trailing spaces are significant to make
>> # option removal work correctly.
>> -   options=" $(__git ${cmd/_/ } --git-completion-helper) $incl "
>> +   options=" $(__git ${cmd/_/ } 
>> --git-completion-helper=$GIT_COMPLETION_OPTIONS) $incl "
>
> This approach is rather different from what I had envisioned. Rather
> than asking --git-completion-helper to do the filtering, my thought
> was that it should unconditionally dump _all_ options but annotate
> them, and then git-completion.bash can filter however it sees fit. For
> instance, --git-completion-helper might annotate "dangerous" options
> with a "!" ("!--force" or "--force!" or "--force:!" or whatever).
>
> The benefit of this approach is that it more easily supports future
> enhancements. For instance, options which only make sense in certain
> contexts could be annotated to indicate such. An example are the
> --continue, --abort, --skip options for git-rebase which only make
> sense when a rebase session is active. One could imagine these options
> being annotated something like this:
>
> --abort:rebasing
> --continue:rebasing
> --skip:rebasing
>
> where git-completion.bash understands the "rebasing" annotation as
> meaning that these options only make sense when "rebase-apply" is
> present. (In fact, the annotation could be more expressive, such as
> "--abort:exists(rebase-apply)", but that might be overkill.)

I agree. I went a bit off track with this ...=all. But yes some form
of annotation will be needed long term to describe these options in
details. We haven't gotten the annotation in struct option[] to this
level yet, it's a bit hard to see what will be needed here. Let's drop
42/42 for now. I will study git-completion.bash more to see what it
needs and revisit this at some point in future.
-- 
Duy

[PATCH 2/2] Makefile: suppress a sparse warning for pack-revindex.c

[Ramsay Jones]

Sparse has, for a long time, been issuing the following warning against
the pack-revindex.c file:

  SP pack-revindex.c
  pack-revindex.c:64:23: warning: memset with byte count of 262144

This results from a unconditional check, with a hard-coded limit, which
is really only appropriate for the kernel source code. (The check is for
a 'large' byte count in a call to memcpy(), memset(), copy_from_user()
and copy_to_user() functions).

A recent release of sparse (v0.5.1) has introduced some options to allow
this check to be turned off (-Wno-memcpy-max-count) or to specify the
actual limit used (-fmemcpy-max-count=COUNT), rather than a hard-coded
limit of 10.

In order to suppress the warning, add a target for pack-revindex.sp that
adds the '-Wno-memcpy-max-count' option to the SPARSE_FLAGS variable.

Signed-off-by: Ramsay Jones 
---
 Makefile | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Makefile b/Makefile
index 1a9b23b67..7f40f7673 100644
--- a/Makefile
+++ b/Makefile
@@ -2176,6 +2176,8 @@ gettext.sp gettext.s gettext.o: EXTRA_CPPFLAGS = \
 http-push.sp http.sp http-walker.sp remote-curl.sp imap-send.sp: SPARSE_FLAGS 
+= \
-DCURL_DISABLE_TYPECHECK
 
+pack-revindex.sp: SPARSE_FLAGS += -Wno-memcpy-max-count
+
 ifdef NO_EXPAT
 http-walker.sp http-walker.s http-walker.o: EXTRA_CPPFLAGS = -DNO_EXPAT
 endif
-- 
2.16.0

[PATCH 1/2] config.mak.uname: remove SPARSE_FLAGS setting for cygwin

[Ramsay Jones]

Since commit f66450ae9 ("cygwin: Remove the Win32 l/stat() implementation",
2013-06-22), the cygwin build has not used the WIN32 API/header files.
This means that the '-isystem /usr/include/w32api' option to sparse is
no longer necessary (to allow sparse to find the WIN32 header files).
In addition, the '-Wno-one-bit-signed-bitfield' option can be removed,
since the warning suppressed by that option was only provoked by a WIN32
header file.

Signed-off-by: Ramsay Jones 
---
 config.mak.uname | 1 -
 1 file changed, 1 deletion(-)

diff --git a/config.mak.uname b/config.mak.uname
index 685a80d13..6a1d0de0c 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -182,7 +182,6 @@ ifeq ($(uname_O),Cygwin)
NO_ST_BLOCKS_IN_STRUCT_STAT = YesPlease
X = .exe
UNRELIABLE_FSTAT = UnfortunatelyYes
-   SPARSE_FLAGS = -isystem /usr/include/w32api -Wno-one-bit-signed-bitfield
OBJECT_CREATION_USES_RENAMES = UnfortunatelyNeedsTo
MMAP_PREVENTS_DELETE = UnfortunatelyYes
COMPAT_OBJS += compat/cygwin.o
-- 
2.16.0

[PATCH 0/2] misc sparse updates

[Ramsay Jones]

These patches are based on v2.16, but a test merge to master, next and
pu are all clean.

Ramsay Jones (2):
  config.mak.uname: remove SPARSE_FLAGS setting for cygwin
  Makefile: suppress a sparse warning for pack-revindex.c

 Makefile | 2 ++
 config.mak.uname | 1 -
 2 files changed, 2 insertions(+), 1 deletion(-)

-- 
2.16.0

[PATCH 2/2] t5556: replace test_i18ngrep with a simple grep

[Ramsay Jones]

Attempting to grep the output of test_i18ngrep will not work under a
poison build, since the output is (almost) guaranteed not to have the
string you are looking for. In this case, the output of test_i18ngrep
is further filtered by a simple piplined grep to exclude an '... remote
end hung up unexpectedly' warning message. Use a regular 'grep -E' to
replace the call to test_i18ngrep in the filter pipeline.

Also, remove a useless invocation of 'sort' as the final element of the
pipeline.

Signed-off-by: Ramsay Jones 
---
 t/t5536-fetch-conflicts.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t5536-fetch-conflicts.sh b/t/t5536-fetch-conflicts.sh
index 2e42cf331..38381df5e 100755
--- a/t/t5536-fetch-conflicts.sh
+++ b/t/t5536-fetch-conflicts.sh
@@ -22,7 +22,7 @@ verify_stderr () {
cat >expected &&
# We're not interested in the error
# "fatal: The remote end hung up unexpectedly":
-   test_i18ngrep -E '^(fatal|warning):' actual 
| sort &&
+   grep -E '^(fatal|warning):' actual &&
test_i18ncmp expected actual
 }
 
-- 
2.16.0

[PATCH 1/2] t4151: consolidate multiple calls to test_i18ngrep

[Ramsay Jones]

Attempting to grep the output of test_i18ngrep will not work under a
poison build, since the output is (almost) guaranteed not to have the
string you are looking for. In this case, we have a test_i18ngrep call
which attempts to filter the contents of a file, which was itself the
result of a call to test_i18ngrep. In this case, we can achieve the
same effect with a single call to test_i18ngrep (without creating the
intermediate file), since the second regular expression can be used
without change to filter the original input.

Also, replace a call to test_i18ncmp with test_cmp, since the content
being compared is not subject to i18n anyway.

Signed-off-by: Ramsay Jones 
---
 t/t4151-am-abort.sh | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/t/t4151-am-abort.sh b/t/t4151-am-abort.sh
index 9473c2779..16432781d 100755
--- a/t/t4151-am-abort.sh
+++ b/t/t4151-am-abort.sh
@@ -46,9 +46,8 @@ do
 
test_expect_success "am$with3 --skip continue after failed am$with3" '
test_must_fail git am$with3 --skip >output &&
-   test_i18ngrep "^Applying" output >output.applying &&
-   test_i18ngrep "^Applying: 6$" output.applying &&
-   test_i18ncmp file-2-expect file-2 &&
+   test_i18ngrep "^Applying: 6$" output &&
+   test_cmp file-2-expect file-2 &&
test ! -f .git/MERGE_RR
'
 
-- 
2.16.0

[PATCH 0/2] test_i18ngrep

[Ramsay Jones]

These patches resulted from an experiment of yours [1], I wrote these up
last year, then promptly forgot about them! ;-)

These patches were built on top of v2.16, and the second patch has a simple
conflict with commit 93b4b0313c ("t5536: let 'test_i18ngrep' read the file
without redirection", 2018-02-08), which is in the 'next' branch.

The conflict looks like so:

  $ git diff
  diff --cc t/t5536-fetch-conflicts.sh
  index 644736b8a,38381df5e..0
  --- a/t/t5536-fetch-conflicts.sh
  +++ b/t/t5536-fetch-conflicts.sh
  @@@ -22,7 -22,7 +22,11 @@@ verify_stderr () 
  cat >expected &&
  # We're not interested in the error
  # "fatal: The remote end hung up unexpectedly":
  ++<<< HEAD
   +  test_i18ngrep -E '^(fatal|warning):' error | grep -v 'hung up' 
>actual | sort &&
  ++===
  +   grep -E '^(fatal|warning):' actual &&
  ++>>> master-i18n
  test_i18ncmp expected actual
}

  $ 
  
The resolution is to simply take the 'master-i18n' text. However, if you
prefer, I can rebuild these patches on top of 'next' and re-submit. Just
let me know.

Note that I replaced an 'test_i18ngrep -E' with 'grep -E' rather than egrep.
(the grep man page claims that egrep, fgrep and rgrep are deprecated, but I
think that has been the case for as long as I can remember, so don't hold
your breath!).

[1] 
https://public-inbox.org/git/%3cxmqqvahawirr@gitster.mtv.corp.google.com%3E/

Ramsay Jones (2):
  t4151: consolidate multiple calls to test_i18ngrep
  t5556: replace test_i18ngrep with a simple grep

 t/t4151-am-abort.sh| 5 ++---
 t/t5536-fetch-conflicts.sh | 2 +-
 2 files changed, 3 insertions(+), 4 deletions(-)

-- 
2.16.0

REQUEST NEW TRANSLATION (INDONESIAN/id_ID)

[halo]

Hello git-l10n Team

I want to join to this project as a translator for Indonesian language 
(ID)
I have read the README file located in the 
https://github.com/git-l10n/git-po/blob/master/po/README directory


I also have a fork repository master (git-l10n) to my repository 
(anaufalm), and also I have edited the TEAMS file by adding my name as a 
translator for Indonesia (id). And also I created a new file `id.po` 
which I copy from file` ca.po` as the source. Because I not find 
original file as english, like `en.po`.


Furthermore, if approved, I will translate the file asap.

Thank you.

---

My repository (fork): https://github.com/anaufalm/git-id
PR link request TEAMS: https://github.com/git-l10n/git-po/pull/288
PR link add id.po file: https://github.com/git-l10n/git-po/pull/289

[PATCH 2/3] config: respect `pager.config` in list/get-mode only

[Martin Ågren]

Similar to de121ffe5 (tag: respect `pager.tag` in list-mode only,
2017-08-02), use the DELAY_PAGER_CONFIG-mechanism to only respect
`pager.config` when we are listing or "get"ing config.

Some getters give at most one line of output, but it is much easier to
document and understand that we page all of --get[-*] and --list, than
to divide the (current and future) getters into "pages" and "doesn't".

This fixes the failing test added in the previous commit. Also adapt the
test for whether `git config foo.bar bar` respects `pager.config`.

Signed-off-by: Martin Ågren 
---
 Documentation/git-config.txt | 5 +
 t/t7006-pager.sh | 6 +++---
 builtin/config.c | 8 
 git.c| 2 +-
 4 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
index 14da5fc157..ccc8f0bcff 100644
--- a/Documentation/git-config.txt
+++ b/Documentation/git-config.txt
@@ -233,6 +233,11 @@ See also <>.
using `--file`, `--global`, etc) and `on` when searching all
config files.
 
+CONFIGURATION
+-
+`pager.config` is only respected when listing configuration, i.e., when
+`--list`, `--get` or any of `--get-*` is used.
+
 [[FILES]]
 FILES
 -
diff --git a/t/t7006-pager.sh b/t/t7006-pager.sh
index 5a7b757c94..869a0359a8 100755
--- a/t/t7006-pager.sh
+++ b/t/t7006-pager.sh
@@ -245,13 +245,13 @@ test_expect_success TTY 'git branch --set-upstream-to 
ignores pager.branch' '
! test -e paginated.out
 '
 
-test_expect_success TTY 'git config respects pager.config when setting' '
+test_expect_success TTY 'git config ignores pager.config when setting' '
rm -f paginated.out &&
test_terminal git -c pager.config config foo.bar bar &&
-   test -e paginated.out
+   ! test -e paginated.out
 '
 
-test_expect_failure TTY 'git config --edit ignores pager.config' '
+test_expect_success TTY 'git config --edit ignores pager.config' '
rm -f paginated.out editor.used &&
write_script editor <<-\EOF &&
touch editor.used
diff --git a/builtin/config.c b/builtin/config.c
index ab5f95476e..9a57a0caff 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -48,6 +48,11 @@ static int show_origin;
 #define ACTION_GET_COLORBOOL (1<<14)
 #define ACTION_GET_URLMATCH (1<<15)
 
+/* convenience macro for "ACTION_LIST | ACTION_GET_*" */
+#define ACTION_LIST_OR_GET (ACTION_LIST | ACTION_GET | ACTION_GET_ALL | \
+   ACTION_GET_REGEXP | ACTION_GET_COLOR | \
+   ACTION_GET_COLORBOOL | ACTION_GET_URLMATCH)
+
 #define TYPE_BOOL (1<<0)
 #define TYPE_INT (1<<1)
 #define TYPE_BOOL_OR_INT (1<<2)
@@ -594,6 +599,9 @@ int cmd_config(int argc, const char **argv, const char 
*prefix)
usage_with_options(builtin_config_usage, 
builtin_config_options);
}
 
+   if (actions & ACTION_LIST_OR_GET)
+   setup_auto_pager("config", 0);
+
if (actions == ACTION_LIST) {
check_argc(argc, 0, 0);
if (config_with_options(show_all_config, NULL,
diff --git a/git.c b/git.c
index c870b9719c..e5c9b8729d 100644
--- a/git.c
+++ b/git.c
@@ -389,7 +389,7 @@ static struct cmd_struct commands[] = {
{ "column", cmd_column, RUN_SETUP_GENTLY },
{ "commit", cmd_commit, RUN_SETUP | NEED_WORK_TREE },
{ "commit-tree", cmd_commit_tree, RUN_SETUP },
-   { "config", cmd_config, RUN_SETUP_GENTLY },
+   { "config", cmd_config, RUN_SETUP_GENTLY | DELAY_PAGER_CONFIG },
{ "count-objects", cmd_count_objects, RUN_SETUP },
{ "credential", cmd_credential, RUN_SETUP_GENTLY },
{ "describe", cmd_describe, RUN_SETUP },
-- 
2.16.1.72.g5be1f00a9a

[PATCH 3/3] config: change default of `pager.config` to "on"

[Martin Ågren]

This is similar to ff1e72483 (tag: change default of `pager.tag` to
"on", 2017-08-02) and is safe now that we do not consider `pager.config`
at all when we are not listing or getting configuration. This change
will help with listing large configurations, but will not hurt users of
`git config --edit` as it would have before the previous commit.

Signed-off-by: Martin Ågren 
---
 Documentation/git-config.txt |  2 +-
 t/t7006-pager.sh | 12 ++--
 builtin/config.c |  2 +-
 3 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
index ccc8f0bcff..78074cf3b2 100644
--- a/Documentation/git-config.txt
+++ b/Documentation/git-config.txt
@@ -236,7 +236,7 @@ See also <>.
 CONFIGURATION
 -
 `pager.config` is only respected when listing configuration, i.e., when
-`--list`, `--get` or any of `--get-*` is used.
+`--list`, `--get` or any of `--get-*` is used. The default is to use a pager.
 
 [[FILES]]
 FILES
diff --git a/t/t7006-pager.sh b/t/t7006-pager.sh
index 869a0359a8..47f7830eb1 100755
--- a/t/t7006-pager.sh
+++ b/t/t7006-pager.sh
@@ -261,22 +261,22 @@ test_expect_success TTY 'git config --edit ignores 
pager.config' '
test -e editor.used
 '
 
-test_expect_success TTY 'git config --get defaults to not paging' '
+test_expect_success TTY 'git config --get defaults to paging' '
rm -f paginated.out &&
test_terminal git config --get foo.bar &&
-   ! test -e paginated.out
+   test -e paginated.out
 '
 
 test_expect_success TTY 'git config --get respects pager.config' '
rm -f paginated.out &&
-   test_terminal git -c pager.config config --get foo.bar &&
-   test -e paginated.out
+   test_terminal git -c pager.config=false config --get foo.bar &&
+   ! test -e paginated.out
 '
 
-test_expect_success TTY 'git config --list defaults to not paging' '
+test_expect_success TTY 'git config --list defaults to paging' '
rm -f paginated.out &&
test_terminal git config --list &&
-   ! test -e paginated.out
+   test -e paginated.out
 '
 
 
diff --git a/builtin/config.c b/builtin/config.c
index 9a57a0caff..61808a93cb 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -600,7 +600,7 @@ int cmd_config(int argc, const char **argv, const char 
*prefix)
}
 
if (actions & ACTION_LIST_OR_GET)
-   setup_auto_pager("config", 0);
+   setup_auto_pager("config", 1);
 
if (actions == ACTION_LIST) {
check_argc(argc, 0, 0);
-- 
2.16.1.72.g5be1f00a9a

[PATCH 1/3] t7006: add tests for how git config paginates

[Martin Ågren]

The next couple of commits will change how `git config` handles
`pager.config`, similar to how de121ffe5 (tag: respect `pager.tag` in
list-mode only, 2017-08-02) and ff1e72483 (tag: change default of
`pager.tag` to "on", 2017-08-02) changed `git tag`. Similar work has
also been done to `git branch`.

Add tests in this area to make sure that we don't regress and so that
the upcoming commits can be made clearer by adapting the tests. Add some
tests for `--list` and `--get`, one for `--edit`, and one for simple
config-setting.

In particular, use `test_expect_failure` to document that we currently
respect the pager-configuration with `--edit`. The current behavior is
buggy since the pager interferes with the editor and makes the end
result completely broken. See also b3ee740c8 (t7006: add tests for how
git tag paginates, 2017-08-02).

Remove the test added in commit 3ba7e6e29a (config: run
setup_git_directory_gently() sooner, 2010-08-05) since it has some
overlap with these. We could leave it or tweak it, or place new tests
like these next to it, but let's instead make the tests for `git config`
similar to the ones for `git tag` and `git branch`, and place them after
those.

Signed-off-by: Martin Ågren 
---
 t/t7006-pager.sh | 42 +++---
 1 file changed, 35 insertions(+), 7 deletions(-)

diff --git a/t/t7006-pager.sh b/t/t7006-pager.sh
index f5f46a95b4..5a7b757c94 100755
--- a/t/t7006-pager.sh
+++ b/t/t7006-pager.sh
@@ -110,13 +110,6 @@ test_expect_success TTY 'configuration can disable pager' '
! test -e paginated.out
 '
 
-test_expect_success TTY 'git config uses a pager if configured to' '
-   rm -f paginated.out &&
-   test_config pager.config true &&
-   test_terminal git config --list &&
-   test -e paginated.out
-'
-
 test_expect_success TTY 'configuration can enable pager (from subdir)' '
rm -f paginated.out &&
mkdir -p subdir &&
@@ -252,6 +245,41 @@ test_expect_success TTY 'git branch --set-upstream-to 
ignores pager.branch' '
! test -e paginated.out
 '
 
+test_expect_success TTY 'git config respects pager.config when setting' '
+   rm -f paginated.out &&
+   test_terminal git -c pager.config config foo.bar bar &&
+   test -e paginated.out
+'
+
+test_expect_failure TTY 'git config --edit ignores pager.config' '
+   rm -f paginated.out editor.used &&
+   write_script editor <<-\EOF &&
+   touch editor.used
+   EOF
+   EDITOR=./editor test_terminal git -c pager.config config --edit &&
+   ! test -e paginated.out &&
+   test -e editor.used
+'
+
+test_expect_success TTY 'git config --get defaults to not paging' '
+   rm -f paginated.out &&
+   test_terminal git config --get foo.bar &&
+   ! test -e paginated.out
+'
+
+test_expect_success TTY 'git config --get respects pager.config' '
+   rm -f paginated.out &&
+   test_terminal git -c pager.config config --get foo.bar &&
+   test -e paginated.out
+'
+
+test_expect_success TTY 'git config --list defaults to not paging' '
+   rm -f paginated.out &&
+   test_terminal git config --list &&
+   ! test -e paginated.out
+'
+
+
 # A colored commit log will begin with an appropriate ANSI escape
 # for the first color; the text "commit" comes later.
 colorful() {
-- 
2.16.1.72.g5be1f00a9a

Re: [PATCH v3 42/42] git-completion.bash: add GIT_COMPLETION_OPTIONS=all config

[Ævar Arnfjörð Bjarmason]

On Sat, Feb 10 2018, Duy Nguyen jotted:

> On Fri, Feb 09, 2018 at 03:19:57PM +0100, Ævar Arnfjörð Bjarmason wrote:
>>
>> On Fri, Feb 09 2018, Nguyễn Thái Ngọc Duy jotted:
>>
>> > By default, some option names (mostly --force, scripting related or for
>> > internal use) are not completable for various reasons. When
>> > GIT_COMPLETION_OPTIONS is set to all, all options (except hidden ones)
>> > are completable.
>> >
>> > Signed-off-by: Nguyễn Thái Ngọc Duy 
>> > ---
>> >  contrib/completion/git-completion.bash |  6 +-
>> >  parse-options.c| 11 +++
>> >  2 files changed, 12 insertions(+), 5 deletions(-)
>> >
>> > diff --git a/contrib/completion/git-completion.bash 
>> > b/contrib/completion/git-completion.bash
>> > index 0ddf40063b..0cfa489a8e 100644
>> > --- a/contrib/completion/git-completion.bash
>> > +++ b/contrib/completion/git-completion.bash
>> > @@ -36,6 +36,10 @@
>> >  #
>> >  # When set to "1", do not include "DWIM" suggestions in git-checkout
>> >  # completion (e.g., completing "foo" when "origin/foo" exists).
>> > +#
>> > +#   GIT_COMPLETION_OPTIONS
>> > +#
>> > +# When set to "all", complete all possible options
>>
>> I was going to suggest some wording like:
>>
>> When set to "all", include options considered unsafe such as --force
>> in the completion.
>>
>> However per your cover letter it's not just used for that:
>>
>>  10 --force
>>   4 --rerere-autoupdate
>>   1 --unsafe-paths
>>   1 --thin
>>   1 --overwrite-ignore
>>   1 --open-files-in-pager
>>   1 --null
>>   1 --ext-grep
>>   1 --exit-code
>>   1 --auto
>>
>> I wonder if we shouldn't just make this only about --force, I don't see
>> why "git grep --o" should only show --or but not
>> --open-files-in-pager, and e.g. "git grep --" is already verbose so
>> we're not saving much by excluding those.
>>
>> Then this could just become:
>>
>> GIT_COMPLETION_SHOWUNSAFEOPTIONS=1
>>
>> Or other similar boolean variable, for consistency with all the "*SHOW*
>> variables already in git-completion.bash.
>
> No. You're asking for a second default. I'm not adding plenty of
> GIT_COMPLETION_* variables for that. You either have all options, or
> you convince people that --force should be part of the current
> default.

No sorry, I mean that IMO the current patch you have could be simplified
where instead of saying "=all" there's just another variable that only
hides "dangerous" options, i.e. only "--force" (unless I've missed
another supposedly dangerous one).

But as previously discussed I think it just makes sense to stop doing
this conditionally and include --force too, the only stuff we should
hide is stuff like rebase --continue when not in an interactive rebase.

> Or you could push for a generic mechanism that allows you to customize
> your own default. Something like the below patch could give you what
> you want with:
>
> GIT_COMPLETION_OPTIONS=all
> GIT_COMPLETION_EXCLUDES=--open-files-in-pager # and some more
> . /path/to/git-completion.bash
>
> I'm not going to make a real patch for this since people may want to
> ignore --foo in one command and complete --foo in others... I'm just
> not interested in trying to cover all cases.
>
> -- 8< --
> diff --git a/contrib/completion/git-completion.bash 
> b/contrib/completion/git-completion.bash
> index 0cfa489a8e..9ca0d80cd7 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -40,6 +40,10 @@
>  #   GIT_COMPLETION_OPTIONS
>  #
>  # When set to "all", complete all possible options
> +#
> +#   GIT_COMPLETION_EXCLUDES
> +#
> +# Exclude some options from the complete list
>
>  case "$COMP_WORDBREAKS" in
>  *:*) : great ;;
> @@ -298,7 +302,7 @@ __gitcomp_builtin ()
>   # commands, e.g. "git remote add" becomes remote_add.
>   local cmd="$1"
>   local incl="$2"
> - local excl="$3"
> + local excl="$3 $GIT_COMPLETION_EXCLUDES"
>
>   local var=__gitcomp_builtin_"${cmd/-/_}"
>   local options
> -- 8< --

Re: What's cooking in git.git (Jan 2018, #04; Wed, 31)

[Ævar Arnfjörð Bjarmason]

On Sat, Feb 10 2018, Duy Nguyen jotted:

> On Sat, Feb 10, 2018 at 1:37 AM, Ævar Arnfjörð Bjarmason
>  wrote:
>>
>> On Thu, Feb 01 2018, Junio C. Hamano jotted:
>>
>>> * nd/fix-untracked-cache-invalidation (2018-01-24) 5 commits
>>>  - dir.c: stop ignoring opendir() error in open_cached_dir()
>>>  - update-index doc: note a fixed bug in the untracked cache
>>>  - dir.c: fix missing dir invalidation in untracked code
>>>  - dir.c: avoid stat() in valid_cached_dir()
>>>  - status: add a failing test showing a core.untrackedCache bug
>>>
>>>  Some bugs around "untracked cache" feature have been fixed.
>>>
>>>  Will merge to 'next'.
>>
>> I think you / Nguyễn may not have seen my
>> <87d11omi2o@evledraar.gmail.com>
>> (https://public-inbox.org/git/87d11omi2o@evledraar.gmail.com/)
>
> I have. But since you wrote "I haven't found... yet", I assumed you
> were still on it. You didn't give me much info to follow anyway.

Haven't had time to dig further, sorry, and won't be able to share the
repository. Is there some UC inspection command that can be run on the
relevant path / other thing that'll be indicative of what went wrong?

>> As noted there I think it's best to proceed without the "dir.c: stop
>> ignoring opendir[...]" patch.
>>
>> It's going to be a bad regression in 2.17 if it ends up spewing pagefuls
>> of warnings in previously working repos if the UC is on.
>
> "previously working" is a strong word when opendir() starts to
> complain. Sure we can suppress/ignore the error messages but I don't
> think it's a healthy attitude. Unreported bugs can't be fixed.

I mean that for the user it returned the right "git status" info and
didn't print errors, but yeah, the index may have been in some
internally funny state.

> We could perhaps stop reporting after we have printed like ... 5 lines
> or so? That keeps the noise level down a bit and probably still give
> enough indicator to at least repair the cache.

Re: [PATCH 5/8] rebase: introduce the --recreate-merges option

[Jacob Keller]

On Thu, Feb 8, 2018 at 11:13 PM, Johannes Sixt  wrote:
> Am 09.02.2018 um 07:11 schrieb Sergey Organov:
>>
>> Johannes Schindelin  writes:
>>>
>>> Let me explain the scenario which comes up plenty of times in my work
>>> with
>>> Git for Windows. We have a thicket of some 70 branches on top of
>>> git.git's
>>> latest release. These branches often include fixup! and squash! commits
>>> and even more complicated constructs that rebase cannot handle at all at
>>> the moment, such as reorder-before! and reorder-after! (for commits that
>>> really need to go into a different branch).
>>
>>
>> I sympathize, but a solution that breaks even in simple cases can't be
>> used reliably to solve more complex problems, sorry. Being so deep
>> into your problems, I think you maybe just aren't seeing forest for the
>> trees [1].
>
>
> Hold your horses! Dscho has a point here. --preserve-merges --first-parent
> works only as long as you don't tamper with the side branches. If you make
> changes in the side branches during the same rebase operation, this
> --first-parent mode would undo that change. (And, yes, its result would be
> called an "evil merge", and that scary name _should_ frighten you!)
>
> -- Hannes

This is the reason I agree with Johannes, in regards to why
recreate-merges approach is correct.

Yes, an ideal system would be one which correctly, automatically
re-creates the merge *as if* a human had re-merged the two newly
re-created side branches, and preserves any changes in the result of
the merge, such as cases we call "evil merges" which includes
necessary changes to resolve conflicts properly.

However, I would state that such a system, in order to cause the least
surprise to a user must be correct against arbitrary removal, reorder,
and addition of new commits on both the main and topic side branches
for which we are re-creating the merges.

This is problematic, because something like how --preserve-merges
--first-parent does not work under this case.

As a user of the tool, I may be biased because I already read and
understand how recreate-merges is expected to work, but it makes sense
to me that the re-creation of the merge merely re-does the merge and
any modfications in the original would have to be carried over.

I don't know what process we could use to essentially move the changes
from the original merge into the new copy. What ever solution we have
would need to have a coherent user interface and be presentable in
some manner.

One way to think about the contents we're wanting to keep, rather than
the full tree result of the merge which we had before, what we
actually want to keep in some sense is the resulting "diff" as shown
by something like the condensed --combined output. This is obviously
not really a diff that we can apply.

And even if we could apply it, since the merge is occurring, we can't
exactly use 3-way merge conflict in order to actually apply the old
changes into the new merged setup? Could something like rerere logic
work here to track what was done and then re-apply it to the new merge
we create? And once we apply it, we need to be able to handle any
conflicts that occur because of deleting, adding, or re-ordering
commits on the branches we're merging... so in some sense we could
have "conflicts of conflicts" which is a scenario that I don't yet
have a good handle on how this would be presented to the user.

Thanks,
Jake

[PATCH v3 0/3] Add "git rebase --show-current-patch"

[Nguyễn Thái Ngọc Duy]

Compared to v2:

- the potential loss of errno before it's printed out in builtin/am.c
  is fixed.
- keep update_ref() in sequencer.c non-fatal like this rest
- rename ORIG_COMMIT to REBASE_HEAD

Interdiff:

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 6da9296bf8..0b29e48221 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -253,7 +253,7 @@ leave out at most one of A and B, in which case it defaults 
to HEAD.
 --show-current-patch::
Show the current patch in an interactive rebase or when rebase
is stopped because of conflicts. This is the equivalent of
-   `git show ORIG_COMMIT`.
+   `git show REBASE_HEAD`.
 
 -m::
 --merge::
diff --git a/builtin/am.c b/builtin/am.c
index bf9b356340..21aedec41f 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -1011,7 +1011,7 @@ static void am_setup(struct am_state *state, enum 
patch_format patch_format,
 
if (mkdir(state->dir, 0777) < 0 && errno != EEXIST)
die_errno(_("failed to create directory '%s'"), state->dir);
-   delete_ref(NULL, "ORIG_COMMIT", NULL, REF_NO_DEREF);
+   delete_ref(NULL, "REBASE_HEAD", NULL, REF_NO_DEREF);
 
if (split_mail(state, patch_format, paths, keep_cr) < 0) {
am_destroy(state);
@@ -,7 +,7 @@ static void am_next(struct am_state *state)
 
oidclr(>orig_commit);
unlink(am_path(state, "original-commit"));
-   delete_ref(NULL, "ORIG_COMMIT", NULL, REF_NO_DEREF);
+   delete_ref(NULL, "REBASE_HEAD", NULL, REF_NO_DEREF);
 
if (!get_oid("HEAD", ))
write_state_text(state, "abort-safety", oid_to_hex());
@@ -1443,8 +1443,8 @@ static int parse_mail_rebase(struct am_state *state, 
const char *mail)
 
oidcpy(>orig_commit, _oid);
write_state_text(state, "original-commit", oid_to_hex(_oid));
-   update_ref("am", "ORIG_COMMIT", _oid,
-  NULL, 0, UPDATE_REFS_DIE_ON_ERR);
+   update_ref("am", "REBASE_HEAD", _oid,
+  NULL, REF_NO_DEREF, UPDATE_REFS_DIE_ON_ERR);
 
return 0;
 }
@@ -2127,6 +2127,7 @@ static void am_abort(struct am_state *state)
 static int show_patch(struct am_state *state)
 {
struct strbuf sb = STRBUF_INIT;
+   const char *patch_path;
int len;
 
if (!is_null_oid(>orig_commit)) {
@@ -2140,10 +2141,10 @@ static int show_patch(struct am_state *state)
return ret;
}
 
-   len = strbuf_read_file(, am_path(state, msgnum(state)), 0);
+   patch_path = am_path(state, msgnum(state));
+   len = strbuf_read_file(, patch_path, 0);
if (len < 0)
-   die_errno(_("failed to read '%s'"),
- am_path(state, msgnum(state)));
+   die_errno(_("failed to read '%s'"), patch_path);
 
setup_pager();
write_in_full(1, sb.buf, sb.len);
diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index deea688e0e..8777805c9f 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -439,7 +439,7 @@ __git_refs ()
track=""
;;
*)
-   for i in HEAD FETCH_HEAD ORIG_HEAD MERGE_HEAD 
ORIG_COMMIT; do
+   for i in HEAD FETCH_HEAD ORIG_HEAD MERGE_HEAD 
REBASE_HEAD; do
case "$i" in
$match*)
if [ -e "$dir/$i" ]; then
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index ef72bd5871..a613156bcb 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -199,14 +199,14 @@ make_patch () {
 
 die_with_patch () {
echo "$1" > "$state_dir"/stopped-sha
-   git update-ref ORIG_COMMIT "$1"
+   git update-ref REBASE_HEAD "$1"
make_patch "$1"
die "$2"
 }
 
 exit_with_patch () {
echo "$1" > "$state_dir"/stopped-sha
-   git update-ref ORIG_COMMIT "$1"
+   git update-ref REBASE_HEAD "$1"
make_patch $1
git rev-parse --verify HEAD > "$amend"
gpg_sign_opt_quoted=${gpg_sign_opt:+$(git rev-parse --sq-quote 
"$gpg_sign_opt")}
@@ -843,7 +843,7 @@ To continue rebase after editing, run:
exit
;;
 show-current-patch)
-   exec git show ORIG_COMMIT --
+   exec git show REBASE_HEAD --
;;
 esac
 
@@ -860,7 +860,7 @@ fi
 
 orig_head=$(git rev-parse --verify HEAD) || die "$(gettext "No HEAD?")"
 mkdir -p "$state_dir" || die "$(eval_gettext "Could not create temporary 
\$state_dir")"
-rm -f "$(git rev-parse --git-path ORIG_COMMIT)"
+rm -f "$(git rev-parse --git-path REBASE_HEAD)"
 
 : > "$state_dir"/interactive || die "$(gettext "Could not mark as 
interactive")"
 write_basic_state
diff --git a/git-rebase--merge.sh b/git-rebase--merge.sh
index 70966c32c3..957688f236 100644
--- a/git-rebase--merge.sh
+++ 

[PATCH v3 1/3] am: add --show-current-patch

[Nguyễn Thái Ngọc Duy]

Pointing the user to $GIT_DIR/rebase-apply may encourage them to mess
around in there, which is not a good thing. With this, the user does
not have to keep the path around somewhere (because after a couple of
commands, the path may be out of scrollback buffer) when they need to
look at the patch.

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 Documentation/git-am.txt   |  6 -
 builtin/am.c   | 32 ++
 contrib/completion/git-completion.bash |  2 +-
 t/t4150-am.sh  |  5 
 4 files changed, 39 insertions(+), 6 deletions(-)

diff --git a/Documentation/git-am.txt b/Documentation/git-am.txt
index 12879e4029..0f426ae874 100644
--- a/Documentation/git-am.txt
+++ b/Documentation/git-am.txt
@@ -16,7 +16,7 @@ SYNOPSIS
 [--exclude=] [--include=] [--reject] [-q | --quiet]
 [--[no-]scissors] [-S[]] [--patch-format=]
 [( | )...]
-'git am' (--continue | --skip | --abort)
+'git am' (--continue | --skip | --abort | --show-current-patch)
 
 DESCRIPTION
 ---
@@ -167,6 +167,10 @@ default.   You can use `--no-utf8` to override this.
 --abort::
Restore the original branch and abort the patching operation.
 
+--show-current-patch::
+   Show the patch being applied when "git am" is stopped because
+   of conflicts.
+
 DISCUSSION
 --
 
diff --git a/builtin/am.c b/builtin/am.c
index acfe9d3c8c..07abfb8f83 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -1831,8 +1831,7 @@ static void am_run(struct am_state *state, int resume)
git_config_get_bool("advice.amworkdir", 
_amworkdir);
 
if (advice_amworkdir)
-   printf_ln(_("The copy of the patch that failed 
is found in: %s"),
-   am_path(state, "patch"));
+   printf_ln(_("Use 'git am --show-current-patch' 
to see the failed patch"));
 
die_user_resolve(state);
}
@@ -2121,6 +2120,23 @@ static void am_abort(struct am_state *state)
am_destroy(state);
 }
 
+static int show_patch(struct am_state *state)
+{
+   struct strbuf sb = STRBUF_INIT;
+   const char *patch_path;
+   int len;
+
+   patch_path = am_path(state, msgnum(state));
+   len = strbuf_read_file(, patch_path, 0);
+   if (len < 0)
+   die_errno(_("failed to read '%s'"), patch_path);
+
+   setup_pager();
+   write_in_full(1, sb.buf, sb.len);
+   strbuf_release();
+   return 0;
+}
+
 /**
  * parse_options() callback that validates and sets opt->value to the
  * PATCH_FORMAT_* enum value corresponding to `arg`.
@@ -2149,7 +2165,8 @@ enum resume_mode {
RESUME_APPLY,
RESUME_RESOLVED,
RESUME_SKIP,
-   RESUME_ABORT
+   RESUME_ABORT,
+   RESUME_SHOW_PATCH
 };
 
 static int git_am_config(const char *k, const char *v, void *cb)
@@ -2171,6 +2188,7 @@ int cmd_am(int argc, const char **argv, const char 
*prefix)
int patch_format = PATCH_FORMAT_UNKNOWN;
enum resume_mode resume = RESUME_FALSE;
int in_progress;
+   int ret = 0;
 
const char * const usage[] = {
N_("git am [] [( | )...]"),
@@ -2249,6 +2267,9 @@ int cmd_am(int argc, const char **argv, const char 
*prefix)
OPT_CMDMODE(0, "abort", ,
N_("restore the original branch and abort the patching 
operation."),
RESUME_ABORT),
+   OPT_CMDMODE(0, "show-current-patch", ,
+   N_("show the patch being applied."),
+   RESUME_SHOW_PATCH),
OPT_BOOL(0, "committer-date-is-author-date",
_date_is_author_date,
N_("lie about committer date")),
@@ -2359,11 +2380,14 @@ int cmd_am(int argc, const char **argv, const char 
*prefix)
case RESUME_ABORT:
am_abort();
break;
+   case RESUME_SHOW_PATCH:
+   ret = show_patch();
+   break;
default:
die("BUG: invalid resume value");
}
 
am_state_release();
 
-   return 0;
+   return ret;
 }
diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index 3683c772c5..56ca445fa8 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1077,7 +1077,7 @@ _git_am ()
 {
__git_find_repo_path
if [ -d "$__git_repo_path"/rebase-apply ]; then
-   __gitcomp "--skip --continue --resolved --abort"
+   __gitcomp "--skip --continue --resolved --abort 
--show-current-patch"
return
fi
case "$cur" in
diff --git a/t/t4150-am.sh b/t/t4150-am.sh
index 73b67b4280..23abf42abc 100755
--- a/t/t4150-am.sh
+++ b/t/t4150-am.sh
@@ -662,6 +662,11 @@ test_expect_success 'am 

[PATCH v3 3/3] rebase: introduce and use pseudo-ref REBASE_HEAD

[Nguyễn Thái Ngọc Duy]

The new command `git rebase --show-current-patch` is useful for seeing
the commit related to the current rebase state. Some however may find
the "git show" command behind it too limiting. You may want to
increase context lines, do a diff that ignores whitespaces...

For these advanced use cases, the user can execute any command they
want with the new pseudo ref REBASE_HEAD.

This also helps show where the stopped commit is from, which is hard
to see from the previous patch which implements --show-current-patch.

Helped-by: Tim Landscheidt 
Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 Documentation/git-rebase.txt   | 3 ++-
 builtin/am.c   | 4 
 contrib/completion/git-completion.bash | 2 +-
 git-rebase--interactive.sh | 5 -
 git-rebase--merge.sh   | 4 +++-
 git-rebase.sh  | 1 +
 sequencer.c| 4 
 t/t3400-rebase.sh  | 3 ++-
 t/t3404-rebase-interactive.sh  | 5 -
 9 files changed, 25 insertions(+), 6 deletions(-)

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 7ef9577472..0b29e48221 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -252,7 +252,8 @@ leave out at most one of A and B, in which case it defaults 
to HEAD.
 
 --show-current-patch::
Show the current patch in an interactive rebase or when rebase
-   is stopped because of conflicts.
+   is stopped because of conflicts. This is the equivalent of
+   `git show REBASE_HEAD`.
 
 -m::
 --merge::
diff --git a/builtin/am.c b/builtin/am.c
index 37219fceb0..21aedec41f 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -1011,6 +1011,7 @@ static void am_setup(struct am_state *state, enum 
patch_format patch_format,
 
if (mkdir(state->dir, 0777) < 0 && errno != EEXIST)
die_errno(_("failed to create directory '%s'"), state->dir);
+   delete_ref(NULL, "REBASE_HEAD", NULL, REF_NO_DEREF);
 
if (split_mail(state, patch_format, paths, keep_cr) < 0) {
am_destroy(state);
@@ -1110,6 +,7 @@ static void am_next(struct am_state *state)
 
oidclr(>orig_commit);
unlink(am_path(state, "original-commit"));
+   delete_ref(NULL, "REBASE_HEAD", NULL, REF_NO_DEREF);
 
if (!get_oid("HEAD", ))
write_state_text(state, "abort-safety", oid_to_hex());
@@ -1441,6 +1443,8 @@ static int parse_mail_rebase(struct am_state *state, 
const char *mail)
 
oidcpy(>orig_commit, _oid);
write_state_text(state, "original-commit", oid_to_hex(_oid));
+   update_ref("am", "REBASE_HEAD", _oid,
+  NULL, REF_NO_DEREF, UPDATE_REFS_DIE_ON_ERR);
 
return 0;
 }
diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index 2bd30d68cf..8777805c9f 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -439,7 +439,7 @@ __git_refs ()
track=""
;;
*)
-   for i in HEAD FETCH_HEAD ORIG_HEAD MERGE_HEAD; do
+   for i in HEAD FETCH_HEAD ORIG_HEAD MERGE_HEAD 
REBASE_HEAD; do
case "$i" in
$match*)
if [ -e "$dir/$i" ]; then
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 0c0f8abbf9..a613156bcb 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -199,12 +199,14 @@ make_patch () {
 
 die_with_patch () {
echo "$1" > "$state_dir"/stopped-sha
+   git update-ref REBASE_HEAD "$1"
make_patch "$1"
die "$2"
 }
 
 exit_with_patch () {
echo "$1" > "$state_dir"/stopped-sha
+   git update-ref REBASE_HEAD "$1"
make_patch $1
git rev-parse --verify HEAD > "$amend"
gpg_sign_opt_quoted=${gpg_sign_opt:+$(git rev-parse --sq-quote 
"$gpg_sign_opt")}
@@ -841,7 +843,7 @@ To continue rebase after editing, run:
exit
;;
 show-current-patch)
-   exec git show "$(cat "$state_dir/stopped-sha")" --
+   exec git show REBASE_HEAD --
;;
 esac
 
@@ -858,6 +860,7 @@ fi
 
 orig_head=$(git rev-parse --verify HEAD) || die "$(gettext "No HEAD?")"
 mkdir -p "$state_dir" || die "$(eval_gettext "Could not create temporary 
\$state_dir")"
+rm -f "$(git rev-parse --git-path REBASE_HEAD)"
 
 : > "$state_dir"/interactive || die "$(gettext "Could not mark as 
interactive")"
 write_basic_state
diff --git a/git-rebase--merge.sh b/git-rebase--merge.sh
index 0a96dfae37..957688f236 100644
--- a/git-rebase--merge.sh
+++ b/git-rebase--merge.sh
@@ -57,6 +57,7 @@ call_merge () {
echo "$msgnum" >"$state_dir/msgnum"
cmt="$(cat "$state_dir/cmt.$msgnum")"
echo "$cmt" > "$state_dir/current"
+   git update-ref REBASE_HEAD 

[PATCH v3 2/3] rebase: add --show-current-patch

[Nguyễn Thái Ngọc Duy]

It is useful to see the full patch while resolving conflicts in a
rebase. The only way to do it now is

less .git/rebase-*/patch

which could turn out to be a lot longer to type if you are in a
linked worktree, or not at top-dir. On top of that, an ordinary user
should not need to peek into .git directory. The new option is
provided to examine the patch.

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 Documentation/git-rebase.txt   |  6 -
 builtin/am.c   | 11 +
 contrib/completion/git-completion.bash |  4 ++--
 git-rebase--am.sh  |  3 +++
 git-rebase--interactive.sh |  3 +++
 git-rebase--merge.sh   |  3 +++
 git-rebase.sh  |  7 +-
 t/t3400-rebase.sh  | 33 ++
 t/t3404-rebase-interactive.sh  |  5 
 9 files changed, 71 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 8a861c1e0d..7ef9577472 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -12,7 +12,7 @@ SYNOPSIS
[ []]
 'git rebase' [-i | --interactive] [options] [--exec ] [--onto ]
--root []
-'git rebase' --continue | --skip | --abort | --quit | --edit-todo
+'git rebase' --continue | --skip | --abort | --quit | --edit-todo | 
--show-current-patch
 
 DESCRIPTION
 ---
@@ -250,6 +250,10 @@ leave out at most one of A and B, in which case it 
defaults to HEAD.
 --edit-todo::
Edit the todo list during an interactive rebase.
 
+--show-current-patch::
+   Show the current patch in an interactive rebase or when rebase
+   is stopped because of conflicts.
+
 -m::
 --merge::
Use merging strategies to rebase.  When the recursive (default) merge
diff --git a/builtin/am.c b/builtin/am.c
index 07abfb8f83..37219fceb0 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -2126,6 +2126,17 @@ static int show_patch(struct am_state *state)
const char *patch_path;
int len;
 
+   if (!is_null_oid(>orig_commit)) {
+   const char *av[4] = { "show", NULL, "--", NULL };
+   char *new_oid_str;
+   int ret;
+
+   av[1] = new_oid_str = xstrdup(oid_to_hex(>orig_commit));
+   ret = run_command_v_opt(av, RUN_GIT_CMD);
+   free(new_oid_str);
+   return ret;
+   }
+
patch_path = am_path(state, msgnum(state));
len = strbuf_read_file(, patch_path, 0);
if (len < 0)
diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index 56ca445fa8..2bd30d68cf 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1992,11 +1992,11 @@ _git_rebase ()
 {
__git_find_repo_path
if [ -f "$__git_repo_path"/rebase-merge/interactive ]; then
-   __gitcomp "--continue --skip --abort --quit --edit-todo"
+   __gitcomp "--continue --skip --abort --quit --edit-todo 
--show-current-patch"
return
elif [ -d "$__git_repo_path"/rebase-apply ] || \
 [ -d "$__git_repo_path"/rebase-merge ]; then
-   __gitcomp "--continue --skip --abort --quit"
+   __gitcomp "--continue --skip --abort --quit 
--show-current-patch"
return
fi
__git_complete_strategy && return
diff --git a/git-rebase--am.sh b/git-rebase--am.sh
index 14c50782e0..c931891cbc 100644
--- a/git-rebase--am.sh
+++ b/git-rebase--am.sh
@@ -27,6 +27,9 @@ skip)
move_to_original_branch
return
;;
+show-current-patch)
+   exec git am --show-current-patch
+   ;;
 esac
 
 if test -z "$rebase_root"
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index d47bd29593..0c0f8abbf9 100644
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -840,6 +840,9 @@ To continue rebase after editing, run:
 
exit
;;
+show-current-patch)
+   exec git show "$(cat "$state_dir/stopped-sha")" --
+   ;;
 esac
 
 comment_for_reflog start
diff --git a/git-rebase--merge.sh b/git-rebase--merge.sh
index 06a4723d4d..0a96dfae37 100644
--- a/git-rebase--merge.sh
+++ b/git-rebase--merge.sh
@@ -137,6 +137,9 @@ skip)
finish_rb_merge
return
;;
+show-current-patch)
+   exec git show "$(cat "$state_dir/current")" --
+   ;;
 esac
 
 mkdir -p "$state_dir"
diff --git a/git-rebase.sh b/git-rebase.sh
index fd72a35c65..41c915d18c 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -45,6 +45,7 @@ abort! abort and check out the original branch
 skip!  skip current patch and continue
 edit-todo! edit the todo list during an interactive rebase
 quit!  abort but keep HEAD where it is
+show-current-patch! show the patch file being applied or merged
 "
 . git-sh-setup
 set_reflog_action rebase
@@ -245,7 +246,7