Thread (135 messages) flat view 135 messages, 9 authors, 2020-03-17

Re: [PATCH v5 20/20] rebase: rename the two primary rebase backends

From: Jonathan Nieder <hidden>
Date: 2020-03-12 17:55:54

Elijah Newren wrote:
On Thu, Mar 12, 2020 at 8:13 AM Emily Shaffer [off-list ref] wrote:
quoted
This broke quite a few upstream users for us today when we rolled out a
next with this patch added on top. To shim around the post-commit hook
issue, we had set a system config for all our users to use
merge.backend=am; the machinery is pretty intolerant to a wrongly
configured backend name (die() rather than a warning and fallback).

Would it make more sense to deal with an unrecognized backend by falling
back to the default backend, instead?
[...]
Sorry for the pain.  The earlier part of the series had only ever made
it to next, and was reverted there, and thus, in my thinking, in the
new cycle no one would have ever seen the intermediate state.  (Oops,
I forgot about cases where people tried out next towards the end of
last cycle before it was reverted and decided to set config based upon
it.)
Right, I'm mostly interested in this for the future: do we expect the
list of backends to only grow over time, or do we want to support
removing and renaming backends?  In the latter case, how can we support
people sharing config between multiple Git versions with their
merge.backend settings?
I'm a little worried about ignoring the setting and just picking one;
if the setting has been marked and they set it to e.g. "appply" (one
too many p's), then does it really make sense to just show a warning
but continue using the backend they didn't want, especially since they
may miss the warning among the rest of the output?  I'd rather go the
route of improving the message, perhaps:
        _("Unknown rebase.backend config setting: %s")
Would that work for you?
What if we support multiple merge.backend values, with semantics:

- last recognized value wins
- if no value is specified, use the default
- if values are specified but none are recognized, error out with a
  clear error message

?
Also, I thought that you and Jonathan were going to be changing the
post-commit hook handling[1][2].  Does this mean that you've punted on
that, and I need to make some kind of change here to get you to switch
over?
The setting was a stopgap; our interest in this upstream is primarily
from the point of view of "we ran into this, so let's take the
opportunity to help others that might run into similar issues in the
future".

Thanks,
Jonathan
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help