# [PATCH 0/2] am: foreign patch support fixes 9 messages from 2011-08-29 to 2011-08-31. Participants: Giuseppe Bilotta, Junio C Hamano, Sverre Rabbelier. Thread: https://gitlist.dev/t/28248 ## Giuseppe Bilotta, 2011-08-29 16:44 Subject: [PATCH 0/2] am: foreign patch support fixes Message-ID: <1314636247-26125-1-git-send-email-giuseppe.bilotta@gmail.com> URL: https://gitlist.dev/e/1314636247-26125-1-git-send-email-giuseppe.bilotta%40gmail.com ``` Two small patches to fix/enhance support for foreign patchset. The first patch adds support for hg patches, which have been detected (but not supported) for a while. I've used it to import a couple of patches successfully. The second patch fixes a rather long-standing issue with stgit patches, when Author was used instead of From. Apparently not many patches with this format are encountered in the wild, since nobody had an issue with it so far. Giuseppe Bilotta (2): am: preliminary support for hg patches am: fix stgit patch mangling git-am.sh | 36 +++++++++++++++++++++++++++++++++++- 1 files changed, 35 insertions(+), 1 deletions(-) -- 1.7.7.rc0.331.g25483.dirty ``` ## Giuseppe Bilotta, 2011-08-29 16:44 Subject: [PATCH 1/2] am: preliminary support for hg patches Message-ID: <1314636247-26125-2-git-send-email-giuseppe.bilotta@gmail.com> URL: https://gitlist.dev/e/1314636247-26125-2-git-send-email-giuseppe.bilotta%40gmail.com In-Reply-To: <1314636247-26125-1-git-send-email-giuseppe.bilotta@gmail.com> ``` Signed-off-by: Giuseppe Bilotta --- git-am.sh | 34 ++++++++++++++++++++++++++++++++++ 1 files changed, 34 insertions(+), 0 deletions(-) diff --git a/git-am.sh b/git-am.sh index 4fff195..729ee51 100755 --- a/git-am.sh +++ b/git-am.sh @@ -311,6 +311,40 @@ split_patches () this= msgnum= ;; + hg) + this=0 + for hg in "$@" + do + this=`expr "$this" + 1` + msgnum=`printf "%0${prec}d" $this` + # hg stores changeset metadata in #-commented lines preceding + # the commit message and diff(s). The only metadata we care about + # are the User and Date (Node ID and Parent are hashes which are + # only relevant to the hg repository and thus not useful to us) + # Since we cannot guarantee that the commit message is in git-friendly + # format, we put no Subject: line and just consume all of the message + # as the body + perl -M'POSIX qw(strftime)' -ne 'BEGIN { $subject = 0 } + if ($subject) { print ; } + elsif (/^\# User /) { s/\# User/From:/ ; print ; } + elsif (/^\# Date /) { + my ($hashsign, $str, $time, $tz) = split ; + $tz = sprintf "%+05d", (0-$tz)/36; + print "Date: " . + strftime("%a, %d %b %Y %H:%M:%S ", + localtime($time)) + . "$tz\n"; + } elsif (/^\# /) { next ; } + else { + print "\n", $_ ; + $subject = 1; + } + ' < "$hg" > "$dotest/$msgnum" || clean_abort + done + echo "$this" > "$dotest/last" + this= + msgnum= + ;; *) if test -n "$patch_format" ; then clean_abort "$(eval_gettext "Patch format \$patch_format is not supported.")" -- 1.7.7.rc0.331.g25483.dirty ``` ## Giuseppe Bilotta, 2011-08-29 16:44 Subject: [PATCH 2/2] am: fix stgit patch mangling Message-ID: <1314636247-26125-3-git-send-email-giuseppe.bilotta@gmail.com> URL: https://gitlist.dev/e/1314636247-26125-3-git-send-email-giuseppe.bilotta%40gmail.com In-Reply-To: <1314636247-26125-1-git-send-email-giuseppe.bilotta@gmail.com> ``` Signed-off-by: Giuseppe Bilotta --- git-am.sh | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/git-am.sh b/git-am.sh index 729ee51..14696d4 100755 --- a/git-am.sh +++ b/git-am.sh @@ -295,7 +295,7 @@ split_patches () perl -ne 'BEGIN { $subject = 0 } if ($subject > 1) { print ; } elsif (/^\s+$/) { next ; } - elsif (/^Author:/) { print s/Author/From/ ; } + elsif (/^Author:/) { s/Author/From/ ; print ;} elsif (/^(From|Date)/) { print ; } elsif ($subject) { $subject = 2 ; -- 1.7.7.rc0.331.g25483.dirty ``` ## Junio C Hamano, 2011-08-29 16:57 Subject: Re: [PATCH 1/2] am: preliminary support for hg patches Message-ID: <7v62lg6tr3.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7v62lg6tr3.fsf%40alter.siamese.dyndns.org In-Reply-To: <1314636247-26125-2-git-send-email-giuseppe.bilotta@gmail.com> ``` Giuseppe Bilotta writes: > Signed-off-by: Giuseppe Bilotta I'll leave nitpicking of this patch and helping to improve it to people who actually have to deal with Hg generated patches for now. > + hg) > + this=0 > + for hg in "$@" > + do > + this=`expr "$this" + 1` > + msgnum=`printf "%0${prec}d" $this` > + # hg stores changeset metadata in #-commented lines preceding > + # the commit message and diff(s). The only metadata we care about > + # are the User and Date (Node ID and Parent are hashes which are > + # only relevant to the hg repository and thus not useful to us) > + # Since we cannot guarantee that the commit message is in git-friendly > + # format, we put no Subject: line and just consume all of the message > + # as the body Personally I am a bit worried about the phoney "diff --git" output Hg seems to (be able to) produce. Do they have "index ..." line that express the blob object names in git terms (implausible), for example? We _might_ want to strip s/diff --git /diff / so that apply won't be confused if that turns out to be a problem. Thanks. > + perl -M'POSIX qw(strftime)' -ne 'BEGIN { $subject = 0 } > + if ($subject) { print ; } > + elsif (/^\# User /) { s/\# User/From:/ ; print ; } > + elsif (/^\# Date /) { > + my ($hashsign, $str, $time, $tz) = split ; > + $tz = sprintf "%+05d", (0-$tz)/36; > + print "Date: " . > + strftime("%a, %d %b %Y %H:%M:%S ", > + localtime($time)) > + . "$tz\n"; > + } elsif (/^\# /) { next ; } > + else { > + print "\n", $_ ; > + $subject = 1; > + } > + ' < "$hg" > "$dotest/$msgnum" || clean_abort > + done > + echo "$this" > "$dotest/last" > + this= > + msgnum= > + ;; > *) > if test -n "$patch_format" ; then > clean_abort "$(eval_gettext "Patch format \$patch_format is not supported.")" ``` ## Giuseppe Bilotta, 2011-08-29 17:51 Subject: Re: [PATCH 1/2] am: preliminary support for hg patches Message-ID: URL: https://gitlist.dev/e/CAOxFTcyqGCB3TcS2CmFuVXqrCP2H-1aBDv3JJVKrNp-Q8Zahmg%40mail.gmail.com In-Reply-To: <7v62lg6tr3.fsf@alter.siamese.dyndns.org> ``` On Mon, Aug 29, 2011 at 6:57 PM, Junio C Hamano wrote: > Giuseppe Bilotta writes: > >> Signed-off-by: Giuseppe Bilotta > > I'll leave nitpicking of this patch and helping to improve it to people > who actually have to deal with Hg generated patches for now. [snip] >> +                     # hg stores changeset metadata in #-commented lines preceding >> +                     # the commit message and diff(s). The only metadata we care about >> +                     # are the User and Date (Node ID and Parent are hashes which are >> +                     # only relevant to the hg repository and thus not useful to us) >> +                     # Since we cannot guarantee that the commit message is in git-friendly >> +                     # format, we put no Subject: line and just consume all of the message >> +                     # as the body > > Personally I am a bit worried about the phoney "diff --git" output Hg > seems to (be able to) produce. Do they have "index ..." line that express > the blob object names in git terms (implausible), for example? We _might_ > want to strip s/diff --git /diff / so that apply won't be confused if that > turns out to be a problem. Nope, it doesn't have index .... lines. Still, the patches seems to apply correctly. Well, the couple of patches I tested did, at least, even though they were marked as diff --git and they were lacking the index ... lines. -- Giuseppe "Oblomov" Bilotta ``` ## Junio C Hamano, 2011-08-29 21:05 Subject: Re: [PATCH 1/2] am: preliminary support for hg patches Message-ID: <7vd3fo53oe.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vd3fo53oe.fsf%40alter.siamese.dyndns.org In-Reply-To: ``` Giuseppe Bilotta writes: > Nope, it doesn't have index .... lines. Still, the patches seems to > apply correctly. Well, the couple of patches I tested did, at least, > even though they were marked as diff --git and they were lacking the > index ... lines. Does "am -3" do the right thing when the patch does not apply cleanly, for example? What about renaming patches? Until we are reasonably sure that we can grok it reasonably well, I'd sleep better if we stripped " --git" part from a patch that is known to be produced by somebody that does not fully re-implement "git diff". ``` ## Giuseppe Bilotta, 2011-08-30 08:28 Subject: Re: [PATCH 1/2] am: preliminary support for hg patches Message-ID: URL: https://gitlist.dev/e/CAOxFTczyNtyLWyXppj%3D0UW_zeD3t%2BrDtzt-vwqXkwvWOTdxi2g%40mail.gmail.com In-Reply-To: <7vd3fo53oe.fsf@alter.siamese.dyndns.org> ``` On Mon, Aug 29, 2011 at 11:05 PM, Junio C Hamano wrote: > Giuseppe Bilotta writes: > >> Nope, it doesn't have index .... lines. Still, the patches seems to >> apply correctly. Well, the couple of patches I tested did, at least, >> even though they were marked as diff --git and they were lacking the >> index ... lines. > > Does "am -3" do the right thing when the patch does not apply cleanly, for > example? three-way merges are impossible because all hash information is being stripped (hg stores them in the Node ID and Parent metadata, which we strip, and has no index metadata for the actual diff blocks). This is correctly detected by -3, with Applying: Threeway test fatal: sha1 information is lacking or useless (dir.h). Repository lacks necessary blobs to fall back on 3-way merge. Cannot fall back to three-way merge. Patch failed at 0001 Threeway test The message is a bit misleading (it's not the repo lacking the blobs, it's the patch missing the information), but the process fails as expected. > What about renaming patches? They lack similarity indices, but they seem to be properly formated (and the simple cases I tested apply correctly). In fact, stripping the --git from an hg patch containing a rename makes the patch unusable: error: datetime.move: does not exist in index Patch failed at 0001 Move So I think that keeping the --git is the right choice. -- Giuseppe "Oblomov" Bilotta ``` ## Junio C Hamano, 2011-08-30 17:02 Subject: Re: [PATCH 1/2] am: preliminary support for hg patches Message-ID: <7vwrdu3ka0.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vwrdu3ka0.fsf%40alter.siamese.dyndns.org In-Reply-To: ``` Giuseppe Bilotta writes: > ... This is > correctly detected by -3, with > > Applying: Threeway test > fatal: sha1 information is lacking or useless (dir.h). > Repository lacks necessary blobs to fall back on 3-way merge. > Cannot fall back to three-way merge. > Patch failed at 0001 Threeway test > > The message is a bit misleading (it's not the repo lacking the blobs, > it's the patch missing the information), but the process fails as > expected. > >> What about renaming patches? > > They lack similarity indices, but they seem to be properly formated > (and the simple cases I tested apply correctly). These were exactly what I wanted to know. Thanks for experimenting. > So I think that keeping the --git is the right choice. Yeah, sounds like we are safe and better off keeping it. ``` ## Sverre Rabbelier, 2011-08-31 14:22 Subject: Re: [PATCH 1/2] am: preliminary support for hg patches Message-ID: URL: https://gitlist.dev/e/CAGdFq_gerRi6REiU4HRqD4SgxBA3obMofAzw7HE4Qkxy82k7Sw%40mail.gmail.com In-Reply-To: <7vwrdu3ka0.fsf@alter.siamese.dyndns.org> ``` Heya, On Tue, Aug 30, 2011 at 19:02, Junio C Hamano wrote: > These were exactly what I wanted to know. Thanks for experimenting. And thanks for working on this, Giuseppe! -- Cheers, Sverre Rabbelier ```