{"thread":{"id":"48104","subject":"[PATCH] filter-branch: consider refs/replace can refer to an object other than commit","startedAt":"2018-03-21T10:35:22Z","lastAt":"2018-04-08T23:11:01Z","messageCount":20,"participants":["Yuki Kokubun","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"342397","messageId":"20180321103504.9483-1-orga.chem.job@gmail.com","threadId":"48104","inReplyTo":null,"subject":"[PATCH] filter-branch: consider refs/replace can refer to an object other than commit","fromName":"Yuki Kokubun","fromEmail":"orga.chem.job@gmail.com","sentAt":"2018-03-21T10:35:04Z","receivedAt":"2018-03-21T10:35:22Z","isPatch":true,"sender":{"key":"orga.chem.job@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1124024?v=4"},"body":"\"git filter-branch -- --all\" can be confused when refs that refer to objects\nother than commits exists.\nBecause \"git rev-parse --all\" that is internally used can return refs that\nrefer to an object other than commit. But it is not considered in the phase of\nupdating refs. Such refs can be created by \"git replace\" with objects other than\ncommits.\n\nSigned-off-by: Yuki Kokubun <orga.chem.job@gmail.com>\n---\n git-filter-branch.sh     |  3 +++\n t/t7003-filter-branch.sh | 13 +++++++++++++\n 2 files changed, 16 insertions(+)\n\ndiff --git a/git-filter-branch.sh b/git-filter-branch.sh\nindex 1b7e4b2cd..a9fafe64f 100755\n--- a/git-filter-branch.sh\n+++ b/git-filter-branch.sh\n@@ -479,6 +479,9 @@ do\n \t# avoid rewriting a ref twice\n \ttest -f \"$orig_namespace$ref\" && continue\n \n+\t# refs/replace can point to a non commit object\n+\ttest $(git cat-file -t \"$ref\") = commit || continue\n+\n \tsha1=$(git rev-parse \"$ref\"^0)\n \trewritten=$(map $sha1)\n \ndiff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh\nindex 7cb60799b..efeaf5887 100755\n--- a/t/t7003-filter-branch.sh\n+++ b/t/t7003-filter-branch.sh\n@@ -470,4 +470,17 @@ test_expect_success 'tree-filter deals with object name vs pathname ambiguity' '\n \tgit show HEAD:$ambiguous\n '\n \n+test_expect_success 'rewrite repository including refs/replace that point to non commit object' '\n+\ttest_when_finished \"git reset --hard original\" &&\n+\ttree=$(git rev-parse HEAD^{tree}) &&\n+\ttest_when_finished \"git replace -d $tree\" &&\n+\techo A >new &&\n+\tgit add new &&\n+\tnew_tree=$(git write-tree) &&\n+\tgit replace $tree $new_tree &&\n+\tgit reset --hard HEAD &&\n+\tgit filter-branch -f -- --all >filter-output 2>&1 &&\n+\t! fgrep fatal filter-output\n+'\n+\n test_done\n-- \n2.16.2.18.g09cb46d\n\n"},{"id":"342410","messageId":"1521646299-16193-1-git-send-email-orga.chem.job@gmail.com","threadId":"48104","inReplyTo":"20180321103504.9483-1-orga.chem.job@gmail.com","subject":"[PATCH] filter-branch: consider refs can refer to an object other than commit or tag","fromName":"Yuki Kokubun","fromEmail":"orga.chem.job@gmail.com","sentAt":"2018-03-21T15:31:39Z","receivedAt":"2018-03-21T15:32:00Z","isPatch":true,"sender":{"key":"orga.chem.job@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1124024?v=4"},"body":"\"git filter-branch -- --all\" can be confused when refs that refer to objects\nother than commits or tags exists.\nBecause \"git rev-parse --all\" that is internally used can return refs that\nrefer to an object other than commit or tag. But it is not considered in the\nphase of updating refs. Such refs can be created by \"git replace\" with\nobjects other than commits or trees.\n\nSigned-off-by: Yuki Kokubun <orga.chem.job@gmail.com>\n---\nThis patch fix the bug of the first patch.\nThe first patch did not consider tags.\n\n git-filter-branch.sh     | 11 ++++++++++-\n t/t7003-filter-branch.sh | 13 +++++++++++++\n 2 files changed, 23 insertions(+), 1 deletion(-)\n\ndiff --git a/git-filter-branch.sh b/git-filter-branch.sh\nindex 1b7e4b2cd..f7cd97b86 100755\n--- a/git-filter-branch.sh\n+++ b/git-filter-branch.sh\n@@ -251,7 +251,16 @@ done < \"$tempdir\"/backup-refs\n \n # The refs should be updated if their heads were rewritten\n git rev-parse --no-flags --revs-only --symbolic-full-name \\\n-\t--default HEAD \"$@\" > \"$tempdir\"/raw-heads || exit\n+\t--default HEAD \"$@\" > \"$tempdir\"/raw-objects || exit\n+# refs/replace can refer to an object other than commit or tag\n+while read ref\n+do\n+\ttype=$(git cat-file -t \"$ref\")\n+\tif test $type = commit || test $type = tag\n+\tthen\n+\t\techo \"$ref\"\n+\tfi\n+done >\"$tempdir\"/raw-heads <\"$tempdir\"/raw-objects\n sed -e '/^^/d' \"$tempdir\"/raw-heads >\"$tempdir\"/heads\n \n test -s \"$tempdir\"/heads ||\ndiff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh\nindex 7cb60799b..efeaf5887 100755\n--- a/t/t7003-filter-branch.sh\n+++ b/t/t7003-filter-branch.sh\n@@ -470,4 +470,17 @@ test_expect_success 'tree-filter deals with object name vs pathname ambiguity' '\n \tgit show HEAD:$ambiguous\n '\n \n+test_expect_success 'rewrite repository including refs/replace that point to non commit object' '\n+\ttest_when_finished \"git reset --hard original\" &&\n+\ttree=$(git rev-parse HEAD^{tree}) &&\n+\ttest_when_finished \"git replace -d $tree\" &&\n+\techo A >new &&\n+\tgit add new &&\n+\tnew_tree=$(git write-tree) &&\n+\tgit replace $tree $new_tree &&\n+\tgit reset --hard HEAD &&\n+\tgit filter-branch -f -- --all >filter-output 2>&1 &&\n+\t! fgrep fatal filter-output\n+'\n+\n test_done\n-- \n2.16.2.18.g09cb46d\n\n"},{"id":"342427","messageId":"xmqqwoy5pcno.fsf@gitster-ct.c.googlers.com","threadId":"48104","inReplyTo":"1521646299-16193-1-git-send-email-orga.chem.job@gmail.com","subject":"Re: [PATCH] filter-branch: consider refs can refer to an object other than commit or tag","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-03-21T17:21:47Z","receivedAt":"2018-03-21T17:21:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Yuki Kokubun <orga.chem.job@gmail.com> writes:\n\n> \"git filter-branch -- --all\" can be confused when refs that refer to objects\n> other than commits or tags exists.\n> Because \"git rev-parse --all\" that is internally used can return refs that\n> refer to an object other than commit or tag. But it is not considered in the\n> phase of updating refs.\n\nCould you describe what the consequence of that is?  We have a ref\nthat points directly at a blob object, or a ref that points at a tag\nobject that points at a blob object.  The current code leaves both of\nthese refs in \"$tempdir/heads\".  Then...?\n\n\t... goes and looks ...\n\nThere is a loop that looks like this:\n\n\twhile read ref\n\tdo\n\t\tsha1=$(git rev-parse \"$ref^0\")\n\t\t...\n\tdone <\"$tempdir/heads\"\n\nwhich would break on anything but a commit-ish.\n\n>  # The refs should be updated if their heads were rewritten\n>  git rev-parse --no-flags --revs-only --symbolic-full-name \\\n> -\t--default HEAD \"$@\" > \"$tempdir\"/raw-heads || exit\n> +\t--default HEAD \"$@\" > \"$tempdir\"/raw-objects || exit\n> +# refs/replace can refer to an object other than commit or tag\n\nMention of replace refs in the proposed log message gives an easy to\nunderstand example and is a good idea, but this in code comment does\nnot have to single out the replace refs.  A tag can also point at an\nobject with any type, e.g. \"git tag v2.6.11-tree v2.6.11^{tree}\"\nwould make \"refs/tags/v2.6.11-tree\" point at the tree at the top\nlevel of the tree-ish \"v2.6.11\".  It probably is OK to drop this\ncomment altogether.\n\n> +while read ref\n> +do\n> +\ttype=$(git cat-file -t \"$ref\")\n> +\tif test $type = commit || test $type = tag\n> +\tthen\n> +\t\techo \"$ref\"\n> +\tfi\n> +done >\"$tempdir\"/raw-heads <\"$tempdir\"/raw-objects\n>  sed -e '/^^/d' \"$tempdir\"/raw-heads >\"$tempdir\"/heads\n\nSo... is the idea to limit the set of refs to be rewritten to those\nthat point at commits and tags?  As I already alluded to, I do not\nthink you want to accept a ref that points at any tag object---only\nthe ones that point at a tag that points at a commit-ish, so that\nthe code will not barf when doing \"$ref^0\".\n\nSo perhaps\n\n\tgit rev-parse --no-flags ... >\"$tempdir/raw-heads\" || exit\n\n\twhile read ref\n\tdo\n\t\tcase \"$ref\" in ^?*) continue ;; esac\n\t\tif git rev-parse --verify \"$ref^0\" 2>/dev/null\n                then\n\t\t\techo \"$ref\"\n\t\tfi\n\tdone >\"$tempdir/heads\" <\"$tempdir/raw-heads\"\n\nor something?  Note that you do not need the \"sed\" as the loop\nalready excludes the negative revs.\n\n>  test -s \"$tempdir\"/heads ||\n> diff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh\n> index 7cb60799b..efeaf5887 100755\n> --- a/t/t7003-filter-branch.sh\n> +++ b/t/t7003-filter-branch.sh\n> @@ -470,4 +470,17 @@ test_expect_success 'tree-filter deals with object name vs pathname ambiguity' '\n>  \tgit show HEAD:$ambiguous\n>  '\n>  \n> +test_expect_success 'rewrite repository including refs/replace that point to non commit object' '\n> +\ttest_when_finished \"git reset --hard original\" &&\n> +\ttree=$(git rev-parse HEAD^{tree}) &&\n> +\ttest_when_finished \"git replace -d $tree\" &&\n> +\techo A >new &&\n> +\tgit add new &&\n> +\tnew_tree=$(git write-tree) &&\n> +\tgit replace $tree $new_tree &&\n\nPerhaps something like this here:\n\n\tgit tag -a \"tag to a tree\" treetag $new_tree &&\n\ncan tell su how well it works with a tag that points at a tree?\n\n> +\tgit reset --hard HEAD &&\n> +\tgit filter-branch -f -- --all >filter-output 2>&1 &&\n> +\t! fgrep fatal filter-output\n> +'\n> +\n>  test_done\n"},{"id":"342451","messageId":"5ab2b606.86f5620a.c0c10.41b3@mx.google.com","threadId":"48104","inReplyTo":"xmqqwoy5pcno.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH] filter-branch: consider refs can refer to an object other than commit or tag","fromName":"Yuki Kokubun","fromEmail":"orga.chem.job@gmail.com","sentAt":"2018-03-21T20:00:00Z","receivedAt":"2018-03-21T19:44:11Z","isPatch":true,"sender":{"key":"orga.chem.job@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1124024?v=4"},"body":"> Yuki Kokubun <orga.chem.job@gmail.com> writes:\n> \n> > \"git filter-branch -- --all\" can be confused when refs that refer to objects\n> > other than commits or tags exists.\n> > Because \"git rev-parse --all\" that is internally used can return refs that\n> > refer to an object other than commit or tag. But it is not considered in the\n> > phase of updating refs.\n> \n> Could you describe what the consequence of that is?  We have a ref\n> that points directly at a blob object, or a ref that points at a tag\n> object that points at a blob object.  The current code leaves both of\n> these refs in \"$tempdir/heads\".  Then...?\n\nSorry, this is my wrong.\nI wrongly thought only refs/replace can point at a blob or tree object.\n\n> \n> \t... goes and looks ...\n> \n> There is a loop that looks like this:\n> \n> \twhile read ref\n> \tdo\n> \t\tsha1=$(git rev-parse \"$ref^0\")\n> \t\t...\n> \tdone <\"$tempdir/heads\"\n> \n> which would break on anything but a commit-ish.\n> \n> >  # The refs should be updated if their heads were rewritten\n> >  git rev-parse --no-flags --revs-only --symbolic-full-name \\\n> > -\t--default HEAD \"$@\" > \"$tempdir\"/raw-heads || exit\n> > +\t--default HEAD \"$@\" > \"$tempdir\"/raw-objects || exit\n> > +# refs/replace can refer to an object other than commit or tag\n> \n> Mention of replace refs in the proposed log message gives an easy to\n> understand example and is a good idea, but this in code comment does\n> not have to single out the replace refs.  A tag can also point at an\n> object with any type, e.g. \"git tag v2.6.11-tree v2.6.11^{tree}\"\n> would make \"refs/tags/v2.6.11-tree\" point at the tree at the top\n> level of the tree-ish \"v2.6.11\".  It probably is OK to drop this\n> comment altogether.\n\nOK, I'm gonna drop the incorrect comment.\n\n> \n> > +while read ref\n> > +do\n> > +\ttype=$(git cat-file -t \"$ref\")\n> > +\tif test $type = commit || test $type = tag\n> > +\tthen\n> > +\t\techo \"$ref\"\n> > +\tfi\n> > +done >\"$tempdir\"/raw-heads <\"$tempdir\"/raw-objects\n> >  sed -e '/^^/d' \"$tempdir\"/raw-heads >\"$tempdir\"/heads\n> \n> So... is the idea to limit the set of refs to be rewritten to those\n> that point at commits and tags?  As I already alluded to, I do not\n> think you want to accept a ref that points at any tag object---only\n> the ones that point at a tag that points at a commit-ish, so that\n> the code will not barf when doing \"$ref^0\".\n> \n> So perhaps\n> \n> \tgit rev-parse --no-flags ... >\"$tempdir/raw-heads\" || exit\n> \n> \twhile read ref\n> \tdo\n> \t\tcase \"$ref\" in ^?*) continue ;; esac\n> \t\tif git rev-parse --verify \"$ref^0\" 2>/dev/null\n>                 then\n> \t\t\techo \"$ref\"\n> \t\tfi\n> \tdone >\"$tempdir/heads\" <\"$tempdir/raw-heads\"\n> \n> or something?  Note that you do not need the \"sed\" as the loop\n> already excludes the negative revs.\n\nI feel using \"git rev-parse --verify\" is a good way as you said.\nI'm gonna modify the patch to use it.\n\n> \n> >  test -s \"$tempdir\"/heads ||\n> > diff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh\n> > index 7cb60799b..efeaf5887 100755\n> > --- a/t/t7003-filter-branch.sh\n> > +++ b/t/t7003-filter-branch.sh\n> > @@ -470,4 +470,17 @@ test_expect_success 'tree-filter deals with object name vs pathname ambiguity' '\n> >  \tgit show HEAD:$ambiguous\n> >  '\n> >  \n> > +test_expect_success 'rewrite repository including refs/replace that point to non commit object' '\n> > +\ttest_when_finished \"git reset --hard original\" &&\n> > +\ttree=$(git rev-parse HEAD^{tree}) &&\n> > +\ttest_when_finished \"git replace -d $tree\" &&\n> > +\techo A >new &&\n> > +\tgit add new &&\n> > +\tnew_tree=$(git write-tree) &&\n> > +\tgit replace $tree $new_tree &&\n> \n> Perhaps something like this here:\n> \n> \tgit tag -a \"tag to a tree\" treetag $new_tree &&\n> \n> can tell su how well it works with a tag that points at a tree?\n\nSounds good. I'm gonna add such tags to the test case.\n\n> \n> > +\tgit reset --hard HEAD &&\n> > +\tgit filter-branch -f -- --all >filter-output 2>&1 &&\n> > +\t! fgrep fatal filter-output\n> > +'\n> > +\n> >  test_done\n"},{"id":"342454","messageId":"xmqqpo3xnqq4.fsf@gitster-ct.c.googlers.com","threadId":"48104","inReplyTo":"5ab2b606.86f5620a.c0c10.41b3@mx.google.com","subject":"Re: [PATCH] filter-branch: consider refs can refer to an object other than commit or tag","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-03-21T20:00:51Z","receivedAt":"2018-03-21T20:00:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Yuki Kokubun <orga.chem.job@gmail.com> writes:\n\n>> Yuki Kokubun <orga.chem.job@gmail.com> writes:\n>> \n>> > \"git filter-branch -- --all\" can be confused when refs that refer to objects\n>> > other than commits or tags exists.\n>> > Because \"git rev-parse --all\" that is internally used can return refs that\n>> > refer to an object other than commit or tag. But it is not considered in the\n>> > phase of updating refs.\n>> \n>> Could you describe what the consequence of that is?  We have a ref\n>> that points directly at a blob object, or a ref that points at a tag\n>> object that points at a blob object.  The current code leaves both of\n>> these refs in \"$tempdir/heads\".  Then...?\n>\n> Sorry, this is my wrong.\n> I wrongly thought only refs/replace can point at a blob or tree object.\n\nNo need to be sorry.  You still need to describe what (bad things)\nhappen if we do not filter out refs that do not point at committish\nin the proposed log message.  \n\nIOW, can you elaborate and clarify your \"can be confused\" at the\nbeginning?\n"},{"id":"342504","messageId":"5ab3bd79.d14d650a.be5c0.8544@mx.google.com","threadId":"48104","inReplyTo":"xmqqpo3xnqq4.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH] filter-branch: consider refs can refer to an object other than commit or tag","fromName":"Yuki Kokubun","fromEmail":"orga.chem.job@gmail.com","sentAt":"2018-03-22T14:26:10Z","receivedAt":"2018-03-22T14:28:15Z","isPatch":true,"sender":{"key":"orga.chem.job@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1124024?v=4"},"body":"> Yuki Kokubun <orga.chem.job@gmail.com> writes:\n> \n> >> Yuki Kokubun <orga.chem.job@gmail.com> writes:\n> >> \n> >> > \"git filter-branch -- --all\" can be confused when refs that refer to objects\n> >> > other than commits or tags exists.\n> >> > Because \"git rev-parse --all\" that is internally used can return refs that\n> >> > refer to an object other than commit or tag. But it is not considered in the\n> >> > phase of updating refs.\n> >> \n> >> Could you describe what the consequence of that is?  We have a ref\n> >> that points directly at a blob object, or a ref that points at a tag\n> >> object that points at a blob object.  The current code leaves both of\n> >> these refs in \"$tempdir/heads\".  Then...?\n> >\n> > Sorry, this is my wrong.\n> > I wrongly thought only refs/replace can point at a blob or tree object.\n> \n> No need to be sorry.  You still need to describe what (bad things)\n> happen if we do not filter out refs that do not point at committish\n> in the proposed log message.  \n> \n> IOW, can you elaborate and clarify your \"can be confused\" at the\n> beginning?\n\nI meant the confusion is abnormal messages from the output of \"git filter-branch -- --all\".\nFor example, this is an output of \"git filter-branch -- --all\":\n\nRewrite bcdbd016c77df3d5641a3cf820b2ed46ba7bf3b4 (5/5) (0 seconds passed, remaining 0 predicted)\nWARNING: Ref 'refs/heads/master' is unchanged\nWARNING: Ref 'refs/heads/no-newline' is unchanged\nWARNING: Ref 'refs/heads/original' is unchanged\nerror: object 1bf53b49c26465454e4ac377f2ed3f91bb1d6ac1 is a tree, not a commit\nerror: object 1bf53b49c26465454e4ac377f2ed3f91bb1d6ac1 is a tree, not a commit\nfatal: ambiguous argument 'refs/replace/8a2016f3730cad8309c110f819c855403ed0a5b9^0': unknown revision or path not in the working tree.\nUse '--' to separate paths from revisions, like this:\n'git <command> [<revision>...] -- [<file>...]'\nWARNING: Ref 'refs/replace/8a2016f3730cad8309c110f819c855403ed0a5b9' is unchanged\nWARNING: Ref 'refs/tags/add-file' is unchanged\nWARNING: Ref 'refs/tags/file' is unchanged\nerror: object 1bf53b49c26465454e4ac377f2ed3f91bb1d6ac1 is a tree, not a commit\nerror: object 1bf53b49c26465454e4ac377f2ed3f91bb1d6ac1 is a tree, not a commit\nfatal: ambiguous argument 'refs/tags/treetag^0': unknown revision or path not in the working tree.\nUse '--' to separate paths from revisions, like this:\n'git <command> [<revision>...] -- [<file>...]'\nWARNING: Ref 'refs/tags/treetag' is unchanged\n\nYou can see a lot of terrible messages such as \"error\" and \"fatal\".\nBut on the whole, the result of \"git filter-branch -- --all\" is not so abnormal.\nSo, this is a just problem about abonormal messages.\n\nI think this messages should be suppressed.\nHow do you feel about it?\n"},{"id":"342521","messageId":"xmqq370sm4cv.fsf@gitster-ct.c.googlers.com","threadId":"48104","inReplyTo":"5ab3bd79.d14d650a.be5c0.8544@mx.google.com","subject":"Re: [PATCH] filter-branch: consider refs can refer to an object other than commit or tag","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-03-22T17:01:36Z","receivedAt":"2018-03-22T17:01:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Yuki Kokubun <orga.chem.job@gmail.com> writes:\n\n>> Yuki Kokubun <orga.chem.job@gmail.com> writes:\n>> \n>> >> Yuki Kokubun <orga.chem.job@gmail.com> writes:\n>> >> \n>> >> > \"git filter-branch -- --all\" can be confused when refs that refer to objects\n>> >> > other than commits or tags exists.\n>> ...\n>\n> I meant the confusion is abnormal messages from the output of \"git filter-branch -- --all\".\n\nOK, so it is not that the program logic gets confused and ends up\nperforming a wrong rewrite, but mostly that it gives confusing\nmessages.\n\n> For example, this is an output of \"git filter-branch -- --all\":\n>\n> Rewrite bcdbd016c77df3d5641a3cf820b2ed46ba7bf3b4 (5/5) (0 seconds passed, remaining 0 predicted)\n> WARNING: Ref 'refs/heads/master' is unchanged\n> WARNING: Ref 'refs/heads/no-newline' is unchanged\n> WARNING: Ref 'refs/heads/original' is unchanged\n\nThese are worth keeping, as I think existing users expect to see\nthem.\n\n> error: object 1bf53b49c26465454e4ac377f2ed3f91bb1d6ac1 is a tree, not a commit\n> error: object 1bf53b49c26465454e4ac377f2ed3f91bb1d6ac1 is a tree, not a commit\n> fatal: ambiguous argument 'refs/replace/8a2016f3730cad8309c110f819c855403ed0a5b9^0': unknown revision or path not in the working tree.\n> Use '--' to separate paths from revisions, like this:\n> 'git <command> [<revision>...] -- [<file>...]'\n> WARNING: Ref 'refs/replace/8a2016f3730cad8309c110f819c855403ed0a5b9' is unchanged\n> WARNING: Ref 'refs/tags/add-file' is unchanged\n> WARNING: Ref 'refs/tags/file' is unchanged\n> error: object 1bf53b49c26465454e4ac377f2ed3f91bb1d6ac1 is a tree, not a commit\n> error: object 1bf53b49c26465454e4ac377f2ed3f91bb1d6ac1 is a tree, not a commit\n> fatal: ambiguous argument 'refs/tags/treetag^0': unknown revision or path not in the working tree.\n> Use '--' to separate paths from revisions, like this:\n> 'git <command> [<revision>...] -- [<file>...]'\n> WARNING: Ref 'refs/tags/treetag' is unchanged\n\nI think these warning messages should be kept, especially if we are\nto keep the warning messages for the unchanged branches.  However,\nthe internal error messages are unwanted--these are implementation\ndetails that reach the conclusion, i.e. the ref we were asked to\nrewrite ended up being unchanged hence we did not touch it.\n\nHowever, if we pre-filter to limit the refs in \"$tempdir/heads\" to\nthose that are committish (i.e. those that pass \"$ref^0\") like the\npatch and subsequent discussion suggests, wouldn't we lose the\nwarning for these replace refs and non-committish tags.  We perhaps\ncould do something like:\n\n\tgit rev-parse --no-flags ... >\"$tempdir/raw-heads\" || exit\n\n\twhile read ref\n\tdo\n\t\tcase \"$ref\" in ^?*) continue ;; esac\n\t\tif git rev-parse --verify \"$ref^0\" 2>/dev/null\n                then\n\t\t\techo \"$ref\"\n\t\telse\n\t\t\twarn \"WARNING: not rewriting '$ref' (not a committish)\"\n\t\tfi\n\tdone >\"$tempdir/heads\" <\"$tempdir/raw-heads\"\n\n(note: the else clause is new, relative to my earlier suggestion).\n"},{"id":"342555","messageId":"5ab46520.0352650a.cc02b.a177@mx.google.com","threadId":"48104","inReplyTo":"xmqq370sm4cv.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH] filter-branch: consider refs can refer to an object other than commit or tag","fromName":"Yuki Kokubun","fromEmail":"orga.chem.job@gmail.com","sentAt":"2018-03-23T02:15:39Z","receivedAt":"2018-03-23T02:23:33Z","isPatch":true,"sender":{"key":"orga.chem.job@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1124024?v=4"},"body":"> However, if we pre-filter to limit the refs in \"$tempdir/heads\" to\n> those that are committish (i.e. those that pass \"$ref^0\") like the\n> patch and subsequent discussion suggests, wouldn't we lose the\n> warning for these replace refs and non-committish tags.  We perhaps\n> could do something like:\n> \n> \tgit rev-parse --no-flags ... >\"$tempdir/raw-heads\" || exit\n> \n> \twhile read ref\n> \tdo\n> \t\tcase \"$ref\" in ^?*) continue ;; esac\n> \t\tif git rev-parse --verify \"$ref^0\" 2>/dev/null\n>                 then\n> \t\t\techo \"$ref\"\n> \t\telse\n> \t\t\twarn \"WARNING: not rewriting '$ref' (not a committish)\"\n> \t\tfi\n> \tdone >\"$tempdir/heads\" <\"$tempdir/raw-heads\"\n> \n> (note: the else clause is new, relative to my earlier suggestion).\n\nI agree these suggestions.\nI'm gonna send a new patch that follow it.\n"},{"id":"342560","messageId":"20180323050913.5188-1-orga.chem.job@gmail.com","threadId":"48104","inReplyTo":"5ab46520.0352650a.cc02b.a177@mx.google.com","subject":"[PATCH v2] filter-branch: fix errors caused by refs that cannot be used with ^0","fromName":"Yuki Kokubun","fromEmail":"orga.chem.job@gmail.com","sentAt":"2018-03-23T05:09:13Z","receivedAt":"2018-03-23T05:27:13Z","isPatch":true,"sender":{"key":"orga.chem.job@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1124024?v=4"},"body":"\"git filter-branch -- --all\" print unwanted error messages when refs that\ncannot be used with ^0 exist. Such refs can be created by \"git replace\" with\ntrees or blobs. Also, \"git tag\" with trees or blobs can create such refs.\n---\n git-filter-branch.sh     | 14 ++++++++++++--\n t/t7003-filter-branch.sh | 14 ++++++++++++++\n 2 files changed, 26 insertions(+), 2 deletions(-)\n\ndiff --git a/git-filter-branch.sh b/git-filter-branch.sh\nindex 1b7e4b2cd..41efecb28 100755\n--- a/git-filter-branch.sh\n+++ b/git-filter-branch.sh\n@@ -251,8 +251,18 @@ done < \"$tempdir\"/backup-refs\n \n # The refs should be updated if their heads were rewritten\n git rev-parse --no-flags --revs-only --symbolic-full-name \\\n-\t--default HEAD \"$@\" > \"$tempdir\"/raw-heads || exit\n-sed -e '/^^/d' \"$tempdir\"/raw-heads >\"$tempdir\"/heads\n+\t--default HEAD \"$@\" > \"$tempdir\"/raw-refs || exit\n+while read ref\n+do\n+\tcase \"$ref\" in ^?*) continue ;; esac\n+\n+\tif git rev-parse --verify \"$ref\"^0 >/dev/null 2>&1\n+\tthen\n+\t\techo \"$ref\"\n+\telse\n+\t\twarn \"WARNING: not rewriting '$ref' (not a committish)\"\n+\tfi\n+done >\"$tempdir\"/heads <\"$tempdir\"/raw-refs\n \n test -s \"$tempdir\"/heads ||\n \tdie \"You must specify a ref to rewrite.\"\ndiff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh\nindex 7cb60799b..04f79f32b 100755\n--- a/t/t7003-filter-branch.sh\n+++ b/t/t7003-filter-branch.sh\n@@ -470,4 +470,18 @@ test_expect_success 'tree-filter deals with object name vs pathname ambiguity' '\n \tgit show HEAD:$ambiguous\n '\n \n+test_expect_success 'rewrite repository including refs that point at non-commit object' '\n+\ttest_when_finished \"git reset --hard original\" &&\n+\ttree=$(git rev-parse HEAD^{tree}) &&\n+\ttest_when_finished \"git replace -d $tree\" &&\n+\techo A >new &&\n+\tgit add new &&\n+\tnew_tree=$(git write-tree) &&\n+\tgit replace $tree $new_tree &&\n+\tgit tag -a -m \"tag to a tree\" treetag $new_tree &&\n+\tgit reset --hard HEAD &&\n+\tgit filter-branch -f -- --all >filter-output 2>&1 &&\n+\t! fgrep fatal filter-output\n+'\n+\n test_done\n-- \n2.16.2.18.g09cb46d\n\n"},{"id":"342676","messageId":"xmqqvadmilx5.fsf@gitster-ct.c.googlers.com","threadId":"48104","inReplyTo":"20180323050913.5188-1-orga.chem.job@gmail.com","subject":"Re: [PATCH v2] filter-branch: fix errors caused by refs that cannot be used with ^0","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-03-23T20:20:22Z","receivedAt":"2018-03-23T20:20:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Yuki Kokubun <orga.chem.job@gmail.com> writes:\n\n> \"git filter-branch -- --all\" print unwanted error messages when refs that\n> cannot be used with ^0 exist.\n\nIt is not incorrect per-se, but if I were writing this, I'd say\n\"... when refs that point at objects that are not committish\" or\nsomething like that, as that is much closer to people (both end\nusers and Git developers) than the \"^0\" implementation detail.\n\n> Such refs can be created by \"git replace\" with\n> trees or blobs. Also, \"git tag\" with trees or blobs can create such refs.\n\nTaking two paragraphs above together, you wrote an excellent\ndescription of the problem to be solved (i.e. what would be seen by\nusers and under what condition the symptom would trigger).  If there\nis a single obvious solution that would trivially follow from the\nproblem description, it is perfectly fine to stop here.  Otherwise,\nit would help to describe how it is solved next.  Something as brief\nlike\n\n\tFilter these problematic refs out early, and warn that these\n\trefs are left unwritten while doing so.\n\nwould suffice in this case, I think.  We _could_ insert\n\n\tbefore they are seen by the logic to see which refs have\n\tbeen modified and which have been left intact (which is\n\twhere the unwanted error messages come from),\n\nbetween \"early,\" and \"and warn\", if we wanted to.\n\n> ---\n>  git-filter-branch.sh     | 14 ++++++++++++--\n>  t/t7003-filter-branch.sh | 14 ++++++++++++++\n>  2 files changed, 26 insertions(+), 2 deletions(-)\n\nThe diff looks good.  \n\nPlease sign-off your patch (cf. Documentation/SubmittingPatches).\n\nThanks.\n"},{"id":"342839","messageId":"5ab6a823.8369630a.f525.c93a@mx.google.com","threadId":"48104","inReplyTo":"xmqqvadmilx5.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH] filter-branch: consider refs can refer to an object other than commit or tag","fromName":"Yuki Kokubun","fromEmail":"orga.chem.job@gmail.com","sentAt":"2018-03-24T19:29:53Z","receivedAt":"2018-03-24T19:34:01Z","isPatch":true,"sender":{"key":"orga.chem.job@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1124024?v=4"},"body":">> \"git filter-branch -- --all\" print unwanted error messages when refs that\n>> cannot be used with ^0 exist.\n>\n> It is not incorrect per-se, but if I were writing this, I'd say\n> \"... when refs that point at objects that are not committish\" or\n> something like that, as that is much closer to people (both end\n> users and Git developers) than the \"^0\" implementation detail.\n\nI agree that readability to users is important than the implementation detail.\nSo, I'm gonna change the commit message.\n\n>> Such refs can be created by \"git replace\" with\n>> trees or blobs. Also, \"git tag\" with trees or blobs can create such refs.\n> \n> Taking two paragraphs above together, you wrote an excellent\n> description of the problem to be solved (i.e. what would be seen by\n> users and under what condition the symptom would trigger).  If there\n> is a single obvious solution that would trivially follow from the\n> problem description, it is perfectly fine to stop here.  Otherwise,\n> it would help to describe how it is solved next.  Something as brief\n> like\n> \n> \tFilter these problematic refs out early, and warn that these\n> \trefs are left unwritten while doing so.\n> \n> would suffice in this case, I think.  We _could_ insert\n> \n> \tbefore they are seen by the logic to see which refs have\n> \tbeen modified and which have been left intact (which is\n> \twhere the unwanted error messages come from),\n> \n> between \"early,\" and \"and warn\", if we wanted to.\n\nI think the detailed description is better than the shorter one in this case.\nSo I'm gonna follow to detailed one.\n"},{"id":"342840","messageId":"1521920467-6091-1-git-send-email-orga.chem.job@gmail.com","threadId":"48104","inReplyTo":"xmqqvadmilx5.fsf@gitster-ct.c.googlers.com","subject":"[PATCH v3] filter-branch: fix errors caused by refs that point at non-committish","fromName":"Yuki Kokubun","fromEmail":"orga.chem.job@gmail.com","sentAt":"2018-03-24T19:41:07Z","receivedAt":"2018-03-24T19:41:23Z","isPatch":true,"sender":{"key":"orga.chem.job@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1124024?v=4"},"body":"\"git filter-branch -- --all\" print error messages when refs that point at\nobjects that are not committish. Such refs can be created by \"git replace\" with\ntrees or blobs. And also \"git tag\" with trees or blobs can create such refs.\n\nFilter these problematic refs out early, before they are seen by the logic to\nsee which refs have been modified and which have been left intact (which is\nwhere the unwanted error messages come from), and warn that these refs are left\nunwritten while doing so.\n---\n git-filter-branch.sh     | 14 ++++++++++++--\n t/t7003-filter-branch.sh | 14 ++++++++++++++\n 2 files changed, 26 insertions(+), 2 deletions(-)\n\ndiff --git a/git-filter-branch.sh b/git-filter-branch.sh\nindex 1b7e4b2..41efecb 100755\n--- a/git-filter-branch.sh\n+++ b/git-filter-branch.sh\n@@ -251,8 +251,18 @@ done < \"$tempdir\"/backup-refs\n \n # The refs should be updated if their heads were rewritten\n git rev-parse --no-flags --revs-only --symbolic-full-name \\\n-\t--default HEAD \"$@\" > \"$tempdir\"/raw-heads || exit\n-sed -e '/^^/d' \"$tempdir\"/raw-heads >\"$tempdir\"/heads\n+\t--default HEAD \"$@\" > \"$tempdir\"/raw-refs || exit\n+while read ref\n+do\n+\tcase \"$ref\" in ^?*) continue ;; esac\n+\n+\tif git rev-parse --verify \"$ref\"^0 >/dev/null 2>&1\n+\tthen\n+\t\techo \"$ref\"\n+\telse\n+\t\twarn \"WARNING: not rewriting '$ref' (not a committish)\"\n+\tfi\n+done >\"$tempdir\"/heads <\"$tempdir\"/raw-refs\n \n test -s \"$tempdir\"/heads ||\n \tdie \"You must specify a ref to rewrite.\"\ndiff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh\nindex 7cb6079..04f79f3 100755\n--- a/t/t7003-filter-branch.sh\n+++ b/t/t7003-filter-branch.sh\n@@ -470,4 +470,18 @@ test_expect_success 'tree-filter deals with object name vs pathname ambiguity' '\n \tgit show HEAD:$ambiguous\n '\n \n+test_expect_success 'rewrite repository including refs that point at non-commit object' '\n+\ttest_when_finished \"git reset --hard original\" &&\n+\ttree=$(git rev-parse HEAD^{tree}) &&\n+\ttest_when_finished \"git replace -d $tree\" &&\n+\techo A >new &&\n+\tgit add new &&\n+\tnew_tree=$(git write-tree) &&\n+\tgit replace $tree $new_tree &&\n+\tgit tag -a -m \"tag to a tree\" treetag $new_tree &&\n+\tgit reset --hard HEAD &&\n+\tgit filter-branch -f -- --all >filter-output 2>&1 &&\n+\t! fgrep fatal filter-output\n+'\n+\n test_done\n-- \n1.9.1\n\n"},{"id":"342895","messageId":"xmqqr2o8f7hd.fsf@gitster-ct.c.googlers.com","threadId":"48104","inReplyTo":"1521920467-6091-1-git-send-email-orga.chem.job@gmail.com","subject":"Re: [PATCH v3] filter-branch: fix errors caused by refs that point at non-committish","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-03-25T16:25:02Z","receivedAt":"2018-03-25T16:25:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Yuki Kokubun <orga.chem.job@gmail.com> writes:\n\n> \"git filter-branch -- --all\" print error messages when refs that point at\n> objects that are not committish.\n\nGrammo (third-person singular 'prints' misspelt without 's'; the\n\"when\" clause has a complex subject but no verb).\n\nPerhaps this will salvage what you meant:\n\n\t\"git filter-branch -- --all\" prints error messages when\n\tprocessing refs that point at objects that are not\n\tcommittish.\n\n> Such refs can be created by \"git replace\" with\n> trees or blobs. And also \"git tag\" with trees or blobs can create such refs.\n>\n> Filter these problematic refs out early, before they are seen by the logic to\n> see which refs have been modified and which have been left intact (which is\n> where the unwanted error messages come from), and warn that these refs are left\n> unwritten while doing so.\n> ---\n\nPlease sign-off your patch (cf. Documentation/SubmittingPatches).\n\nOtherwise this round looks good.\n\nThanks.\n\n>  git-filter-branch.sh     | 14 ++++++++++++--\n>  t/t7003-filter-branch.sh | 14 ++++++++++++++\n>  2 files changed, 26 insertions(+), 2 deletions(-)\n>\n> diff --git a/git-filter-branch.sh b/git-filter-branch.sh\n> index 1b7e4b2..41efecb 100755\n> --- a/git-filter-branch.sh\n> +++ b/git-filter-branch.sh\n> @@ -251,8 +251,18 @@ done < \"$tempdir\"/backup-refs\n>  \n>  # The refs should be updated if their heads were rewritten\n>  git rev-parse --no-flags --revs-only --symbolic-full-name \\\n> -\t--default HEAD \"$@\" > \"$tempdir\"/raw-heads || exit\n> -sed -e '/^^/d' \"$tempdir\"/raw-heads >\"$tempdir\"/heads\n> +\t--default HEAD \"$@\" > \"$tempdir\"/raw-refs || exit\n> +while read ref\n> +do\n> +\tcase \"$ref\" in ^?*) continue ;; esac\n> +\n> +\tif git rev-parse --verify \"$ref\"^0 >/dev/null 2>&1\n> +\tthen\n> +\t\techo \"$ref\"\n> +\telse\n> +\t\twarn \"WARNING: not rewriting '$ref' (not a committish)\"\n> +\tfi\n> +done >\"$tempdir\"/heads <\"$tempdir\"/raw-refs\n>  \n>  test -s \"$tempdir\"/heads ||\n>  \tdie \"You must specify a ref to rewrite.\"\n> diff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh\n> index 7cb6079..04f79f3 100755\n> --- a/t/t7003-filter-branch.sh\n> +++ b/t/t7003-filter-branch.sh\n> @@ -470,4 +470,18 @@ test_expect_success 'tree-filter deals with object name vs pathname ambiguity' '\n>  \tgit show HEAD:$ambiguous\n>  '\n>  \n> +test_expect_success 'rewrite repository including refs that point at non-commit object' '\n> +\ttest_when_finished \"git reset --hard original\" &&\n> +\ttree=$(git rev-parse HEAD^{tree}) &&\n> +\ttest_when_finished \"git replace -d $tree\" &&\n> +\techo A >new &&\n> +\tgit add new &&\n> +\tnew_tree=$(git write-tree) &&\n> +\tgit replace $tree $new_tree &&\n> +\tgit tag -a -m \"tag to a tree\" treetag $new_tree &&\n> +\tgit reset --hard HEAD &&\n> +\tgit filter-branch -f -- --all >filter-output 2>&1 &&\n> +\t! fgrep fatal filter-output\n> +'\n> +\n>  test_done\n"},{"id":"342903","messageId":"5ab7d35a.9060650a.fe7a7.04ef@mx.google.com","threadId":"48104","inReplyTo":"xmqqr2o8f7hd.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v3] filter-branch: fix errors caused by refs that point at non-committish","fromName":"Yuki Kokubun","fromEmail":"orga.chem.job@gmail.com","sentAt":"2018-03-25T16:39:45Z","receivedAt":"2018-03-25T16:50:39Z","isPatch":true,"sender":{"key":"orga.chem.job@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1124024?v=4"},"body":"References: <xmqqr2o8f7hd.fsf@gitster-ct.c.googlers.com>\n\t<xmqqvadmilx5.fsf@gitster-ct.c.googlers.com>\n\t<5ab46520.0352650a.cc02b.a177@mx.google.com>\n\t<20180323050913.5188-1-orga.chem.job@gmail.com> \nContent-Type: text/plain\n\n> Grammo (third-person singular 'prints' misspelt without 's'; the\n> \"when\" clause has a complex subject but no verb).\n> \n> Perhaps this will salvage what you meant:\n> \n> \t\"git filter-branch -- --all\" prints error messages when\n> \tprocessing refs that point at objects that are not\n> \tcommittish.\n\nThanks. I'm gonna fix it.\n\n> Please sign-off your patch (cf. Documentation/SubmittingPatches).\n\nOK I'm gonna add it.\n\nI greatly appreciate for your kind review.\nI couldn't write this patch without your help.\nThanks.\n"},{"id":"342905","messageId":"1521996898-7052-1-git-send-email-orga.chem.job@gmail.com","threadId":"48104","inReplyTo":"xmqqr2o8f7hd.fsf@gitster-ct.c.googlers.com","subject":"[PATCH v4] filter-branch: fix errors caused by refs that point at non-committish","fromName":"Yuki Kokubun","fromEmail":"orga.chem.job@gmail.com","sentAt":"2018-03-25T16:54:58Z","receivedAt":"2018-03-25T16:55:09Z","isPatch":true,"sender":{"key":"orga.chem.job@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1124024?v=4"},"body":"\"git filter-branch -- --all\" prints error messages when processing refs that\npoint at objects that are not committish. Such refs can be created by\n\"git replace\" with trees or blobs. And also \"git tag\" with trees or blobs can\ncreate such refs.\n\nFilter these problematic refs out early, before they are seen by the logic to\nsee which refs have been modified and which have been left intact (which is\nwhere the unwanted error messages come from), and warn that these refs are left\nunwritten while doing so.\n\nSigned-off-by: Yuki Kokubun <orga.chem.job@gmail.com>\n---\n git-filter-branch.sh     | 14 ++++++++++++--\n t/t7003-filter-branch.sh | 14 ++++++++++++++\n 2 files changed, 26 insertions(+), 2 deletions(-)\n\ndiff --git a/git-filter-branch.sh b/git-filter-branch.sh\nindex 1b7e4b2cd..41efecb28 100755\n--- a/git-filter-branch.sh\n+++ b/git-filter-branch.sh\n@@ -251,8 +251,18 @@ done < \"$tempdir\"/backup-refs\n \n # The refs should be updated if their heads were rewritten\n git rev-parse --no-flags --revs-only --symbolic-full-name \\\n-\t--default HEAD \"$@\" > \"$tempdir\"/raw-heads || exit\n-sed -e '/^^/d' \"$tempdir\"/raw-heads >\"$tempdir\"/heads\n+\t--default HEAD \"$@\" > \"$tempdir\"/raw-refs || exit\n+while read ref\n+do\n+\tcase \"$ref\" in ^?*) continue ;; esac\n+\n+\tif git rev-parse --verify \"$ref\"^0 >/dev/null 2>&1\n+\tthen\n+\t\techo \"$ref\"\n+\telse\n+\t\twarn \"WARNING: not rewriting '$ref' (not a committish)\"\n+\tfi\n+done >\"$tempdir\"/heads <\"$tempdir\"/raw-refs\n \n test -s \"$tempdir\"/heads ||\n \tdie \"You must specify a ref to rewrite.\"\ndiff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh\nindex 7cb60799b..04f79f32b 100755\n--- a/t/t7003-filter-branch.sh\n+++ b/t/t7003-filter-branch.sh\n@@ -470,4 +470,18 @@ test_expect_success 'tree-filter deals with object name vs pathname ambiguity' '\n \tgit show HEAD:$ambiguous\n '\n \n+test_expect_success 'rewrite repository including refs that point at non-commit object' '\n+\ttest_when_finished \"git reset --hard original\" &&\n+\ttree=$(git rev-parse HEAD^{tree}) &&\n+\ttest_when_finished \"git replace -d $tree\" &&\n+\techo A >new &&\n+\tgit add new &&\n+\tnew_tree=$(git write-tree) &&\n+\tgit replace $tree $new_tree &&\n+\tgit tag -a -m \"tag to a tree\" treetag $new_tree &&\n+\tgit reset --hard HEAD &&\n+\tgit filter-branch -f -- --all >filter-output 2>&1 &&\n+\t! fgrep fatal filter-output\n+'\n+\n test_done\n-- \n2.16.2.18.g09cb46d\n\n"},{"id":"342906","messageId":"5ab7d550.dc96620a.4b512.feab@mx.google.com","threadId":"48104","inReplyTo":"1521996898-7052-1-git-send-email-orga.chem.job@gmail.com","subject":"Re: [PATCH v4] filter-branch: fix errors caused by refs that point at non-committish","fromName":"Yuki Kokubun","fromEmail":"orga.chem.job@gmail.com","sentAt":"2018-03-25T16:45:45Z","receivedAt":"2018-03-25T16:59:01Z","isPatch":true,"sender":{"key":"orga.chem.job@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1124024?v=4"},"body":"References: <1521996898-7052-1-git-send-email-orga.chem.job@gmail.com>\nContent-Type: text/plain\n\nSorry, I forgot add a line of \"Reviewed-by\".\nI'm gonna send the fixed patch again.\n"},{"id":"342908","messageId":"1521997310-7200-1-git-send-email-orga.chem.job@gmail.com","threadId":"48104","inReplyTo":"1521996898-7052-1-git-send-email-orga.chem.job@gmail.com","subject":"[PATCH v5] filter-branch: fix errors caused by refs that point at non-committish","fromName":"Yuki Kokubun","fromEmail":"orga.chem.job@gmail.com","sentAt":"2018-03-25T17:01:50Z","receivedAt":"2018-03-25T17:02:02Z","isPatch":true,"sender":{"key":"orga.chem.job@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1124024?v=4"},"body":"\"git filter-branch -- --all\" prints error messages when processing refs that\npoint at objects that are not committish. Such refs can be created by\n\"git replace\" with trees or blobs. And also \"git tag\" with trees or blobs can\ncreate such refs.\n\nFilter these problematic refs out early, before they are seen by the logic to\nsee which refs have been modified and which have been left intact (which is\nwhere the unwanted error messages come from), and warn that these refs are left\nunwritten while doing so.\n\nSigned-off-by: Yuki Kokubun <orga.chem.job@gmail.com>\nReviewed-by: Junio C Hamano <gitster@pobox.com>\n---\n git-filter-branch.sh     | 14 ++++++++++++--\n t/t7003-filter-branch.sh | 14 ++++++++++++++\n 2 files changed, 26 insertions(+), 2 deletions(-)\n\ndiff --git a/git-filter-branch.sh b/git-filter-branch.sh\nindex 1b7e4b2cd..41efecb28 100755\n--- a/git-filter-branch.sh\n+++ b/git-filter-branch.sh\n@@ -251,8 +251,18 @@ done < \"$tempdir\"/backup-refs\n \n # The refs should be updated if their heads were rewritten\n git rev-parse --no-flags --revs-only --symbolic-full-name \\\n-\t--default HEAD \"$@\" > \"$tempdir\"/raw-heads || exit\n-sed -e '/^^/d' \"$tempdir\"/raw-heads >\"$tempdir\"/heads\n+\t--default HEAD \"$@\" > \"$tempdir\"/raw-refs || exit\n+while read ref\n+do\n+\tcase \"$ref\" in ^?*) continue ;; esac\n+\n+\tif git rev-parse --verify \"$ref\"^0 >/dev/null 2>&1\n+\tthen\n+\t\techo \"$ref\"\n+\telse\n+\t\twarn \"WARNING: not rewriting '$ref' (not a committish)\"\n+\tfi\n+done >\"$tempdir\"/heads <\"$tempdir\"/raw-refs\n \n test -s \"$tempdir\"/heads ||\n \tdie \"You must specify a ref to rewrite.\"\ndiff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh\nindex 7cb60799b..04f79f32b 100755\n--- a/t/t7003-filter-branch.sh\n+++ b/t/t7003-filter-branch.sh\n@@ -470,4 +470,18 @@ test_expect_success 'tree-filter deals with object name vs pathname ambiguity' '\n \tgit show HEAD:$ambiguous\n '\n \n+test_expect_success 'rewrite repository including refs that point at non-commit object' '\n+\ttest_when_finished \"git reset --hard original\" &&\n+\ttree=$(git rev-parse HEAD^{tree}) &&\n+\ttest_when_finished \"git replace -d $tree\" &&\n+\techo A >new &&\n+\tgit add new &&\n+\tnew_tree=$(git write-tree) &&\n+\tgit replace $tree $new_tree &&\n+\tgit tag -a -m \"tag to a tree\" treetag $new_tree &&\n+\tgit reset --hard HEAD &&\n+\tgit filter-branch -f -- --all >filter-output 2>&1 &&\n+\t! fgrep fatal filter-output\n+'\n+\n test_done\n-- \n2.16.2.18.g09cb46d\n\n"},{"id":"342910","messageId":"xmqqy3igdqob.fsf@gitster-ct.c.googlers.com","threadId":"48104","inReplyTo":"1521996898-7052-1-git-send-email-orga.chem.job@gmail.com","subject":"Re: [PATCH v4] filter-branch: fix errors caused by refs that point at non-committish","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-03-25T17:13:24Z","receivedAt":"2018-03-25T17:13:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Yuki Kokubun <orga.chem.job@gmail.com> writes:\n\n> \"git filter-branch -- --all\" prints error messages when processing refs that\n> point at objects that are not committish. Such refs can be created by\n> \"git replace\" with trees or blobs. And also \"git tag\" with trees or blobs can\n> create such refs.\n>\n> Filter these problematic refs out early, before they are seen by the logic to\n> see which refs have been modified and which have been left intact (which is\n> where the unwanted error messages come from), and warn that these refs are left\n> unwritten while doing so.\n>\n> Signed-off-by: Yuki Kokubun <orga.chem.job@gmail.com>\n> ---\n>  git-filter-branch.sh     | 14 ++++++++++++--\n>  t/t7003-filter-branch.sh | 14 ++++++++++++++\n>  2 files changed, 26 insertions(+), 2 deletions(-)\n\nGood.  Will queue.  Thanks.\n\n>\n> diff --git a/git-filter-branch.sh b/git-filter-branch.sh\n> index 1b7e4b2cd..41efecb28 100755\n> --- a/git-filter-branch.sh\n> +++ b/git-filter-branch.sh\n> @@ -251,8 +251,18 @@ done < \"$tempdir\"/backup-refs\n>  \n>  # The refs should be updated if their heads were rewritten\n>  git rev-parse --no-flags --revs-only --symbolic-full-name \\\n> -\t--default HEAD \"$@\" > \"$tempdir\"/raw-heads || exit\n> -sed -e '/^^/d' \"$tempdir\"/raw-heads >\"$tempdir\"/heads\n> +\t--default HEAD \"$@\" > \"$tempdir\"/raw-refs || exit\n> +while read ref\n> +do\n> +\tcase \"$ref\" in ^?*) continue ;; esac\n> +\n> +\tif git rev-parse --verify \"$ref\"^0 >/dev/null 2>&1\n> +\tthen\n> +\t\techo \"$ref\"\n> +\telse\n> +\t\twarn \"WARNING: not rewriting '$ref' (not a committish)\"\n> +\tfi\n> +done >\"$tempdir\"/heads <\"$tempdir\"/raw-refs\n>  \n>  test -s \"$tempdir\"/heads ||\n>  \tdie \"You must specify a ref to rewrite.\"\n> diff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh\n> index 7cb60799b..04f79f32b 100755\n> --- a/t/t7003-filter-branch.sh\n> +++ b/t/t7003-filter-branch.sh\n> @@ -470,4 +470,18 @@ test_expect_success 'tree-filter deals with object name vs pathname ambiguity' '\n>  \tgit show HEAD:$ambiguous\n>  '\n>  \n> +test_expect_success 'rewrite repository including refs that point at non-commit object' '\n> +\ttest_when_finished \"git reset --hard original\" &&\n> +\ttree=$(git rev-parse HEAD^{tree}) &&\n> +\ttest_when_finished \"git replace -d $tree\" &&\n> +\techo A >new &&\n> +\tgit add new &&\n> +\tnew_tree=$(git write-tree) &&\n> +\tgit replace $tree $new_tree &&\n> +\tgit tag -a -m \"tag to a tree\" treetag $new_tree &&\n> +\tgit reset --hard HEAD &&\n> +\tgit filter-branch -f -- --all >filter-output 2>&1 &&\n> +\t! fgrep fatal filter-output\n> +'\n> +\n>  test_done\n"},{"id":"342938","messageId":"xmqqtvt3ewdn.fsf@gitster-ct.c.googlers.com","threadId":"48104","inReplyTo":"5ab7d35a.9060650a.fe7a7.04ef@mx.google.com","subject":"Re: [PATCH v3] filter-branch: fix errors caused by refs that point at non-committish","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-03-25T20:24:52Z","receivedAt":"2018-03-25T20:24:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Yuki Kokubun <orga.chem.job@gmail.com> writes:\n\n> References: <xmqqr2o8f7hd.fsf@gitster-ct.c.googlers.com>\n> \t<xmqqvadmilx5.fsf@gitster-ct.c.googlers.com>\n> \t<5ab46520.0352650a.cc02b.a177@mx.google.com>\n> \t<20180323050913.5188-1-orga.chem.job@gmail.com> \n> Content-Type: text/plain\n>\n>> Grammo (third-person singular 'prints' misspelt without 's'; the\n>> \"when\" clause has a complex subject but no verb).\n>> \n>> Perhaps this will salvage what you meant:\n>> \n>> \t\"git filter-branch -- --all\" prints error messages when\n>> \tprocessing refs that point at objects that are not\n>> \tcommittish.\n>\n> Thanks. I'm gonna fix it.\n>\n>> Please sign-off your patch (cf. Documentation/SubmittingPatches).\n>\n> OK I'm gonna add it.\n>\n> I greatly appreciate for your kind review.\n> I couldn't write this patch without your help.\n> Thanks.\n\nHeh, it's a team effort, and it is not like I am helping your effort\nto build \"open source contribution credit\" for your coursework ;-)\n\nThank *you* for contributing to make Git a better system.\n\n\n\n"},{"id":"344121","messageId":"xmqqr2np8fb6.fsf@gitster-ct.c.googlers.com","threadId":"48104","inReplyTo":"5ab7d550.dc96620a.4b512.feab@mx.google.com","subject":"Re: [PATCH v4] filter-branch: fix errors caused by refs that point at non-committish","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-04-08T23:10:53Z","receivedAt":"2018-04-08T23:11:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Yuki Kokubun <orga.chem.job@gmail.com> writes:\n\n> References: <1521996898-7052-1-git-send-email-orga.chem.job@gmail.com>\n> Content-Type: text/plain\n>\n> Sorry, I forgot add a line of \"Reviewed-by\".\n> I'm gonna send the fixed patch again.\n\nDo not worry too much about this.  Reviewed-by: added by patch\nauthor is seen rather rarely, because the following sequence of\nevents need to happen:\n\n - The author sends a patch vN\n\n - A reviewer reviews vN and says \"This is now Reviewed-by: me\"\n\n - The author re-submits patch vN+1, where the only difference\n   between vN and vN+1 is essentially _nothing_.  Except for the\n   addition of \"Reviewed-by:\" by the reviewer.\n\nBut our typical patch injestion cycle is much faster and often the\nlast step does not happen ;-)\n"}]}