Re: [RFC PATCH] Introduce "precious" file concept

[Jacob Keller]

On Tue, Nov 27, 2018 at 1:45 AM Per Lundberg  wrote:
>
> On 11/26/18 5:55 PM, Duy Nguyen wrote:
> > On Mon, Nov 26, 2018 at 4:47 PM Ævar Arnfjörð Bjarmason
> >  wrote:
> >> Some of the solutions overlap with this thing you want, but I think it's
> >> worth keeping the distinction between the two in mind.
> >
> > On the other hand all use cases should be considered. It's going to be
> > a mess to have "trashable" attribute that applies to some commands
> > while "precious" to some others (and even worse when they overlap,
> > imagine having to define both in .gitattributes)
>
> Agree - I think it would be a very bad idea to have a "mix" of both
> trashable and precious. IMO, we should try to find which one of these
> concepts suits most general use cases best and causes less churn for
> existing scripts/users' existing "semantic expectations", and pick that one.
> --
> Per Lundberg

Personally, I would rather err on the side which requires the least
interaction from users to avoid silently clobbering an ignored file.

Either Duy's solution with a sort of "untracked" reflog, or the
garbage/trashable notion.

I don't like the idea of precious because it means people have to know
and remember to opt in, and it's quite possible they will not do so
until after they've lost real data.

I'd only have trashable apply in the case where it was implicit. i.e.
git clean -fdx would still delete them, as this is an explicit
operation that (hopefully?) users know will delete data.

Thanks,
Jake

Re: git log -S or -G

[Jacob Keller]

On Mon, Oct 8, 2018 at 8:22 PM Jeff King  wrote:
>
> On Tue, Oct 09, 2018 at 08:09:32AM +0900, Junio C Hamano wrote:
>
> > Julia Lawall  writes:
> >
> > >> Doing the same for -S is much harder at the machinery level, as it
> > >> performs its thing without internally running "diff" twice, but just
> > >> counts the number of occurrences of 'foo'---that is sufficient for
> > >> its intended use, and more efficient.
> > >
> > > There is still the question of whether the number of occurrences of foo
> > > decreases or increases.
> >
> > Hmph, taking the changes that makes the number of hits decrease
> > would catch a subset of "changes that removes 'foo' only---I am not
> > interested in the ones that adds 'foo'".  It will avoid getting
> > confused by a change that moves an existing 'foo' to another place
> > in the same file (as the number of hits does not change), but at the
> > same time, it will miss a change that genuinely removes an existing
> > 'foo' and happens to add a 'foo' at a different place in the same
> > file that is unrelated to the original 'foo'.  Depending on the
> > definition of "I am only interested in removed ones", that may or
> > may not be acceptable.
>
> I think that is the best we could do for "-S", though, which is
> inherently about counting hits.
>
> For "-G", we are literally grepping the diff. It does not seem
> unreasonable to add the ability to grep only "-" or "+" lines, and the
> interface for that should be pretty straightforward (a tri-state flag to
> look in remove, added, or both lines).
>
> -Peff

Yea. I know I've wanted something like this in the past.

Thanks,
Jake

Re: [PATCH v3] coccicheck: process every source file at once

[Jacob Keller]

On Fri, Oct 5, 2018 at 12:00 PM Jeff King  wrote:
> That's OK, too, assuming people would actually want to use it. I'm also
> OK shipping this (with the "make -j" fix you suggested) and seeing if
> anybody actually complains. I assume there are only a handful of people
> running coccicheck in the first place.
>
> -Peff

Ok. I can go this route if we have consensus on the "break it and see
if someone complains" route.

Regards,
Jake

Re: [PATCH 1/1] rebase -i: introduce the 'break' command

[Jacob Keller]

