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

Re: [RFC/PATCH Second draft] Fast forward strategies allow, never, and only

From
Junio C Hamano <gitster@pobox.com>
Date
Mar 19, 2008, 20:35 UTC
Message-ID
<7vskym310l.fsf@gitster.siamese.dyndns.org>
In-Reply-To
<402c10cd0803172127u480276c9s4f9d716b4912ad5e@mail.gmail.com>
"Sverre Hvammen Johansen" <hvammen@gmail.com> writes:
Show 14 quoted lines
> New fast forward strategies, only, is introduced.  This new fast
> forward strategy prevents real merges.
>
> FF strategy "only" fails if the specified heads and HEAD can not
> be reduced down to only one real parent.  The only allowed
> outcome is a fast forward unless HEAD is up to date with
> the specified heads.
>
> This patch also uses the real heads found instead of those
> specified for real merges.  This means that merge startegies
> that only take two heads can now accept more than two heads
> if they can be reduced down to only two real heads.  However,
> fast-forward of head in combination with a real merge is
> handled as before.

This might be easier to review if split into two parts. Code suffling to do --ff/--no-ff => ff={allow,never} and documentation updates to improve the description of these two options in the first patch, and addition of "only" to code and the updated docuemntation in the second.

Show 31 quoted lines
> +If your workflow is always to branch from the special branch
> +("master") when working on a topic and merge that back to "master", if
> +you happen to have worked only on a single topic and the "master" was
> +never advanced during the time you worked on that topic, merging the
> +topic back to "master" will result in a fast-forward.  When you look
> +back that history, you will not be able to tell where the topic
> +started and ended by following the ancestry chain of the "master"
> +branch.
> +
> +Using "never fast forward" policy on such a special branch will be a
> +way to make sure that all commits on the first-parent ancestry of that
> +special branch will be merges from something else.  From the history
> +you can determine where the topic started and ended.
> +
> +The following shows two branches forked off from "master".  The branch
> +"master" have merged in changes from branch "topicA" twice and
> +"topicB" once:
> +
> +------------
> +         o---o---o---o---o  topicA
> +        /     \           \
> +    ---*-------*-------*---*  master
> +      /         \     /
> +                 o---o  topicB
> +------------
> +
> +The first merge of topicA or the only merge of topicB would have
> +resulted in a fast forward without '--ff=never'.  The last merge of
> +topicA would have failed with '--ff=only'.  Topic A consist of those
> +commits that can be reached from master^2 without passing through any
> +of the first-parent ancestries of master.

If you remove one sentence "The last merge ... =only'." from it, this part is a very good write-up on how "never fast forward" could be useful, and might even be a worthy addition to the current documentation. However it lacks a crucial bit of information: it is _not enough_ to just use --no-ff to maintain the "special status" of "master". You also need to prevent direct committing to it. So while --no-ff is an ingredient you can construct such a workflow out of, it by itself is not the whole solution. You need additional discipline.

Show 6 quoted lines
> +However, if your workflow require a linear history for the special
> +branch ("master"), topic branches must be rebased before merging them
> +back to "master".  A pull or a merge from the "master branch of a
> +topic branch may accidentally introduce a merge commit that was not
> +already in the topic branch if the topic that were merged was not
> +properly rebased.  This will creating a none linear history.
This,...
Show 7 quoted lines
> +Using "only fast forward" policy ensures that whenever a pull or a
> +merge is performed it will fail unless the merge can be resolved as a
> +fast forward.  This will however not guarantee a linear history since
> +the topic branches that are merged in may have merge commits recorded.
>
> +You may therefor need to use this policy on the topic branches as
> +well.

combined with the above, would make "only" an incomplete implementation of the goal you stated earlier, i.e. "to force a completely linear history", but I think you can trivially fix this by making sure that there is no merge commit in ORIG_HEAD..MERGE_HEAD and refusing if you find one. And by fixing the implementation, you do not have to make excuses like the above two and half paragraphs.

Having said that, I doubt what you are trying to achieve is to force a totally linear history, as that is quite useless policy unless you are talking about a one-man project.

When your project has gained meaningful number of developers, there will always be more than one nontrivial changes outstanding. Forcing a linear history everywhere means whenever somebody wants his changes accepted, he needs to fetch + rebase whenever anybody else's change is accepted to the special "master". The more developers you have, the narrower window between acceptance of changes to the "master" becomes, and eventually the window will become shorter than the time needed to rebase and retest any nontrivial change on top of the tip of "master". At that point, nobody can get anything but a trivially rebasable and untested series accepted. Such a policy would encourage people to merge only "early half" of their series (because they do not have enough time to rebase the full series) in a poorly tested shape. So in that sense it is not just "practicaly useless", but can be actively harmful by encouraging a bad workflow.

