git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [RFC/PATCH 1/4] Add git-sequencer shell prototype

From
Johannes Schindelin <johannes.schindelin@gmx.de>
Date
Jul 4, 2008, 01:03 UTC
Message-ID
<alpine.DEB.1.00.0807040252490.2849@eeepc-johanness>
In-Reply-To
<20080704003857.GG6677@leksak.fem-net>
Hi,
On Fri, 4 Jul 2008, Stephan Beyer wrote:
Show 9 quoted lines
> 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.

Show 50 quoted lines
> > > 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.

Show 30 quoted lines
> 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.
Show 16 quoted lines
> > > 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

Previous: Stephan BeyerNext: Stephan Beyer
Message 30 of 52 in “git sequencer prototype”
  1. Stephan BeyerJul 1, 2008
  2. 1/4 Add git-sequencer shell prototypeStephan Beyer, Jul 1, 2008
  3. 2/4 Add git-sequencer prototype documentationStephan Beyer, Jul 1, 2008
  4. 3/4 Add git-sequencer test suite (t3350)Stephan Beyer, Jul 1, 2008
  5. 4/4 Migrate git-am to use git-sequencerStephan Beyer, Jul 1, 2008
  6. git-rebase-i migration to sequencerStephan Beyer, Jul 1, 2008
  7. 1/2 Make rebase--interactive use OPTIONS_SPECStephan Beyer, Jul 1, 2008
  8. 2/2 Migrate git-rebase--i to use git-sequencerStephan Beyer, Jul 1, 2008
  9. Stephan BeyerJul 5, 2008
  10. Junio C HamanoJul 5, 2008
  11. Jakub NarebskiJul 1, 2008
  12. Stephan BeyerJul 1, 2008
  13. Jakub NarebskiJul 1, 2008
  14. Stephan BeyerJul 1, 2008
  15. Jakub NarebskiJul 2, 2008
  16. Junio C HamanoJul 2, 2008
  17. Stephan BeyerJul 2, 2008
  18. 2/4 Add git-sequencer prototype documentationStephan Beyer, Jul 5, 2008
  19. Jakub NarebskiJul 8, 2008
  20. Stephan BeyerJul 8, 2008
  21. Karl HasselströmJul 9, 2008
  22. Junio C HamanoJul 3, 2008
  23. Johannes SchindelinJul 3, 2008
  24. Stephan BeyerJul 3, 2008
  25. Junio C HamanoJul 3, 2008
  26. Stephan BeyerJul 3, 2008
  27. Stephan BeyerJul 3, 2008
  28. Johannes SchindelinJul 3, 2008
  29. Stephan BeyerJul 4, 2008
  30. Johannes SchindelinJul 4, 2008
  31. Stephan BeyerJul 4, 2008
  32. Allow cherry-picking root commitsJohannes Schindelin, Jul 4, 2008
  33. Stephan BeyerJul 4, 2008
  34. Junio C HamanoJul 6, 2008
  35. Johannes SchindelinJul 6, 2008
  36. t3503: Add test case for identical filesStephan Beyer, Jul 6, 2008
  37. Stephan BeyerJul 6, 2008
  38. Johannes SchindelinJul 6, 2008
  39. Stephan BeyerJul 4, 2008
  40. Johannes SchindelinJul 4, 2008
  41. Junio C HamanoJul 3, 2008
  42. Jakub NarebskiJul 3, 2008
  43. Stephan BeyerJul 3, 2008
  44. 1/4 Add git-sequencer shell prototypeStephan Beyer, Jul 5, 2008
  45. Junio C HamanoJul 1, 2008
  46. Stephan BeyerJul 1, 2008
  47. Alex RiesenJul 4, 2008
  48. Junio C HamanoJul 4, 2008
  49. Stephan BeyerJul 4, 2008
  50. Alex RiesenJul 5, 2008
  51. Thomas AdamJul 5, 2008
  52. Johannes SchindelinJul 5, 2008

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.