On Fri, Oct 5, 2018 at 1:17 AM Junio C Hamano  wrote:
> If one wants to emulate this with the versions of Git that are
> currently deployed, would it be sufficient to insert "exec false"
> instead of "break"?
>
> The reason I am asking is *not* to imply that we do not need this
> new feature.  It is because I vaguely recall seeing a request to add
> 'pause' to the insn set and "exec false" was mentioned as a more
> general alternative long time ago.  I am trying to see if this is a
> recurring request/wish, because it would reinforce that this new
> feature would be a good addition if that is the case.
>
> I suspect that "exec false" would give a message that looks like a
> complaint ("'false' failed so we are giving you control back to fix
> things" or something like that), and having a dedicated way to pause
> the execution without alarming the user is a good idea.
>
> I think the earlier request asked for 'pause' (I didn't dig the list
> archive very carefully, though), and 'stop' may also be a possible
> verb, but I tend to agree with this patch that 'break' is probably
> the best choice, simply because it begins with 'b' in the
> abbreviated form, a letter that is not yet used by others (unlike
> 'pause' or 'stop' that would want 'p' and 's' that are already
> taken)..
>

Yea. I use "exec false" all the time for this purpose, but it's a bit
confusing, and it does cause rebase to indicate that a command failed.

I think adding a builtin command to do this is a good idea, and I
think break is a reasonable verb, (especially considering the
shorthand "b").

Regards,
Jake

Re: [PATCH v3] coccicheck: process every source file at once

[Jacob Keller]

On Tue, Oct 2, 2018 at 1:18 PM Jacob Keller  wrote:
>
> On Tue, Oct 2, 2018 at 1:07 PM Jacob Keller  wrote:
> >
> > From: Jacob Keller 
> >
> > make coccicheck is used in order to apply coccinelle semantic patches,
> > and see if any of the transformations found within contrib/coccinelle/
> > can be applied to the current code base.
> >
> > Pass every file to a single invocation of spatch, instead of running
> > spatch once per source file.
> >
> > This reduces the time required to run make coccicheck by a significant
> > amount of time:
> >
> > Prior timing of make coccicheck
> >   real6m14.090s
> >   user25m2.606s
> >   sys 1m22.919s
> >
> > New timing of make coccicheck
> >   real1m36.580s
> >   user7m55.933s
> >   sys 0m18.219s
> >
> > This is nearly a 4x decrease in the time required to run make
> > coccicheck. This is due to the overhead of restarting spatch for every
> > file. By processing all files at once, we can amortize this startup cost
> > across the total number of files, rather than paying it once per file.
> >
> > Signed-off-by: Jacob Keller 
> > ---
>
> Forgot to add what changed. I dropped the subshell and "||" bit around
> invoking spatch.
>
> Thanks,
> Jake
>

Junio, do you want me to update the commit message on my side with the
memory concerns? Or could you update it to mention memory as a noted
trade off.

Thanks,
Jake

>
> >  Makefile | 6 ++
> >  1 file changed, 2 insertions(+), 4 deletions(-)
> >
> > diff --git a/Makefile b/Makefile
> > index df1df9db78da..da692ece9e12 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -2715,10 +2715,8 @@ endif
> >  %.cocci.patch: %.cocci $(COCCI_SOURCES)
> > @echo '' SPATCH $<; \
> > ret=0; \
> > -   for f in $(COCCI_SOURCES); do \
> > -   $(SPATCH) --sp-file $< $$f $(SPATCH_FLAGS) || \
> > -   { ret=$$?; break; }; \
> > -   done >$@+ 2>$@.log; \
> > +   $(SPATCH) --sp-file $< $(COCCI_SOURCES) $(SPATCH_FLAGS) >$@+ 
> > 2>$@.log; \
> > +   ret=$$?; \
> > if test $$ret != 0; \
> > then \
> > cat $@.log; \
> > --
> > 2.18.0.219.gaf81d287a9da
> >

Re: [PATCH] coccicheck: process every source file at once

[Jacob Keller]

On Wed, Oct 3, 2018 at 8:05 AM SZEDER Gábor  wrote:
> In fact, it works finer than ever, because running 1.0.0 with this
> patch on Travis CI notices a possible memmove() -> MOVE_ARRAY()
> conversion:
>
>   https://travis-ci.org/szeder/git/jobs/436542684#L576
>
> Surprisingly, running 1.0.0 without this patch, or running 1.0.4
> locally either with or without this patch doesn't notice that
> memmove() call.  Presumably that's why Jonathan could kind-of "revert"
> my conversion from f919ffebed (Use MOVE_ARRAY, 2018-01-22) in his
> 6a1a79fd14 (object: move grafts to object parser, 2018-05-15) without
> anyone noticing.
>

That seems very odd...

Thanks,
Jake

Re: [PATCH] coccicheck: process every source file at once

[Jacob Keller]

On Wed, Oct 3, 2018 at 3:17 AM SZEDER Gábor  wrote:
>
> On Tue, Oct 02, 2018 at 03:55:19PM -0400, Jeff King wrote:
> > On Tue, Oct 02, 2018 at 12:16:42PM -0700, Jacob Keller wrote:
> >
> > > make coccicheck is used in order to apply coccinelle semantic patches,
> > > and see if any of the transformations found within contrib/coccinelle/
> > > can be applied to the current code base.
> > >
> > > Pass every file to a single invocation of spatch, instead of running
> > > spatch once per source file.
> > >
> > > This reduces the time required to run make coccicheck by a significant
> > > amount of time:
> > >
> > > Prior timing of make coccicheck
> > >   real6m14.090s
> > >   user25m2.606s
> > >   sys 1m22.919s
> > >
> > > New timing of make coccicheck
> > >   real1m36.580s
> > >   user7m55.933s
> > >   sys 0m18.219s
> >
> > Yay! This is a nice result.
> >
> > It's also one of the things that Julia suggested in an earlier thread.
> > One thing I wasn't quite sure about after digging into the various
> > versions (1.0.4 on Debian stable/unstable, 1.0.6 in experimental, and
> > pre-1.0.7 at the bleeding edge) was whether the old versions would be
> > similarly helped (or work at all).
> >
> > I just replicated your results with 1.0.4.deb-3+b2 from Debian stable.
> > It's possible there are older versions floating around, but for
> > something developer-only like this, I think "in Debian stable" is a
> > reasonable enough cutoff.
>
> Linux build jobs on Travis CI run Ubuntu Trusty 14.04 LTS, and
> therefore our static analysis build job still runs 1.0.0~rc19.deb-3.
>
> This patch appears to work fine with that version, too, though note
> that I also changed the build job to don't run two parallel jobs; for
> the reason see below.
>
> > > This is nearly a 4x decrease in the time required to run make
> > > coccicheck. This is due to the overhead of restarting spatch for every
> > > file. By processing all files at once, we can amortize this startup cost
> > > across the total number of files, rather than paying it once per file.
> >
> > One minor side effect is that we lose the opportunity to parallelize
> > quite as much. However, I think the reduction in total CPU makes it well
> > worth that (and we still have 8 cocci files and growing, which should
> > keep at least 8 cores busy).
>
> One major side effect, however, is the drastically increased memory
> usage resulting from processing all files at once.  With your patch on
> top of current master:
>
>   $ for cocci in contrib/coccinelle/*.cocci ; do command time -f 'max RSS: 
> %MkB' make ${cocci}.patch ; done
>SPATCH contrib/coccinelle/array.cocci
>   max RSS: 1537068kB
>SPATCH contrib/coccinelle/commit.cocci
>   max RSS: 1510920kB
>SPATCH contrib/coccinelle/free.cocci
>   max RSS: 1393712kB
>SPATCH contrib/coccinelle/object_id.cocci
>SPATCH result: contrib/coccinelle/object_id.cocci.patch
>   max RSS: 1831700kB
>SPATCH contrib/coccinelle/qsort.cocci
>   max RSS: 1384960kB
>SPATCH contrib/coccinelle/strbuf.cocci
>   max RSS: 1395800kB
>SPATCH contrib/coccinelle/swap.cocci
>   max RSS: 1393620kB
>SPATCH contrib/coccinelle/xstrdup_or_null.cocci
>   max RSS: 1371716kB
>
> Without your patch the max RSS lingers around 87048kB - 101980kB,
> meaning ~15x - 18x increase
>

Quite a significant increase.


> This might cause quite the surprise to developers who are used to run
> 'make -jN coccicheck'; my tiny laptop came to a grinding halt with
> -j4.
>
> I think this side effect should be mentioned in the commit message.
>

Yes I agree. I hadn't considered the memory problem.

Thanks,
Jake

Re: [PATCH] more oideq/hasheq conversions

[Jacob Keller]

On Tue, Oct 2, 2018 at 2:19 PM Jeff King  wrote:
>
> We added faster equality-comparison functions for hashes in
> 14438c4497 (introduce hasheq() and oideq(), 2018-08-28). A
> few topics were in-flight at the time, and can now be
> converted. This covers all spots found by "make coccicheck"
> in master (the coccicheck results were tweaked by hand for
> style).
>
> Signed-off-by: Jeff King 
> ---
> Jake: I was surprised that this was not a "patch 2" on top of your
> earlier coccicheck patch. Apologies if you were planning to send it out.
>

Nope, I hadn't gotten around to this yet, thanks!

The conversions also look good to me :)

Regards,
Jake

Re: [PATCH] coccicheck: process every source file at once

[Jacob Keller]

On Tue, Oct 2, 2018 at 1:31 PM Jeff King  wrote:
> Actually, I guess we do not need to save $? at all, since we have only a
> single process to care about. So even simpler:
>
>   spatch ... 2>$@+ 2>$@.log ||
>   {
> cat $@.log
> exit 1
>   }
>   # if we get here, we were successful
>   mv $@+ $@ ;# etc
>
> would work. That's missing all the Makefile=required backslashes and
> semicolons, of course. ;)
>

I opted to drop to just save the return, immediately after calling.
It's a bit less code change, and I think the result is as clear as the
above would be. This way we do drop the subshell, not that it matters
much in the end...

Thanks,
Jake

> -Peff

Re: [PATCH v3] coccicheck: process every source file at once

[Jacob Keller]

On Tue, Oct 2, 2018 at 1:07 PM Jacob Keller  wrote:
>
> From: Jacob Keller 
>
> make coccicheck is used in order to apply coccinelle semantic patches,
> and see if any of the transformations found within contrib/coccinelle/
> can be applied to the current code base.
>
> Pass every file to a single invocation of spatch, instead of running
> spatch once per source file.
>
> This reduces the time required to run make coccicheck by a significant
> amount of time:
>
> Prior timing of make coccicheck
>   real6m14.090s
>   user25m2.606s
>   sys 1m22.919s
>
> New timing of make coccicheck
>   real1m36.580s
>   user7m55.933s
>   sys 0m18.219s
>
> This is nearly a 4x decrease in the time required to run make
> coccicheck. This is due to the overhead of restarting spatch for every
> file. By processing all files at once, we can amortize this startup cost
> across the total number of files, rather than paying it once per file.
>
> Signed-off-by: Jacob Keller 
> ---

Forgot to add what changed. I dropped the subshell and "||" bit around
invoking spatch.

Thanks,
Jake


>  Makefile | 6 ++
>  1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index df1df9db78da..da692ece9e12 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -2715,10 +2715,8 @@ endif
>  %.cocci.patch: %.cocci $(COCCI_SOURCES)
> @echo '' SPATCH $<; \
> ret=0; \
> -   for f in $(COCCI_SOURCES); do \
> -   $(SPATCH) --sp-file $< $$f $(SPATCH_FLAGS) || \
> -   { ret=$$?; break; }; \
> -   done >$@+ 2>$@.log; \
> +   $(SPATCH) --sp-file $< $(COCCI_SOURCES) $(SPATCH_FLAGS) >$@+ 
> 2>$@.log; \
> +   ret=$$?; \
> if test $$ret != 0; \
> then \
> cat $@.log; \
> --
> 2.18.0.219.gaf81d287a9da
>

[PATCH v3] coccicheck: process every source file at once

[Jacob Keller]

From: Jacob Keller 

make coccicheck is used in order to apply coccinelle semantic patches,
and see if any of the transformations found within contrib/coccinelle/
can be applied to the current code base.

Pass every file to a single invocation of spatch, instead of running
spatch once per source file.

This reduces the time required to run make coccicheck by a significant
amount of time:

Prior timing of make coccicheck
  real6m14.090s
  user25m2.606s
  sys 1m22.919s

New timing of make coccicheck
  real1m36.580s
  user7m55.933s
  sys 0m18.219s

This is nearly a 4x decrease in the time required to run make
coccicheck. This is due to the overhead of restarting spatch for every
file. By processing all files at once, we can amortize this startup cost
across the total number of files, rather than paying it once per file.

Signed-off-by: Jacob Keller 
---
 Makefile | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/Makefile b/Makefile
index df1df9db78da..da692ece9e12 100644
--- a/Makefile
+++ b/Makefile
@@ -2715,10 +2715,8 @@ endif
 %.cocci.patch: %.cocci $(COCCI_SOURCES)
@echo '' SPATCH $<; \
ret=0; \
-   for f in $(COCCI_SOURCES); do \
-   $(SPATCH) --sp-file $< $$f $(SPATCH_FLAGS) || \
-   { ret=$$?; break; }; \
-   done >$@+ 2>$@.log; \
+   $(SPATCH) --sp-file $< $(COCCI_SOURCES) $(SPATCH_FLAGS) >$@+ 2>$@.log; \
+   ret=$$?; \
if test $$ret != 0; \
then \
cat $@.log; \
-- 
2.18.0.219.gaf81d287a9da

Re: [PATCH v2] coccicheck: process every source file at once

[Jacob Keller]

On Tue, Oct 2, 2018 at 1:03 PM Jacob Keller  wrote:
>
> From: Jacob Keller 
>
> make coccicheck is used in order to apply coccinelle semantic patches,
> and see if any of the transformations found within contrib/coccinelle/
> can be applied to the current code base.
>
> Pass every file to a single invocation of spatch, instead of running
> spatch once per source file.
>
> This reduces the time required to run make coccicheck by a significant
> amount of time:
>
> Prior timing of make coccicheck
>   real6m14.090s
>   user25m2.606s
>   sys 1m22.919s
>
> New timing of make coccicheck
>   real1m36.580s
>   user7m55.933s
>   sys 0m18.219s
>
> This is nearly a 4x decrease in the time required to run make
> coccicheck. This is due to the overhead of restarting spatch for every
> file. By processing all files at once, we can amortize this startup cost
> across the total number of files, rather than paying it once per file.
>
> Signed-off-by: Jacob Keller 
> ---

Woops, ignore this version, it doesn't quite work.

Thanks,
Jake

[PATCH v2] coccicheck: process every source file at once

[Jacob Keller]

From: Jacob Keller 

make coccicheck is used in order to apply coccinelle semantic patches,
and see if any of the transformations found within contrib/coccinelle/
can be applied to the current code base.

Pass every file to a single invocation of spatch, instead of running
spatch once per source file.

This reduces the time required to run make coccicheck by a significant
amount of time:

Prior timing of make coccicheck
  real6m14.090s
  user25m2.606s
  sys 1m22.919s

New timing of make coccicheck
  real1m36.580s
  user7m55.933s
  sys 0m18.219s

This is nearly a 4x decrease in the time required to run make
coccicheck. This is due to the overhead of restarting spatch for every
file. By processing all files at once, we can amortize this startup cost
across the total number of files, rather than paying it once per file.

Signed-off-by: Jacob Keller 
---
 Makefile | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/Makefile b/Makefile
index df1df9db78da..b9947f3f51ec 100644
--- a/Makefile
+++ b/Makefile
@@ -2715,10 +2715,8 @@ endif
 %.cocci.patch: %.cocci $(COCCI_SOURCES)
@echo '' SPATCH $<; \
ret=0; \
-   for f in $(COCCI_SOURCES); do \
-   $(SPATCH) --sp-file $< $$f $(SPATCH_FLAGS) || \
-   { ret=$$?; break; }; \
-   done >$@+ 2>$@.log; \
+   ( $(SPATCH) --sp-file $< $(COCCI_SOURCES) $(SPATCH_FLAGS) || \
+   { ret=$$?; }; ) >$@+ 2>$@.log; \
if test $$ret != 0; \
then \
cat $@.log; \
-- 
2.18.0.219.gaf81d287a9da

Re: [PATCH] coccicheck: process every source file at once

[Jacob Keller]

On Tue, Oct 2, 2018 at 12:55 PM Jeff King  wrote:
>
> On Tue, Oct 02, 2018 at 12:16:42PM -0700, Jacob Keller wrote:
>
> > make coccicheck is used in order to apply coccinelle semantic patches,
> > and see if any of the transformations found within contrib/coccinelle/
> > can be applied to the current code base.
> >
> > Pass every file to a single invocation of spatch, instead of running
> > spatch once per source file.
> >
> > This reduces the time required to run make coccicheck by a significant
> > amount of time:
> >
> > Prior timing of make coccicheck
> >   real6m14.090s
> >   user25m2.606s
> >   sys 1m22.919s
> >
> > New timing of make coccicheck
> >   real1m36.580s
> >   user7m55.933s
> >   sys 0m18.219s
>
> Yay! This is a nice result.
>
> It's also one of the things that Julia suggested in an earlier thread.
> One thing I wasn't quite sure about after digging into the various
> versions (1.0.4 on Debian stable/unstable, 1.0.6 in experimental, and
> pre-1.0.7 at the bleeding edge) was whether the old versions would be
> similarly helped (or work at all).
>
> I just replicated your results with 1.0.4.deb-3+b2 from Debian stable.
> It's possible there are older versions floating around, but for
> something developer-only like this, I think "in Debian stable" is a
> reasonable enough cutoff.
>

Good. I hadn't checked back too far, but I know support for multiple
files has existed since quite a while.

> > This is nearly a 4x decrease in the time required to run make
> > coccicheck. This is due to the overhead of restarting spatch for every
> > file. By processing all files at once, we can amortize this startup cost
> > across the total number of files, rather than paying it once per file.
>
> One minor side effect is that we lose the opportunity to parallelize
> quite as much. However, I think the reduction in total CPU makes it well
> worth that (and we still have 8 cocci files and growing, which should
> keep at least 8 cores busy).
>

I don't think we do any less than previously, because we are doing
each file in a for-loop, so those would all be serial anyways.

> I think recent versions of Coccinelle will actually parallelize
> internally, too, but my 1.0.4 doesn't seem to. That's probably what I
> was thinking of earlier (but this is still a win even without that).
>
> It looks like this also fixes a problem I ran into when doing the oideq
> work, which is that the patch for a header file would get shown multiple
> times (once for each file that includes it). So this is faster _and_
> more correct. Double-yay.
>

Woot :D

> > diff --git a/Makefile b/Makefile
> > index df1df9db78da..b9947f3f51ec 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -2715,10 +2715,8 @@ endif
> >  %.cocci.patch: %.cocci $(COCCI_SOURCES)
> >   @echo '' SPATCH $<; \
> >   ret=0; \
> > - for f in $(COCCI_SOURCES); do \
> > - $(SPATCH) --sp-file $< $$f $(SPATCH_FLAGS) || \
> > - { ret=$$?; break; }; \
> > - done >$@+ 2>$@.log; \
> > + ( $(SPATCH) --sp-file $< $(COCCI_SOURCES) $(SPATCH_FLAGS) || \
> > + { ret=$$?; }; ) >$@+ 2>$@.log; \
>
> This looks pretty straight-forward. I wondered if we could get rid of
> the "ret" handling, since we don't need to pass the error back out of
> the loop. But it's also used for the "show the log only on error" logic
> below:
>

We could probably get rid of it by doing the spatch without an ||, and
then just save $?.

> >   if test $$ret != 0; \
> >   then \
> >   cat $@.log; \
>
> The subshell could be a {}, though, I think, but it's not that big a
> deal either way.
>
> -Peff

[PATCH] coccicheck: process every source file at once

[Jacob Keller]

From: Jacob Keller 

make coccicheck is used in order to apply coccinelle semantic patches,
and see if any of the transformations found within contrib/coccinelle/
can be applied to the current code base.

Pass every file to a single invocation of spatch, instead of running
spatch once per source file.

This reduces the time required to run make coccicheck by a significant
amount of time:

Prior timing of make coccicheck
  real6m14.090s
  user25m2.606s
  sys 1m22.919s

New timing of make coccicheck
  real1m36.580s
  user7m55.933s
  sys 0m18.219s

This is nearly a 4x decrease in the time required to run make
coccicheck. This is due to the overhead of restarting spatch for every
file. By processing all files at once, we can amortize this startup cost
across the total number of files, rather than paying it once per file.

Signed-off-by: Jacob Keller 
---
 Makefile | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/Makefile b/Makefile
index df1df9db78da..b9947f3f51ec 100644
--- a/Makefile
+++ b/Makefile
@@ -2715,10 +2715,8 @@ endif
 %.cocci.patch: %.cocci $(COCCI_SOURCES)
@echo '' SPATCH $<; \
ret=0; \
-   for f in $(COCCI_SOURCES); do \
-   $(SPATCH) --sp-file $< $$f $(SPATCH_FLAGS) || \
-   { ret=$$?; break; }; \
-   done >$@+ 2>$@.log; \
+   ( $(SPATCH) --sp-file $< $(COCCI_SOURCES) $(SPATCH_FLAGS) || \
+   { ret=$$?; }; ) >$@+ 2>$@.log; \
if test $$ret != 0; \
then \
cat $@.log; \
-- 
2.18.0.219.gaf81d287a9da

Re: [PATCH v2 0/1] Make 'git commit' not accidentally lose staged content

[Jacob Keller]

On Mon, Sep 17, 2018 at 12:26 PM Junio C Hamano  wrote:
> FWIW, I didn't mean to say that we should give users a way to
> recover.  Your "commit -a" or "commit $path" protection just
> prevents the situation from happening, and I think it is sufficient.
>
> The sole point I wanted to raise by bringing up the above was that
> we should have the same degree of protection against "add $path" or
> "add -u".
>
> Of course, "index log" is interesting and it may even turn out to be
> useful (I was skeptical about "reference log" the same way, but it
> turned out to be useful without burdening the system too heavily),
> and it may even remove the need for the "do not accidentally lose
> information by adding more to the index" protection.  But until that
> happens, if we are to have such a protection, we would wnat to give
> the same degree of protection to "commit" and "add".

I think having both is good. There are a lot of ways to accidentally
throw away work, and it's pretty frustrating to have it happen. But
the reflog is also somewhat complicated, and I've definitely seen a
lot of developers who've never heard of it, and struggle with the
concept.

I personally think having the nice "it looks like you're about to
throw away all your changes, are you sure" style of protection using
something like --clobber-index is useful as a mode, even if we have an
index log of sorts. Having it be default helps new people, even if it
does get in the way of someone who knows what they're doing. Having it
be configurable, to me, sort of defeats the point, since it means
having to tell people to turn this on.

I personally don't mind having to type an extended option to clobber
when I know it's what I want, but I can see that being painful.

However, if we had a reflog for the index, this becomes less of a
problem since recovery is much easier.

Thanks,
Jake

Re: [PATCH v2 0/1] Make 'git commit' not accidentally lose staged content

[Jacob Keller]

On Mon, Sep 17, 2018 at 11:15 AM Jeff King  wrote:
>
> On Mon, Sep 17, 2018 at 07:29:26PM +0200, Duy Nguyen wrote:
>
> > > On the other hand, if I am keeping some change that should never be
> > > in a commit in the working tree file, and building the contents in
> > > the index using "add -p" to incrementally, it would be the same
> > > disaster as you are trying to prevent if I by mistake did a whole
> > > path 'add', even if I catch myself doing so before running 'commit'
> > > i.e.
> > >
> > > edit X
> > > git add -p X
> > > git diff --cached X
> > > git diff X
> > > ... repeat the above number of times ...
> > > git add X ;# OOPS!
> > > git add . ;# OOPS! even worse!
> > >
> > > Even though this does not involve "git commit -a" or "git commit X",
> > > an unrecoverable damage that requires redoing the manual work is
> > > already done.
> >
> > I don't see a good way to get to recover this situation. I could go
> > back to the "index log" idea, where we keep a log of index changes (or
> > just "interesting" changes). That way there's no behavior change at
> > all. The user who accidentally updates/deletes something can always
> > retrieve the old content back (assuming that they realize quickly
> > since we can't keep very long log).
>
> FWIW, I like that approach much better, since:
>
>   1. It does not bother or restrict anybody in their workflow; instead,
>  they pay the complexity price only when they know they have made a
>  mistake.
>
>   2. It covers many more cases (e.g., just doing the wrong thing via
>  "add -p").
>

I also think this is a better approach for the same reasons.

> A naive index log would be pretty cheap in CPU, at least for POSIX-ish
> systems. You could just hard link "index" to "index.N" before renaming
> "index.lock" over "index". But I guess if you have a gigantic index,
> that's less appealing. So maybe storing the equivalent of a "--raw" diff
> between the two index states would make more sense (and after all, you
> don't really need the stat-cache or cache-tree). It would cost more to
> reconstruct the index on the fly, but then the point is that you would
> create these logs a lot more than you access them.
>
> > I've been thinking about allowing to undo worktree changes too (e.g.
> > accidental "git reset --hard") and this log can cover it as well.
>
> I like that, too. It's a little more costly just because it may involve
> object-db writes, but I think in most cases it would be fine. I almost
> always "git stash" away discarded changes these days instead of "git
> reset --hard", because it effectively provides this kind of log.
>

Obviously we do eventually turn the index into a tree, which is used
by the commit. Would it be possible to simply somehow store these
trees, and have commands which blow the tree away simply instead, save
it? I'm not sure how costly that is.

Thanks,
Jake

Re: [PATCH v2 0/1] Make 'git commit' not accidentally lose staged content

[Jacob Keller]

On Mon, Sep 17, 2018 at 10:09 AM Junio C Hamano  wrote:
>
> It usually is safer (simply because you do not have to think about
> it) to start a behaviour change like this as a strict opt-in to gain
> confidence.

I tend to agree, however.. in this case it could be considered safer
to err on the side of not throwing away the index which could have
crafted changes in it.

> The approach to check if the contents in the index matches that in
> the HEAD per-path (i.e. "The contents we are adding to the index is
> whole working tree contents for that path.  But the index already
> has contents different from HEAD for the path---are we losing
> information by doing this?"), is a very good one.  But for the
> protection to be effective, I think "git commit" and "git add"
> should be covered the same way, ideally with the same code and
> possibly the same configuration knob and/or command line option to
> control the behaviour.

Checking both commit and add makes sense to me.

>
> If the information loss caused by the "add/commit X" or "add
> -u/commit -a" is so serious that this new feature deserves to become
> the default (which I do not yet think it is the case, by the way),
> then we could even forbid "commit X" or "commit -a" when the paths
> involved has difference between the index and the HEAD, without any
> configuration knob or command line override for "commit", and then
> tell the users to use "git add/rm" _with_ the override before coming
> back to "git commit".

I was going to suggest we have some sort of reflog equivalent for the
index, but Duy seems to discuss that in a follow-on mail.

>
> How should this new check intract with paths added with "add -N", by
> the way?

Re: [PATCH/RFC] commit: new option to abort -a something is already staged

[Jacob Keller]

On Fri, Aug 24, 2018 at 7:42 AM Duy Nguyen  wrote:
>
> On Fri, Aug 24, 2018 at 5:02 AM Jacob Keller  wrote:
> >
> > On Thu, Aug 23, 2018 at 9:28 AM Junio C Hamano  wrote:
> > > I think the above example forgets "-a" on the final "git commit"
> > > step.  With it added, I can understand the concern (and I am sure
> > > you would, too).
> > >
> > > The user is trying to add everything done in the working tree, and
> > > "commit -a" would catch all changes to paths that were already
> > > tracked, but a separate "add" is necessary for newly created paths.
> > > But adding a new path means the index no longer matches HEAD, and
> > > the "commit -a" at the final step sweeps the changes to already
> > > tracked paths---failing that because there "already is something
> > > staged" will break the workflow.
> >
> > Right. I think this would need to be able to understand the case of
> > "different only by new files".
>
> OK so the rules I'm going to try to implement is, if the  version in
> the index is not the same as one in HEAD or one in worktree, then "git
> commit -a" fails.
>
> The unwritten line is, if the path in question does not exist in HEAD,
> then the first condition "as one in HEAD" is dropped, naturally. This
> addresses the "git add new-file" problem, but if you have made more
> changes in new-file in worktree after "git add" and do "git commit -a"
> then you still get rejected because it fails the second condition.
>
> File removal should be considered as well. But I don't foresee any
> problem there. Resolving merges, replacing higher stage entries with
> stage 0 will be accepted at "git commit -a" as usual.
>
> Let me know if we should tweak these rules (and how).
> --
> Duy

This seems reasonable to me. It might trigger a few issues with people
doing "git add new_file ; $EDITOR new_file ; git commit -a" but... I
know I've accidentally done "git commit -a" and had problems.

What about "git commit "? That's another case where I've
accidentally lost some work for a similar reason.

Thanks,
Jake

Re: [PATCH/RFC] commit: new option to abort -a something is already staged

[Jacob Keller]

On Thu, Aug 23, 2018 at 9:28 AM Junio C Hamano  wrote:
> I think the above example forgets "-a" on the final "git commit"
> step.  With it added, I can understand the concern (and I am sure
> you would, too).
>
> The user is trying to add everything done in the working tree, and
> "commit -a" would catch all changes to paths that were already
> tracked, but a separate "add" is necessary for newly created paths.
> But adding a new path means the index no longer matches HEAD, and
> the "commit -a" at the final step sweeps the changes to already
> tracked paths---failing that because there "already is something
> staged" will break the workflow.

Right. I think this would need to be able to understand the case of
"different only by new files".

Thanks,
Jake

Re: [PATCH/RFC] commit: new option to abort -a something is already staged

[Jacob Keller]

On Mon, Aug 20, 2018 at 8:43 AM Nguyễn Thái Ngọc Duy  wrote:
> Instead, let's handle just this problem for now. This new option
> commit.preciousDirtyIndex, if set to false, will not allow `commit -a`
> to continue if the final index is different from the existing one. I
> don't think this can be achieved with hooks because the hooks don't
> know about "-a" or different commit modes.
>

Here you say setting it to "false" enables this check but...

> +commit.preciousDirtyIndex::
> +   If some changes are partially staged in the index (i.e.
> +   "git commit -a" and "git commit" commit different changes), reject
> +   "git commit -a".
> +

Here, sounds like it is enabled by setting it to true.

Thanks,
Jake

Re: [ANNOUNCE] Git v2.19.0-rc0

[Jacob Keller]

On Thu, Aug 23, 2018 at 5:16 PM Jeff King  wrote:
>
> On Thu, Aug 23, 2018 at 08:06:37PM -0400, Jeff King wrote:
>
> > This almost works:
> >
> >   @@
> >   expression a, b;
> >   statement s;
> >   @@
> >   - if (oidcmp(a, b)) s
> >   + if (!oideq(a, b)) s
> >
> > [...]
>
> > So I really do want some way of saying "all of the block, no matter what
> > it is". Or of leaving it out as context.
>
> Aha. The magic invocation is:
>
>   @@
>   expression a, b;
>   statement s;
>   @@
>   - if (oidcmp(a, b))
>   + if (!oideq(a, b))
>   s
>
> I would have expected that you could replace "s" with "...", but that
> does not seem to work.
>
> -Peff

Odd...

What about..

- if (oidcmp(a,b))
+ if(!oideq(a,b))
  { ... }

Or maybe you need to use something like

  <...
- if (oidcmp(a,b))
+ if (!oideq(a,b))
  ...>

Hmm. Yea, semantic patches are a bit confusing overall sometimes.

But it looks like you got something which works?

Thanks,
Jake

Re: [ANNOUNCE] Git v2.19.0-rc0

[Jacob Keller]

On Thu, Aug 23, 2018 at 9:30 AM Jeff King  wrote:
> I think that audit isn't actually too bad (but definitely not something
> we should do for v2.19). The cocci patch I showed earlier hits most of
> them. It misses the negated ones (e.g., "if (oidcmp(...))"). I'm not
> sure if there's a way to ask coccinelle only for oidcmp()
> used in a boolean context.
>

You can look for explicitly "if (oidcmp(...))" though. I don't know if
you can catch *any* use which degrades to boolean outside of an if
statement, but I wouldn't expect there to be too many of those?

Thanks,
Jake

Re: [PATCH 01/17] cache: update object ID functions for the_hash_algo

[Jacob Keller]

On Sun, Jul 8, 2018 at 9:05 PM Eric Sunshine  wrote:
>
> On Sun, Jul 8, 2018 at 10:38 PM Jacob Keller  wrote:
> > On Sun, Jul 8, 2018 at 4:39 PM brian m. carlson
> >  wrote:
> > >  static inline int oidcmp(const struct object_id *oid1, const struct 
> > > object_id *oid2)
> > >  {
> > > -   return hashcmp(oid1->hash, oid2->hash);
> > > +   return memcmp(oid1->hash, oid2->hash, the_hash_algo->rawsz);
> > >  }
> >
> > Just curious, what's the reasoning for not using the hashcmp anymore?
>
> hashcmp() is specific to SHA-1 (for instance, it hardocdes
> GIT_SHA1_RAWSZ). oidcmp() is meant as the hash-agnostic replacement
> for hashcmp(), so it doesn't make sense to continue implementing
> oidcmp() in terms of hashcmp() (the latter of which will eventually be
> retired, presumably).

Fair. I just saw that hashcmp was also updated to use the_hash_algo,
but if we're going to drop it eventually, then there's zero reason to
keep implementing oidcmp in terms of it, so... makes sense to me!

Thanks,
Jake

Re: [PATCH 00/17] object_id part 14

[Jacob Keller]

On Sun, Jul 8, 2018 at 4:39 PM brian m. carlson
 wrote:
>
> This is the fourteenth series of patches to switch to using struct
> object_id and the_hash_algo.  This series converts several core pieces
> to use struct object_id, including the oid* and hex functions.
>
> All of these patches have been tested with both SHA-1 and a 256-bit
> hash.
>

I read through the series, and didn't spot anything odd, except for
the question about reasoning for why we use memcmp directly over using
hashcmp. I don't think that's any sort of blocker, it just seemed an
odd decision to me.

Thanks,
Jake

Re: [PATCH 01/17] cache: update object ID functions for the_hash_algo

[Jacob Keller]

On Sun, Jul 8, 2018 at 4:39 PM brian m. carlson
 wrote:
>  static inline int oidcmp(const struct object_id *oid1, const struct 
> object_id *oid2)
>  {
> -   return hashcmp(oid1->hash, oid2->hash);
> +   return memcmp(oid1->hash, oid2->hash, the_hash_algo->rawsz);
>  }
>

Just curious, what's the reasoning for not using the hashcmp anymore?

Thanks,
Jake

Re: [PATCH 0/8] Reroll of sb/diff-color-move-more

[Jacob Keller]

On Thu, May 17, 2018 at 12:46 PM, Stefan Beller  wrote:
>>> * sb/diff-color-move-more (2018-04-25) 7 commits
>>...
>>>
>>>  Will merge to 'next'.
>>
>>I did not get around to fix it up, there are still review
>>comments outstanding. (The test is broken in the last commit.)
>
> This is a reroll of sb/diff-color-move-more, with the test fixed as well
> as another extra patch, that would have caught the bad test.
>
> The range diff is below.
>
> Thanks,
> Stefan
>
> Stefan Beller (8):
>   xdiff/xdiff.h: remove unused flags
>   xdiff/xdiffi.c: remove unneeded function declarations
>   diff.c: do not pass diff options as keydata to hashmap
>   diff.c: adjust hash function signature to match hashmap expectation
>   diff.c: add a blocks mode for moved code detection
>   diff.c: decouple white space treatment from move detection algorithm
>   diff.c: add --color-moved-ignore-space-delta option
>   diff: color-moved white space handling options imply color-moved
>

I've been using this locally, and it's really nice. One question I
had, are there plans to make the whitespace options configurable? I
really like the option for enabling lines to count as moved when the
whitespace changes uniformly, (it helps make changes more obvious when
doing indentation changes such as wrapping code within a block).
However, it's rather a long option name to type out. I didn't see any
obvious config options to enable it by default though.

Thanks,
Jake

Re: [PATCH] add -p: fix counting empty context lines in edited patches

[Jacob Keller]

On Fri, Jun 1, 2018 at 10:46 AM, Phillip Wood  wrote:
> From: Phillip Wood 
>
> recount_edited_hunk() introduced in commit 2b8ea7f3c7 ("add -p:
> calculate offset delta for edited patches", 2018-03-05) required all
> context lines to start with a space, empty lines are not counted. This
> was intended to avoid any recounting problems if the user had
> introduced empty lines at the end when editing the patch. However this
> introduced a regression into 'git add -p' as it seems it is common for
> editors to strip the trailing whitespace from empty context lines when
> patches are edited thereby introducing empty lines that should be
> counted. 'git apply' knows how to deal with such empty lines and POSIX
> states that whether or not there is an space on an empty context line
> is implementation defined [1].
>
> Fix the regression by counting lines consist solely of a newline as
> well as lines starting with a space as context lines and add a test to
> prevent future regressions.
>
> [1] http://pubs.opengroup.org/onlinepubs/9699919799/utilities/diff.html
>
> Reported-by: Mahmoud Al-Qudsi 
> Reported-by: Oliver Joseph Ash 
> Reported-by: Jeff Felchner 
> Signed-off-by: Phillip Wood 
> ---
> My apologies to everyone who was affected by this regression.
>

Ahhh I suspect this is why my edited code in add -p was sometimes
failing to apply!

Thanks,
Jake

>  git-add--interactive.perl  |  2 +-
>  t/t3701-add-interactive.sh | 43 ++
>  2 files changed, 44 insertions(+), 1 deletion(-)
>
> diff --git a/git-add--interactive.perl b/git-add--interactive.perl
> index ab022ec073..bb6f249f03 100755
> --- a/git-add--interactive.perl
> +++ b/git-add--interactive.perl
> @@ -1047,7 +1047,7 @@ sub recount_edited_hunk {
> $o_cnt++;
> } elsif ($mode eq '+') {
> $n_cnt++;
> -   } elsif ($mode eq ' ') {
> +   } elsif ($mode eq ' ' or $_ eq "\n") {
> $o_cnt++;
> $n_cnt++;
> }
> diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh
> index e5c66f7500..f1bb879ea4 100755
> --- a/t/t3701-add-interactive.sh
> +++ b/t/t3701-add-interactive.sh
> @@ -175,6 +175,49 @@ test_expect_success 'real edit works' '
> diff_cmp expected output
>  '
>
> +test_expect_success 'setup file' '
> +   test_write_lines a "" b "" c >file &&
> +   git add file &&
> +   test_write_lines a "" d "" c >file
> +'
> +
> +test_expect_success 'setup patch' '
> +   SP=" " &&
> +   NULL="" &&
> +   cat >patch <<-EOF
> +   @@ -1,4 +1,4 @@
> +a
> +   $NULL
> +   -b
> +   +f
> +   $SP
> +   c
> +   EOF
> +'
> +
> +test_expect_success 'setup expected' '
> +   cat >expected <<-EOF
> +   diff --git a/file b/file
> +   index b5dd6c9..f910ae9 100644
> +   --- a/file
> +   +++ b/file
> +   @@ -1,5 +1,5 @@
> +a
> +   $SP
> +   -f
> +   +d
> +   $SP
> +c
> +   EOF
> +'
> +
> +test_expect_success 'edit can strip spaces from empty context lines' '
> +   test_write_lines e n q | git add -p 2>error &&
> +   test_must_be_empty error &&
> +   git diff >output &&
> +   diff_cmp expected output
> +'
> +
>  test_expect_success 'skip files similarly as commit -a' '
> git reset &&
> echo file >.gitignore &&
> --
> 2.17.0
>

Re: commit-graph: change in "best" merge-base when ambiguous

[Jacob Keller]

On Mon, May 21, 2018 at 2:54 PM, Jeff King  wrote:
> Yes, I think this is clearly a case where all of the single merge-bases
> we could show are equally good. And I don't think we should promise to
> show a particular one, but I _do_ think it's friendly for us to have
> deterministic tie-breakers (we certainly don't now).
>
> -Peff

Right. I think we should probably have some mechanism that will allow
us to always give the same answer, even if it's somewhat arbitrary.

Regards,
Jake

Re: [PATCH v4 3/4] string-list: provide `string_list_appendf()`

[Jacob Keller]

On Sun, May 20, 2018 at 3:17 AM, Martin Ågren  wrote:
> +/**
> + * Add formatted string to the end of `list`. This function ignores
> + * the value of `list->strdup_strings` and always appends a freshly
> + * allocated string, so you will probably not want to use it with
> + * `strdup_strings = 0`.
> + */
> +struct string_list_item *string_list_appendf(struct string_list *list,
> +const char *fmt, ...);
> +

Would it make sense to verify that strdup_strings == 0? I guess we'd
have to use die or BUG(), but that would mean that the program could
crash..

I doubt this could be verified at compilation time

Thanks,
Jake

Re: Add option to git to ignore binary files unless force added

[Jacob Keller]

On Fri, May 18, 2018 at 4:31 AM, Anmol Sethi  wrote:
> This definitely works but it would be more clean to just have git ignore the 
> binary files from the get go.
>

Sure it'd be more convenient for you. But there are loads of possible
combinations, and the idea of what constitutes unwanted files is
hugely variable to each user. We don't really want to end up
supporting a million different ways to do this, and the hooks
interface provides a reasonable method for rejecting commits with
unwanted contents.


Thanks,
Jake

Re: [PATCH v2 1/3] merge: setup `opts` later in `checkout_fast_forward()`

[Jacob Keller]

On Thu, May 17, 2018 at 2:48 PM, Junio C Hamano  wrote:
> Martin Ågren  writes:
>
>> After we initialize the various fields in `opts` but before we actually
>> use them, we might return early. Move the initialization further down,
>> to immediately before we use `opts`.
>>
>> This limits the scope of `opts` and will help a later commit fix a
>> memory leak without having to worry about those early returns.
>>
>> This patch is best viewed using something like this (note the tab!):
>> --color-moved --anchored="trees[nr_trees] = parse_tree_indirect"
>
> This side remark is interesting because it totally depends on how
> you look at it.  I think "initialize opts late" and "attempt to
> parse the trees first and fail early" are the sides of the same
> coin, and the diff shown without the anchor matches the latter,
> which is also perfectly acceptable interpretation of what this patch
> does.
>

Yes. I like that we have tools available to show diffs in different
hopefully meaningful ways.

I happen to like when the diff matches my mental map of the change
after reading the commit message, so having the author indicate how
best to view it is useful, but definitely cool to see that we can get
different interpretations.

Thanks,
Jake

Re: Add option to git to ignore binary files unless force added

[Jacob Keller]

On Wed, May 16, 2018 at 5:45 PM, Anmol Sethi  wrote:
> I think it’d be great to have an option to have git ignore binary files. My 
> repositories are always source only, committing a binary is always a mistake. 
> At the moment, I have to configure the .gitignore to ignore every binary file 
> and that gets tedious. Having git ignore all binary files would be great.
>
> This could be achieved via an option in .gitconfig or maybe a special line in 
> .gitignore.
>
> I just want to never accidentally commit a binary again.
>
> --
> Best,
> Anmol
>

I believe you can do a couple things. There should be a hook which you
can modify to validate that there are no binary files on
pre-commit[1], or pre-push[2] to verify that you never push commits
with binaries in them.

You could also implement the update hook on the server if you control
it, to allow it to block pushes which contain binary files.

Thanks,
Jake

[1]https://git-scm.com/docs/githooks#_pre_commit
[2]https://git-scm.com/docs/githooks#_pre_push
[3]https://git-scm.com/docs/githooks#update

Re: [PATCH v2 1/3] merge: setup `opts` later in `checkout_fast_forward()`

[Jacob Keller]

On Wed, May 16, 2018 at 12:29 PM, Martin Ågren  wrote:
> On 16 May 2018 at 18:41, Stefan Beller  wrote:
>> On Wed, May 16, 2018 at 9:30 AM, Martin Ågren  wrote:
>>>
>>> This patch is best viewed using something like this (note the tab!):
>>> --color-moved --anchored="  trees[nr_trees] = parse_tree_indirect"
>>
>> Heh! Having a "is best viewed" paragraph is the new shiny thing in
>> commit messages as 'git log origin/pu --grep "is best viewed"' tells me.
>
> :-)
>
>> Regarding the anchoring, I wonder if we can improve it by ignoring
>> whitespaces or just looking for substrings, or by allowing regexes or ...
>
> FWIW, because my first naive attempt failed (for some reason I did not
> consider the leading tab part of the "line" so I did not provide it), I
> had the same thought. Ignoring leading whitespace seemed easy enough in
> the implementation.
>
> Then I started thinking about all the ways in which whitespace can be
> ignored. My reaction in the end was to not try and open that can right
> there and then. I did not think about regexes.
>
> I guess this boils down to the usage. Copying the line to anchor on from
> an editor could run into these kind of whitespace-issues, and shell
> escaping. Typing an anchor could become easier with regexes since one
> could skip typing common substrings and just anchor on /unique-part/.
>
> Martin