But your "merge has to fast-forward, but the merged branch can have nonlinear history itself" semantics is different from "require linear history". For example, if you want the top-level integrator to do absolutely _nothing_ (not even a trivial merge), in order to shift the burden of integration testing to contributors, then use of such semantics (which is _different_ from "completely linear") by the top-level integrator could achieve that. Nobody can ask the top-level to pull which would result in a tree that he built and tested himself. The top-level integrator would say "Your tree is not a fast-forward from mine, because I merged with somebody else between the time you prepared your tree and I learned about it. Please try again", and in response, the contributor can pull from the top-level integrator and re-test the merge result and ask again. With luck, the top-level integrator may not have merged with anybody else and a pull from that contributor would fast-forward.

So if that is what you are trying to achieve, you need to update your description. If you aim for "Totally linear", I think many people will find it is practically useless, but if you are aiming for something different, you should advertise it as such.

I think the scaling issue (iow "narrowing window") is the same either way, and the documentaion should warn about it to the users. Unclueful people may think, without really thinking ;-), that it is a good thing in any occassions to require totally linear history, unless downsides are also explained.

Show 28 quoted lines
> diff --git a/git-merge.sh b/git-merge.sh
> index 7dbbb1d..a98cd77 100755
> --- a/git-merge.sh
> +++ b/git-merge.sh
> @@ -12,7 +12,7 @@ summary              show a diffstat at the end of the merge
>  n,no-summary         don't show a diffstat at the end of the merge
>  squash               create a single commit instead of doing a merge
>  commit               perform a commit if the merge sucesses (default)
> -ff                   allow fast forward (default)
> +ff?                  fast forward options
>  s,strategy=          merge strategy to use
>  m,message=           message to be used for the merge commit (if any)
>  "
> @@ -35,7 +35,7 @@ no_fast_forward_strategies='subtree ours'
>  no_trivial_strategies='recursive recur subtree ours'
>  use_strategies=
>
> -allow_fast_forward=t
> +fast_forward=allow
>  allow_trivial_merge=t
>  squash= no_commit=
>
> @@ -153,8 +153,6 @@ parse_config () {
>                 --summary)
>                         show_diffstat=t ;;
>                 --squash)
> -                       test "$allow_fast_forward" = t ||
> -                               die "You cannot combine --squash with --no-ff."
I do not think you defended why it is good idea to drop this sanity check.
Show 16 quoted lines
>                         squash=t no_commit=t ;;
>                 --no-squash)
>                         squash= no_commit= ;;
> @@ -163,18 +161,33 @@ parse_config () {
>                 --no-commit)
>                         no_commit=t ;;
>                 --ff)
> -                       allow_fast_forward=t ;;
> +                       case "$2" in
> +                       allow|never|only)
> +                               fast_forward=$2; shift ;;
> +                       -*)
> +                               fast_forward=allow ;;
> +                       *)
> +                               die "Available fast-forward strategies
> are: allow, newer, and only" ;;

Strategies? "fast forward options" you used in the option description is a reasonable one, and it would be better to keep it consistent.

I didn't look at the rest of the patch (yet).
Previous: Sverre Hvammen JohansenNext: Sverre Hvammen Johansen
Message 20 of 27 in “Fast forward strategies allow, never, and only”
  1. Fast forward strategies allow, never, and onlySverre Hvammen Johansen, Mar 11, 2008
  2. Sverre Hvammen JohansenMar 11, 2008
  3. Ping YinMar 11, 2008
  4. Junio C HamanoMar 11, 2008
  5. Sverre Hvammen JohansenMar 12, 2008
  6. Sverre Hvammen JohansenMar 16, 2008
  7. Sverre Hvammen JohansenMar 14, 2008
  8. Jakub NarebskiMar 11, 2008
  9. Sverre Hvammen JohansenMar 12, 2008
  10. Junio C HamanoMar 12, 2008
  11. Sverre Hvammen JohansenMar 12, 2008
  12. Sverre Hvammen JohansenMar 18, 2008
  13. Ping YinMar 18, 2008
  14. Sverre Hvammen JohansenMar 18, 2008
  15. Jon LoeligerMar 18, 2008
  16. Jakub NarebskiMar 18, 2008
  17. Sverre Hvammen JohansenMar 19, 2008
  18. Jakub NarebskiMar 19, 2008
  19. Sverre Hvammen JohansenMar 20, 2008
  20. Junio C HamanoMar 19, 2008
  21. Sverre Hvammen JohansenMar 20, 2008
  22. Junio C HamanoMar 22, 2008
  23. Sverre Hvammen JohansenMar 26, 2008
  24. Sverre Hvammen JohansenMar 31, 2008
  25. Fast forward strategies allow, never, and onlySverre Hvammen Johansen, Apr 20, 2008
  26. Junio C HamanoApr 22, 2008
  27. Sverre Hvammen JohansenApr 24, 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.