On Mon, Mar 10, 2008 at 10:19 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> "Sverre Hvammen Johansen" <hvammen@xxxxxxxxx> writes:
>
> > @@ -153,8 +153,8 @@ parse_config () {
> > --summary)
> > show_diffstat=t ;;
> > --squash)
> > - test "$allow_fast_forward" = t ||
> > - die "You cannot combine --squash with --no-ff."
> > + test "$fast_forward" = allow ||
> > + die "You cannot combine --squash with --ff=never."
>
> Why does the user get this message after saying --ff=only?
Bug. What should the semantic be? To me it makes sense that a squash
oweride the ff options instead of giving a error, but specifying a ff
option after --squash is an error.
> How does this code parse "git merge --ff my_other_branch"?
It is getting late so I need to get back to you about this.
> Shouldn't you issue the same error message for these two inputs?
>
> "git merge --ff=never --squash"
> "git merge --squash --ff=never"
Allow the first one since --ff=never can be in the config file and
give error on the last one.
> What does this complex double loop compute differently from what "git
> show-branch --independent" gives you? Aside from that you will run slower
> but you can take more than 25 branches?
The main issue is that show-branch --independent does not give me the
desired order for these branches. I want the first branch to be head
or something that can be fast forwarded from head. The second branch
should be the next branch in the specified list that have not been
eliminated or something that can be fast forwarded from this, and so
on and so forth. This is an absolute requirement for the first
argument (head). If show-branch had a documented order and meet the
absolute requirement above I would prefer show-branch --independentr
instead of this nasty loop.
> More generally, I doubt it is really useful to let the user throw millions
> of potentially duplicate refs and have the merge silently record a
> filtered out results. Yes, you made the process of culling duplicates too
> chatty in the above part of the patch, and fmt-merge-msg will hopefully
> still show what the user gave on the command line, but the heads used by
> the real merge process now is very different from it. The merge comment
> is totally disconnected from the reality. Why is this an improvement?
We already do this in the case where we have head pluss one branch.
If it results in only one real parent we throw one of them away
resulting in a fast forward or an "up to date". The suggested patch
is just a generalization over this to the case where we have head and
more than one branch.
> If the goal is to allow Octopus that is more complex than the simplest
> kind, don't. Octopus was deliberately written to allow the most simple
> kind and nothing else for a reason: bisectability.
That is not the goal.
> The user should know what he is merging; throwing many heads that he does
> not even know how they relate to each other, and call the resulting mess a
> merge feels like a sure way to encourage a bad workflow.
We do merges all the time without knowing what we actually are
merging. That is something that happen in many work flows. I assume
that you don't want a real merge in the case that you are "up to date"
with your remote or your head can be fast forward. For the users
convenience we do a fast forward or report it to be "up to date".
Exactly the same argument holds where there are more than one remote
involved. The user may not know who is ahead and who is behind and he
usually want the commit to record a simple history as possible.
> > +then
> > + real_parents="$@"
> > + ff_head=$head
> > +else
> > + find_real_parents "$@"
> > +fi
>
> This part is simply unacceptable. At least please do not needlessly call
> find_real_parents in the most common case of giving only one remote head.
I now keep common_b in common so subsequent calls to git merge-base
--all can be optimized away for the most common case.
--
Sverre Hvammen Johansen
--
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
