| Message ID | CA+55aFzstE-+NzfSAWMEokB7-rYsZOcZe9Ez-LxPNOKnciJ3UQ@mail.gmail.com |
|---|---|
| State | Not Applicable |
| Delegated to: | David Miller |
| Headers | show |
Linus Torvalds <torvalds@linux-foundation.org> writes: > So I suspect we should just apply this patch, but I didn't check > exacty what the failed tests are - except for the first one, that just > compares against a canned response (and the canned response should > just be changed). After applying your patch and running $ perl -pi -e 'if (/\ttag /) { s/754b754407bf032e9a2f9d5a9ad05ca79a6b228f/6c9dec2b923228c9ff994c6cfe4ae16c12408dc5/; s/0567da4d5edd2ff4bb292a465ba9e64dcad9536b/c61a82b60967180544e3c19f819ddbd0c9f89899/; s/6134ee8f857693b96ff1cc98d3e2fd62b199e5a8/525b7fb068d59950d185a8779dc957c77eed73ba/; }' t/t5515/fetch.* to unpeel the three tags used in the test 5515 that used to expect FETCH_HEAD to have peeled tags to expect tag objects themselves instead, all tests passes. > although I suspect it was just a fairly mindless case of "make it a > commit, because the merge needs the commit" - never mind that the > merge would peel it anyway. I am 100% sure the machinery that comes up with the tree (or half-merged conflicted state) does not mind being fed tags. After all they need to peel them down to commits for common ancestor discovery, and they need to further peel them down to trees to perform three-way merges. However we would need to audit so that we do not accidentally record the tag object names in the "parent" headers in the merge commits, which is what I'll be doing next. -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Junio C Hamano <gitster@pobox.com> writes: > However we would need to audit so that we do not accidentally record the > tag object names in the "parent" headers in the merge commits, which is > what I'll be doing next. Just reporting my findings. builtin/merge.c was updated to use want_commit() that uses peel_to_type() to commit to make sure we do not get fed anything funky, and also uses struct commit_list to pass around list of parents to be recorded, so we should be OK in this department. -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Nov 4, 2011 at 2:22 PM, Junio C Hamano <junio@pobox.com> wrote: > > builtin/merge.c was updated to use want_commit() that uses peel_to_type() > to commit to make sure we do not get fed anything funky, and also uses > struct commit_list to pass around list of parents to be recorded, so we > should be OK in this department. I'm pretty sure people have already done "git merge v3.1" kind of things using local tags (where no peeling of FETCH_HEAD has been done). See git log --merges --grep 'Merge.*v[23]\.[0-9]' for a ton of examples of this (and there's something odd going on: we have "Merge commit .." and "Merge tag ..", and I suspect the latter is people editing it to be correct by hand, but I dunno). So this has always worked, methinks. However - exactly beause git apparently makes it do that "Merge commit " message, I suspect we've peeled things too early and too much. We've peeled it so early that once again something thinks it's a commit, not a tag. So if anything, I suspect "git merge" not only peels, but peels too much (or at the wrong point). We should probably peel as late as possible. But it's a small detail. Linus -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
builtin/fetch.c | 3 +-- 1 files changed, 1 insertions(+), 2 deletions(-) diff --git a/builtin/fetch.c b/builtin/fetch.c index 91731b909aeb..494a7f9976f8 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -436,8 +436,7 @@ static int store_updated_refs(const char *raw_url, const char *remote_name, } note[note_len] = '\0'; fprintf(fp, "%s\t%s\t%s", - sha1_to_hex(commit ? commit->object.sha1 : - rm->old_sha1), + sha1_to_hex(rm->old_sha1), rm->merge ? "" : "not-for-merge", note); for (i = 0; i < url_len; ++i)