Simpler approach is to just match substring instead. Then, the user
can decide how much of the string is required to get the anchor they
wanted.

Thanks,
Jake

Re: [PATCH v2 1/3] merge: setup `opts` later in `checkout_fast_forward()`

[Jacob Keller]

On Wed, May 16, 2018 at 9:41 AM, Stefan Beller  wrote:
> + Jonathan Tan for a side discussion on anchoring.
>
> On Wed, May 16, 2018 at 9:30 AM, Martin Ågren  wrote:
>>
>> This patch is best viewed using something like this (note the tab!):
>> --color-moved --anchored="  trees[nr_trees] = parse_tree_indirect"
>
> Heh! Having a "is best viewed" paragraph is the new shiny thing in
> commit messages as 'git log origin/pu --grep "is best viewed"' tells me.
>
> Regarding the anchoring, I wonder if we can improve it by ignoring
> whitespaces or just looking for substrings, or by allowing regexes or ...
>
> Thanks,
> Stefan

I think expanding it to be regexp would be nicest. To be honest, I
already thought it was substring based

It'd be *really* cool if we had a way for a commit messages (or maybe
notes?) to indicate the anchor so that git show could (optionally)
figure out the anchor automatically. It's been REALLY useful for me
when showing diffs to be able to provide a better idea of what a human
*actually* did vs what the smallest diff was.

Thanks,
Jake

Re: [PATCH v2 02/18] Add a new builtin: branch-diff

[Jacob Keller]

On Sat, May 5, 2018 at 6:05 PM, Igor Djordjevic
 wrote:
> Hi Dscho,
>
> On 05/05/2018 23:57, Johannes Schindelin wrote:
>>
>> > > This builtin does not do a whole lot so far, apart from showing a
>> > > usage that is oddly similar to that of `git tbdiff`. And for a
>> > > good reason: the next commits will turn `branch-diff` into a
>> > > full-blown replacement for `tbdiff`.
>> >
>> > One minor point about the name: will it become annoying as a tab
>> > completion conflict with git-branch?
>>
>> I did mention this in the commit message of 18/18:
>>
>> Without this patch, we would only complete the `branch-diff` part but
>> not the options and other arguments.
>>
>> This of itself may already be slightly disruptive for well-trained
>> fingers that assume that `git braorimas` would expand to
>> `git branch origin/master`, as we now no longer automatically append a
>> space after completing `git branch`: this is now ambiguous.
>>
>> > It feels really petty complaining about the name, but I just want
>> > to raise the point, since it will never be easier to change than
>> > right now.
>>
>> I do hear you. Especially since I hate `git cherry` every single
>> time that I try to tab-complete `git cherry-pick`.
>>
>> > (And no, I don't really have another name in mind; I'm just
>> > wondering if "subset" names like this might be a mild annoyance in
>> > the long run).
>>
>> They totally are, and if you can come up with a better name, I am
>> really interested in changing it before this hits `next`, even.
>
> I gave this just a quick glance so might be I`m missing something
> obvious or otherwise well-known here, bur why not `diff-branch` instead?
>
> From user interface perspective, I would (personally) rather expect a
> command that does "diff of branches" to belong to "diff family" of
> commands (just operating on branches, instead of "branch" command
> knowing to "diff itself"), and I see we already have `diff-files`,
> `diff-index` and `diff-tree`, for what that`s worth.
>
> Heck, I might even expect something like `git diff --branch ...` to work,
> but I guess that is yet a different matter :)
>
> Thanks, Buga

I like diff-branch, though I suppose that also conflicts with diff too.

Thanks,
Jake

Re: Fetching tags overwrites existing tags

[Jacob Keller]

