# [PATCH] filter-branch: Grok special characters in tag names 6 messages from 2008-08-21 to 2008-08-21. Participants: Johannes Sixt, Brandon Casey, Junio C Hamano. Thread: https://gitlist.dev/t/15137 ## Johannes Sixt, 2008-08-21 14:45 Subject: [PATCH] filter-branch: Grok special characters in tag names Message-ID: <1219329911-31620-1-git-send-email-johannes.sixt@telecom.at> URL: https://gitlist.dev/e/1219329911-31620-1-git-send-email-johannes.sixt%40telecom.at ``` The tag rewriting code used a 'sed' expression to substitute the new tag name into the corresponding field of the annotated tag object. But this is problematic if the tag name contains special characters. In particular, if the tag name contained a slash, then the 'sed' expression had a syntax error. We now protect against this by using 'printf' to assemble the tag header. Signed-off-by: Johannes Sixt --- git-filter-branch.sh | 12 +++++++----- t/t7003-filter-branch.sh | 8 ++++++++ 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/git-filter-branch.sh b/git-filter-branch.sh index 2688254..81392ad 100755 --- a/git-filter-branch.sh +++ b/git-filter-branch.sh @@ -412,15 +412,17 @@ if [ "$filter_tag_name" ]; then echo "$ref -> $new_ref ($sha1 -> $new_sha1)" if [ "$type" = "tag" ]; then - new_sha1=$(git cat-file tag "$ref" | + new_sha1=$( ( printf 'object %s\ntype commit\ntag %s\n' \ + "$new_sha1" "$new_ref" + git cat-file tag "$ref" | sed -n \ -e "1,/^$/{ - s/^object .*/object $new_sha1/ - s/^type .*/type commit/ - s/^tag .*/tag $new_ref/ + /^object /d + /^type /d + /^tag /d }" \ -e '/^-----BEGIN PGP SIGNATURE-----/q' \ - -e 'p' | + -e 'p' ) | git mktag) || die "Could not create new tag object for $ref" if git cat-file tag "$ref" | \ diff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh index 233254f..95f13a8 100755 --- a/t/t7003-filter-branch.sh +++ b/t/t7003-filter-branch.sh @@ -254,4 +254,12 @@ test_expect_success 'Tag name filtering strips gpg signature' ' test_cmp expect actual ' +test_expect_success 'Tag name filtering allows slashes in tag names' ' + git tag -m tag-with-slash X/1 && + git cat-file tag X/1 | sed -e s,X/1,X/2, > expect && + git filter-branch -f --tag-name-filter "echo X/2" && + git cat-file tag X/2 > actual && + test_cmp expect actual +' + test_done -- 1.6.0.137.g19b5f8c ``` ## Brandon Casey, 2008-08-21 16:38 Subject: Re: [PATCH] filter-branch: Grok special characters in tag names Message-ID: URL: https://gitlist.dev/e/NWVPkDIELqWBHTU58gfzDqO8HR575ZDJVO2pYdPMtqv9aBLzMLvyZg%40cipher.nrlssc.navy.mil In-Reply-To: <1219329911-31620-1-git-send-email-johannes.sixt@telecom.at> ``` Johannes Sixt wrote: > diff --git a/git-filter-branch.sh b/git-filter-branch.sh > index 2688254..81392ad 100755 > --- a/git-filter-branch.sh > +++ b/git-filter-branch.sh > @@ -412,15 +412,17 @@ if [ "$filter_tag_name" ]; then > echo "$ref -> $new_ref ($sha1 -> $new_sha1)" > > if [ "$type" = "tag" ]; then > - new_sha1=$(git cat-file tag "$ref" | > + new_sha1=$( ( printf 'object %s\ntype commit\ntag %s\n' \ > + "$new_sha1" "$new_ref" > + git cat-file tag "$ref" | > sed -n \ > -e "1,/^$/{ > - s/^object .*/object $new_sha1/ > - s/^type .*/type commit/ > - s/^tag .*/tag $new_ref/ > + /^object /d > + /^type /d > + /^tag /d Junio complained that my initial version of this was fragile which has similarities with the above. Initially, I was blindly changing the first line to contain "object...", second line to "type...", etc. Would something like the following be equivalent _and_ clearer? Emphasis on "and" because both are necessary, not because I strongly feel it to be so. diff --git a/git-filter-branch.sh b/git-filter-branch.sh index a324cf0..11c5c04 100755 --- a/git-filter-branch.sh +++ b/git-filter-branch.sh @@ -419,9 +419,12 @@ if [ "$filter_tag_name" ]; then new_sha1=$(git cat-file tag "$ref" | sed -n \ -e "1,/^$/{ - s/^object .*/object $new_sha1/ - s/^type .*/type commit/ - s/^tag .*/tag $new_ref/ + /^object .*/c\\ +object $new_sha1 + /^type .*/c\\ +type commit + /^tag .*/c\\ +tag $new_ref }" \ -e '/^-----BEGIN PGP SIGNATURE-----/q' \ -e 'p' | -brandon ``` ## Johannes Sixt, 2008-08-21 19:07 Subject: Re: [PATCH] filter-branch: Grok special characters in tag names Message-ID: <200808212107.33474.johannes.sixt@telecom.at> URL: https://gitlist.dev/e/200808212107.33474.johannes.sixt%40telecom.at In-Reply-To: ``` On Donnerstag, 21. August 2008, Brandon Casey wrote: > Johannes Sixt wrote: > > diff --git a/git-filter-branch.sh b/git-filter-branch.sh > > index 2688254..81392ad 100755 > > --- a/git-filter-branch.sh > > +++ b/git-filter-branch.sh > > @@ -412,15 +412,17 @@ if [ "$filter_tag_name" ]; then > > echo "$ref -> $new_ref ($sha1 -> $new_sha1)" > > > > if [ "$type" = "tag" ]; then > > - new_sha1=$(git cat-file tag "$ref" | > > + new_sha1=$( ( printf 'object %s\ntype commit\ntag %s\n' \ > > + "$new_sha1" "$new_ref" > > + git cat-file tag "$ref" | > > sed -n \ > > -e "1,/^$/{ > > - s/^object .*/object $new_sha1/ > > - s/^type .*/type commit/ > > - s/^tag .*/tag $new_ref/ > > + /^object /d > > + /^type /d > > + /^tag /d > > Junio complained that my initial version of this was fragile which has > similarities with the above. Initially, I was blindly changing the first > line to contain "object...", second line to "type...", etc. I don't see what's wrong with that. "object" must be the first line anyway, otherwise git mktag chokes. > Would something like the following be equivalent _and_ clearer? Emphasis on > "and" because both are necessary, not because I strongly feel it to be so. > > diff --git a/git-filter-branch.sh b/git-filter-branch.sh > index a324cf0..11c5c04 100755 > --- a/git-filter-branch.sh > +++ b/git-filter-branch.sh > @@ -419,9 +419,12 @@ if [ "$filter_tag_name" ]; then > new_sha1=$(git cat-file tag "$ref" | > sed -n \ > -e "1,/^$/{ > - s/^object .*/object $new_sha1/ > - s/^type .*/type commit/ > - s/^tag .*/tag $new_ref/ > + /^object .*/c\\ > +object $new_sha1 > + /^type .*/c\\ > +type commit > + /^tag .*/c\\ > +tag $new_ref > }" \ > -e '/^-----BEGIN PGP SIGNATURE-----/q' \ > -e 'p' | This looks fine, too. (The '.*' in /^foo .*/c\\ is unnecessary.) -- Hannes ``` ## Brandon Casey, 2008-08-21 19:35 Subject: Re: [PATCH] filter-branch: Grok special characters in tag names Message-ID: URL: https://gitlist.dev/e/ON1COPVuxZx0fvgxvAxMTeg1TI9wGwA6G-XV3THb5Y5XvUoIzNchBg%40cipher.nrlssc.navy.mil In-Reply-To: <200808212107.33474.johannes.sixt@telecom.at> ``` Johannes Sixt wrote: > (The '.*' in /^foo .*/c\\ is unnecessary.) Yeah, I noticed after I sent it. -brandon ``` ## Junio C Hamano, 2008-08-21 20:26 Subject: Re: [PATCH] filter-branch: Grok special characters in tag names Message-ID: <7v3akyi07f.fsf@gitster.siamese.dyndns.org> URL: https://gitlist.dev/e/7v3akyi07f.fsf%40gitster.siamese.dyndns.org In-Reply-To: ``` Brandon Casey writes: > Junio complained that my initial version of this was fragile which has > similarities with the above. Initially, I was blindly changing the first line > to contain "object...", second line to "type...", etc. Was it more about not limiting the munging to only the header part? In any case, I think what Hannes has in the patch is fine (although I did not look the lines that follow outside the context). > Would something like the following be equivalent _and_ clearer? Emphasis > on "and" because both are necessary, not because I strongly feel it to > be so. I was bitten by a/i/c followed by literal text that behave differently with various implementations of sed, and learned to stay away from the construct long time ago. Things might have gotten better these days, but old habit and gut-reaction is hard to shake off. ``` ## Brandon Casey, 2008-08-21 21:34 Subject: Re: [PATCH] filter-branch: Grok special characters in tag names Message-ID: URL: https://gitlist.dev/e/yncKq2pMiBaRiN-VbWFQsKLk4CfOXCHjFSB_xG9FtqweMeZ_6Hwf-w%40cipher.nrlssc.navy.mil In-Reply-To: <7v3akyi07f.fsf@gitster.siamese.dyndns.org> ``` Junio C Hamano wrote: > Brandon Casey writes: > >> Junio complained that my initial version of this was fragile which has >> similarities with the above. Initially, I was blindly changing the first line >> to contain "object...", second line to "type...", etc. > > Was it more about not limiting the munging to only the header part? No. I originally had something like 1c\object $new_sha1 2c\tag commit ... blindly changing line one to "object ...", line two to "tag ..." etc. That aspect is what you commented about. So it would have modified the proper parts of a well-formed tag. I took your comments to be concerned with future proofing or dealing with a corrupt/flawed tag (allowing the flaws to propagate so mktag would error out), but you didn't state that explicitly. You also educated me about addresses like 1,/regex/ which was used to limit the munging to only the header part. > In > any case, I think what Hannes has in the patch is fine (although I did not > look the lines that follow outside the context). That's fine. -brandon ```