Re: [PATCHv3 2/2] Warnings before amending published history

[Junio C Hamano <gitster@pobox.com>]

Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:

> Lucien Kong <Lucien.Kong@ensimag.imag.fr> writes:
>
>>  builtin/commit.c              |   82 ++++++++++++++++++++++++++
>>  sha1_name.c                   |   95 +++++++-----------------------
>>  sha1_name.h                   |  130 +++++++++++++++++++++++++++++++++++++++++
>
> I'm surprised that you need such a big patch. Basically, you're making
> all static functions in sha1_name.c public. If you really need such
> intrusive change, then you should at least explain why in the commit
> message, and most preferably split the patch into one refactoring patch
> to expose the functions and one to use them.
>
> But I suspect what you're looking for is already in cache.h.

I do not think I am going to take this nor the rebase patch that
bases its decision on the hardcoded "Does the commit appear in the
history of refs/remotes/*anything*?" logic.

At least, there should be "Here is a list of the branches I promised
others that I am not to going to rewind." even if you are going to
make its default value to be "for-each-ref refs/remotes/".  It is
too inflexible to be useful otherwise.  Not only in the contributor
and integrator workflow, but a simple "Alice asks Bob to pull from
her Github repository" will be hurt on the "I fixed up the issues
you raised. Could you please take another look" round.  Besides, I
won't be able to amend things outside 'next' but are in 'pu' ;-).

The logic in the patch in this thread to check each ref~$n is not
even worth commenting on, but as to the logic in the other "rebase"
one, I think it is wasteful to ask "what are the refs that can reach
this commit?" when what you really want to know is "is there any ref
among this set that can reach this commit?" (the former needs to
keep a lot more state).  It should be something like looking at the
output of:

	git rev-list <list commits you are going to touch here> \
		--not <list tips of refs you have published>

and make sure all the commits you are going to touch appear in the
result.  Any missing one is reachable from the refs you have
published and you may not want to rebase.

It may be an interesting thought experiment to see if you can take
advantage of the inherent ancestry relationship among the list of
commits you are going to touch. The later commits that will be
replayed in a rebase are very likely to be children of earlier one,
so in theory, if you can identify the set of topologically earliest
commits that will be replayed, you only need to check them, and if
you can cheaply come up with that set of earliest commits, the above
rev-list may become cheaper.