{"thread":{"id":"46539","subject":"[PATCH 0/2] filter-branch: support for incremental update + fix for ancient tag format","startedAt":"2017-08-08T08:48:06Z","lastAt":"2017-08-09T20:23:50Z","messageCount":12,"participants":["Ian Campbell","Junio C Hamano","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"325793","messageId":"1502179560.2735.22.camel@hellion.org.uk","threadId":"46539","inReplyTo":null,"subject":"[PATCH 0/2] filter-branch: support for incremental update + fix for ancient tag format","fromName":"Ian Campbell","fromEmail":"ijc@hellion.org.uk","sentAt":"2017-08-08T08:06:00Z","receivedAt":"2017-08-08T08:48:06Z","isPatch":true,"sender":{"key":"ijc@hellion.org.uk","avatar":"https://avatars.githubusercontent.com/u/12985729?v=4"},"body":"Hi,\n\nI've long (since 2013, urk!) been carrying these two changes to git-\nfilter-branch in the split out devicetree source tree[0] which extracts\nall the device tree sources from the Linux kernel source tree.\n\nI think it's about time I sent them here, sorry for the rather extreme\ndelay! I've rebased to 2.14 and retested, I've also pushed a copy to\n[1] where Travis seems happy.\n\nIan.\n\n[0] https://git.kernel.org/pub/scm/linux/kernel/git/devicetree/devicetree-rebasing.git/\n[1] https://github.com/ijc/git/tree/git-filter-branch\n"},{"id":"325794","messageId":"20170808080620.9536-1-ijc@hellion.org.uk","threadId":"46539","inReplyTo":"1502179560.2735.22.camel@hellion.org.uk","subject":"[PATCH 1/2] filter-branch: Add --state-branch to hold pickled copy of ref map","fromName":"Ian Campbell","fromEmail":"ijc@hellion.org.uk","sentAt":"2017-08-08T08:06:19Z","receivedAt":"2017-08-08T08:48:09Z","isPatch":true,"sender":{"key":"ijc@hellion.org.uk","avatar":"https://avatars.githubusercontent.com/u/12985729?v=4"},"body":"Allowing for incremental updates of large trees.\n\nI have been using this as part of the device tree extraction from the Linux\nkernel source since 2013, about time I sent the patch upstream!\n\nSigned-off-by: Ian Campbell <ijc@hellion.org.uk>\n---\n git-filter-branch.sh | 39 ++++++++++++++++++++++++++++++++++++++-\n 1 file changed, 38 insertions(+), 1 deletion(-)\n\ndiff --git a/git-filter-branch.sh b/git-filter-branch.sh\nindex 3a74602ef..d07db3fee 100755\n--- a/git-filter-branch.sh\n+++ b/git-filter-branch.sh\n@@ -86,7 +86,7 @@ USAGE=\"[--setup <command>] [--env-filter <command>]\n \t[--parent-filter <command>] [--msg-filter <command>]\n \t[--commit-filter <command>] [--tag-name-filter <command>]\n \t[--subdirectory-filter <directory>] [--original <namespace>]\n-\t[-d <directory>] [-f | --force]\n+\t[-d <directory>] [-f | --force] [--state-branch <branch>]\n \t[--] [<rev-list options>...]\"\n \n OPTIONS_SPEC=\n@@ -106,6 +106,7 @@ filter_msg=cat\n filter_commit=\n filter_tag_name=\n filter_subdir=\n+state_branch=\n orig_namespace=refs/original/\n force=\n prune_empty=\n@@ -181,6 +182,9 @@ do\n \t--original)\n \t\torig_namespace=$(expr \"$OPTARG/\" : '\\(.*[^/]\\)/*$')/\n \t\t;;\n+\t--state-branch)\n+\t\tstate_branch=\"$OPTARG\"\n+\t\t;;\n \t*)\n \t\tusage\n \t\t;;\n@@ -252,6 +256,20 @@ export GIT_INDEX_FILE\n # map old->new commit ids for rewriting parents\n mkdir ../map || die \"Could not create map/ directory\"\n \n+if [ -n \"$state_branch\" ] ; then\n+\tstate_commit=`git show-ref -s \"$state_branch\"`\n+\tif [ -n \"$state_commit\" ] ; then\n+\t\techo \"Populating map from $state_branch ($state_commit)\" 1>&2\n+\t\tgit show \"$state_commit\":filter.map |\n+\t\t    perl -n -e 'm/(.*):(.*)/ or die;\n+\t\t\t\topen F, \">../map/$1\" or die;\n+\t\t\t\tprint F \"$2\" or die;\n+\t\t\t\tclose(F) or die'\n+\telse\n+\t\techo \"Branch $state_branch does not exist. Will create\" 1>&2\n+\tfi\n+fi\n+\n # we need \"--\" only if there are no path arguments in $@\n nonrevs=$(git rev-parse --no-revs \"$@\") || exit\n if test -z \"$nonrevs\"\n@@ -544,6 +562,25 @@ if [ \"$filter_tag_name\" ]; then\n \tdone\n fi\n \n+if [ -n \"$state_branch\" ] ; then\n+\techo \"Saving rewrite state to $state_branch\" 1>&2\n+\tSTATE_BLOB=$(ls ../map |\n+\t    perl -n -e 'chomp();\n+\t\t\topen F, \"<../map/$_\" or die;\n+\t\t\tchomp($f = <F>); print \"$_:$f\\n\";' |\n+\t    git hash-object -w --stdin )\n+\tSTATE_TREE=$(/bin/echo -e \"100644 blob $STATE_BLOB\\tfilter.map\" | git mktree)\n+\tSTATE_PARENT=$(git show-ref -s \"$state_branch\")\n+\tunset GIT_AUTHOR_NAME GIT_AUTHOR_EMAIL GIT_AUTHOR_DATE\n+\tunset GIT_COMMITTER_NAME GIT_COMMITTER_EMAIL GIT_COMMITTER_DATE\n+\tif [ -n \"$STATE_PARENT\" ] ; then\n+\t    STATE_COMMIT=$(/bin/echo \"Sync\" | git commit-tree \"$STATE_TREE\" -p \"$STATE_PARENT\")\n+\telse\n+\t    STATE_COMMIT=$(/bin/echo \"Sync\" | git commit-tree \"$STATE_TREE\" )\n+\tfi\n+\tgit update-ref \"$state_branch\" \"$STATE_COMMIT\"\n+fi\n+\n cd \"$orig_dir\"\n rm -rf \"$tempdir\"\n \n-- \n2.11.0\n\n"},{"id":"325795","messageId":"20170808080620.9536-2-ijc@hellion.org.uk","threadId":"46539","inReplyTo":"1502179560.2735.22.camel@hellion.org.uk","subject":"[PATCH 2/2] filter-branch: Handle rewritting (very) old style tags which lack tagger","fromName":"Ian Campbell","fromEmail":"ijc@hellion.org.uk","sentAt":"2017-08-08T08:06:20Z","receivedAt":"2017-08-08T08:48:11Z","isPatch":true,"sender":{"key":"ijc@hellion.org.uk","avatar":"https://avatars.githubusercontent.com/u/12985729?v=4"},"body":"Such as v2.6.12-rc2..v2.6.13-rc3 in the Linux kernel source tree.\n\nInsert a fake tag header, since newer `git mktag` wont accept the input\notherwise:\n\n    $ git cat-file tag v2.6.12-rc2\n    object 1da177e4c3f41524e886b7f1b8a0c1fc7321cac2\n    type commit\n    tag v2.6.12-rc2\n\n    Linux v2.6.12-rc2 release\n    -----BEGIN PGP SIGNATURE-----\n    Version: GnuPG v1.2.4 (GNU/Linux)\n\n    iD8DBQBCbW8ZF3YsRnbiHLsRAgFRAKCq/TkuDaEombFABkPqYgGCgWN2lQCcC0qc\n    wznDbFU45A54dZC8RZ5JxyE=\n    =ESRP\n    -----END PGP SIGNATURE-----\n\n    $ git cat-file tag v2.6.12-rc2 | git mktag\n    error: char76: could not find \"tagger \"\n    fatal: invalid tag signature file\n    $ git cat-file tag v2.6.13-rc4 | git mktag\n    7eab951de91d95875ba34ec4c599f37e1208db93\n\nSigned-off-by: Ian Campbell <ijc@hellion.org.uk>\n---\n git-filter-branch.sh | 3 +++\n 1 file changed, 3 insertions(+)\n\ndiff --git a/git-filter-branch.sh b/git-filter-branch.sh\nindex d07db3fee..6927aa2da 100755\n--- a/git-filter-branch.sh\n+++ b/git-filter-branch.sh\n@@ -540,6 +540,9 @@ if [ \"$filter_tag_name\" ]; then\n \t\t\tnew_sha1=$( ( printf 'object %s\\ntype commit\\ntag %s\\n' \\\n \t\t\t\t\t\t\"$new_sha1\" \"$new_ref\"\n \t\t\t\tgit cat-file tag \"$ref\" |\n+\t\t\t\tawk '/^tagger/\t{ tagged=1 }\n+\t\t\t\t     /^$/\t{ if (!tagged && !done) { print \"tagger Unknown <unknown@example.com> 0 +0000\" } ; done=1 }\n+\t\t\t\t     //\t\t{ print }' |\n \t\t\t\tsed -n \\\n \t\t\t\t    -e '1,/^$/{\n \t\t\t\t\t  /^object /d\n-- \n2.11.0\n\n"},{"id":"325858","messageId":"xmqq4lthbwyi.fsf@gitster.mtv.corp.google.com","threadId":"46539","inReplyTo":"20170808080620.9536-1-ijc@hellion.org.uk","subject":"Re: [PATCH 1/2] filter-branch: Add --state-branch to hold pickled copy of ref map","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-08-08T20:56:05Z","receivedAt":"2017-08-08T20:56:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ian Campbell <ijc@hellion.org.uk> writes:\n\n> Allowing for incremental updates of large trees.\n\n\"by doing what\" is missing.  And ...\n\n>\n> I have been using this as part of the device tree extraction from the Linux\n> kernel source since 2013, about time I sent the patch upstream!\n\n... this does not help understanding what is going on.  It belongs\nto the space after three dashes.\n\nPerhaps\n\n\tSubject: filter-branch: stash away ref map in a branch\n\n\tWith \"--state-branch=<branchname>\" option, the mapping from\n\told object names and filtered ones in ./map/ directory is\n\tstashed away in the object database, and the one from the\n\tprevious run is read to populate the ./map/ directory,\n\tallowing for incremental updates of large trees.\n\nor something?\n\n>\n> Signed-off-by: Ian Campbell <ijc@hellion.org.uk>\n> ---\n>  git-filter-branch.sh | 39 ++++++++++++++++++++++++++++++++++++++-\n>  1 file changed, 38 insertions(+), 1 deletion(-)\n>\n> diff --git a/git-filter-branch.sh b/git-filter-branch.sh\n> index 3a74602ef..d07db3fee 100755\n> --- a/git-filter-branch.sh\n> +++ b/git-filter-branch.sh\n> @@ -86,7 +86,7 @@ USAGE=\"[--setup <command>] [--env-filter <command>]\n>  \t[--parent-filter <command>] [--msg-filter <command>]\n>  \t[--commit-filter <command>] [--tag-name-filter <command>]\n>  \t[--subdirectory-filter <directory>] [--original <namespace>]\n> -\t[-d <directory>] [-f | --force]\n> +\t[-d <directory>] [-f | --force] [--state-branch <branch>]\n>  \t[--] [<rev-list options>...]\"\n>  \n>  OPTIONS_SPEC=\n> @@ -106,6 +106,7 @@ filter_msg=cat\n>  filter_commit=\n>  filter_tag_name=\n>  filter_subdir=\n> +state_branch=\n>  orig_namespace=refs/original/\n>  force=\n>  prune_empty=\n> @@ -181,6 +182,9 @@ do\n>  \t--original)\n>  \t\torig_namespace=$(expr \"$OPTARG/\" : '\\(.*[^/]\\)/*$')/\n>  \t\t;;\n> +\t--state-branch)\n> +\t\tstate_branch=\"$OPTARG\"\n> +\t\t;;\n>  \t*)\n>  \t\tusage\n>  \t\t;;\n> @@ -252,6 +256,20 @@ export GIT_INDEX_FILE\n>  # map old->new commit ids for rewriting parents\n>  mkdir ../map || die \"Could not create map/ directory\"\n>  \n> +if [ -n \"$state_branch\" ] ; then\n> +\tstate_commit=`git show-ref -s \"$state_branch\"`\n\nI hate to nitpick styles, especially on this script that already has\nexisting violations, but for completeness:\n\nStyle: we prefer to write $(command substitution) instead.\nStyle: we prefer to write \"if test\", not \"if [\".\nStyle: we prefer to avoid ';' and write \"if test condtion\" and\n       \"then\" on different lines.\n\nIt is a bit curious use of \"show-ref\".  It is not wrong per-se, but\n\"git rev-parse\" may be more common.  I do not care too deeply either\nway, though.\n\nDon't we want to make sure the value given to --state-branch is a\nfull refname, not just a branch name?  What happens when you say \n\n\tfilter-branch --state-branch master\n\nby mistake?  \"show-ref -s\" is likely to show your refs/heads/master,\nand other master branches that appear as remote-tracking branches for\nthe remotes you interact with.\n\n> +\tif [ -n \"$state_commit\" ] ; then\n> +\t\techo \"Populating map from $state_branch ($state_commit)\" 1>&2\n> +\t\tgit show \"$state_commit\":filter.map |\n> +\t\t    perl -n -e 'm/(.*):(.*)/ or die;\n> +\t\t\t\topen F, \">../map/$1\" or die;\n> +\t\t\t\tprint F \"$2\" or die;\n> +\t\t\t\tclose(F) or die'\n\nThe process calling this perl script, which carefully diagnoses\nmalformed input and dies, does not seem to do anything when it sees\nerrors.  Intended?\n\n> +\telse\n> +\t\techo \"Branch $state_branch does not exist. Will create\" 1>&2\n> +\tfi\n> +fi\n> +\n>  # we need \"--\" only if there are no path arguments in $@\n>  nonrevs=$(git rev-parse --no-revs \"$@\") || exit\n>  if test -z \"$nonrevs\"\n> @@ -544,6 +562,25 @@ if [ \"$filter_tag_name\" ]; then\n>  \tdone\n>  fi\n>  \n> +if [ -n \"$state_branch\" ] ; then\n> +\techo \"Saving rewrite state to $state_branch\" 1>&2\n> +\tSTATE_BLOB=$(ls ../map |\n> +\t    perl -n -e 'chomp();\n> +\t\t\topen F, \"<../map/$_\" or die;\n> +\t\t\tchomp($f = <F>); print \"$_:$f\\n\";' |\n\nI see it somewhat gross to pipe the output of \"/bin/ls\" to a Perl\nscript, instead of iterating over \"while (<../map/*>)\" inside the\nscript itself.\n\n> +\t    git hash-object -w --stdin )\n> +\tSTATE_TREE=$(/bin/echo -e \"100644 blob $STATE_BLOB\\tfilter.map\" | git mktree)\n> +\tSTATE_PARENT=$(git show-ref -s \"$state_branch\")\n\nDon't you already have this in $state_commit?\n\nOne advantage of reading $state_branch again at this point is to\ndetect mistakes of running more than one filter-branch (which may\ncause you to read $STATE_PARENT that is different from $state_commit\nyou read earlier), but I do not think that is being done here, so...\n\n> +\tunset GIT_AUTHOR_NAME GIT_AUTHOR_EMAIL GIT_AUTHOR_DATE\n> +\tunset GIT_COMMITTER_NAME GIT_COMMITTER_EMAIL GIT_COMMITTER_DATE\n\nHmph.  I can see that you are trying not to be affected by the\ncommitters and authors of the commits on the branch being filtered\n(which are set by finish_ident shell function), but I wonder if we\ncould (and more importantly \"want to\") do better to preserve the\nreal committer the user who runs the script may have in the\nenvironment before running it.  I guess it does not matter that\nmuch, as long as the user has properly user.{name,email} configured\nelsewhere without relying on the environment variable.\n\n> +\tif [ -n \"$STATE_PARENT\" ] ; then\n> +\t    STATE_COMMIT=$(/bin/echo \"Sync\" | git commit-tree \"$STATE_TREE\" -p \"$STATE_PARENT\")\n> +\telse\n> +\t    STATE_COMMIT=$(/bin/echo \"Sync\" | git commit-tree \"$STATE_TREE\" )\n> +\tfi\n> +\tgit update-ref \"$state_branch\" \"$STATE_COMMIT\"\n> +fi\n> +\n>  cd \"$orig_dir\"\n>  rm -rf \"$tempdir\"\n\nDespite all the above comments, I like what you are trying to\nachieve here.  Thanks for sharing.\n"},{"id":"325859","messageId":"xmqqzib9ai63.fsf@gitster.mtv.corp.google.com","threadId":"46539","inReplyTo":"20170808080620.9536-2-ijc@hellion.org.uk","subject":"Re: [PATCH 2/2] filter-branch: Handle rewritting (very) old style tags which lack tagger","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-08-08T21:00:52Z","receivedAt":"2017-08-08T21:01:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ian Campbell <ijc@hellion.org.uk> writes:\n\n> Such as v2.6.12-rc2..v2.6.13-rc3 in the Linux kernel source tree.\n>\n> Insert a fake tag header, since newer `git mktag` wont accept the input\n> otherwise:\n>\n>     $ git cat-file tag v2.6.12-rc2\n>     object 1da177e4c3f41524e886b7f1b8a0c1fc7321cac2\n>     type commit\n>     tag v2.6.12-rc2\n>\n>     Linux v2.6.12-rc2 release\n>     -----BEGIN PGP SIGNATURE-----\n>     Version: GnuPG v1.2.4 (GNU/Linux)\n>\n>     iD8DBQBCbW8ZF3YsRnbiHLsRAgFRAKCq/TkuDaEombFABkPqYgGCgWN2lQCcC0qc\n>     wznDbFU45A54dZC8RZ5JxyE=\n>     =ESRP\n>     -----END PGP SIGNATURE-----\n>\n>     $ git cat-file tag v2.6.12-rc2 | git mktag\n>     error: char76: could not find \"tagger \"\n>     fatal: invalid tag signature file\n>     $ git cat-file tag v2.6.13-rc4 | git mktag\n>     7eab951de91d95875ba34ec4c599f37e1208db93\n>\n> Signed-off-by: Ian Campbell <ijc@hellion.org.uk>\n> ---\n>  git-filter-branch.sh | 3 +++\n>  1 file changed, 3 insertions(+)\n>\n> diff --git a/git-filter-branch.sh b/git-filter-branch.sh\n> index d07db3fee..6927aa2da 100755\n> --- a/git-filter-branch.sh\n> +++ b/git-filter-branch.sh\n> @@ -540,6 +540,9 @@ if [ \"$filter_tag_name\" ]; then\n>  \t\t\tnew_sha1=$( ( printf 'object %s\\ntype commit\\ntag %s\\n' \\\n>  \t\t\t\t\t\t\"$new_sha1\" \"$new_ref\"\n>  \t\t\t\tgit cat-file tag \"$ref\" |\n> +\t\t\t\tawk '/^tagger/\t{ tagged=1 }\n> +\t\t\t\t     /^$/\t{ if (!tagged && !done) { print \"tagger Unknown <unknown@example.com> 0 +0000\" } ; done=1 }\n> +\t\t\t\t     //\t\t{ print }' |\n>  \t\t\t\tsed -n \\\n>  \t\t\t\t    -e '1,/^$/{\n>  \t\t\t\t\t  /^object /d\n\nWhat the change wants to do makes perfect sense, but piping output\nfrom awk into sed looks somewhat gross.  Perhaps we'd want to roll\nwhat the existing sed script is trying to do into this new awk\nscript?\n\n\n"},{"id":"325926","messageId":"1502264598.2735.30.camel@hellion.org.uk","threadId":"46539","inReplyTo":"xmqqzib9ai63.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 2/2] filter-branch: Handle rewritting (very) old style tags which lack tagger","fromName":"Ian Campbell","fromEmail":"ijc@hellion.org.uk","sentAt":"2017-08-09T07:43:18Z","receivedAt":"2017-08-09T07:43:25Z","isPatch":true,"sender":{"key":"ijc@hellion.org.uk","avatar":"https://avatars.githubusercontent.com/u/12985729?v=4"},"body":"On Tue, 2017-08-08 at 14:00 -0700, Junio C Hamano wrote:\n> > @@ -540,6 +540,9 @@ if [ \"$filter_tag_name\" ]; then\n> > >  \t\t\tnew_sha1=$( ( printf 'object %s\\ntype commit\\ntag %s\\n' \\\n> > >  \t\t\t\t\t\t\"$new_sha1\" \"$new_ref\"\n> > >  \t\t\t\tgit cat-file tag \"$ref\" |\n> > > > +\t\t\t\tawk '/^tagger/\t{ tagged=1 }\n> > > > > +\t\t\t\t     /^$/\t{ if (!tagged && !done) { print \"tagger Unknown <unknown@example.com> 0 +0000\" } ; done=1 }\n> > > > +\t\t\t\t     //\t\t{ print }' |\n> > >  \t\t\t\tsed -n \\\n> > >  \t\t\t\t    -e '1,/^$/{\n> > >  \t\t\t\t\t  /^object /d\n> \n> What the change wants to do makes perfect sense, but piping output\n> from awk into sed looks somewhat gross.  Perhaps we'd want to roll\n> what the existing sed script is trying to do into this new awk\n> script?\n\nI'm far from an awk guru but I think (unit tested in isolation only)\nthat such script would look something like (I also inverted/renamed\ndone into header since it seemed clearer):\n\n    BEGIN    \t    \t    \t    \t    \t    { header=1 }\n    /^tagger /    \t    \t    \t    \t    { tagged=1 }\n    /^$/    \t    \t    \t    \t    \t    { if (!tagged && header) { print \"tagger Unknown <    unknown@example.com    > 0 +0000\" } ; header=0 }\n\n    /^-----BEGIN PGP SIGNATURE-----/    \t    { exit(0) }\n\n    //    \t    \t    \t    \t    \t    { if (!header || $0 !~ /^(object|type|tag )/) { print } }\n\n    Ian.\n"},{"id":"325927","messageId":"1502265467.2735.32.camel@hellion.org.uk","threadId":"46539","inReplyTo":"xmqq4lthbwyi.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 1/2] filter-branch: Add --state-branch to hold pickled copy of ref map","fromName":"Ian Campbell","fromEmail":"ijc@hellion.org.uk","sentAt":"2017-08-09T07:57:47Z","receivedAt":"2017-08-09T07:57:54Z","isPatch":true,"sender":{"key":"ijc@hellion.org.uk","avatar":"https://avatars.githubusercontent.com/u/12985729?v=4"},"body":"On Tue, 2017-08-08 at 13:56 -0700, Junio C Hamano wrote:\n> Ian Campbell <ijc@hellion.org.uk> writes:\n> \n> > Allowing for incremental updates of large trees.\n> \n> \"by doing what\" is missing.  And ...\n> \n> >\n> > I have been using this as part of the device tree extraction from\n> the Linux\n> > kernel source since 2013, about time I sent the patch upstream!\n> \n> ... this does not help understanding what is going on.  It belongs\n> to the space after three dashes.\n> \n> Perhaps\n> \n> \tSubject: filter-branch: stash away ref map in a branch\n> \n> \tWith \"--state-branch=<branchname>\" option, the mapping from\n> \told object names and filtered ones in ./map/ directory is\n> \tstashed away in the object database, and the one from the\n> \tprevious run is read to populate the ./map/ directory,\n> \tallowing for incremental updates of large trees.\n> \n> or something?\n\nYes, thanks that is a lot better.\n\nI'll address the feedback (style nits and all) in the coming weeks,\nheads up that I might be a bit slow, got a busy week this week followed\nby 3 weeks of travel (which might mean no time for hacking or lots,\nhard to say ;-))\n\n> Don't we want to make sure the value given to --state-branch is a\n> full refname, not just a branch name?  What happens when you say \n> \n> \tfilter-branch --state-branch master\n> \n> by mistake?  \"show-ref -s\" is likely to show your refs/heads/master,\n> and other master branches that appear as remote-tracking branches for\n> the remotes you interact with.\n\nI've been using this as `--state-branch refs/heads/filter-state` which\ncreates a local/visible filter-state branch which I also push to a\nremote, so I also have a `refs/remotes/state/filter-state` too.\n\nWhat is the correct way to check for a full ref name? Is it as simple\nas checking for a refs/heads/ prefix or is there a better way?\n\n> > +\tif [ -n \"$state_commit\" ] ; then\n> > +\t\techo \"Populating map from $state_branch\n> ($state_commit)\" 1>&2\n> > +\t\tgit show \"$state_commit\":filter.map |\n> > +\t\t    perl -n -e 'm/(.*):(.*)/ or die;\n> > +\t\t\t\topen F, \">../map/$1\" or die;\n> > +\t\t\t\tprint F \"$2\" or die;\n> > +\t\t\t\tclose(F) or die'\n> \n> The process calling this perl script, which carefully diagnoses\n> malformed input and dies, does not seem to do anything when it sees\n> errors.  Intended?\n\nI hadn't realised the script wasn't using `set -e`. I'll sort this with\nsome local error handling.\n\n> \n> > +\telse\n> > +\t\techo \"Branch $state_branch does not exist. Will\n> create\" 1>&2\n> > +\tfi\n> > +fi\n> > +\n> >  # we need \"--\" only if there are no path arguments in $@\n> >  nonrevs=$(git rev-parse --no-revs \"$@\") || exit\n> >  if test -z \"$nonrevs\"\n> > @@ -544,6 +562,25 @@ if [ \"$filter_tag_name\" ]; then\n> >  \tdone\n> >  fi\n> >  \n> > +if [ -n \"$state_branch\" ] ; then\n> > +\techo \"Saving rewrite state to $state_branch\" 1>&2\n> > +\tSTATE_BLOB=$(ls ../map |\n> > +\t    perl -n -e 'chomp();\n> > +\t\t\topen F, \"<../map/$_\" or die;\n> > +\t\t\tchomp($f = <F>); print \"$_:$f\\n\";' |\n> \n> I see it somewhat gross to pipe the output of \"/bin/ls\" to a Perl\n> script, instead of iterating over \"while (<../map/*>)\" inside the\n> script itself.\n\nI considered cleaning this up too as I was forward porting, but weirdly\nit appeared to microbenchmark slower that way, I don't remember the\nmagnitude of the difference (and the test script is on another machine\nright now). I'll revisit that and if it isn't too much slower I'll\nswitch to the saner looking all in Perl method.\n\n> > +\tunset GIT_AUTHOR_NAME GIT_AUTHOR_EMAIL GIT_AUTHOR_DATE\n> > +\tunset GIT_COMMITTER_NAME GIT_COMMITTER_EMAIL\n> GIT_COMMITTER_DATE\n> \n> Hmph.  I can see that you are trying not to be affected by the\n> committers and authors of the commits on the branch being filtered\n> (which are set by finish_ident shell function), but I wonder if we\n> could (and more importantly \"want to\") do better to preserve the\n> real committer the user who runs the script may have in the\n> environment before running it.  I guess it does not matter that\n> much, as long as the user has properly user.{name,email} configured\n> elsewhere without relying on the environment variable.\n\nI'm glad you spotted this because I couldn't remember ;-)\n\nI'll stash these in a bunch of ORIG_FOO near the top and then reset\nthem at an appropriate point (I'll use ORIG_GIT_DIR as the pattern).\n\n> Despite all the above comments, I like what you are trying to\n> achieve here.  Thanks for sharing.\n\nThanks for the review and feedback.\n\n\nIan.\n"},{"id":"325933","messageId":"20170809102040.l5sb6ukqh2225zqm@sigill.intra.peff.net","threadId":"46539","inReplyTo":"20170808080620.9536-2-ijc@hellion.org.uk","subject":"Re: [PATCH 2/2] filter-branch: Handle rewritting (very) old style tags which lack tagger","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-08-09T10:20:41Z","receivedAt":"2017-08-09T10:20:49Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Aug 08, 2017 at 09:06:20AM +0100, Ian Campbell wrote:\n\n> Such as v2.6.12-rc2..v2.6.13-rc3 in the Linux kernel source tree.\n> \n> Insert a fake tag header, since newer `git mktag` wont accept the input\n> otherwise:\n\nHmm. Now your resulting tag will have this crufty \"unknown@example.com\"\nheader baked into it, won't it?\n\nShould we instead make git-mktag more lenient (possibly with a\ncommand-line option to reduce accidental omissions)?\n\n-Peff\n"},{"id":"325961","messageId":"xmqqlgms7nbl.fsf@gitster.mtv.corp.google.com","threadId":"46539","inReplyTo":"20170809102040.l5sb6ukqh2225zqm@sigill.intra.peff.net","subject":"Re: [PATCH 2/2] filter-branch: Handle rewritting (very) old style tags which lack tagger","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-08-09T15:50:06Z","receivedAt":"2017-08-09T15:50:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Tue, Aug 08, 2017 at 09:06:20AM +0100, Ian Campbell wrote:\n>\n>> Such as v2.6.12-rc2..v2.6.13-rc3 in the Linux kernel source tree.\n>> \n>> Insert a fake tag header, since newer `git mktag` wont accept the input\n>> otherwise:\n>\n> Hmm. Now your resulting tag will have this crufty \"unknown@example.com\"\n> header baked into it, won't it?\n>\n> Should we instead make git-mktag more lenient (possibly with a\n> command-line option to reduce accidental omissions)?\n\nThat sounds sensible. Thanks for injecting a dose of sanity.\n"},{"id":"325982","messageId":"1502305353.2735.33.camel@hellion.org.uk","threadId":"46539","inReplyTo":"xmqqlgms7nbl.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 2/2] filter-branch: Handle rewritting (very) old style tags which lack tagger","fromName":"Ian Campbell","fromEmail":"ijc@hellion.org.uk","sentAt":"2017-08-09T19:02:33Z","receivedAt":"2017-08-09T19:03:28Z","isPatch":true,"sender":{"key":"ijc@hellion.org.uk","avatar":"https://avatars.githubusercontent.com/u/12985729?v=4"},"body":"On Wed, 2017-08-09 at 08:50 -0700, Junio C Hamano wrote:\n> Jeff King <peff@peff.net> writes:\n> \n> > On Tue, Aug 08, 2017 at 09:06:20AM +0100, Ian Campbell wrote:\n> >\n> >> Such as v2.6.12-rc2..v2.6.13-rc3 in the Linux kernel source tree.\n> >> \n> >> Insert a fake tag header, since newer `git mktag` wont accept the\n> input\n> >> otherwise:\n> >\n> > Hmm. Now your resulting tag will have this crufty \"unknown@example.\n> com\"\n> > header baked into it, won't it?\n> >\n> > Should we instead make git-mktag more lenient (possibly with a\n> > command-line option to reduce accidental omissions)?\n> \n> That sounds sensible. Thanks for injecting a dose of sanity.\n\nIndeed. I'll add a --allow-missing-tagger option (suggestions for a\nsnappier name accepted!) and pass it unconditionally from the filter-\nbranch script.\n\nIan.\n\n"},{"id":"325983","messageId":"xmqqo9ro5zgc.fsf@gitster.mtv.corp.google.com","threadId":"46539","inReplyTo":"1502305353.2735.33.camel@hellion.org.uk","subject":"Re: [PATCH 2/2] filter-branch: Handle rewritting (very) old style tags which lack tagger","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-08-09T19:10:59Z","receivedAt":"2017-08-09T19:11:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ian Campbell <ijc@hellion.org.uk> writes:\n\n> Indeed. I'll add a --allow-missing-tagger option (suggestions for a\n> snappier name accepted!) and pass it unconditionally from the filter-\n> branch script.\n\nThanks.  That's much better.\n"},{"id":"325984","messageId":"20170809202342.si5d72s44v3xywvq@sigill.intra.peff.net","threadId":"46539","inReplyTo":"1502305353.2735.33.camel@hellion.org.uk","subject":"Re: [PATCH 2/2] filter-branch: Handle rewritting (very) old style tags which lack tagger","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-08-09T20:23:42Z","receivedAt":"2017-08-09T20:23:50Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Aug 09, 2017 at 08:02:33PM +0100, Ian Campbell wrote:\n\n> > > Should we instead make git-mktag more lenient (possibly with a\n> > > command-line option to reduce accidental omissions)?\n> > \n> > That sounds sensible. Thanks for injecting a dose of sanity.\n> \n> Indeed. I'll add a --allow-missing-tagger option (suggestions for a\n> snappier name accepted!) and pass it unconditionally from the filter-\n> branch script.\n\nI think that name is the right amount of snappy. It's not meant to be\nused very often. :)\n\n-Peff\n"}]}