Hi,
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 ;-)
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).
> > 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.
Perhaps I got Junio wrong, too.
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
If Junio meant with
> (1) The committer information would not record the user and time of the
> sequencer operation, which actually may be a good thing.
that he thinks the first variant is the way to go, I strongly disagree.
But perhaps I'm getting everyone wrong these days ;)
> > > > (2) When $what is revert, this codepath shouldn't be exercised,
> > > > should it?
> > >
> > > Yes.
> >
> > I haven't done a check intentionally, but there was a stupid thinko.
> > So you're right.
> >
> > But: this will only be a bug if the commit that _comes next in the
> > original history_ is to be reverted.
>
> Does not matter. It's a bug.
>
> A bug is almost always in the details, a corner-case, but it almost always
> needs fixing nevertheless.
Of course ;)
> > Nonetheless, purely tested:
>
> "Nevertheless", maybe? "untested", maybe?
No, I tested it once. ;-)
(For the new single-quoted variant I've changed the author name in
t3350).
> > 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.
(I mean, you should be in a safe state to do git sequencer --edit and
correct "hours" to "ours'.)
Regards,
Stephan
--
Stephan Beyer <s-beyer@xxxxxxx>, PGP 0x6EDDD207FCC5040F
--
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
