Re: equal-tree-merges as way to make rebases fast-forward-able

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Nanako Shiraishi <nanako3@xxxxxxxxxxx> writes:

> I think you can record a merge commit that has an unusual 
> list of parents for this. For example, you can record the 
> latest version twice, as the first and the second parents, 
> and make the previous version the third parent. Because 
> such a merge can't be created with git-merge command, you 
> can reliably tell that it is an unusual 'marker' merge.

That's too ugly a hack, and hints an undesirable attitude that this will
be the last feature that needs such cleverness, without leaving the door
open for others who need similar "magic marker" capability added to commit
objects.  The approach will not scale, unless you consider "first and
second parents being the same means it is 'rebased branches magic', and
first, second and third parents being the same means some other matic, 
etc." as scalable.

As I said, if the approach this series takes turns out to be useful, it is
Ok to implement it as a new header in the commit object if necessary.

We try very hard to avoid adding random headers to commits because a
commit that records the same history should be named the same way in the
object store namespace and adding random headers will make it easier to
create the same commit with different names, but what we are discussing
is a special purpose pseudo commit and it _is_ a feature that the object
name of a commit, after SHA-1 hashing, is different with and without the
special header in this case.

> No matter what techinique is used to mark the special 
> 'marker', if it happens in real life for two or more people 
> who worked independantly to arrive at the same conclusion, 
> I don't think dismissing it as 'by chance' and discarding 
> the contribution from the second branch is a good solution. 
> If git is meant to work smoothly in projects where more than 
> one person see and accept patches from the same origin, the 
> condition is not met 'by chance'; the tool is by design 
> supposed to handle it as a regular situation.

I said the same thing, and I agree that two (or more) people creating the
same state should be treated as a normal event.  But I am not so sure
about a merge that binds two such histories together.  The person who
makes such a merge can go on without making one as far as tree-state is
concerned (i.e. such a merge will result in the same tree with both
parents), and the _only_ reason a merge between the two branches is
created is to cauterize one (or both) of the branches, declare that
everything that happened in the branch is now part of this branch.
In other words, such a merge is an operation to purely affect the history
and not contents.

As J6t pointed out, when we tell the revision walking machinery to limit
by path, we already simplify the history we show to the user, and if path
happens to name the whole tree, such a history is already simplified to
show only one side of the story.  So perhaps it is not as grave an offence
to ignore contribution from one side when both of the parents of a merge
record the same tree as I originally thought.  It also justifies not
introducing a new header in commits.  The implementation of Bernhard's
series might become simpler if it used the trick to use "." pathspec
internally instead of introducing a new traversal option to the revision
machinery.

> On the other hand, if you made the marker reliable, I think 
> you don't have to disable this feature by default like you 
> said in your (2).

That is true, but as you may be able to tell, I am undecided if the marker
should be _that_ explicit, or should be implicit and the fact that a merge
that whose all parents have the same tree should make it automatically a
marker (iow, I earlier said "misidentify" but now I am wondering if it
makes sense to define any merge with the property an "alternative history
binding marker", no matter how it was created).

> As a side note, I have a bug to report. I tried this sequence 
> of commands to make sure git-merge doesn't record the same 
> parent twice (the last git-merge is made on the slave branch 
> and tries to have slave, master and slave as its three 
> parents).
>
>  % git init
>  % echo hello >world
>  % git add . ; git commit -m first
>  % echo again >world
>  % git commit -a -m master
>  % git checkout -b slave master^
>  % echo again >world
>  % git commit -a -m slave
>  % git merge master slave
>
> But I got the "usage: ..." error message from git-merge.

Well, you did not quote the usage string you got, but it should have began
like this:

    usage: git merge [options] <remote>...
       or: git merge [options] <msg> HEAD <remote>

The parser misinterpreted your request as the latter form (which is
ancient and probably predates your involvement with the git project),
noticed that you did not give any <remote> commit, and then gave the
usage message.

I think we really should start deprecating the ancient form, but the
original sample script using this syntax from Linus was copied by many
people and are still found everywhere, I think, and people may still
use their scripts that were written with the ancient syntax.

In any case, at least this patch will make it start behaving a bit
more sanely.

-- >8 --
Subject: Do not misidentify "git merge foo HEAD" as an old-style invocation

This was misinterpreted as an ancient style "git merge <message> HEAD
<commit> <commit>..." that merges one (or more) <commit> into the current
branch and record the resulting commit with the given message.  Then a
later sanity check found that there is no <commit> specified and gave
a usage message.

Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx>
---

diff --git a/builtin-merge.c b/builtin-merge.c
index e95c5dc..e5cf795 100644
--- a/builtin-merge.c
+++ b/builtin-merge.c
@@ -792,7 +792,7 @@ static int suggest_conflicts(void)
 static struct commit *is_old_style_invocation(int argc, const char **argv)
 {
 	struct commit *second_token = NULL;
-	if (argc > 1) {
+	if (argc > 2) {
 		unsigned char second_sha1[20];
 
 		if (get_sha1(argv[1], second_sha1))

--
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

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]