On Fri, Apr 27, 2018 at 12:13 PM, Bryan Turner  wrote:
> On Fri, Apr 27, 2018 at 12:08 PM, Wink Saville  wrote:
>>
>> The other change was rather than using 
>> ""+refs/tags/*:refs/remote-tags/$name/*"
>> I've changed it to "+refs/tags/*:refs/remote/tags/$name/*" which seems 
>> cleaner.
>> Again, if remote-tags is preferred I'll change it back.
>
>
> From looking at the code, it looks like you mean
> "+refs/tags/*:refs/remotes/tags/$name/*".
>
> The issue with that approach is that it collides with a remote named
> "tags". "refs/remote-tags", on the other hand, represents a new-to-Git
> path, one that won't already be in use by any other standard
> functionality. That seems like a better approach than hoping no one
> out there will call one of their remotes "tags".
>
> Bryan

Note that my suggestion was very specific "remote" not pluralized,
which is obviously a bit confusing, since there's remote and
"remotes".

The goal being that you put "remote//" followed by the full
remote ref minus the refs prefix.

It specifically is attempting to avoid the problem of expanding
"remotes". Unfortunately, I don't have a better alternative format,
and i very much want to avoid having to do "remote-tags",
"remote-notes", "remote-replaces", "remote-meta" etc...

In that spirit, I'm working to hopefully propose something today.

Thanks,
Jake

Re: [PATCH 00/18] Add `branch-diff`, a `tbdiff` lookalike

[Jacob Keller]

On Thu, May 3, 2018 at 11:05 AM, Ævar Arnfjörð Bjarmason
 wrote:
>
> On Thu, May 03 2018, Johannes Schindelin wrote:
>
>> The incredibly useful `git-tbdiff` tool to compare patch series (say, to see
>> what changed between two iterations sent to the Git mailing list) is slightly
>> less useful for this developer due to the fact that it requires the 
>> `hungarian`
>> and `numpy` Python packages which are for some reason really hard to build in
>> MSYS2. So hard that I even had to give up, because it was simply easier to
>> reimplement the whole shebang as a builtin command.
>>
>> The project at https://github.com/trast/tbdiff seems to be dormant, anyway.
>> Funny (and true) story: I looked at the open Pull Requests to see how active
>> that project is, only to find to my surprise that I had submitted one in 
>> August
>> 2015, and that it was still unanswered let alone merged.
>
> I've been using branch-diff and haven't found issues with it yet, it
> works like tbdiff but better. Faster, uses the same diff as git
> (better), and spews to the pager by default.

I'm hoping to take a look at this as well, I remember looking into
tbdiff in the past, but also had trouble getting it to work. I've
tried a variety of similar things, including 4-way parent diffs, but
nothing quite gave the results I expected.

Thanks!

Regards,
Jake

Re: [PATCH v3 00/12] get_short_oid UI improvements

[Jacob Keller]

On Wed, May 2, 2018 at 6:45 AM, Derrick Stolee  wrote:
> On 5/2/2018 8:42 AM, Derrick Stolee wrote:
>>
>> On 5/1/2018 2:40 PM, Ævar Arnfjörð Bjarmason wrote:
>>>
>>> The biggest change in v3 is the no change at all to the code, but a
>>> lengthy explanation of why I didn't go for Derrick's simpler
>>> implementation. Maybe I'm wrong about that, but I felt uneasy
>>> offloading undocumented (or if I documented it, it would only be for
>>> this one edge-case) magic on the oid_array API. Instead I'm just
>>> making this patch a bit more complex.
>>
>>
>> I think that's fair. Thanks for going along with me on the thought
>> experiment.
>
>
> Also, v3 looks good to me.
>
> Reviewed-by: Derrick Stolee 
>

I also reviewed this, and it looks good to me as well.

Thanks,
Jake

Re: [RFC PATCH v4 0/3] Optional sub hierarchy for remote tags

[Jacob Keller]

On Tue, May 1, 2018 at 4:24 PM, Junio C Hamano <gits...@pobox.com> wrote:
> Jacob Keller <jacob.kel...@gmail.com> writes:
>
>> I also agree, I'd prefer if we aim for the mapping to be something
>> which works for all refs in the future, even if such support isn't
>> added now, which is why i've proposed using "refs/remote//" so
>> that a tag would go from
>>
>> refs/tags/v1.7
>>
>> to
>>
>> refs/remote//tags/v1.7
>>
>> Ideally, we could work to update "refs/remotes/" to go to
>> "refs/remote//heads" as well.
>
> This is *not* imcompatible with having refs/remote-tags/* as an
> interim solution.

Sure. I'm just proposing that we pick a name that all the refs can move to now.

>
> We'll have to support refs/remotes// anyway long after
> we start using refs/remote//heads/ by (1) switching
> the fetch refspecs newer "git clone" writes to the latter format,

Ofcourse we'll have to support this, and i didn't mean to imply we wouldn't.

I was just hoping to avoid having even more places to check in the future.

> and (2) extending the dwim table to try both formats.  Having Wink's
> solution as an interim step adds one more entry to (2) but the
> machinery is already there.  And it does not change (1), either.
>

Sure, we could. And yes, we have to do (1), which means we have to do
(2) anyways. But we can still pick something which is more easily
expandable than refs/remotes/ was.

Thanks,
Jake

Re: [RFC PATCH v4 0/3] Optional sub hierarchy for remote tags

[Jacob Keller]

On Tue, May 1, 2018 at 12:24 PM, Ævar Arnfjörð Bjarmason
 wrote:
> Thanks a lot for working on this, it's great to have something the
> direction of getting rid of this discrepancy between the 1=1 mapping for
> branches, but the 1=many mapping for tags. My recent patch series and
> the pruneTags feature I added in 2.17.0 are really just hacks around not
> having that.
>
> My concern with your patches is not that they're not implementing some
> mythical future where we're mapping each 's refs to
> remotes/ and local refs to local/ ref>, which is what we should really be aiming for and would forever get
> us out of this rut of 1=many and local & remote refs existing in the
> same namespace, but that they might make it harder to get there.
>

I also agree, I'd prefer if we aim for the mapping to be something
which works for all refs in the future, even if such support isn't
added now, which is why i've proposed using "refs/remote//" so
that a tag would go from

refs/tags/v1.7

to

refs/remote//tags/v1.7

Ideally, we could work to update "refs/remotes/" to go to
"refs/remote//heads" as well. This allows obtaining any ref and
mapping it safely per remote. I don't really like the fact that we
can't simply re-use "remotes", nor do I like the fact that "remote" is
very close to "remotes" increasing the chance of typos... historically
I suggested using "tracking" but I don't really like that either..
because honestly there *is* no good name left for this purpose once
"remotes" became only about branches.

The reason I care about this is that I do want to be able to share the
notes refs in a way that allows easy merging, which currently is not
really doable without a lot of work from all users of notes knowing
exactly how you plan to work on them.

Thanks,
Jake

> So specifically, your 1/3 writes this to the config:
>
>   [remote "gbenchmark"]
> url = g...@github.com:google/benchmark
> fetch = +refs/heads/*:refs/remotes/gbenchmark/*
> fetch = +refs/tags/*:refs/remote-tags/gbenchmark/*
> tagopt = --remote-tags
>
> If the user clones with your patch, and then uses an older git version
> on the same repo (a use case we need to support) that older version
> doesn't know about --remote-tags, and will fetch them all.
>
> As a workaround for that maybe we'll need something like:
>
>   [remote "gbenchmark"]
> url = g...@github.com:google/benchmark
> fetch = +refs/heads/*:refs/remotes/gbenchmark/*
> fetch = +refs/tags/*:refs/remote-tags/gbenchmark/*
> tagStyle = remote
> tagopt = --no-tags
>
> Or whatever, i.e. something where only the new version will fetch the
> tags and ignore the tagopt option (which I never liked anyway). It's a
> hack, but at least you don't end up with crap it your ref namespace by
> flip-flopping between versions.
>
> Then as I alluded to in my
> https://public-inbox.org/git/20180429202100.32353-6-ava...@gmail.com/ we
> have a lot of stuff that hardcodes special behaviors for
> refs/{tags,heads}/, including but not limited to:
>
> git grep -C2 -e TAG_REFSPEC -e tag_refspec -e '"refs/tags/' -- '*.[ch]'
>
> So maybe we need to start this series with some set of patches where we
> make the currently hardcoded behavior for refs/{heads,tags}/
> configurable.
>
> Sorry about this "nice shed you built, how about you make a cathedral
> instead?" E-Mail. I really don't think we should make perfect the enemy
> of the good, but at the same time it would be unfortunate if we can't
> get perfect because we settled for good.

Re: Branch deletion question / possible bug?

[Jacob Keller]

On Fri, Apr 27, 2018 at 5:29 PM, Tang (US), Pik S  wrote:
> Hi,
>
> I discovered that I was able to delete the feature branch I was in, due to 
> some fat fingering on my part and case insensitivity.  I never realized this 
> could be done before.  A quick google search did not give me a whole lot to 
> work with...
>
> Steps to reproduce:
> 1. Create a feature branch, "editCss"
> 2. git checkout master
> 3. git checkout editCSS
> 4. git checkout editCss
> 5. git branch -d editCSS
>

Are you running on a case-insensitive file system? What version of
git? I thought I recalled seeing commits to help avoid creating
branches of the same name with separate case when we know we're on a
file system which is case-insensitive..

> Normally, it should have been impossible for a user to delete the branch 
> they're on.  And the deletion left me in a weird state that took a while to 
> dig out of.
>
> I know this was a user error, but I was also wondering if this was a bug.

If we have not yet done this, I think we should. Long term this would
be fixed by using a separate format to store refs than the filesystem,
which has a few projects being worked on but none have been put into a
release.

Thanks,
Jake

>
>
> Thanks,
>
> Pik Tang
>

Re: Fetching tags overwrites existing tags

[Jacob Keller]

On Fri, Apr 27, 2018 at 12:08 PM, Wink Saville  wrote:
> On Thu, Apr 26, 2018 at 4:24 PM, Junio C Hamano  wrote:
>> Junio C Hamano  writes:
>>
>>
>> Hence (1) we should detect and error out when --prefix-tags is used
>> with mirror fetch near where we do the same for track used without
>> mirror fetch already, (2) detect and error out when --prefix-tags is
>> used with track, and (3) add "+refs/tags/*:refs/remote-tags/$name/*"
>> just once without paying attention to track here.  We may not even
>> want add_remote_tags() helper function if we go that route.
>>
>
> I've replied to the thread using format-email/send-email with the
> subject: "[RFC PATCH v2] Teach remote add the --prefix-tags option",
> but I misspelled Junio's email address :(
>
> I've tried to address the issues pointed out by Junio. But I've choosen
> not to do "(2) detect and error out when --prefix-tags is used with track".
> My thinking is tags are independent of tracking and it seems reasonable
> that they sould be included if requested. If I'm wrong I'll certainly fix it.
>
> The other change was rather than using 
> ""+refs/tags/*:refs/remote-tags/$name/*"
> I've changed it to "+refs/tags/*:refs/remote/tags/$name/*" which seems 
> cleaner.
> Again, if remote-tags is preferred I'll change it back.

The only main concern I have with "remote" is that it is very similar
but not exactly the same as "remotes". Unfortunately, it is not
possible in *every* circumstance to use remotes.

Personally, I'd prefer we used "refs/remote//tags" rather
than "refs/remote/tags/", and possibly plan to migrate
from refs/remotes// to
refs/remote//heads/

This is mostly so that future additions of things like notes,
replaces, or really *any* refs would automatically drop into
"refs/remotes//",
which is a longer term goal I've had for a while (though i haven't
been able to put much time to it at present). Historically, I proposed
using "tracking" instead of "remote", but I am ok with any name we
choose as long as it doesn't create conflicts.

Thanks,
Jake

>
> One other question, I'm not sure "--prefix-tags" is the best name for
> the option,
> maybe "--sub-tags" or "--nested-tags" or ...
>
> -- Wink

Re: git merge banch w/ different submodule revision

[Jacob Keller]

On Fri, Apr 27, 2018 at 5:24 PM, Elijah Newren  wrote:
> I would expect that a different example involving non-linear history
> would behave the same, if both sides update the submodule in a fashion
> that is just fast-forwarding and one commit contains the other in its
> history.  I'm curious if you have a counter example.

My interpretation of the counter example was that the two submodule
updates were not linear (i.e. one did not contain the other after
updating).

I could be wrong, so more clarification from Lief would be helpful in
illuminating the problem.

Thanks,
Jake

Re: git merge banch w/ different submodule revision

[Jacob Keller]

On Thu, Apr 26, 2018 at 10:56 AM, Stefan Beller  wrote:
> We often treating a submodule as a file from the superproject, but not always.
> And in case of a merge, git seems to be a bit smarter than treating it
> as a textfile
> with two different lines.

Sure, but a submodule is checked out "at a commit", so if two branches
of history are merged, and they conflict over which place the
submodule is at shouldn't that produce a conflict??

I mean, how is the merge algorithm supposed to know which is right?
The patch you linked appears to be able to resolve it to the one which
contains both commits.. but that may not actually be true since you
can rewind submodules since they're *pointers* to commits, not commits
themselves.

I'm not against that as a possible strategy to merge submodules, but
it seems like not necessarily something you would always want...

Thanks,
Jake

Re: Fetching tags overwrites existing tags

[Jacob Keller]

On Tue, Apr 24, 2018 at 5:52 PM, Junio C Hamano  wrote:
> Wink Saville  writes:
>
>> Ideally I would have liked the tags fetched from gbenchmark to have a prefix
>> of gbenchmark/, like the branches have, maybe something like:
>>
>> $ git fetch --tags gbenchmark
>> ...
>>  * [new branch]  v2  -> gbenchmark/v2
>>  * [new tag] v0.0.9  -> gbenchmark/v0.0.9
>>  * [new tag] v0.1.0  -> gbenchmark/v0.1.0
>>  * [new tag] v1.0.0  -> gbenchmark/v1.0.0
>>  * [new tag] v1.1.0  -> gbenchmark/v1.1.0
>>  * [new tag] v1.2.0  -> gbenchmark/v1.2.0
>>  * [new tag] v1.3.0  -> gbenchmark/v1.3.0
>>  * [new tag] v1.4.0  -> gbenchmark/v1.4.0
>
> The tag namespace (refs/tags/) is considered a shared resource (I am
> not saying that that is the only valid world model---I am merely
> explaining why things are like they are), hence the auto-following
> tags will bring them to refs/tags/ (and I do not think there is no
> way to configure auto-following to place them elsewhere).
>
> But you could configure things yourself.
>
> $ git init victim && cd victim
> $ git remote add origin ../git.git
> $ git config --add remote.origin.fetch \
>   "+refs/tags/*:refs/remote-tags/origin/*"
> $ tail -n 4 .git/config
> [remote "origin"]
> url = ../git.git/
> fetch = +refs/heads/*:refs/remotes/origin/*
> fetch = +refs/tags/*:refs/remote-tags/origin/*
> $ git fetch --no-tags
>
> The "--no-tags" option serves to decline the auto-following tags to
> refs/tags/ hierarchy; once your repository is configured this way,
> your initial and subsequent "git fetch" will copy refs it finds in
> refs/tags/ hierarchy over there to your refs/remote-tags/origin/
> hierarchy locally.

It should be noted, that remote-tags would not be integrated into "git
tag" or many other places in git commands, so it may be significantly
less visible.

Thanks,
Jake

Re: Fetching tags overwrites existing tags

[Jacob Keller]

On Tue, Apr 24, 2018 at 12:57 PM, Wink Saville  wrote:
> If have a repository with a tag "v1.0.0" and I add a remote repository
> which also has a tag "v1.0.0" tag is overwritten.
>
> Google found [1] from 2011 and option 3 is what I'd like to see. Has it been
> implemented and I just don't see it?
>
> [1]: https://groups.google.com/forum/#!topic/git-version-control/0l_rJFyTE60
>
>
> Here is an example demonstrating what I see:
>
> $ echo abc > abc.txt
> $ git init .
> Initialized empty Git repository in
> /home/wink/prgs/git/investigate-fetch-tags/.git/
> $ git add *
> $ git commit -m "Initial commit"
> [master (root-commit) 1116fdc] Initial commit
>  1 file changed, 1 insertion(+)
>  create mode 100644 abc.txt
> $ git tag v1.0.0
> $ git remote add gbenchmark g...@github.com:google/benchmark
> $ git log --graph --format="%h %s %d"
> * 1116fdc Initial commit  (HEAD -> master, tag: v1.0.0)
> $ git fetch --tags gbenchmark
> warning: no common commits
> remote: Counting objects: 4400, done.
> remote: Compressing objects: 100% (15/15), done.
> remote: Total 4400 (delta 5), reused 5 (delta 3), pack-reused 4382
> Receiving objects: 100% (4400/4400), 1.33 MiB | 2.81 MiB/s, done.
> Resolving deltas: 100% (2863/2863), done.
> From github.com:google/benchmark
>  * [new branch]  clangtidy   -> gbenchmark/clangtidy
>  * [new branch]  iter_report -> gbenchmark/iter_report
>  * [new branch]  master  -> gbenchmark/master
>  * [new branch]  releasing   -> gbenchmark/releasing
>  * [new branch]  reportercleanup -> gbenchmark/reportercleanup
>  * [new branch]  rmheaders   -> gbenchmark/rmheaders
>  * [new branch]  v2  -> gbenchmark/v2
>  * [new tag] v0.0.9  -> v0.0.9
>  * [new tag] v0.1.0  -> v0.1.0
>  t [tag update]  v1.0.0  -> v1.0.0
>  * [new tag] v1.1.0  -> v1.1.0
>  * [new tag] v1.2.0  -> v1.2.0
>  * [new tag] v1.3.0  -> v1.3.0
>  * [new tag] v1.4.0  -> v1.4.0
> $ git log --graph --format="%h %s %d"
> * 1116fdc Initial commit  (HEAD -> master)
>
> As you can see the tag on 1116fdc is gone, v1.0.0 tag has been updated
> and now its pointing to the tag in gbenchmark:
>
> $ git log -5 --graph --format="%h %s %d" v1.0.0
> *   cd525ae Merge pull request #171 from eliben/update-doc-userealtime
>  (tag: v1.0.0)
> |\
> | * c7ab1b9 Update README to mention UseRealTime for wallclock time
> measurements.
> |/
> * f662e8b Rename OS_MACOSX macro to new name BENCHMARK_OS_MACOSX. Fix #169
> *   0a1f484 Merge pull request #166 from disconnect3d/master
> |\
> | * d2917bc Fixes #165: CustomArguments ret type in README
> |/
>
> Ideally I would have liked the tags fetched from gbenchmark to have a prefix
> of gbenchmark/, like the branches have, maybe something like:
>

That would require a complete redesign of how we handle remotes. I've
proposed ideas in the past but never had time and they didn't gain
much traction.

It's a known limitation that the tags namespace can only hold a single
tag name (even if remotes differ in what that tag is). I *thought*
that the tags should not be updated after you fetch it once, but it
seems this is not the behavior we get now?

My basic idea was to fetch *all* remote refs into a
refs///* such that *every* ref in a
remote can be determined by something like
"refs/tracking/origin/tags/name" instead of
"refs/remotes/origin/name", and then tags would have to be updated to
check for tags in each remote as well as locally. Additionally, you
could update the tool to warn when two remotes have the same tag at
different refs, and allow disambiguation.

Ideally, "origin/branch" should still DWIM, same for "tag" should work
unless there are conflicts.

Unfortunately, it's a pretty big change in how remotes are handled,
and I never had time to actually work towards a POC or implementation.
Mostly, we ended up on bikeshedding what the name should be now that
we can't use "refs/remotes" due to backwards compatibility. I don't
really like "tracking" as a name, but it was the best I could come up
with.

(Note, the impetus for this proposal was actually to allow easy
sharing of notes and other specialized refs).

Thanks,
Jake

> $ git fetch --tags gbenchmark
> ...
>  * [new branch]  v2  -> gbenchmark/v2
>  * [new tag] v0.0.9  -> gbenchmark/v0.0.9
>  * [new tag] v0.1.0  -> gbenchmark/v0.1.0
>  * [new tag] v1.0.0  -> gbenchmark/v1.0.0
>  * [new tag] v1.1.0  -> gbenchmark/v1.1.0
>  * [new tag] v1.2.0  -> gbenchmark/v1.2.0
>  * [new tag] v1.3.0  -> gbenchmark/v1.3.0
>  * [new tag] v1.4.0  -> gbenchmark/v1.4.0
>
>
> -- Wink

Re: [PATCH 1/2] merge: setup `opts` later in `checkout_fast_forward()`

[Jacob Keller]

On Mon, Apr 23, 2018 at 10:13 PM, Martin Ågren  wrote:
> After we initialize the various fields in `opts` but before we actually
> use them, we might return early. Move the initialization further down,
> to immediately before we use `opts`.
>
> This limits the scope of `opts` and will help a subsequent commit fix a
> memory leak without having to worry about those early returns.
>
> Signed-off-by: Martin Ågren 
> ---
>  merge.c | 32 +---
>  1 file changed, 17 insertions(+), 15 deletions(-)
>
> diff --git a/merge.c b/merge.c
> index f06a4773d4..f123658e58 100644
> --- a/merge.c
> +++ b/merge.c
> @@ -94,8 +94,24 @@ int checkout_fast_forward(const struct object_id *head,
> return -1;
>
> memset(, 0, sizeof(trees));
> -   memset(, 0, sizeof(opts));
> memset(, 0, sizeof(t));
> +
> +   trees[nr_trees] = parse_tree_indirect(head);
> +   if (!trees[nr_trees++]) {
> +   rollback_lock_file(_file);
> +   return -1;
> +   }
> +   trees[nr_trees] = parse_tree_indirect(remote);
> +   if (!trees[nr_trees++]) {
> +   rollback_lock_file(_file);
> +   return -1;
> +   }
> +   for (i = 0; i < nr_trees; i++) {
> +   parse_tree(trees[i]);
> +   init_tree_desc(t+i, trees[i]->buffer, trees[i]->size);
> +   }
> +
> +   memset(, 0, sizeof(opts));
> if (overwrite_ignore) {
> memset(, 0, sizeof(dir));

I'm guessing the diff algorithm simply found that this was a more
compact representation of the change? It's a bit confusing when your
description indicates you "moved" some code down, but it looks like
you moved code up.

Thanks,
Jake

> dir.flags |= DIR_SHOW_IGNORED;
> @@ -112,20 +128,6 @@ int checkout_fast_forward(const struct object_id *head,
> opts.fn = twoway_merge;
> setup_unpack_trees_porcelain(, "merge");
>
> -   trees[nr_trees] = parse_tree_indirect(head);
> -   if (!trees[nr_trees++]) {
> -   rollback_lock_file(_file);
> -   return -1;
> -   }
> -   trees[nr_trees] = parse_tree_indirect(remote);
> -   if (!trees[nr_trees++]) {
> -   rollback_lock_file(_file);
> -   return -1;
> -   }
> -   for (i = 0; i < nr_trees; i++) {
> -   parse_tree(trees[i]);
> -   init_tree_desc(t+i, trees[i]->buffer, trees[i]->size);
> -   }
> if (unpack_trees(nr_trees, t, )) {
> rollback_lock_file(_file);
> return -1;
> --
> 2.17.0
>

Re: Git enhancement request - checkout (clone) set modified dates to commit date

[Jacob Keller]

On Sun, Apr 22, 2018 at 5:23 PM, Andrew Wolfe  wrote:
> Kevin, thanks for your feedback.
>
> You have a reasonable point, because usually you don't put the outputs of a 
> build into version control, but the build script checks them for being 
> current.
>
> On the other hand, when you change branches, your existing output directories 
> are worthless problems anyway — even if you have all the interdependencies 
> correct.  Because of this, I'm inclined to consider this use case as 
> intrinsically hazardous.  When I do a checkout, I always want to purge all 
> the intermediate and end targets regardless.

Not every build has this problem, and certainly I think some of the
most common build software would not (Make). It's fairly easy to fix
this by using a git hook to update files post checkout (you can look
up the timestamp of each file's commit time, or any other time and use
touch to do this yourself).

Thanks,
Jake

Re: [PATCH v6 00/15] rebase -i: offer to recreate commit topology

[Jacob Keller]

On Fri, Apr 20, 2018 at 1:26 AM, Johannes Schindelin
<johannes.schinde...@gmx.de> wrote:
> Hi Jake,
>
> On Thu, 19 Apr 2018, Jacob Keller wrote:
>
>> On Wed, Apr 18, 2018 at 9:24 PM, Sergey Organov <sorga...@gmail.com> wrote:
>> > Johannes Schindelin <johannes.schinde...@gmx.de> writes:
>> >
>> >> On Fri, 13 Apr 2018, Phillip Wood wrote:
>> >>
>> >>> On 12/04/18 23:02, Johannes Schindelin wrote:
>> >>> >
>> >>> > [...]
>> >>> >
>> >>> > So: the order of the 3-way merges does matter.
>> >>> >
>> >>> > [...]
>> >>>
>> >>> Those conflicts certainly look intimidating (and the ones in your later
>> >>> reply with the N way merge example still look quite complicated). One
>> >>> option would be just to stop and have the user resolve the conflicts
>> >>> after each conflicting 3-way merge rather than at the end of all the
>> >>> merges. There are some downsides: there would need to be a way to
>> >>> explain to the user that this is an intermediate step (and what that
>> >>> step was); the code would have to do some book keeping to know where it
>> >>> had got to; and it would stop and prompt the user to resolve conflicts
>> >>> more often which could be annoying but hopefully they'd be clearer to
>> >>> resolve because they weren't nested.
>> >>
>> >> I thought about that. But as I pointed out: the order of the merges *does*
>> >> matter. Otherwise we force the user to resolve conflicts that they
>> >> *already* resolved during this rebase...
>> >
>> > How it's relevant to what Phillip suggested? How the order of taking 2
>> > steps, A and B, affects an ability to stop after the first step? It's
>> > still either "A,stop,B" or "B,stop,A", depending on the chosen order.
>> >
>> > What's the _actual_ problem here, if any?
>> >
>> > -- Sergey
>>
>> I believe the order of the merges changes which ones cause conflicts,
>
> That is a correct interpretation of the example I showed.
>
>> but it's possible to generate pre-images (i.e. a set of parents to
>> merge) which cause conflicts regardless of which ordering we pick, so
>> I'm not sure there is a "best ordering".
>
> In general, there is no best ordering, you are right. There is no silver
> bullet.
>
> I am not satisfied with stating that and then leaving it at that.
>
> In the example I presented, you can see that there are common cases where
> there *is* a best ordering. In the wrong order, even if you would force
> the user to resolve the merge conflict in an intermediate merge (which
> would introduce a nightmare for the user interface, I am sure you see
> that), then the next merge would *again* show merge conflicts.
>
> And I, for one, am *really* certain what my decision would be when offered
> the two options 1) force the user to resolve merge conflicts *twice*, or
> 2) reorder the intermediate merges and present the user with exactly one
> set of merge conflicts.
>
> So it is irrelevant that there might not be a "best order" in the general
> case, when in the common cases quite frequently there is.
>
> It is just another example where theory disagrees with practice. Don't get
> me wrong: it is good to start with theory. And likewise it is simply
> necessary to continue from there, and put your theory to the test. And
> then you need to turn this into something practical.
>
> Ciao,
> Dscho

I recall you suggested an approach of "try one way, if there are
conflicts, check the other way and see if it had conflicts".

And I also agree that forcing the user to resolve conflicts in the
middle of the operation is a huge nightmare of a user interface,
probably worse than the issues with nested merge conflicts.

Thanks,
Jake

Re: [PATCH v6 00/15] rebase -i: offer to recreate commit topology

[Jacob Keller]

On Wed, Apr 18, 2018 at 9:24 PM, Sergey Organov  wrote:
> Johannes Schindelin  writes:
>
>> Hi Phillip,
>>
>> On Fri, 13 Apr 2018, Phillip Wood wrote:
>>
>>> On 12/04/18 23:02, Johannes Schindelin wrote:
>>> >
>>> > [...]
>>> >
>>> > So: the order of the 3-way merges does matter.
>>> >
>>> > [...]
>>>
>>> Those conflicts certainly look intimidating (and the ones in your later
>>> reply with the N way merge example still look quite complicated). One
>>> option would be just to stop and have the user resolve the conflicts
>>> after each conflicting 3-way merge rather than at the end of all the
>>> merges. There are some downsides: there would need to be a way to
>>> explain to the user that this is an intermediate step (and what that
>>> step was); the code would have to do some book keeping to know where it
>>> had got to; and it would stop and prompt the user to resolve conflicts
>>> more often which could be annoying but hopefully they'd be clearer to
>>> resolve because they weren't nested.
>>
>> I thought about that. But as I pointed out: the order of the merges *does*
>> matter. Otherwise we force the user to resolve conflicts that they
>> *already* resolved during this rebase...
>
> How it's relevant to what Phillip suggested? How the order of taking 2
> steps, A and B, affects an ability to stop after the first step? It's
> still either "A,stop,B" or "B,stop,A", depending on the chosen order.
>
> What's the _actual_ problem here, if any?
>
> -- Sergey

I believe the order of the merges changes which ones cause conflicts,
but it's possible to generate pre-images (i.e. a set of parents to
merge) which cause conflicts regardless of which ordering we pick, so
I'm not sure there is a "best ordering".

Thanks,
Jake

Re: Optimizing writes to unchanged files during merges?

[Jacob Keller]

On Tue, Apr 17, 2018 at 10:27 AM, Lars Schneider
<larsxschnei...@gmail.com> wrote:
>
>> On 16 Apr 2018, at 19:45, Jacob Keller <jacob.kel...@gmail.com> wrote:
>>
>> On Mon, Apr 16, 2018 at 10:43 AM, Jacob Keller <jacob.kel...@gmail.com> 
>> wrote:
>>> On Mon, Apr 16, 2018 at 9:07 AM, Lars Schneider
>>> <larsxschnei...@gmail.com> wrote:
>>>> What if Git kept a LRU list that contains file path, content hash, and
>>>> mtime of any file that is removed or modified during a checkout. If a
>>>> file is checked out later with the exact same path and content hash,
>>>> then Git could set the mtime to the previous value. This way the
>>>> compiler would not think that the content has been changed since the
>>>> last rebuild.
>>>
>>> That would only work until they actuall *did* a build on the second
>>> branch, and upon changing back, how would this detect that it needs to
>>> update mtime again? I don't think this solution really works.
>>> Ultimately, the problem is that the build tool relies on the mtime to
>>> determine what to rebuild. I think this would cause worse problems
>>> because we *wouldn't* rebuild in the case. How is git supposed to know
>>> that we rebuilt when switching branches or not?
>>>
>>> Thanks,
>>> Jake
>>
>> I think a better solution for your problem would be to extend the
>> build system you're using to avoid rebuilding when the contents
>> haven't changed since last build (possibly by using hashes?). At the
>> very least, I would not want this to be default, as it could possibly
>> result in *no* build when there should be one, which is far more
>> confusing to debug.
>
> I am 100% with you that this is a build system issue. But changing
> the build system for many teams in a large organization is really
> hard. That's why I wondered if Git could help with a shortcut.
> Looks like there is no shortcut (see my other reply in this thread).
>
> Thanks
> Lars

Right. I think that solutions involving hooks or scripts which "fix"
the mtimes are the best bet for this problem then, given that building
it into git would cause problems for other users. (And personally I
would always ere on the side of causing rebuilds unless we're 100%
sure)

Thanks,
Jake

Re: man page for "git remote set-url" seems confusing/contradictory

[Jacob Keller]

On Tue, Apr 17, 2018 at 8:34 AM, Robert P. J. Day <rpj...@crashcourse.ca> wrote:
> On Tue, 17 Apr 2018, Junio C Hamano wrote:
>
>> Jacob Keller <jacob.kel...@gmail.com> writes:
>>
>> > Things won't work so well if you set the push url and fetch url to
>> > different repositories. Git assumes that refs updated by "push"
>> > will also be reflected via "fetch".
>> >
>> > I don't know offhand what will break, but likely something will.
>> > For one, when you fetch again it will rewind your remotes after
>> > the push.
>>
>> Exactly.  I still haven't fully embraced it myself, but for a long
>> time, "git push" pretends as if it fetched from that remote and
>> updates the corresponding remote tracking branches (if you have
>> any), so you'll be inviting inconsistent behaviour if you set your
>> fetch and push URLs pointing at two logically separate places.
>
>   ... snip ...
>
>   oh, i totally buy all that now, i'm just suggesting that the man
> page might be tweaked to make that more obvious. in "man git-remote",
> under "set-url", remember that it reads:
>
> "Note that the push URL and the fetch URL, even though they can be set
> differently, must still refer to the same place."
>
>   i think it would be useful to be more specific about what "can be
> set differently" means, since a lot of readers might not immediately
> appreciate that it means just, say, the transport protocols. it never
> hurts to add that little bit of detail.
>
> rday
>
>

Maybe something like:

"Note that the push URL and the fetch URL, even though they can be set
differently, are expected to refer to the same repository. For
example, the fetch URL might use an unauthenticated transport, while
the push URL generally requires authentication" Then follow this bit
with the mention of multiple remotes.

(I think "repository" conveys the meaning better than "place".
Technically, the URLs can be completely different as long as they end
up contacting the same real server repository.)

Thanks,
Jake

Re: man page for "git remote set-url" seems confusing/contradictory

[Jacob Keller]

On Mon, Apr 16, 2018 at 4:13 PM, Junio C Hamano <gits...@pobox.com> wrote:
> Jacob Keller <jacob.kel...@gmail.com> writes:
>
>> Things won't work so well if you set the push url and fetch url to
>> different repositories. Git assumes that refs updated by "push" will
>> also be reflected via "fetch".
>>
>> I don't know offhand what will break, but likely something will. For
>> one, when you fetch again it will rewind your remotes after the push.
>
> Exactly.  I still haven't fully embraced it myself, but for a long
> time, "git push" pretends as if it fetched from that remote and
> updates the corresponding remote tracking branches (if you have
> any), so you'll be inviting inconsistent behaviour if you set your
> fetch and push URLs pointing at two logically separate places.
>
> This is a tangent, but there probably should be a boolean that
> disables this feature in "git push" per destination repository,
> i.e. "when pushing into this repository, pretend that we immediately
> fetched from the refs we just pushed to and update the remote
> tracking branches we have for them: yes/no".  It is not entirely
> implausible to envision an overly smart remote repository that turns
> a non-fast-forward push into an automatic rebase when it is safe to
> do so, instead of failing such a push, and you'd disable the "assume
> what we pushed would appear there" when talking to such a remote.
>

Not to mention something like gerrit which uses magical references
"refs/publish/branchname" which don't actually get generated on the
server.

Thanks,
Jake

Re: Draft of Git Rev News edition 38

[Jacob Keller]

On Mon, Apr 16, 2018 at 3:30 PM, Christian Couder
<christian.cou...@gmail.com> wrote:
> On Mon, Apr 16, 2018 at 5:19 PM, Sergey Organov <sorga...@gmail.com> wrote:
>> Kaartic Sivaraam <kaartic.sivar...@gmail.com> writes:
>>
>>> 1. I see the following sentence in the "Rebasing merges: a jorney to the
>>> ultimate solution (Road Clear) (written by Jacob Keller)" article
>>>
>>>   "A few examples were tried, but it was proven that the original
>>>   concept did not work, as dropped commits could end up being
>>>   replaid into the merge commits, turning them into "evil"
>>>   merges."
>>>
>>> I'm not sure if 'replaid' is proper English assuming the past tense of
>>> replay was intended there (which I think is 'replayed').
>>
>> It could have meant, say, "reapplied", -- we need to ask the author.
>
> Yeah it could but I would say that it is not very likely compared to
> "replayed", so I changed it to "replayed". And yeah I can change it to
> something else if Jake (who is Cc'ed) prefers.
>
>> While we are at it, please also consider to replace "original concept"
>> by "original algorithm", as it didn't work due to a mistake in the
>> algorithm as opposed to failure of the concept itself.
>
> Ok, it's now "original algorithm".
>
> Thanks,
> Christian.

Replayed is accurate.

Thanks,
Jake

Re: man page for "git remote set-url" seems confusing/contradictory

[Jacob Keller]

On Mon, Apr 16, 2018 at 9:19 AM, Robert P. J. Day  wrote:
> On Mon, 16 Apr 2018, Andreas Schwab wrote:
>
>> On Apr 16 2018, "Robert P. J. Day"  wrote:
>>
>> > i don't understand how you can clearly set the fetch and push URLs
>> > of a remote independently, while the man page nonetheless insists
>> > that "even though they can be set differently, must still refer to
>> > the same place". how can they be set differently yet still must
>> > refer to the same place?
>>
>> They could be using different transport methods.  For example, for
>> fetching the unauthenticated git: method could be used, but for
>> pushing an authenticated method like ssh: is usually needed.
>
>   ok, point taken, but does that mean the two must actually refer to
> the same "place"? it seems that i'm perfectly free to use this command
> to set the push and fetch URLs to totally different locations.
>
> rday

Things won't work so well if you set the push url and fetch url to
different repositories. Git assumes that refs updated by "push" will
also be reflected via "fetch".

I don't know offhand what will break, but likely something will. For
one, when you fetch again it will rewind your remotes after the push.

The URLs could be different for lots of reasons, but they need to
ultimately link to the same remote repository. As Andreas said, the
most likely reason is differing transport protocols.

Thanks,
Jake

Re: [PATCH] completion: reduce overhead of clearing cached --options

[Jacob Keller]

On Sat, Apr 14, 2018 at 6:27 AM, Jakub Narebski  wrote:
> SZEDER Gábor  writes:
>> On Fri, Apr 13, 2018 at 11:44 PM, Jakub Narebski  wrote:
>>> SZEDER Gábor  writes:

 In Bash we can do better: run the 'compgen -v __gitcomp_builtin_'
 builtin command, which lists the same variables, but without a
 pipeline and 'sed' it can do so with lower overhead.
>>>
>>> What about ZSH?
>>
>> Nothing, ZSH is unaffected by this patch.
>
> All right, so for ZSH we would need LC_ALL=C trick, or come with some
> equivalent of 'compgen -v __gitcomp_builtin_' for this shell.
>
> Good patch, though it does not solve whole of the problem.
>
> Best,
> --
> Jakub Narębski

Is ZSH actually affected by the broken set behavior, though?

Thanks,
Jake

Re: "proper" way to deactivate push for a remote?

[Jacob Keller]

On Mon, Apr 16, 2018 at 7:03 AM, Robert P. J. Day  wrote:
>
>  another feature i've seen for the very first time ... working with
> kubernetes so i checked it out of github, and part of the instructions
> for that is to make sure you don't accidentally try to push back to
> the github remote, so the directions suggest:
>
> $ git remote add upstream https://github.com/kubernetes/kubernetes.git
> $ git remote set-url --push upstream no_push
>
>   fair enough, i just assumed the word "no_push" was some magical
> keyword in that context, but as i read it, all you need to do is put
> *some* invalid URL value there, is that correct?
>
>   and is that the accepted way to do that? what about just deleting
> that line from .git/config? is that valid, or is there a different
> recommendation for doing that? thanks.
>
> rday

I've done this in the past by using a push hook which just reports an
error. That's how I'd do it.

I think if you also set the push url to the empty string it would also work.

Thanks,
Jake

Re: Optimizing writes to unchanged files during merges?

[Jacob Keller]

On Mon, Apr 16, 2018 at 10:43 AM, Jacob Keller <jacob.kel...@gmail.com> wrote:
> On Mon, Apr 16, 2018 at 9:07 AM, Lars Schneider
> <larsxschnei...@gmail.com> wrote:
>> What if Git kept a LRU list that contains file path, content hash, and
>> mtime of any file that is removed or modified during a checkout. If a
>> file is checked out later with the exact same path and content hash,
>> then Git could set the mtime to the previous value. This way the
>> compiler would not think that the content has been changed since the
>> last rebuild.
>
> That would only work until they actuall *did* a build on the second
> branch, and upon changing back, how would this detect that it needs to
> update mtime again? I don't think this solution really works.
> Ultimately, the problem is that the build tool relies on the mtime to
> determine what to rebuild. I think this would cause worse problems
> because we *wouldn't* rebuild in the case. How is git supposed to know
> that we rebuilt when switching branches or not?
>
> Thanks,
> Jake

I think a better solution for your problem would be to extend the
build system you're using to avoid rebuilding when the contents
haven't changed since last build (possibly by using hashes?). At the
very least, I would not want this to be default, as it could possibly
result in *no* build when there should be one, which is far more
confusing to debug.

Thanks,
Jake

Re: Optimizing writes to unchanged files during merges?

[Jacob Keller]

On Mon, Apr 16, 2018 at 9:07 AM, Lars Schneider
 wrote:
>
>> On 16 Apr 2018, at 04:03, Linus Torvalds  
>> wrote:
>>
>> On Sun, Apr 15, 2018 at 6:44 PM, Junio C Hamano  wrote:
>>>
>>> I think Elijah's corrected was_tracked() also does not care "has
>>> this been renamed".
>>
>> I'm perfectly happy with the slightly smarter patches. My patch was
>> really just an RFC and because I had tried it out.
>>
>>> One thing that makes me curious is what happens (and what we want to
>>> happen) when such a "we already have the changes the side branch
>>> tries to bring in" path has local (i.e. not yet in the index)
>>> changes.  For a dirty file that trivially merges (e.g. a path we
>>> modified since our histories forked, while the other side didn't do
>>> anything, has local changes in the working tree), we try hard to
>>> make the merge succeed while keeping the local changes, and we
>>> should be able to do the same in this case, too.
>>
>> I think it might be nice, but probably not really worth it.
>>
>> I find the "you can merge even if some files are dirty" to be really
>> convenient, because I often keep stupid test patches in my tree that I
>> may not even intend to commit, and I then use the same tree for
>> merging.
>>
>> For example, I sometimes end up editing the Makefile for the release
>> version early, but I won't *commit* that until I actually cut the
>> release. But if I pull some branch that has also changed the Makefile,
>> it's not worth any complexity to try to be nice about the dirty state.
>>
>> If it's a file that actually *has* been changed in the branch I'm
>> merging, and I'm more than happy to just stage the patch (or throw it
>> away - I think it's about 50:50 for me).
>>
>> So I don't think it's a big deal, and I'd rather have the merge fail
>> very early with "that file has seen changes in the branch you are
>> merging" than add any real complexity to the merge logic.
>
> I am happy to see this discussion and the patches, because long rebuilds
> are a constant annoyance for us. We might have been bitten by the exact
> case discussed here, but more often, we have a slightly different
> situation:
>
> An engineer works on a task branch and runs incremental builds — all
> is good. The engineer switches to another branch to review another
> engineer's work. This other branch changes a low-level header file,
> but no rebuild is triggered. The engineer switches back to the previous
> task branch. At this point, the incremental build will rebuild
> everything, as the compiler thinks that the low-level header file has
> been changed (because the mtime is different).
>
> Of course, this problem can be solved with a separate worktree. However,
> our engineers forget about that sometimes, and then, they are annoyed by
> a 4h rebuild.
>
> Is this situation a problem for others too?
> If yes, what do you think about the following approach:
>
> What if Git kept a LRU list that contains file path, content hash, and
> mtime of any file that is removed or modified during a checkout. If a
> file is checked out later with the exact same path and content hash,
> then Git could set the mtime to the previous value. This way the
> compiler would not think that the content has been changed since the
> last rebuild.

That would only work until they actuall *did* a build on the second
branch, and upon changing back, how would this detect that it needs to
update mtime again? I don't think this solution really works.
Ultimately, the problem is that the build tool relies on the mtime to
determine what to rebuild. I think this would cause worse problems
because we *wouldn't* rebuild in the case. How is git supposed to know
that we rebuilt when switching branches or not?

Thanks,
Jake

>
> I think that would fix the problem that our engineers run into and also
> the problem that Linus experienced during the merge, wouldn't it?
>
> Thanks,
> Lars

Re: Draft of Git Rev News edition 38

[Jacob Keller]

On Mon, Apr 16, 2018 at 5:29 AM, Sergey Organov <sorga...@gmail.com> wrote:
> Hi Christian,
>
> Christian Couder <christian.cou...@gmail.com> writes:
>> On Mon, Apr 16, 2018 at 12:11 AM, Christian Couder
>> <christian.cou...@gmail.com> wrote:
>>>
>>> A draft of a new Git Rev News edition is available here:
>>>
>>>   
>>> https://github.com/git/git.github.io/blob/master/rev_news/drafts/edition-38.md
>>
>> The draft has just been updated with 2 articles contributed by Jake
>> about rebasing merges, so I am cc'ing more people involved in those
>> discussions.
>
> I find this section of the draft pretty close to my own vision of what
> and how has been discussed, except for a few issues.
>
> [all quotations below are taken from the draft]
>
>> Some discussion about --preserve-merges and compatibility with scripts
>> (i.e. should we change or fix it? or should we deprecate it?)
>> followed.
>>
>>Rebasing merges: a jorney to the ultimate solution (Road Clear)
>>(written by Jacob Keller)
>
> What article by Jacob is actually meant here I have no idea, please
> check, as this one, and the RFC this refers to, was written by me, not
> by Jacob, and it is the outline of potential method of actually rebasing
> merges that is discussed in the next paragraph, so it likely belongs
> right after the next paragraph:

I believe he meant that the summary on git rev news was written by me,
that's all :)

>
>> After the discussions in the above article Sergey posted an outline of a
>> potential method for actually rebasing a merge (as opposed to recreating
>> it from scratch) which used a process of git cherry-pick -mN of the
>> merge onto each topic branch being merged, and then merging the result.
>
> The reference to:
>
> Rebasing merges: a jorney to the ultimate solution (Road Clear)
> (written by Sergey Organov)
>
> belongs here, if at all.
>
> In addition, I'd like to see a minor edition to the following:
>
>> Sergey replied that he thinks the solution produces the same result as
>> his updated strategy.
>
> This has been said in the context that assumed lack of conflicts during
> application of both strategies. Something like this, maybe:
>
> "Sergey replied that he thinks the solution produces the same result as
> his updated strategy, at least when none of the strategies produce any
> conflicts."
>
> Next, this is very close, but not exactly right:
>
>> Further suggestions to the strategy were proposed and tested, ultimately
>> resulting in Sergey proposing the addition of using the original merge
>> commit as a merge base during the final step.
>
> This was not an addition, this was a fix of particular mistake in the
> original RFC that has been revealed during testing. I didn't get it
> right at first that it's original merge commit that must be used as
> merge base, so my original proposal ended up implicitly using wrong
> merge base, that is the one computed by "git merge-base U1' U2'".
>
> Something along these lines may fit better:
>
> "Further suggestions to the strategy were proposed and tested,
> ultimately resulting in Sergey proposing the fix to his method,
> specifically using the original merge commit as a merge base during the
> final step."
>
> I'd also like a reference to the final fixed [RFC v2] be added right
> here. The reference is:
>
> https://public-inbox.org/git/87r2oxe3o1@javad.com/
>
> Thanks a lot!
>
> -- Sergey

Yep that all sounds right to me also.

Thanks,
Jake

git blame completion add completion?

[Jacob Keller]

Hi,

I use git blame sometimes to blame against specific revisions, i.e.
"git blame  -- " and it would be nice if blame could
figure out how to complete the  or revision reference..

I tried to take a stab at adding support for this, but I couldn't
figure out which completion function to use...

I want "git blame " to complete files like normal, (as this is
the most general use case), but to have "git blame xyz" to
complete revisions if there's any that match, (while still also
completing files). Additionally, it should only complete a revision
for the first argument, and go back to strict file names after.

In an *ideal* world, it'd be nice if it could figure out say:

git blame  xyz

and complete xyz as a file found in branch, not actually checking
local files but the ls-files of that branch instead. (i.e. it should
be able to complete a file that was deleted in the current checkout
but exists on that ref).

any ideas on where to start with that?

Thanks,
Jake

Re: [PATCH v6 00/15] rebase -i: offer to recreate commit topology

[Jacob Keller]

On Thu, Apr 12, 2018 at 3:02 PM, Johannes Schindelin
<johannes.schinde...@gmx.de> wrote:
> Hi Jake,
>
> On Thu, 12 Apr 2018, Jacob Keller wrote:
>
>> On Wed, Apr 11, 2018 at 10:42 PM, Sergey Organov <sorga...@gmail.com> wrote:
>> >
>> > Jacob Keller <jacob.kel...@gmail.com> writes:
>> >> On Wed, Apr 11, 2018 at 6:13 AM, Sergey Organov <sorga...@gmail.com> 
>> >> wrote:
>> >>> It was rather --recreate-merges just a few weeks ago, and I've seen
>> >>> nobody actually commented either in favor or against the
>> >>> --rebase-merges.
>> >>>
>> >>> git rebase --rebase-merges
>> >>
>> >> I'm going to jump in here and say that *I* prefer --rebase-merges, as
>> >> it clearly mentions merge commits (which is the thing that changes).
>> >
>> > OK, thanks, it's fair and the first argument in favor of
>> > --rebase-merges I see.
>>
>> I'd be ok with "--keep-merges" also.
>
> My main argument against --keep-merges is that there is no good short
> option for it: -k and -m are already taken. And I really like my `git
> rebase -kir` now...
>

Right, that's unfortunate.

> A minor argument in favor of `--rebase-merges` vs `--keep-merges` is that
> we do not really keep the merge commits, we rewrite them. In the version
> as per this here patch series, we really create recursive merges from
> scratch.

I also don't have a strong opinion in regards to --keep vs --rebase..

>
> In the later patch series on which I am working, we use a variation of
> Phillip's strategy which can be construed as a generalization of the
> cherry-pick to include merges: for a cherry-pick, we perform a 3-way merge
> between the commit and HEAD, with the commit's parent commit as merge
> base. With Phillip's strategy, we perform a 3-way merge between the merge
> commit and HEAD (i.e. the rebased first parent), with the merge commit's
> first parent as merge base, followed by a 3-way merge with the rebased
> 2nd parent (with the original 2nd parent as merge base), etc
>
> However. This strategy, while it performed well in my initial tests (and
> in Buga's initial tests, too), *does* involve more than one 3-way merge,
> and therefore it risks something very, very nasty: *nested* merge
> conflicts.

Yea, it could. Finding an elegant solution around this would be ideal!
(By elegant, I mean a solution which produces merge conflicts that
users can resolve relatively easily).

>
> Now, I did see nested merge conflicts in the past, very rarely, but that
> can happen, when two developers criss-cross merge each others' `master`
> branch and are really happy to perform invasive changes that our merge
> does not deal well with, such as indentation changes.
>
> When rebasing a merge conflict, however, such nested conflicts can happen
> relatively easily. Not rare at all.

Right. This would be true regardless of what strategy we choose, I think.

>
> I found out about this by doing what I keep preaching in this thred:
> theory is often very nice *right* until the point where it hits reality,
> and then frequently turns really useless, real quickly. Theoretical
> musings can therefore be an utter waste of time, unless accompanied by
> concrete examples.

Agreed.

>
> To start, I built on the example for an "evil merge" that I gave already
> in the very beginning of this insanely chatty thread: if one branch
> changes the signature of a function, and a second branch adds a caller to
> that function, then by necessity a merge between those two branches has to
> change the caller to accommodate the signature change. Otherwise it would
> end up in a broken state.
>
> In my `sequencer-shears` branch at https://github.com/dscho/git, I added
> this as a test case, where I start out with a main.c containing a single
> function called core(). I then create one branch where this function is
> renamed to hi(), and another branch where the function caller() is added
> that calls core(). Then I merge both, amending the merge commit so that
> caller() now calls hi(). So this is the main.c after merging:
>
> int hi(void) {
> printf("Hello, world!\n");
> }
> /* caller */
> void caller(void) {
> hi();
> }
>
> To create the kind of problems that are all too common in my daily work
> (seemingly every time some stable patch in Git for Windows gets
> upstreamed, it changes into an incompatible version, causing merge
> conflicts, and sometimes not only that... but I digress...), I then added
> an "upstream" where some maintainer decided that co

