Hi,
On Fri, 4 Jul 2008, Stephan Beyer wrote:
> On Fri, Jul 04, 2008 at 01:53:21AM +0200, Johannes Schindelin wrote:
> > On Thu, 3 Jul 2008, Stephan Beyer wrote:
> > > Btw, another root commit problem is btw that it's not possible to
> > > cherry-pick root commits.
> >
> > That is a problem to be fixed in cherry-pick, not in sequencer. Care
> > to take care of that?
>
> Not at the moment but that's one of the things I note down for later ;-)
Well, logically, it should come _before_ you use it in sequencer. And you
should use it in sequencer.
> And btw, somehow it is still open for me if builtin sequencer should be
> a git-cherry-pick user (for pick) or if git-cherry-pick should be a
> sequencer user (which would result in a change of usage on cherry-pick
> conflicts).
The thing is, sequencer and cherry-pick _both_ use the same underlying
functionality: cherry-picking a single commit.
So, logically, that part should be moved into libgit.a, and cherry-pick
should use the to-be-introduced sequencer() function.
> > > Johannes Schindelin wrote:
> > > > > > +# Usage: pick_one (cherry-pick|revert) [-*|--edit] sha1
> > > > > > +pick_one () {
> > > > > > + what="$1"
> > > > > > + # we just assume that this is either cherry-pick or revert
> > > > > > + shift
> > > > > > +
> > > > > > + # check for fast-forward if no options are given
> > > > > > + if expr "x$1" : 'x[^-]' >/dev/null
> > > > > > + then
> > > > > > + test "$(git rev-parse --verify "$1^")" = \
> > > > > > + "$(git rev-parse --verify HEAD)" &&
> > > > > > + output git reset --hard "$1" &&
> > > > > > + return
> > > > > > + fi
> > > > > > + test "$1" != '--edit' -a "$what" = 'revert' &&
> > > > > > + what='revert --no-edit'
> > > > >
> > > > > This looks somewhat wrong.
> > > > >
> > > > > When the history looks like ---A---B and we are at A,
> > > > > cherry-picking B can be optimized to just advancing to B, but
> > > > > that optimization has a slight difference (or two) in the
> > > > > semantics.
> > > > >
> > > > > (1) The committer information would not record the user and
> > > > > time of the sequencer operation, which actually may be a
> > > > > good thing.
> > > >
> > > > This is debatable. But I think you are correct, for all the same
> > > > reasons why a merge can result in a fast-forward.
> > >
> > > Dscho, you mean me by referring to 'you' here, right?
> >
> > Nope.
> >
> > > Otherwise I'm a bit confused: "For the same reasons why a merge can
> > > result in a fast-forward we should not do fast forward here" ;-)
> >
> > What I meant: there is no use here to redo it. It has already be
> > done, and redoing just pretends that the girl calling sequencer tried
> > to pretend that she did it.
> >
> > If the merge has been done already, it should not be redone.
> >
> > Only if the user _explicitely_ specified a merge strategy, there _might_
> > be a reason to redo the merge, but I still doubt it.
>
> I don't get the light bulb. You're talking about "the merge", I am
> talking about fast-forward on picks.
Ooops. I think I was talking about a later comment of Junio's.
The thing is, if you try to pick a commit, and the current revision is
already the parent of that commit, I think it would be wrong to redo the
commit, pretending that the current time and committer were applying that
commit.
> I try a simple example just to go sure that we're talking about the
> same.
>
> We have commits
>
> A ---- B ---- C ---- D
> HEAD
>
> A is parent of B, B of C, C of D.
>
> Now we do:
> pick C
> pick --signoff D
> (Assume that the Signed-off-by: line is missing on D)
>
> Without fast-forward, we get
>
> A ---- B ---- C ---- D
> \
> `--- C'---- D'
> HEAD
>
> C' differs in C only in the committer data, perhaps only committer date.
>
> With fast-forward, we get:
>
> A ---- B ---- C ---- D
> \
> `--- D'
> HEAD
IMO the --signoff should check if the sign off is present (must be the
last one, as if we redid that commit), and if it is, fast-forward.
If it is missing, we need to redo the commit.
> > > Johannes Schindelin wrote:
> > > > I'd not check in sequencer for the strategy. Especially given
> > > > that we want to support user-written strategies in the future.
> > >
> > > I don't know how this is planned to look like, but perhaps
> > > --list-strategies may make sense here, too.
> >
> > No. You just do not check for strategies. Period. git-merge does
> > that, and you can easily abort a rebase if you explicitely asked for
> > an invalid strategy.
>
> Hmm, my dream of the "robust sequencing after sanity check passed" is
> dead with your "period".
>
> So I'll have to check what happens, when e.g. "--strategy=hours" is
> used.
Wow. Is that the CVS/SVN merge strategy?
Do not care too much about the rare use cases. Only experts will use -s.
But experts are known to shoot themselves, or inflict other pain on their
bodies, so it is okay to let them suffer from a typo.
Ciao,
Dscho
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
