From: Junio C Hamano <gitster@pobox.com>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Mauro Carvalho Chehab <mchehab@osg.samsung.com>,
Stephen Rothwell <sfr@canb.auug.org.au>,
Linux-Next Mailing List <linux-next@vger.kernel.org>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
Git Mailing List <git@vger.kernel.org>
Subject: Re: linux-next: unnecessary merge in the v4l-dvb tree
Date: Wed, 14 Feb 2018 10:12:44 -0800 [thread overview]
Message-ID: <xmqqfu63o2xv.fsf@gitster-ct.c.googlers.com> (raw)
In-Reply-To: <CA+55aFxkYTeY9h=VHFXi=gbXsnsHCRMAVZ9=1_EsFGSqr0sj9g@mail.gmail.com> (Linus Torvalds's message of "Tue, 13 Feb 2018 09:33:02 -0800")
Linus Torvalds <torvalds@linux-foundation.org> writes:
> On Tue, Feb 13, 2018 at 9:18 AM, Junio C Hamano <gitster@pobox.com> wrote:
>>
>> That makes me wonder if another heuristic I floated earlier is more
>> appropriate. When merging a tag object T, if refs/tags/T exists and
>> it is that tag object, then an updated "merge" would default to "--ff";
>> otherwise, it would keep the current default of creating a merge even
>> when we could fast-forward, in order to record that tag T in the
>> resulting history.
>
> Oooh. Yes, that sounds like the right thing to do.
>
> So the "no fast-forward" logic would trigger only if the name we used
> for merging is one of the temporary ones (ie .git/{FETCH,MERGE}_HEAD),
> not if the mentioned tag is already a normal tag reference.
>
> Then it's very explicitly about "don't lose the signing information".
>
> I'd still have to teach people to use "--only-ff" if they don't do the
> "fetch and merge" model but literally just do "git pull upstream
> vX.Y", but at least the case Mauro describes would automatically just
> DTRT.
>
> Me likey.
The implementation cannot exactly be "did the user give FETCH_HEAD
or v4.16-rc1 from the command line?", because we'd want to catch it
when Mauro says "git fetch linus && git merge v4.16-rc1" and behave
identically as "git pull linus v4.16-rc1" (and the latter internally
gets turned into "git merge FETCH_HEAD").
So, instead, we read the "tag" line from the tag object to learn the
tagname T, see if refs/tags/T exists and points at that object, to
see if we are Mauro who follows your tags, or if we are you who
fetch and merge contributors' "for-linus" signed tag (which I am
assuming you won't contaminate your refs/tags/ hierarchy with).
There are a few fallouts in the testsuite if we go this route. I am
not quite decided if I like the approach.
builtin/merge.c | 42 ++++++++++++++++++++++++++++++++++++++----
t/t6200-fmt-merge-msg.sh | 2 +-
t/t7600-merge.sh | 2 +-
3 files changed, 40 insertions(+), 6 deletions(-)
diff --git a/builtin/merge.c b/builtin/merge.c
index 30264cfd7c..45c7916505 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -33,6 +33,7 @@
#include "sequencer.h"
#include "string-list.h"
#include "packfile.h"
+#include "tag.h"
#define DEFAULT_TWOHEAD (1<<0)
#define DEFAULT_OCTOPUS (1<<1)
@@ -1125,6 +1126,42 @@ static struct commit_list *collect_parents(struct commit *head_commit,
return remoteheads;
}
+static int merging_a_throwaway_tag(struct commit *commit)
+{
+ const char *tag_ref;
+ struct object_id oid;
+
+ /* Are we merging a tag? */
+ if (!merge_remote_util(commit) ||
+ !merge_remote_util(commit)->obj ||
+ merge_remote_util(commit)->obj->type != OBJ_TAG)
+ return 0;
+
+ /*
+ * Now we know we are merging a tag object. Are we downstream
+ * and following the tags from upstream? If so, we must have
+ * the tag object pointed at by "refs/tags/$T" where $T is the
+ * tagname recorded in the tag object. We want to allow such
+ * a "just to catch up" merge to fast-forward.
+ */
+ tag_ref = xstrfmt("refs/tags/%s",
+ ((struct tag *)merge_remote_util(commit)->obj)->tag);
+
+ if (!read_ref(tag_ref, &oid) &&
+ !oidcmp(&oid, &merge_remote_util(commit)->obj->oid))
+ return 0;
+
+ /*
+ * Otherwise, we are playing an integrator's role, making a
+ * merge with a throw-away tag from a contributor with
+ * something like "git pull $contributor $signed_tag".
+ * We want to forbid such a merge from fast-forwarding
+ * by default; otherwise we would not keep the signature
+ * anywhere.
+ */
+ return 1;
+}
+
int cmd_merge(int argc, const char **argv, const char *prefix)
{
struct object_id result_tree, stash, head_oid;
@@ -1322,10 +1359,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
oid_to_hex(&commit->object.oid));
setenv(buf.buf, merge_remote_util(commit)->name, 1);
strbuf_reset(&buf);
- if (fast_forward != FF_ONLY &&
- merge_remote_util(commit) &&
- merge_remote_util(commit)->obj &&
- merge_remote_util(commit)->obj->type == OBJ_TAG)
+ if (fast_forward != FF_ONLY && merging_a_throwaway_tag(commit))
fast_forward = FF_NO;
}
diff --git a/t/t6200-fmt-merge-msg.sh b/t/t6200-fmt-merge-msg.sh
index 2e2fb0e957..a54a52aaa4 100755
--- a/t/t6200-fmt-merge-msg.sh
+++ b/t/t6200-fmt-merge-msg.sh
@@ -512,7 +512,7 @@ test_expect_success 'merge-msg with "merging" an annotated tag' '
test_when_finished "git reset --hard" &&
annote=$(git rev-parse annote) &&
- git merge --no-commit $annote &&
+ git merge --no-commit --no-ff $annote &&
{
cat <<-EOF
Merge tag '\''$annote'\''
diff --git a/t/t7600-merge.sh b/t/t7600-merge.sh
index dfde6a675a..28a1c43ca7 100755
--- a/t/t7600-merge.sh
+++ b/t/t7600-merge.sh
@@ -718,7 +718,7 @@ test_expect_success GPG 'merge --no-edit tag should skip editor' '
git tag -f -s -m "A newer commit" signed &&
git reset --hard c0 &&
- EDITOR=false git merge --no-edit signed &&
+ EDITOR=false git merge --no-edit --no-ff signed &&
git rev-parse signed^0 >expect &&
git rev-parse HEAD^2 >actual &&
test_cmp expect actual
next prev parent reply other threads:[~2018-02-14 18:12 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20180213080036.3bf3a908@canb.auug.org.au>
2018-02-12 21:15 ` linux-next: unnecessary merge in the v4l-dvb tree Linus Torvalds
2018-02-12 21:36 ` Mauro Carvalho Chehab
2018-02-12 21:37 ` Linus Torvalds
2018-02-12 21:44 ` Junio C Hamano
2018-02-12 21:59 ` Linus Torvalds
2018-02-12 23:42 ` Junio C Hamano
2018-02-13 0:21 ` Mauro Carvalho Chehab
2018-02-13 17:18 ` Junio C Hamano
2018-02-13 17:33 ` Linus Torvalds
2018-02-14 18:12 ` Junio C Hamano [this message]
2018-02-15 22:45 ` [PATCH] merge: allow fast-forward when merging a tracked tag Junio C Hamano
2018-02-15 23:34 ` Eric Sunshine
2018-02-16 18:06 ` Junio C Hamano
2018-02-16 21:27 ` [PATCH v2] " Junio C Hamano
2018-02-12 21:37 ` linux-next: unnecessary merge in the v4l-dvb tree Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqfu63o2xv.fsf@gitster-ct.c.googlers.com \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-next@vger.kernel.org \
--cc=mchehab@osg.samsung.com \
--cc=sfr@canb.auug.org.au \
--cc=torvalds@linux-foundation.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).