Re: [PATCH v6 15/15] rebase -i --rebase-merges: add a section to the man page

[Jacob Keller]

On Thu, Apr 12, 2018 at 2:30 AM, Johannes Schindelin
 wrote:
>> I think it would be worthwhile to point out that unlike the other commands
>> this will not preserve untracked files. Maybe something like
>> "Note that unlike the `pick` or `merge` commands or initial checkout when the
>> rebase starts the `reset` command will overwrite any untracked files."
>
> You know what? You just pointed out a bug in my thinking. Previously, I
> thought that this is impossible, that you cannot overwrite untracked files
> because we labeled this revision previously, so the only new files to
> write by `reset` were tracked files previous. But that forgets `exec` and
> `reset` with unlabeled revisions (e.g. for cousins).
>
> So I changed the `reset` command to refuse overwriting untracked files...
>
> Thank you for improving this patch series!
> Dscho

Good catch! This could possibly have bitten someone badly.

Re: [PATCH v6 00/15] rebase -i: offer to recreate commit topology

[Jacob Keller]

On Wed, Apr 11, 2018 at 10:42 PM, Sergey Organov <sorga...@gmail.com> wrote:
> Hi Jacob,
>
> Jacob Keller <jacob.kel...@gmail.com> writes:
>> On Wed, Apr 11, 2018 at 6:13 AM, Sergey Organov <sorga...@gmail.com> wrote:
>>> It was rather --recreate-merges just a few weeks ago, and I've seen
>>> nobody actually commented either in favor or against the
>>> --rebase-merges.
>>>
>>> git rebase --rebase-merges
>>>
>>
>> I'm going to jump in here and say that *I* prefer --rebase-merges, as
>> it clearly mentions merge commits (which is the thing that changes).
>
> OK, thanks, it's fair and the first argument in favor of --rebase-merges
> I see.
>

I'd be ok with "--keep-merges" also. I don't like the idea of
"flatten" as it, to me, means that anyone who wants to understand the
option without prior knowledge must immediately read the man page or
they will be confused. Something like "--rebase-merges" at least my
coworkers got it instantly. The same could be said for "--keep-merges"
too, but so far no one I asked said the immediately understood
"--no-flatten".

Thanks,
Jake

Re: [PATCH v6 00/15] rebase -i: offer to recreate commit topology

[Jacob Keller]

On Wed, Apr 11, 2018 at 6:13 AM, Sergey Organov  wrote:
> It was rather --recreate-merges just a few weeks ago, and I've seen
> nobody actually commented either in favor or against the
> --rebase-merges.
>
> git rebase --rebase-merges
>

I'm going to jump in here and say that *I* prefer --rebase-merges, as
it clearly mentions merge commits (which is the thing that changes).

I hadn't mentioned this before, because it was a suggestion that
someone else made and it seemed that Johannes liked it, so I didn't
think further discussion was worthwhile.

Thanks,
Jake

