Re: [RFC PATCH 0/5] Add option to autostage changes when continuing a rebase

[Phillip Wood <phillip.wood@talktalk.net>]

On 22/08/17 16:54, Junio C Hamano wrote:
> Phillip Wood <phillip.wood@talktalk.net> writes:
> 
>>> In other words, instead of
>>>
>>> 	git add -u && git rebase --continue
>>>
>>> you would want a quicker way to say
>>>
>>> 	git rebase --continue $something_here 
>>
>> Exactly
>> ...
>> rebase --continue -a
>>
>> behaves like commit -a in that it commits all updated tracked files and
>> does not take pathspecs.
> 
> Hmph, but what 'a' in "commit -a" wants to convey is "all", and in
> the context of "rebase" we want to avoid saying "all".  A user can
> be confused into thinking "all" refers to "all subsequent commits" 
> not "all paths conflicted".
> 
> Besides, with the $something_here option, the user is explicitly
> telling us that the user finisished the resolution of conflicts left
> in the working tree.  There is nothing "auto" about it.
> 
>> Did you have any further thoughts on what checks if any this new option
>> should make to avoid staging obviously unresolved files?
> 
> The more I think about this, the more I am convinced that it is a
> very bad idea to give such a $something_here option only to "rebase".
> 
> The standard flow for any operation that could stop in the middle
> because the command needs help from the user with conflicts that
> cannot be mechanically resolved is
> 
>  (1) write out conflicted state in the working tree, and mark these
>      paths in the index by leaving them in higher stages
>      (i.e. "ls-files -u" would list them);
> 
>  (2) the user edits them and marks the ones that are now resolved;
> 
>  (3) the user tells us to "continue".
> 
> and this is not limited to "rebase".  "cherry-pick", "am", and
> "revert" all share the above, and even "merge" (which is a single
> commit operation to which "continue" in its original meaning---to
> tell us the user is done with this step and wants us to go on
> processing further commits or patches---does not quite apply) does.
> 
> And "rebase" is an oddball in that it is the only command that we
> could deduce which files in the working tree should be "add"ed, i.e.
> "all the files that are different from HEAD".  All others allow
> users to begin in a dirty working tree (they only require the index
> to be clean), 

Are you sure about that, the second sentence of the cherry-pick man page
is "This requires your working tree to be clean (no modifications from
the HEAD commit).". If you pass '--no-commit' then this is relaxed so
that the index can differ from HEAD but it is not clear from the man
page if the working tree can differ from the index (potentially that
could give different conflicts in the index and working tree). I'm not
sure what a rebase that does not commit changes would look like.

> so your 
> 
> 	git [cherry-pick|am|revert] --continue $something_here
> 
> cannot be "git add -u && git [that command] --continue".  Blindly
> taking any and all files that have modifications from HEAD will
> produce a wrong result.
> 
> You cannot even sanely solve that by first recording the paths that
> are conflicted before giving control back to the user and limit the
> "add" to them, i.e. doing "git add" only for paths that appear in
> "git ls-files -u" output would not catch additional changes the was
> needed in files that did not initially conflict (i.e. "evil merge"
> will have to modify a file that was not involved in the mechanical
> part of the conflict), because out of the files that are dirty in
> the working tree, some are evil merge resolutions and others are
> ones that were dirty from the beginning.

I wonder if git could record the state of the working tree in a
temporary index just before it exits with conflicts. Then when the user
has resolved the conflicts

git [cherry-pick|am|revert] --continue $something_here

could do something like

{ GIT_INDEX_FILE=$TMP_INDEX git diff-files --name-only &&
    git ls-files -u } | git update-index --remove --stdin

to stage the files that the user has edited. This would not catch the
case where a new untracked file has been created that should be added to
the conflict resolution, but I think it is reasonable to expect the user
to manually add new files in this case. It would also not add an
unchanged file whose changes should be part of the conflict resolution
but I can't think of a sensible case where the user might want that at
the moment.

> 
> The only reason "git rebase" can get away without having to worry
> about all of the above is because it insists that the working tree
> is clean before it begins.  In the ideal world, however, we would
> want to lift that and allow it to start in a working tree that has
> an unfinished local modification that does not interfere with the
> rebase, just like all other operations that could stop upon a
> conflict. 

It could be expensive to check that the local modifications will not
interfere with the rebase - wouldn't it have to look at all the files
touched by each commit before starting? What do cherry-pick and am do
here when picking several commits?

 And when that happens, your $something_here option to
> "git rebase --continue" will have to worry about it, too.
> So...
>