Re: [PATCH v5 0/2] *** SUBJECT HERE ***

[Jacob Keller]

On Thu, Apr 5, 2018 at 10:27 PM, Taylor Blau  wrote:
> *** BLURB HERE ***
>

Missing subject and cover letter stuff.. did you submit before you
were ready? or did you not mean to include the cover letter? :)

-Jake

> Taylor Blau (2):
>   builtin/config.c: treat type specifiers singularly
>   builtin/config.c: prefer `--type=bool` over `--bool`, etc.
>
>  Documentation/git-config.txt | 74 +++---
>  builtin/config.c | 77 +++-
>  t/t1300-repo-config.sh   | 29 ++
>  3 files changed, 121 insertions(+), 59 deletions(-)
>
> --
> 2.17.0

Re: [PATCH v2 2/4] push: colorize errors

[Jacob Keller]

On Thu, Apr 5, 2018 at 3:48 PM, Johannes Schindelin
 wrote:
> From: Ryan Dammrose 
>
> This is an attempt to resolve an issue I experience with people that are
> new to Git -- especially colleagues in a team setting -- where they miss
> that their push to a remote location failed because the failure and
> success both return a block of white text.
>
> An example is if I push something to a remote repository and then a
> colleague attempts to push to the same remote repository and the push
> fails because it requires them to pull first, but they don't notice
> because a success and failure both return a block of white text. They
> then continue about their business, thinking it has been successfully
> pushed.
>
> This patch colorizes the errors and hints (in red and yellow,
> respectively) so whenever there is a failure when pushing to a remote
> repository that fails, it is more noticeable.
>
> [jes: fixed a couple bugs, added the color.{advice,push,transport}
> settings, refactored to use want_color_stderr().]
>
> Signed-off-by: Ryan Dammrose ryandammr...@gmail.com
> Signed-off-by: Johannes Schindelin 
>
> squash! push: colorize errors
>
> Stop talking about localized errors

Guessing you intended to remove this part after squashing?

Didn't see anything else to comment on in the actual code.

Thanks,
Jake

Re: Socket activation for GitWeb FastCGI with systemd?

[Jacob Keller]

On Tue, Apr 3, 2018 at 11:53 AM, Alex Ivanov  wrote:
> Hi.
> I want to use systemd as fastcgi spawner for gitweb + nginx.
> The traffic is low and number of users is limited + traversal bots. For that 
> reason I've decided to use following mimimal services
>
> gitweb.socket
> [Unit]
> Description=GitWeb Socket
>
> [Socket]
> ListenStream=/run/gitweb.sock
> Accept=false
>
> [Install]
> WantedBy=sockets.target
>
> gitweb.service
> [Unit]
> Description=GitWeb Service
>
> [Service]
> Type=simple
> ExecStart=/path/to/gitweb.cgi --fcgi
> StandardInput=socket
>
> However this scheme is not resistant to simple DDOS.
> E.g. traversal bots often kill the service by opening non existing path (e.g 
> http://host/?p=repo;a=blob;f=nonexisting/path;hb=HEAD showing in browser 404 
> - Cannot find file) many times consecutively, which leads to
> Apr 03 21:32:10 host systemd[1]: gitweb.service: Start request repeated too 
> quickly.
> Apr 03 21:32:10 host systemd[1]: gitweb.service: Failed with result 
> 'start-limit-hit'.
> Apr 03 21:32:10 host systemd[1]: Failed to start GitWeb service.
> and 502 Bad Gateway in browser. I believe the reason is that gitweb.service 
> dies on failure and if it happens too often, systemd declines to restart the 
> service due to start limit hit.
> So my question is how to correct systemd services for GitWeb to be resistant 
> to such issue? I prefer to use single process to process all clients.
> Thanks.

This sounds like a systemd specific question that might get a better
answer from the systemd mailing list.

That being said, I believe if in this case gitweb is dying due to the
path not existing? You might be able to configure systemd to
understand that the particular exit code for when the path doesn't
exist is a "valid" exit, and not a failure case..

I'm not entirely understanding your goal.. you want each request to
launch the gitweb process, and when it's done you want it to exit? But
if there are multiple connections at once you want it to stay alive
until it services them all? I think the best answer is configure
systemd to understand that the exit code for when the path is invalid
will be counted as a success.

Thanks,
Jake

Re: [RFC PATCH 0/7] Moved code detection: ignore space on uniform indentation

[Jacob Keller]

On Tue, Apr 3, 2018 at 12:00 PM, Stefan Beller  wrote:
 The fun is in the last patch, which allows white space sensitive
 languages to trust the move detection, too. Each block that is marked as
 moved will have the same delta in {in-, de-}dentation.
 I would think this mode might be a reasonable default eventually.
>>>
>>> This sounds like a good idea. "Trust" is probably too strong a word, but
>>> I can see this being useful even in non-whitespace-sensitive languages
>>> with nested blocks (like C).
>>
>> The ability to detect moved code despite whitespace changes would be
>> good, even while showing diffs with the whitespace intact.
>
> That is what the last patch is about.

Right. I was trying to say "I agree with the goal, even if we don't
necessarily allow every possible white-space + color-moved lines
combination" (i.e. to avoid polluting the option space too much)

Thanks,
Jake

Re: [RFC PATCH 0/7] Moved code detection: ignore space on uniform indentation

[Jacob Keller]

On Mon, Apr 2, 2018 at 4:47 PM, Jonathan Tan  wrote:
> On Mon,  2 Apr 2018 15:48:47 -0700
> Stefan Beller  wrote:
>
>> This is a re-attempt of [1], which allows the moved code detection to
>> ignore blanks in various modes.
>>
>> patches 1-5 are refactoring, patch 6 adds all existing white space options
>> of regular diff to the move detection. (I am unsure about this patch,
>> as I presume we want to keep the option space at a minimum if possible).
>
> My preference is to not do this until a need has been demonstrated, but
> this sounds like it could be useful one day. I'll review the patches
> from the viewpoint that we do want this feature.
>
>> The fun is in the last patch, which allows white space sensitive
>> languages to trust the move detection, too. Each block that is marked as
>> moved will have the same delta in {in-, de-}dentation.
>> I would think this mode might be a reasonable default eventually.
>
> This sounds like a good idea. "Trust" is probably too strong a word, but
> I can see this being useful even in non-whitespace-sensitive languages
> with nested blocks (like C).

The ability to detect moved code despite whitespace changes would be
good, even while showing diffs with the whitespace intact.

We may not need *all* the options available, but allowing the internal
settings to enable this but have the user-visible controls be limited
should be totally fine.

Thanks,
Jake

Re: Draft of Git Rev News edition 37

[Jacob Keller]

On Fri, Mar 30, 2018 at 2:02 AM, Johannes Schindelin
<johannes.schinde...@gmx.de> wrote:
> Hi,
>
> On Mon, 19 Mar 2018, Christian Couder wrote:
>
>> On Sun, Mar 18, 2018 at 11:41 PM, Jacob Keller <jacob.kel...@gmail.com> 
>> wrote:
>> >
>> > I don't have a good summary yet, but I think a section about the
>> > discussion regarding the new recreate-merges and rebasing merges
>> > that's been on going might be useful?
>>
>> Yeah sure, we would gladly accept a summary of this discussion.
>
> I would *love* a summary of that discussion, especially since it got
> pretty unwieldy (and partially out of hand, but that part probably does
> not need a lot of detail apart from the adjective "heated").
>
>> > a lot of that discussion occurred prior to git-merge (tho it's been
>> > ongoing since then?).
>>
>> If you want to take the latest discussions into account, the summary
>> could be either split into two parts, one for this edition and the
>> other one for the next edition. Or we could wait and have the whole
>> summary in the next edition.
>
> Jake, I do not know about your availability, but I would love it if you
> could take a stab, as I trust you to be unbiased. I would not trust myself
> to be unbiased because (as everybody saw who cared to read) I got a little
> bit too emotional and should have stayed more professional.
>
> Thanks,
> Dscho

I hope to be able to make a summary of the discussion as best I can.
It may take a bit as there is a lot of mails to read. I agree that a
good summary should come from someone outside the discussion to reduce
emotional bias.

Thanks,
Jake

Re: [PATCH] branch -l: print useful info whilst rebasing a non-local branch

[Jacob Keller]

On Mon, Mar 26, 2018 at 12:44 AM, Eric Sunshine <sunsh...@sunshineco.com> wrote:
> On Mon, Mar 26, 2018 at 3:25 AM, Jeff King <p...@peff.net> wrote:
>> OK, so here's some patches. We could do the first three now, wait a
>> while before the fourth, and then wait a while (or never) on the fifth.
>>
>>   [1/5]: t3200: unset core.logallrefupdates when testing reflog creation
>>   [2/5]: t: switch "branch -l" to "branch --create-reflog"
>>   [3/5]: branch: deprecate "-l" option
>>   [4/5]: branch: drop deprecated "-l" option
>>   [5/5]: branch: make "-l" a synonym for "--list"
>
> The entire series looks good to me. FWIW,
>
> Reviewed-by: Eric Sunshine <sunsh...@sunshineco.com>

Same to me.

Reviewed-by: Jacob Keller <jacob.kel...@gmail.com>

Thanks,
Jake

Re: Should I try to fix rebase interactive preserve-merges bug

[Jacob Keller]

On Sun, Mar 25, 2018 at 10:32 AM, Wink Saville  wrote:
> There is a "TODO known breakage" in t3404-rebase-interactve.sh:
>
>not ok 24 - exchange two commits with -p # TODO known breakage
>
> I'm contemplating trying to fix it. But with --recreate-merges coming
> maybe it's not worth the effort. Should I proceed with attempting a
> fix or is --preserve-merges going to be a synonym for
> --recreate-merges?
>
> -- Wink

AFAIK this breakage of preserve-merges is a design flaw which isn't
really fixable, which is why --recreate-merges is being added.

I believe the plan is to deprecate preserve-merges once
recreate-merges has landed.

Thanks,
Jake

Re: [PATCH] branch -l: print useful info whilst rebasing a non-local branch

[Jacob Keller]

On Sat, Mar 24, 2018 at 9:33 PM, Jeff King  wrote:
> IMHO we should do one of:
>
>   1. Nothing. ;)
>
>   2. Complain about "-l" in list mode to help educate users about the
>  current craziness.
>

I think we should do this at a minimum. It's easy, and it doesn't
break any scripts who are doing something sane.

>   3. Drop "-l" (probably with a deprecation period); it seems unlikely
>  to me that anybody uses it for branch creation, and this would at
>  least reduce the confusion (then it would just be "so why don't we
>  have -l" instead of "why is -l not what I expect").

Personally, I'd prefer this, because it's minimal effort on scripts
part to fix themselves to use the long option name for reflog, and
doesn't cause that much heart burn.

>
>   4. Repurpose "-l" as a shortcut for --list (also after a deprecation
>  period). This is slightly more dangerous in that it may confuse
>  people using multiple versions of Git that cross the deprecation
>  line. But that's kind of what the deprecation period is for...
>
> -Peff

I don't think this is particularly all that valuable, since we default
to list mode so it only helps if you want to pass an argument to the
list mode (since otherwise we'd create a branch). Maybe it could be
useful, but if we did it, I'd do it as a sort of double deprecation
period where we use one period to remove the -l functionality
entirely, before adding anything back. I think the *gain* of having -l
is not really worth it though.

Regards,
Jake

Re: query on git submodule (ignore)

[Jacob Keller]

On Sat, Mar 24, 2018 at 7:17 PM, prashant Nidgunde
 wrote:
> Hello,
>
> I am new to this community ,so please ignore if I am asking anything silly.
>
> Case :
> Today when I built my submodule , and did a git status , it shows as modified.
>
> After reading certain suggestions on web i found out that i can ignore
> that adding a line in .gitmodules
>
> But, I had to add that line manually ( which could be errorprone
> because of typos )
>
>
> Question:
> 1. Is it feasible to build a feature like :
>git submodule "zlib" ignore dirty ( which will
> ignore submodule zlib when its built and dirty  as it has new files in
> its directory)
>
> If this feature is feasible , how do i know if its developed  (
> awaiting merge ) or can I build the patch ?
>

I don't recall such a feature, but I'm sure patches to implement
something would be welcome to be reviewed! (For more information about
submitting patches you could read
https://github.com/git/git/blob/master/Documentation/SubmittingPatches)

I think having an option to automatically write this would be useful.
It may already be possible to do something similar via the git config
command with the -f file argument to edit the .gitmodules file (as it
uses the gitconfig format for its contents). However, this is
definitely not intuitive.

You can read the documentation for the commands using "git help
submodule" and "git help config". Patches are also definitely welcome
for updates to the documentation if it's not clear.


I know also that having a simpler interface to set submodules up so
that they are treated as unchanged would be useful as I have projects
at $dayjob which use submodules, and this is often a complaint against
their use by my co-workers (who sometimes then accidentally commit
re-wind updates to the submodules due to inattentiveness with use of
git add . or git commit -a).

Thanks,
Jake

Re: Draft of Git Rev News edition 37

[Jacob Keller]

On Sun, Mar 18, 2018 at 11:21 AM, Christian Couder
 wrote:
> Hi,
>
> A draft of a new Git Rev News edition is available here:
>
>   
> https://github.com/git/git.github.io/blob/master/rev_news/drafts/edition-37.md
>
> Everyone is welcome to contribute in any section either by editing the
> above page on GitHub and sending a pull request, or by commenting on
> this GitHub issue:
>
>   https://github.com/git/git.github.io/issues/279
>
> You can also reply to this email.
>
> In general all kinds of contribution, for example proofreading,
> suggestions for articles or links, help on the issues in GitHub, and
> so on, are very much appreciated.
>
> I tried to cc everyone who appears in this edition, but maybe I missed
> some people, sorry about that.
>
> Jakub, Markus, Gabriel and me plan to publish this edition on
> Wednesday March 21st.
>
> Thanks,
> Christian.

I don't have a good summary yet, but I think a section about the
discussion regarding the new recreate-merges and rebasing merges
that's been on going might be useful? a lot of that discussion
occurred prior to git-merge (tho it's been ongoing since then?).

Thanks,
Jake

Re: [PATCH 0/2] routines to generate JSON data

[Jacob Keller]

On Fri, Mar 16, 2018 at 2:18 PM, Jeff King  wrote:
>   3. Some other similar format. YAML comes to mind. Last time I looked
>  (quite a while ago), it seemed insanely complex, but I think you
>  could implement only a reasonable subset. OTOH, I think the tools
>  ecosystem for parsing JSON (e.g., jq) is much better.
>

I would personally avoid YAML. It's "easier" for humans to read/parse,
but honestly JSON is already simple enough and anyone who writes C or
javascript can likely parse and hand-write JSON anyways. YAML lacks
built-in parsers for most languages, where as many scripting languages
already have JSON parsing built in, or have more easily attainable
libraries available. In contrast, the YAML libraries are much more
complex and less likely to be available.

That's just my own experience at $dayjob though.

Thanks,
Jake

Re: [RFC] Rebasing merges: a jorney to the ultimate solution(RoadClear)

[Jacob Keller]

On Thu, Mar 8, 2018 at 3:20 AM, Phillip Wood  wrote:
> On 07/03/18 07:26, Johannes Schindelin wrote:
>> Hi Buga,
>>
>> On Tue, 6 Mar 2018, Igor Djordjevic wrote:
>>
>>> On 06/03/2018 19:12, Johannes Schindelin wrote:

>> And I guess being consistent is pretty important, too - if you add new
>> content during merge rebase, it should always show up in the merge,
>> period.
>
> Yes, that should make it easy for the user to know what to expect from
> rebase.

 [...]

 It will be slightly inconsistent. But in a defendable way, I think.
>>>
>>> I like where this discussion is heading, and here`s what I thought
>>> about it :)
>>>
>>> [...]
>>>
>>> Here`s a twist - not letting `merge` trying to be too smart by
>>> figuring out whether passed arguments correspond to rewritten
>>> versions of the original merge parents (which would be too
>>> restrictive, too, I`m afraid), but just be explicit about it, instead!
>>
>> That's the missing piece, I think.
>>
>>> So, it could be something like:
>>>
>>>  merge -C deadbee 123abc:cafecafe 234bcd:bedbedbed
>>
>> I like where this is heading, too, but I do not think that we can do this
>> on a per-MERGE_HEAD basis. The vast majority of merge commits, in
>> practice, have two parents. So the `merge` command would actually only
>> have one revision to merge (because HEAD is the implicit first parent). So
>> that is easy.
>>
>> But as soon as you go octopus, you can either perform an octopus merge, or
>> rebase the original merge commit. You cannot really mix and match here.
>>
>> Unless we reimplement the octopus merge (which works quite a bit
>> differently from the "rebase merge commit" strategy, even if it is
>> incremental, too), which has its own challenges: if there are merge
>> conflicts before merging the last MERGE_HEAD, the octopus merge will exit
>> with status 2, telling you "Should not be doing an octopus.". While we
>> will want to keep merge conflict markers and continue with the "rebase the
>> original merge commit" strategy.
>>
>> And it would slam the door shut for adding support for *other* merge
>> strategies to perform a more-than-two-parents merge.
>>
>> Also, I do not think that it makes a whole lot of sense in practice to let
>> users edit what will be used for "original parent". If the user wants to
>> do complicated stuff, they can already do that, via `exec`. The `merge`
>> command really should be about facilitating common workflows, guiding the
>> user to what is sane.
>>
>> Currently my favorite idea is to introduce a new flag: -R (for "rebase the
>> original merge commit"). It would look like this:
>>
>>   merge -R -C   # 
>>
>> This flag would of course trigger the consistency check (does the number
>> of parents of the original merge commit agree with the parameter list? Was
>> an original merge commit specified to begin with?), and it would not fall
>> back to the recursive merge, but error out if that check failed.
>>
>> Side note: I wonder whether we really need to perform the additional check
>> that ensures that the  refers to the rewritten version of the
>> original merge commit's parent.
>>
>> Second side note: if we can fast-forward, currently we prefer that, and I
>> think we should keep that behavior with -R, too.
>
> I think that would be a good idea to avoid unpleasant surprises.
>
>> If the user wants to force a new merge, they simply remove that -R flag.
>>
>> What do you think?
>
> I did wonder about using 'pick ' for rebasing merges and
> keeping 'merge ...' for recreating them but I'm not sure if that is a
> good idea. It has the advantage that the user cannot specify the wrong
> parents for the merge to be rebased as 'git rebase' would work out if
> the parents have been rebased, but maybe it's a bit magical to use pick
> for merge commits. Also there isn't such a simple way for the user to go
> from 'rabase this merge' to 'recreate this merge' as they'd have to
> write the whole merge line themselves (though I guess something like
> emacs' git-rebase.el would be able to help with that)
>
> Best Wishes
>
> Phillip
>

Since the ultimate commit hashes of newly rebased commits would be
unknown at the time of writing the todo file, I'm not sure how this
would work to specify the parents?

>
>> Ciao,
>> Dscho
>>
>

Re: [RFC] Rebasing merges: a jorney to the ultimate solution(RoadClear)

[Jacob Keller]

On Fri, Mar 2, 2018 at 4:36 AM, Phillip Wood  wrote:
> On 02/03/18 11:17, Phillip Wood wrote:
>>
>> On 02/03/18 01:16, Igor Djordjevic wrote:
>>>
>>> Hi Sergey,
>>>
>>> On 01/03/2018 06:39, Sergey Organov wrote:

>> (3) ---X1---o---o---o---o---o---X2
>>|\   |\
>>| A1---A2---A3---U1  | A1'--A2'--A3'--U1'
>>| \  |
>>|  M |
>>| /  |
>>\-B1---B2---B3---U2  \-B1'--B2'--B3'--U2'
>>
>
> Meh, I hope I`m rushing it now, but for example, if we had decided to
> drop commit A2 during an interactive rebase (so losing A2' from
> diagram above), wouldn`t U2' still introduce those changes back, once
> U1' and U2' are merged, being incorrect/unwanted behavior...? :/
>
> [...]

 Yeah, I now see it myself. I'm sorry for being lazy and not inspecting
 this more carefully in the first place.
>>>
>>> No problem, that`s why we`re discussing it, and I`m glad we`re
>>> aligned now, so we can move forward :)
>>>
> So while your original proposal currently seems like it could be
> working nicely for non-interactive rebase (and might be some simpler
> interactive ones), now hitting/acknowledging its first real use
> limit, my additional quick attempt[1] just tries to aid pretty
> interesting case of complicated interactive rebase, too, where we
> might be able to do better as well, still using you original proposal
> as a base idea :)

 Yes, thank you for pushing me back to reality! :-) The work and thoughts
 you are putting into solving the puzzle are greatly appreciated!
>>>
>>> You`re welcome, and I am enjoying it :)
>>>
 Thinking about it overnight, I now suspect that original proposal had a
 mistake in the final merge step. I think that what you did is a way to
 fix it, and I want to try to figure what exactly was wrong in the
 original proposal and to find simpler way of doing it right.

 The likely solution is to use original UM as a merge-base for final
 3-way merge of U1' and U2', but I'm not sure yet. Sounds pretty natural
 though, as that's exactly UM from which both U1' and U2' have diverged
 due to rebasing and other history editing.
>>>
>>> Yes, this might be it...! ;)
>>>
>>> To prove myself it works, I`ve assembled a pretty crazy `-s ours`
>>> merge interactive rebase scenario, and it seems this passes the test,
>>> ticking all the check boxes (I could think of) :P
>
> Hi Igor
>
>> It is interesting to think what it means to faithfully rebase a '-s
>> ours' merge.
> I should have explained that I mean is a faithful rebase one that
> adheres to the semantics of '-s ours' (i.e. ignores any changes in the
> side branch) or one that merges new changes from the side branch into
> the content of the original merge? In your example you add B4 to B. If
> M' preserves the semantics of '-s ours' then it will not contain the
> changes in B4. I think your result does (correct me if I'm wrong) so it
> is preserving the content of the original merge rather than the
> semantics of it.
>
> Best Wishes
>
> Phillip
>

I believe this was always the outline of this type of approach, as per
Sergey's original email.

We only have the content, and we don't know the semantics (nor, I
think, should we attempt to understand or figure out the semantics).

Regards,
Jake

Re: [RFC] Rebasing merges: a jorney to the ultimate solution(RoadClear)

[Jacob Keller]

On Fri, Mar 2, 2018 at 3:17 AM, Phillip Wood  wrote:
>
> It is interesting to think what it means to faithfully rebase a '-s
> ours' merge. In your example the rebase does not introduce any new
> changes into branch B that it doesn't introduce to branch A. Had it
> added a fixup to branch B1 for example or if the topology was more
> complex so that B ended up with some other changes that the rebase did
> not introduce into A, then M' would contain those extra changes whereas
> '--recreate-merges' with '-s ours' (once it supports it) would not.
>

Unless the method of merging was stored, I don't think we *can*
correctly automate resolving of "-s ours" because all we store is the
resulting content, and we don't know how or why the user generated it
as such. I believe the "correct" solution in any case would be to take
the content we DO know and then ask the user to stop for amendments.

Thanks,
Jake

Re: The case for two trees in a commit ("How to make rebase less modal")

[Jacob Keller]

On Wed, Feb 28, 2018 at 3:30 PM, Stefan Beller  wrote:
> $ git hash-object --stdin -w -t commit < tree c70b4a33a0089f15eb3b38092832388d75293e86
> parent 105d5b91138ced892765a84e771a061ede8d63b8
> author Stefan Beller  1519859216 -0800
> committer Stefan Beller  1519859216 -0800
> tree 5495266479afc9a4bd9560e9feac465ed43fa63a
> test commit
> EOF
> 19abfc3bf1c5d782045acf23abdf7eed81e16669
> $ git fsck |grep 19abfc3bf1c5d782045acf23abdf7eed81e16669
> $
>
> So it is technically possible to create a commit with two tree entries
> and fsck is not complaining.
>
> But why would I want to do that?
>
> There are multiple abstraction levels in Git, I think of them as follows:
> * data structures / object model
> * plumbing
> * porcelain commands to manipulate the repo "at small scale", e.g.
> create a commit/tag
> * porcelain to modify the repo "at larger scale", such as rebase,
> cherrypicking, reverting
>   involving more than 1 commit.
>
> These large scale operations involving multiple commits however
> are all modal in its nature. Before doing anything else, you have to
> finish or abort the rebase or you need expert knowledge how to
> go otherwise.
>
> During the rebase there might be a hard to resolve conflict, which
> you may not want to resolve right now, but defer to later.  Deferring a
> conflict is currently impossible, because precisely one tree is recorded.
>

How does this let you defer a conflict? A future commit which modified
blobs in that tree wouldn't know what version of the trees/blobs to
actually use? Clearly future commits could record their own trees, but
how would they generate the "correct" tree?

Maybe I am missing something here?

Thanks,
Jake

> If we had multiple trees possible in a commit, then all these large scale
> operations would stop being modal and you could just record the unresolved
> merge conflict instead; to come back later and fix it up later.
>
> I'd be advocating for having multiple trees in a commit
> possible locally; it might be a bad idea to publish such trees.
>
> Opinions or other use cases?
>
> Thanks,
> Stefan

Re: [RFC] Rebasing merges: a jorney to the ultimate solution (Road Clear)

[Jacob Keller]

On Tue, Feb 27, 2018 at 3:40 PM, Igor Djordjevic
 wrote:
> On 27/02/2018 20:59, Igor Djordjevic wrote:
>>
>> (3) ---X1---o---o---o---o---o---X2
>>|\   |\
>>| A1---A2---A3---U1  | A1'--A2'--A3'--U1'
>>| \  |
>>|  M |
>>| /  |
>>\-B1---B2---B3---U2  \-B1'--B2'--B3'--U2'
>>
>
> Meh, I hope I`m rushing it now, but for example, if we had decided to
> drop commit A2 during an interactive rebase (so losing A2' from
> diagram above), wouldn`t U2' still introduce those changes back, once
> U1' and U2' are merged, being incorrect/unwanted behavior...? :/
>
> p.s. Looks like Johannes already elaborated on this in the meantime,
> let`s see... (goes reading that other e-mail[1])
>
> [1] 
> https://public-inbox.org/git/nycvar.qro.7.76.6.1802272330290...@zvavag-6oxh6da.rhebcr.pbec.zvpebfbsg.pbz/


In that case, the method won't work well at all, so I think we need a
different approach.

Thanks,
Jake

Re: [RFC] Rebasing merges: a jorney to the ultimate solution (Road Clear)

[Jacob Keller]

On Tue, Feb 27, 2018 at 10:14 AM, Junio C Hamano  wrote:
> Sergey Organov  writes:
>
>> You've already bit this poor thingy to death. Please rather try your
>> teeth on the proposed Trivial Merge (TM) method.
>
> Whatever you do, do *NOT* call any part of your proposal "trivial
> merge", unless you are actually using the term to mean what Git
> calls "trivial merge".  The phrase has an established meaning in Git
> and your attempt to abuse it to mean something entirely different is
> adding unnecessary hindrance for other people to understand what you
> want to perform.

Agreed, I think we need better terminology here, the current words for
(TM) are definitely *not* trivial merges. Same for "angel merge", I
don't think that term really works well either.

The goal of the process is to split the merge apart to its components
for each side branch and then bring them back together after applying
them to the newly rebased branches.

Re: [RFC] Rebasing merges: a jorney to the ultimate solution (Road Clear)

[Jacob Keller]

On Tue, Feb 27, 2018 at 8:21 AM, Johannes Schindelin
<johannes.schinde...@gmx.de> wrote:
> Hi Jake,
>
> On Mon, 26 Feb 2018, Jacob Keller wrote:
>
>> On Mon, Feb 26, 2018 at 4:07 PM, Johannes Schindelin
>> <johannes.schinde...@gmx.de> wrote:
>> >
>> > On Tue, 20 Feb 2018, Igor Djordjevic wrote:
>> >
>> >> I`m really interested in this topic, which seems to (try to) address the
>> >> only "bad feeling" I had with rebasing merges - being afraid of silently
>> >> losing amendments by actually trying to "replay" the merge (where
>> >> additional and possibly important context is missing), instead of really
>> >> "rebasing" it (somehow).
>> >
>> > If those amendments are what you are worried about, why not address them
>> > specifically?
>> >
>> > In other words, rather than performing the equivalent of
>> >
>> > git show ^! | git apply
>> >
>> > (which would of course completely ignore if the rewritten ^2
>> > dropped a patch, amended a patch, or even added a new patch), what you
>> > really want is to figure out what changes the user made when merging, and
>> > what merge strategy was used to begin with.
>> >
>> > To see what I mean, look at the output of `git show 0fd90daba8`: it shows
>> > how conflicts were resolved. By necessity, this is more complicated than a
>> > simple diff: it is *not* as simple as taking a diff between two revisions
>> > and applying that diff to a third revision. There were (at least) three
>> > revisions involved in the original merge commit, and recreating that merge
>> > commit faithfully means to represent the essence of the merge commit
>> > faithfully enough to be able to replay it on a new set of at least three
>> > revisions.  That can be simplified to two-way diffs only in very, very
>> > special circumstances, and in all other cases this simplification will
>> > simply fall on its nose.
>> >
>> > If the proposed solution was to extend `git apply` to process combined
>> > diffs, I would agree that we're on to something. That is not the proposed
>> > solution, though.
>> >
>> > In short: while I am sympathetic to the desire to keep things simple,
>> > the idea to somehow side-step replaying the original merge seems to be
>> > *prone* to be flawed. Any system that cannot accommodate
>> > dropped/changed/added commits on either side of a merge is likely to be
>> > too limited to be useful.
>> >
>>
>>
>> The reason Sergey's solution works is because he cherry picks the
>> merge using each parent first, and then merges the result of those. So
>> each branch of the merge gets one, and then you merge the result of
>> those cherry-picks. This preservers amendments and changes properly,
>> and should result in a good solution.
>
> I saw your patch trying to add a minimal example, and I really want to run
> away screaming.
>
> Do you have any way to describe the idea in a simple, 3-5 lines long
> paragraph?
>
> So far, I just know that it is some sort of confusing criss-cross
> cherry-picking and merging and stuff, but nothing in those steps shouts
> out to me what the *idea* is.
>

Sergey's posted explained it more in detail, at
https://public-inbox.org/git/87y3jtqdyg@javad.com/

I was mostly just attempting to re-create it in a test case to show
that it could work.

> If it would be something like "recreate the old merge, with merge
> conflicts and all, then generate the diff to the actual tree of the merge
> commit, then apply that to the newly-generated merge", I would understand.
>

It's more or less:

Rebase each parent, then cherry-pick -m the original merge to that
parent, then you merge the result of each cherry-pick, then use the
resulting final merged tree to create the merge pointing at the real
parents instead of the cherry-pick merges.

> I would still suspect that -s ours would be a hard nut for that method,
> but I would understand that idea.
>

The goal of the process isn't to know or understand the "-s ours"
strategy, but simply re-create the contents of the original merge
faithfully, while still preserving the changes done when rebasing the
side branches. Thus it should re-create the contents generated by "-s
ours" the first time, but it doesn't need to do or know anything
special about how the content was created.

> Thanks,
> Dscho

[PATCH] test case highlighting possible strategy to rebase merge

[Jacob Keller]

Add a test case highlighting how the process outlined by Sergey Organov
could be used to re-create merges without losing any context.

The test currently only uses new files, with the exception of one part
involving an evil merge. Unfortunately I'm not sure how to represent
merge conflicts that need to be handled. It looks like it should when
there is enough context that the changes don't have direct textual
conflict, but I was unable to figure out how to produce such a change
directly in the test case.

I think this method shows a lot of promise, and is worth investigating
more to see if we can actually make it work in practice without
requiring separate steps.

Signed-off-by: Jacob Keller <jacob.kel...@gmail.com>
---
 t/t7900-rebasing-merge.sh | 178 ++
 1 file changed, 178 insertions(+)
 create mode 100755 t/t7900-rebasing-merge.sh

diff --git a/t/t7900-rebasing-merge.sh b/t/t7900-rebasing-merge.sh
new file mode 100755
index 000..7639425
--- /dev/null
+++ b/t/t7900-rebasing-merge.sh
@@ -0,0 +1,178 @@
+#!/bin/sh
+
+test_description='test showcasing safe method to rebase a merge'
+
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+   echo "file1" >a &&
+   echo "file2" >b &&
+   test_tick &&
+   git add a b &&
+   git commit -m "initial" &&
+   git branch initial &&
+   git branch branch1 &&
+   git branch branch2 &&
+   git checkout branch1 &&
+   test_tick &&
+   echo "a change in file1" >>a &&
+   git add a &&
+   git commit -m "commit1" &&
+   test_tick &&
+   echo "another change in file1" >>a &&
+   git add a &&
+   git commit -m "commit2" &&
+   git checkout branch2 &&
+   test_tick &&
+   echo "a change in file2" >>b &&
+   git add b &&
+   git commit -m "commit3" &&
+   git checkout -B simple-merge branch1 &&
+   git merge -m "simple merge branch2 into branch1" branch2 &&
+   cat >expected <<-\EOF &&
+   file1
+   a change in file1
+   another change in file1
+   EOF
+   test_cmp expected a &&
+   cat >expected <<-\EOF &&
+   file2
+   a change in file2
+   EOF
+   test_cmp expected b
+'
+
+test_expect_success 'trivial rebase case' '
+   git checkout -B new-initial initial &&
+   echo "newfile3" >c &&
+   test_tick &&
+   git add c &&
+   git commit -m "a new file" &&
+   git checkout -B new-branch1 branch1 &&
+   test_tick &&
+   git rebase new-initial &&
+   test_tick &&
+   git cherry-pick -m1 --keep-redundant-commits simple-merge &&
+   git checkout -B new-branch2 branch2 &&
+   test_tick &&
+   git rebase new-initial &&
+   test_tick &&
+   git cherry-pick -m2 --keep-redundant-commits simple-merge &&
+   git checkout -B rebased-simple-merge new-branch1 &&
+   git merge --no-ff -m "rebased simple merge new-branch2 into 
new-branch1" new-branch2 &&
+   git reset --soft HEAD^ &&
+   git update-ref refs/heads/rebased-simple-merge "$(git commit-tree -m 
"finalized rebased simple merge new-branch2 into new-branch1" "$(git 
write-tree)" -p "$(git rev-parse new-branch1^)" -p "$(git rev-parse 
new-branch2^)")" &&
+   git checkout rebased-simple-merge &&
+   cat >expected <<-\EOF &&
+   file1
+   a change in file1
+   another change in file1
+   EOF
+   test_cmp expected a &&
+   cat >expected <<-\EOF &&
+   file2
+   a change in file2
+   EOF
+   test_cmp expected b &&
+   cat >expected <<-\EOF &&
+   newfile3
+   EOF
+   test_cmp expected c
+'
+
+test_expect_success 'evil merge rebase case' '
+   git checkout -B evil-merge simple-merge &&
+   test_tick &&
+   echo "evil change to file2" >>b &&
+   git add b &&
+   git commit --amend -m "evil mergbe branch2 into branch1" &&
+   git checkout -B new-initial initial &&
+   echo "newfile3" >c &&
+   test_tick &&
+   git add c &&
+   git commit -m "a new file" &&
+   git checkout -B new-branch1 branch1 &&
+   test_tick &&
+   git rebase new-initial &&
+   test_

Re: [RFC] Rebasing merges: a jorney to the ultimate solution (Road Clear)

[Jacob Keller]

On Mon, Feb 26, 2018 at 4:07 PM, Johannes Schindelin
 wrote:
> Hi Buga,
>
> On Tue, 20 Feb 2018, Igor Djordjevic wrote:
>
>> I`m really interested in this topic, which seems to (try to) address the
>> only "bad feeling" I had with rebasing merges - being afraid of silently
>> losing amendments by actually trying to "replay" the merge (where
>> additional and possibly important context is missing), instead of really
>> "rebasing" it (somehow).
>
> If those amendments are what you are worried about, why not address them
> specifically?
>
> In other words, rather than performing the equivalent of
>
> git show ^! | git apply
>
> (which would of course completely ignore if the rewritten ^2
> dropped a patch, amended a patch, or even added a new patch), what you
> really want is to figure out what changes the user made when merging, and
> what merge strategy was used to begin with.
>
> To see what I mean, look at the output of `git show 0fd90daba8`: it shows
> how conflicts were resolved. By necessity, this is more complicated than a
> simple diff: it is *not* as simple as taking a diff between two revisions
> and applying that diff to a third revision. There were (at least) three
> revisions involved in the original merge commit, and recreating that merge
> commit faithfully means to represent the essence of the merge commit
> faithfully enough to be able to replay it on a new set of at least three
> revisions.  That can be simplified to two-way diffs only in very, very
> special circumstances, and in all other cases this simplification will
> simply fall on its nose.
>
> If the proposed solution was to extend `git apply` to process combined
> diffs, I would agree that we're on to something. That is not the proposed
> solution, though.
>
> In short: while I am sympathetic to the desire to keep things simple,
> the idea to somehow side-step replaying the original merge seems to be
> *prone* to be flawed. Any system that cannot accommodate
> dropped/changed/added commits on either side of a merge is likely to be
> too limited to be useful.
>


The reason Sergey's solution works is because he cherry picks the
merge using each parent first, and then merges the result of those. So
each branch of the merge gets one, and then you merge the result of
those cherry-picks. This preservers amendments and changes properly,
and should result in a good solution.

I agree that making "git apply" work with combined diffs could also be
another solution, but it may be trickier.

If this *doesn't* work, a test case showing that it doesn't work would
be appreciated. I'm hoping to be able to put together something soon,
but I haven't had time due to $dayjob.

> Ciao,
> Johannes

Re: [PATCH v4 00/12] rebase -i: offer to recreate merge commits

[Jacob Keller]

On Fri, Feb 23, 2018 at 4:35 AM, Johannes Schindelin
 wrote:
> Changes since v3:
>
> - fixed a grammar error in "introduce the `merge` command"'s commit message.
>
> - fixed a couple of resource leaks in safe_append() and do_reset(), pointed
>   out by Eric Sunshine.
>

The interdiff seems incorrect for such a small list of changes, it
appears like large sections of code added by this series appear in the
interdiff without subtractions from the previous versions? Is all that
code new to v3? If not, I'd suspect you accidentally diffed between
the wrong points.

Thanks,
Jake

Re: Git should preserve modification times at least on request

[Jacob Keller]

On Tue, Feb 20, 2018 at 2:05 PM, Peter Backes  wrote:
> Hi Jeff,
>
> On Tue, Feb 20, 2018 at 04:16:34PM -0500, Jeff King wrote:
>> I think there are some references buried somewhere in that wiki, but did
>> you look at any of the third-party tools that store file metadata
>> alongside the files in the repository? E.g.:
>>
>>   https://etckeeper.branchable.com/
>>
>> or
>>
>>   https://github.com/przemoc/metastore
>>
>> I didn't see either of those mentioned in this thread (though I also do
>> not have personal experience with them, either).
>>
>> Modification times are a subset of the total metadata you might care
>> about, so they are solving a much more general problem. Which may also
>> partially answer your question about why this isn't built into git. The
>> general problem gets much bigger when you start wanting to carry things
>> like modes (which git doesn't actually track; we really only care about
>> the executable bit) or extended attributes (acls, etc).
>
> I know about those, but that's not what I am looking for. Those tools
> serve entirely different purposes, ie., tracking file system changes.
> I, however, am specifically interested in version control.
>
> In version control, the user checks out his own copy of the tree for
> working. For this purpose, it is thus pointless to track ownership,
> permissions (except for the x bit), xattrs, or any other metadata. In
> fact, it can be considered the wrong thing to do.
>
> The modification time, however, is special. It clearly has its place in
> version control. It tells us when the last modification was actually
> done to the file. I am often working on some feature, and one part is
> finished and is lying around, but I am still working on other parts in
> other files. Then, maybe after some weeks, the other parts are
> finished. Now, when committing, the information about modification time
> is lost. Maybe some weeks later I want to figure out when I last
> modified those files that were committed. But that information is now
> gone, at least in the git repository. Sure, I could do lots of WIP
> commits, but this would clutter up the history unneccessarly and I
> would have lots of versions that might not even compile, let alone run.

You could have git figure this out by the commit time of the last
commit which modified a file. This gets a bit weird for cherry-picks
or other things like rebase, but that should get what you want.

If you only ever need this information sometimes, you can look it up
by doing something like:

git log -1 --pretty="%cd" -- 

That should show the commit time of the latest commit which touches
that file, which is "essentially" the modify time of the file in terms
of  the version control history.

Obviously, this wouldn't work if you continually amend a change of
multiple files, since git wouldn't track the files separately, and
this only really shows you the time of the last commit.

However, in "version control" sense, this *is* the last time a file
was modified, since it doesn't really care about the stuff that
happens outside of version control.

I'm not really sure if this is enough for you, or if you really want
to store the actual mtime for some reason? (I think you can likely
solve your problem in some other way though).

>
> As far as I remember, bitkeeper had this distinction between checkins
> and commits. You could check in a file at any time, and any number of
> times, and then group all those checkins together with a commit. Git
> seems to have avoided this principle, or have kept it only
> rudimentarily via git add (but git add cannot add more than one version
> of the same file). Perhaps for simplificiation of use, perhaps for
> simplification of implementation, I don't know.
>

You can do lots of commits on a branch and then one merge commit to
merge it into the main line. This is a common strategy used by many
people.

Thanks,
Jake

> I assume, if it were not for the build tool issues, git would have
> tracked mtime from the very start.
>

Maybe. Personally, I would hate having my mtime not be "the time I
checked the file out", since this is intuitive to me at this point.
I'm sure if I lived in a different world I'd be used to that way also,
though.

The build issue *is* important though, because many build systems rely
on the mtime to figure out what to rebuild, and a complete rebuild
isn't a good idea for very large projects.

Thanks,
Jake

> Best wishes
> Peter
> --
> Peter Backes, r...@helen.plasma.xg8.de

Re: Is there any way to "interrupt" a rebase?

[Jacob Keller]

On Tue, Feb 20, 2018 at 12:56 PM, Jeff King  wrote:
> On Tue, Feb 20, 2018 at 12:44:51PM +0100, Johannes Schindelin wrote:
>
>> > It might be even possible to design a new subcommand for the interactive
>> > rebase to facilitate a variation of this strategy (possibly even making
>> > use of the fact that the interactive rebase accumulates mappings between
>> > the original commits and the rewritten ones in
>> > $GIT_DIR/rebase-merge/rewritten-list, intended for use in the post-rewrite
>> > hook).
>>
>> This feature might look somewhat like this:
>>
>>   git rebase --replay-latest-commits 3
>>
>> and it would not even have to look at the `rewritten-list`. All it would
>> do is to put back the latest `pick` from the `done` file (in case of merge
>> conflicts) into the `git-rebase-todo` file, then insert `pick lines for
>> HEAD~3.. at the beginning of that todo file, and then `git reset --hard
>> HEAD~3`.
>
> Keep in mind that the "pick" lines could be "edit", "squash", etc.
>
> I think the general form of your original email's proposal is something
> like: What if we had a "git rebase --rewind" that could "undo" the prior
> command? So if I had a todo file like:
>
>   pick 1
>   edit 2
>   x make test
>   edit 3
>   x make test
>   pick 4
>
> and I failed at the second "make test", then I'd have:
>
>   pick 1
>   edit 2
>   x make test
>   edit 3
>   x make test
>
> in the "done" file, with the final pick remaining in "todo". Could I
> then ask to "rewind" my state by moving "x make test" back to the
> "todo". And two rewinds would get me back to applying patch 3, which I
> could then fix up and re-run my test. Or four rewinds would get me back
> to patch 2, which maybe is where I made the initial mistake.
>
> That's a bit more primitive than what you're proposing in this
> follow-on, because you'd be doing the replay yourself (unless we remap
> the commits). But it's very easy to reason about and implement.
>
> Anyway, just musing at this point. I haven't thought it through, but I
> like the direction of everything you're saying. ;)
>
> -Peff

Using a --rewind that simply tracks the point of each history and can
reset back to each seems a bit more inline with what the original
suggestion is. Sort of like "undo" in an editor might. You could even
add a "rewind=x" so it could go back more than one step at a time, tho
just re-running rewind until you get where you want would be doable as
well.

I like the overall direction of both these suggestions.

Thanks,
Jake

Re: Fetch-hooks

[Jacob Keller]

On Mon, Feb 19, 2018 at 2:50 PM, Leo Gaspard  wrote:
> On 02/19/2018 10:23 PM, Jeff King wrote:
>> [...]
>> If you do go this route, please model it after "pre-receive" rather than
>> "update". We had "update" originally but found it was too limiting for
>> hooks to see only one ref at a time. So we introduced pre-receive. The
>> "update" hook remains for historical reasons, but I don't think we'd
>> want to reproduce the mistake. :)
>
> Hmm, what bothered me with “pre-receive” was that it was an
> all-or-nothing decision, without the ability to allow some references
> through and not others.
>
> Is there a way for “pre-receive” to individually filter hooks? I was
> under the impression that the only way to do that was to use the
> “update” hook, which was the reason I wanted to model it after “update”
> rather than “pre-receive” (my use case being a check independent for
> each pushed ref)

At least in the "push" case I think there is value in saying "if one
ref fails, please fail the entire push, always". This makes sense,
because if a user happens to violate some rule in one thing, they very
likely may not wish any of the push to succeed, and it also creates
problems with whether a push is atomic or not.

For fetch, I could see either method being useful.

Thanks,
Jake

Re: [RFC] Rebasing merges: a jorney to the ultimate solution (Road Clear)

[Jacob Keller]

On Fri, Feb 16, 2018 at 5:08 AM, Sergey Organov  wrote:
> Hi,
>
> By accepting the challenges raised in recent discussion of advanced
> support for history rebasing and editing in Git, I hopefully figured out
> a clean and elegant method of rebasing merges that I think is "The Right
> Way (TM)" to perform this so far troublesome operation. ["(TM)" here has
> second meaning: a "Trivial Merge (TM)", see below.]
>
> Let me begin by outlining the method in git terms, and special thanks
> here must go to "Johannes Sixt"  for his original bright
> idea to use "cherry-pick -m1" to rebase merge commits.
>
> End of preface -- here we go.
>

I hope to take a more detailed look at this, also possibly with some
attempts at re-creating the process by hand to see it in practice.

> Given 2 original branches, b1 and b2, and a merge commit M that joins
> them, suppose we've already rebased b1 to b1', and b2 to b2'. Suppose
> also that B1' and B2' happen to be the tip commits on b1' and b2',
> respectively.
>
> To produce merge commit M' that joins b1' and b2', the following
> operations will suffice:
>
> 1. Checkout b2' and cherry-pick -m2 M, to produce U2' (and new b2').
> 2. Checkout b1' and cherry-pick -m1 M, to produce U1' (and new b1').
> 3. Merge --no-ff new b2' to current new b1', to produce UM'.
> 4. Get rid of U1' and U2' by re-writing parent references of UM' from
>U1' and U2' to  B1' and B2', respectively, to produce M'.
> 5. Mission complete.
>

Seems pretty straight forward, go to each branch and cherry-pick the
merge respective to its relative parent, and then finally re-merge
everything, and consume the intermittent commits.

> Let's now see why and how the method actually works.
>
> Firs off, let me introduce you to my new friend, the Trivial Merge, or
> (TM) for short. By definition, (TM) is a merge that introduces
> absolutely no differences to the sides of the merge. (I also like to
> sometimes call him "Angel Merge", both as the most beautiful of all
> merges, and as direct antithesis to "evil merge".)
>
> One very nice thing about (TM) is that to safely rebase it, it suffices
> to merge its (rebased) parents. It is safe in this case, as (TM) itself
> doesn't posses any content changes, and thus none could be missed by
> replacing it with another merge commit.
>
> I bet most of us have never seen (TM) in practice though, so let's see
> how (TM) can help us handle general case of some random merge. What I'm
> going to do is to create a virtual (TM) and see how it goes from there.
>
> Let's start with this history:
>
>   M
>  / \
> B1  B2
>
> And let's transform it to the following one, contextually equivalent to
> the original, by introducing 2 simple utility commits U1 and U2, and a
> new utility merge commit UM:
>
>   UM
>  /  \
> U1   U2
> ||
> B1   B2
>
> Here content of any of the created UM, U1, and U2 is the same, and is
> exact copy of original content of M. I.e., provided [A] denotes
> "content of commit A", we have:
>
> [UM] = [U1] = [U2] = [M]
>
> Stress again how these changes to the history preserve the exact content
> of the original merge ([UM] = [M]), and how U1 an U2 represent content
> changes due to merge on either side[*], and how neither preceding nor
> subsequent commits content would be affected by the change of
> representation.
>
> Now observe that as [U1] = [UM], and [U2] = [UM], the UM happens to be
> exactly our new friend -- the "Trivial Merge (TM)" his true self,
> introducing zero changes to content.
>
> Next we rebase our new representation of the history and we get:
>
>   UM'
>  /  \
> U1'  U2'
> ||
> B1'  B2'
>
> Here UM' is bare merge of U1' and U2', in exact accordance with the
> method of rebasing a (TM) we've already discussed above, and U1' and U2'
> are rebased versions of U1 and U2, obtained by usual rebasing methods
> for non-merge commits.
>
> (Note, however, that at this point UM' is not necessarily a (TM)
> anymore, so in real implementation it may make sense to check if UM' is
> not a (TM) and stop for possible user amendment.)
>

This might be a bit tricky for a user to understand what the process
is, especially if they don't understand how it's creating special U1'
and U2' commits. However, it *is* the cleanest method I've either seen
or thought of for presenting the conflict to the user.

> Finally, to get to our required merge commit M', we get the content of
> UM' and record two actual parents of the merge:
>
>   M'
>  / \
> B1' B2'
>
> Where [M'] = [UM'].
>
> That's it. Mission complete.
>
> I expect the method to have the following nice features:
>
> - it carefully preserves user changes by rebasing the merge commit
> itself, in a way that is semantically similar to rebasing simple
> (non-merge) commits, yet it allows changes made to branches during
> history editing to propagate over corresponding merge commit that joins
> the branches, even automatically when the changes don't conflict, as
> expected.

Re: git-rebase --undo-skip proposal

[Jacob Keller]

On Wed, Feb 14, 2018 at 5:50 PM, Psidium Guajava  wrote:
> 2018-02-14 22:53 GMT-02:00 Johannes Schindelin :
>> Now, when is the next possible time you can call `git rebase --undo-skip`?
>
> What if the scope were reduced from `--undo-skip` to `--undo-last-skip`?
> Also, further reduce the scope to only allow `--undo-last-skip` during
> a ongoing rebase, not caring about a finished one?
>
> But, this could be so niche that I have doubts if this would ever be used;

I think this is too niche to actually be useful in practice,
especially since figuring out exactly what to replay might be tricky..
I suppose it could keep track of where in the rebase the last skip was
used, and then just go back to rebase and stop there again? That
sounds like just redoing the rebase is easier.. (ie: use the reflog to
go back to before the rebase started, and then re-run it again and
don't skip this time).

Re: git-rebase --undo-skip proposal

[Jacob Keller]

On Wed, Feb 14, 2018 at 4:53 PM, Johannes Schindelin
 wrote:
> Hi,
>
> On Wed, 14 Feb 2018, Psidium Guajava wrote:
>
>> On 2018-02-13 18:42 GMT-02:00 Stefan Beller  wrote:
>> > On Tue, Feb 13, 2018 at 12:22 PM, Psidium Guajava  
>> > wrote:
>> > I think this could also be done with "git rebase --edit-todo", which brings
>> > up the right file in your editor.
>>
>> Yeah that'd would only work if one started a rebase as a interactive
>> one, not am or merge.
>
> I agree that the original proposal was clearly for the non-interactive
> rebase (it talked about .git/rebase-apply/).
>
> The biggest problem with the feature request is not how useful it would
> be: I agree it would be useful. The biggest problem is that it is a little
> bit of an ill-defined problem.
>
> Imagine that you are rebasing 30 patches. Now, let's assume that patch #7
> causes a merge conflict, and you mistakenly call `git rebase --skip`.
>
> Now, when is the next possible time you can call `git rebase --undo-skip`?
> It could be after a merge conflict in #8. Or in interactive rebase, after
> a `pick` that was turned into an `edit`. Or, and this is also entirely
> possible, after the rebase finished with #30 without problems and the
> rebase is actually no longer in progress.
>
> So I do not think that there is a way, in general, to implement this
> feature. Even if you try to remember the state where a `--skip` was
> called, you would remember it in the .git/rebase-apply/ (or
> .git/rebase-merge/) directory, which is cleaned up after the rebase
> concluded successfully. So even then the information required to implement
> the feature would not necessarily be there, still, when it would be needed.
>
> Ciao,
> Johannes

Instead of an "--undo-skip", what if we ask the question of what the
user actually wants?

Generally I'd assume that the user wishes to go back to the rebase and
"pick" the commit back in.

So what if we just make "git rebase --skip" more verbose so that it
more clearly spells out which commit is being skipped? Possibly even
as extra lines of "the following patches were skipped during the
rebase" after it completes?

Then it's up to the user to determine what to do with those commits,
and there are many tools they could use to solve it, "git rebase -i,
git cherry-pick, git reflog to restore to a previous and re-run the
rebase, etc".

Thanks,
Jake

Re: Fetch-hooks

[Jacob Keller]

On Sat, Feb 10, 2018 at 4:21 AM, Jeff King  wrote:
> On Sat, Feb 10, 2018 at 01:37:20AM +0100, Leo Gaspard wrote:
>
>> > Yeah, tag-following may be a little tricky, because it usually wants to
>> > write to refs/tags/. One workaround would be to have your config look
>> > like this:
>> >
>> >   [remote "origin"]
>> >   fetch = +refs/heads/*:refs/quarantine/origin/heads/*
>> >   fetch = +refs/tags/*:refs/quarantine/origin/tags/*
>> >   tagOpt = --no-tags
>> >
>> > That's not exactly the same thing, because it would fetch all tags, not
>> > just those that point to the history on the branches. But in most
>> > repositories and workflows the distinction doesn't matter.
>>
>> Hmm... apart from the implementation complexity (of which I have no
>> idea), is there an argument against the idea of adding a
>> remote..fetchTagsTo refmap similar to remote..fetch but used
>> every time a tag is fetched? (well, maybe not exactly similar to
>> remote..fetch because we know the source is going to be
>> refs/tags/*; so just having the part of .fetch past the ':' would be
>> more like what's needed I guess)
>
> I don't think it would be too hard to implement, and is at least
> logically consistent with the way we handle tags.
>
> If we were designing from scratch, I do think this would all be helped
> immensely by having more separation of refs fetched from remotes. There
> was a proposal in the v1.8 era to fetch everything for a remote,
> including tags, into "refs/remotes/origin/heads/",
> "refs/remotes/origin/tags/", etc. And then we'd teach the lookup side to
> look for tags in each of the remote-tag namespaces.
>
> I still think that would be a good direction to go, but it would be a
> big project (which is why the original stalled).
>

I want to go this direction, but the difficulty has always been
limited time to actually work on such a large project.

Thanks,
Jake

>> The issue with your solution is that if the user runs 'git fetch
>> --tags', he will get the (potentially compromised) tags directly in his
>> refs/tags/.
>
> True.
>
> -Peff

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

[Jacob Keller]

On Mon, Feb 12, 2018 at 11:15 PM, Sergey Organov <sorga...@gmail.com> wrote:
> Hi Jake,
>
> Jacob Keller <jacob.kel...@gmail.com> writes:
>
>> On Mon, Feb 12, 2018 at 12:39 PM, Johannes Schindelin
>> <johannes.schinde...@gmx.de> wrote:
>>> Hi Sergey,
>>>
>>> On Mon, 12 Feb 2018, Sergey Organov wrote:
>>>> > 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.
>>>
>>> Keep repeating it, and it won't become truer.
>>>
>>> If you break formats, you break scripts. Git has *so* many users, there
>>> are very likely some who script *every* part of it.
>>>
>>> We simply cannot do that.
>>>
>>> What we can is deprecate designs which we learned on the way were not only
>>> incomplete from the get-go, but bad overall and hard (or impossible) to
>>> fix. Like --preserve-merges.
>>>
>>> Or for that matter like the design you proposed, to use --first-parent for
>>> --recreate-merges. Or to use --first-parent for some --recreate-merges,
>>> surprising users in very bad ways when it is not used (or when it is
>>> used). I get the impression that you still think it would be a good idea,
>>> even if it should be obvious that it is not.
>>
>> If we consider the addition of new todo list elements as "user
>> breaking", then yes this change would be user-script breaking.
>
> It _is_ user script breaking, provided such script exists. Has anybody
> actually seen one? Not that it's wrong to be extra-cautious about it,
> just curios. Note that to be actually affected, such a script must
> invoke "git rebase -p" _command_ and then tweak its todo output to
> produce outcome.
>
>> Since we did not originally spell out that todo-list items are subject
>> to enhancement by addition of operations in the future, scripts are
>> likely not designed to allow addition of new elements.
>
> Out of curiosity, are you going to spell it now, for the new todo
> format?
>
>> Thus, adding recreate-merges, and deprecating preserve-merges, seems
>> to me to be the correct action to take here.
>
> Yes, sure, provided there is actual breakage, or at least informed
> suspicion there is one.
>
>> One could argue that users should have expected new todo list elements
>> to be added in the future and thus design their scripts to cope with
>> such a thing. If you can convincingly argue this, then I don't
>> necessarily see it as a complete user breaking change to fix
>> preserve-merges in order to allow it to handle re-ordering properly..
>
> I'd not argue this way myself. If there are out-of-git-tree non-human
> users that accept and tweak todo _generated_ by current "git rebase -p"
> _command_, I also vote for a new option.
>

To be fair, I have not seen anything that actually reads the todo list
and tweaks it in such a manner. The closest example is the git garden
shears script, which simply replaces the todo list.

It's certainly *possible* that such a script would exist though,

Thanks,
Jake

>> I think I lean towards agreeing with Johannes, and that adding
>> recreate-merges and removing preserve-merges is the better solution.
>
> On these grounds it is, no objections.
>
> -- Sergey

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

[Jacob Keller]

On Mon, Feb 12, 2018 at 12:39 PM, Johannes Schindelin
 wrote:
> Hi Sergey,
>
> On Mon, 12 Feb 2018, Sergey Organov wrote:
>> > 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.
>
> Keep repeating it, and it won't become truer.
>
> If you break formats, you break scripts. Git has *so* many users, there
> are very likely some who script *every* part of it.
>
> We simply cannot do that.
>
> What we can is deprecate designs which we learned on the way were not only
> incomplete from the get-go, but bad overall and hard (or impossible) to
> fix. Like --preserve-merges.
>
> Or for that matter like the design you proposed, to use --first-parent for
> --recreate-merges. Or to use --first-parent for some --recreate-merges,
> surprising users in very bad ways when it is not used (or when it is
> used). I get the impression that you still think it would be a good idea,
> even if it should be obvious that it is not.

If we consider the addition of new todo list elements as "user
breaking", then yes this change would be user-script breaking.

Since we did not originally spell out that todo-list items are subject
to enhancement by addition of operations in the future, scripts are
likely not designed to allow addition of new elements.

Thus, adding recreate-merges, and deprecating preserve-merges, seems
to me to be the correct action to take here.

One could argue that users should have expected new todo list elements
to be added in the future and thus design their scripts to cope with
such a thing. If you can convincingly argue this, then I don't
necessarily see it as a complete user breaking change to fix
preserve-merges in order to allow it to handle re-ordering properly..

I think I lean towards agreeing with Johannes, and that adding
recreate-merges and removing preserve-merges is the better solution.

Thanks,
Jake

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

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

[Jacob Keller]

On Tue, Feb 6, 2018 at 10:16 PM, Sergey Organov  wrote:
> Johannes Schindelin  writes:
>
> [...]
>
>> +--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.
>
> I wonder why you guys still hold on replaying "merge-the-operation"
> instead of replaying "merge-the-result"? The latter, the merge commit
> itself, no matter how exactly it was created in the first place, is the
> most valuable thing git keeps about the merge, and you silently drop it
> entirely! OTOH, git keeps almost no information about
> "merge-the-operation", so it's virtually impossible to reliably replay
> the operation automatically, and yet you try to.
>

I'm not sure I follow what you mean here?

You mean that you'd want this to actually attempt to re-create the
original merge including conflict resolutions by taking the contents
of the result?

How do you handle if that result has conflicts? What UX do you present
to the user to handle such conflicts? I don't think the normal 3-way
conflicts would even be possible in this case?

Thanks,
Jake

> IMHO that was severe mistake in the original --preserve-merges, and you
> bring with you to this new --recreate-merges... It's sad. Even more sad
> as solution is already known for years:
>
> bc00341838a8faddcd101da9e746902994eef38a
> Author: Johannes Sixt 
> Date:   Sun Jun 16 15:50:42 2013 +0200
>
> rebase -p --first-parent: redo merge by cherry-picking first-parent 
> change
>
> and it works like a charm.
>
> -- Sergey
>

Re: [PATCH 2/8] sequencer: introduce the `merge` command

[Jacob Keller]

On Wed, Jan 31, 2018 at 5:48 AM, Johannes Schindelin
<johannes.schinde...@gmx.de> wrote:
> Hi Jake & Phillip,
>
> On Mon, 29 Jan 2018, Johannes Schindelin wrote:
>
>> On Sat, 20 Jan 2018, Jacob Keller wrote:
>>
>> > On Fri, Jan 19, 2018 at 6:45 AM, Phillip Wood <phillip.w...@talktalk.net> 
>> > wrote:
>> > > On 18/01/18 15:35, Johannes Schindelin wrote:
>> > >>
>> > >> This patch adds the `merge` command, with the following syntax:
>> > >>
>> > >>   merge   
>> > >
>> > > I'm concerned that this will be confusing for users. All of the other
>> > > rebase commands replay the changes in the commit hash immediately
>> > > following the command name. This command instead uses the first
>> > > commit to specify the message which is different to both 'git merge'
>> > > and the existing rebase commands. I wonder if it would be clearer to
>> > > have 'merge -C   ...' instead so it's clear which
>> > > argument specifies the message and which the remote head to merge.
>> > > It would also allow for 'merge -c   ...' in the future
>> > > for rewording an existing merge message and also avoid the slightly
>> > > odd 'merge -  ...'. Where it's creating new merges I'm not sure
>> > > it's a good idea to encourage people to only have oneline commit
>> > > messages by making it harder to edit them, perhaps it could take
>> > > another argument to mean open the editor or not, though as Jake said
>> > > I guess it's not that common.
>> >
>> > I actually like the idea of re-using commit message options like -C,
>> > -c,  and -m, so we could do:
>> >
>> > merge -C  ... to take message from commit
>>
>> That is exactly how the Git garden shears do it.
>>
>> I found it not very readable. That is why I wanted to get away from it in
>> --recreate-merges.
>
> I made up my mind. Even if it is not very readable, it is still better
> than the `merge A B` where the order of A and B magically determines their
> respective roles.
>
>> > merge -c  ...  to take the message from commit and open editor to 
>> > edit
>> > merge -m "" ... to take the message from the quoted test
>> > merge ... to merge and open commit editor with default message
>
> I will probably implement -c, but not -m, and will handle the absence of
> the -C and -c options to construct a default merge message which can then
> be edited.
>
> The -m option just opens such a can of worms with dequoting, that's why I
> do not want to do that.
>

I agree, I don't see a need for "-m".

> BTW I am still trying to figure out how to present the oneline of the
> commit to merge (which is sometimes really helpful because the label might
> be less than meaningful) while *still* allowing for octopus merges.
>
> So far, what I have is this:
>
> merge   
>
> and for octopus:
>
> merge  " ..." ...
>
> I think with the -C syntax, it would become something like
>
> merge -C   # 
>

I like this, especially given you added the "#" for one of the other
new commands as well, (reset I think?)

> and
>
> merge -C   ...
> # Merging: 
> # Merging: 
> # ...
>

I really like this, since you can show each oneline for all the
to-merges for an octopus.

> The only qualm I have about this is that `#` really *is* a valid ref name.
> (Seriously, it is...). So that would mean that I'd have to disallow `#`
> as a label specifically.
>
> Thoughts?
>

I think it's fine to disallow # as a label.

Thanks,
Jake

> Ciao,
> Dscho