{"thread":{"id":"14952","subject":"[PATCH 1/3] filter-branch: Extend test to show rewriting bug","startedAt":"2008-08-12T08:45:56Z","lastAt":"2008-08-12T12:11:06Z","messageCount":5,"participants":["Thomas Rast","Jan Wielemaker"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"86911","messageId":"cover.1218529494.git.trast@student.ethz.ch","threadId":"14952","inReplyTo":"7vsktara5n.fsf@gitster.siamese.dyndns.org","subject":"[PATCH 0/3] filter-branch --subdirectory-filter improvements","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2008-08-12T08:45:56Z","receivedAt":"2008-08-12T08:45:56Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Junio C Hamano wrote:\n> Anything parked in 'pu' is a fair game for replacement later, so please\n> send a replacement series and tell me to drop the previous ones from 'pu'.\n\nSo let's try this one.  The first two do not depend on\n--simplify-merges.\n\n1/3 is new, and extends the --subdirectory-filter test to prove the\nexistence of the bug in current filter-branch.  I hope it helps\nexplain the issue.\n\n2/3 is the same as before[*] modulo changing the test to expect\nsuccess again.\n\nThe third one does depend on --simplify-merges.\n\n3/3 introduces --simplify-merges, which improves the history that\nresults from --subdirectory-filter.  It has absolutely nothing to do\nwith 2/3, except that it touches the same area of code.  (You could\ns/rev-list/rev-list --simplify-merges/ in master:git-filter-branch.sh,\nand get the improved history without the bugfix.)\n\nSorry that I dispersed the patches and v2s randomly across the thread.\n\n- Thomas\n\n[*] http://kerneltrap.org/mailarchive/git/2008/8/8/2867244\n\"[PATCH v2] filter-branch: fix ref rewriting with --subdirectory-filter\"\n\n\nThomas Rast (3):\n  filter-branch: Extend test to show rewriting bug\n  filter-branch: fix ref rewriting with --subdirectory-filter\n  filter-branch: use --simplify-merges\n"},{"id":"86910","messageId":"b6bda5e5028aea161e5d18e0c70c45d5a94dac82.1218529494.git.trast@student.ethz.ch","threadId":"14952","inReplyTo":"cover.1218529494.git.trast@student.ethz.ch","subject":"[PATCH 1/3] filter-branch: Extend test to show rewriting bug","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2008-08-12T08:45:57Z","receivedAt":"2008-08-12T08:45:57Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"This extends the --subdirectory-filter test in t7003 to demonstrate a\nrewriting bug: when rewriting two refs A and B such that B is an\nancestor of A, it fails to rewrite B.\n\nThe underlying issue is that the rev-list invocation at\ngit-filter-branch.sh:332 more or less boils down to\n\n  git rev-list B --boundary ^A\n\nwhich outputs nothing because B is an ancestor of A.\n\nSigned-off-by: Thomas Rast <trast@student.ethz.ch>\n---\n t/t7003-filter-branch.sh |   10 +++++++---\n 1 files changed, 7 insertions(+), 3 deletions(-)\n\ndiff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh\nindex a0ab096..4382baa 100755\n--- a/t/t7003-filter-branch.sh\n+++ b/t/t7003-filter-branch.sh\n@@ -96,13 +96,17 @@ test_expect_success 'filter subdirectory only' '\n \ttest_tick &&\n \tgit commit -m \"again not subdir\" &&\n \tgit branch sub &&\n-\tgit-filter-branch -f --subdirectory-filter subdir refs/heads/sub\n+\tgit branch sub-earlier HEAD~2 &&\n+\tgit-filter-branch -f --subdirectory-filter subdir \\\n+\t\trefs/heads/sub refs/heads/sub-earlier\n '\n \n-test_expect_success 'subdirectory filter result looks okay' '\n+test_expect_failure 'subdirectory filter result looks okay' '\n \ttest 2 = $(git rev-list sub | wc -l) &&\n \tgit show sub:new &&\n-\ttest_must_fail git show sub:subdir\n+\ttest_must_fail git show sub:subdir &&\n+\tgit show sub-earlier:new &&\n+\ttest_must_fail git show sub-earlier:subdir\n '\n \n test_expect_success 'more setup' '\n-- \n1.6.0.rc2.30.gb6bda\n"},{"id":"86912","messageId":"2c31171ea3181f9dfe7c96a2af98e1a5bcf1358a.1218529494.git.trast@student.ethz.ch","threadId":"14952","inReplyTo":"b6bda5e5028aea161e5d18e0c70c45d5a94dac82.1218529494.git.trast@student.ethz.ch","subject":"[PATCH 2/3] filter-branch: fix ref rewriting with --subdirectory-filter","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2008-08-12T08:45:58Z","receivedAt":"2008-08-12T08:45:58Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"The previous ancestor discovery code failed on any refs that are\n(pre-rewrite) ancestors of commits marked for rewriting.  This means\nthat in a situation\n\n   A -- B(topic) -- C(master)\n\nwhere B is dropped by --subdirectory-filter pruning, the 'topic' was\nnot moved up to A as intended, but left unrewritten because we asked\nabout 'git rev-list ^master topic', which does not return anything.\n\nInstead, we use the straightforward\n\n   git rev-list -1 $ref -- $filter_subdir\n\nto find the right ancestor.  To justify this, note that the nearest\nancestor is unique: We use the output of\n\n  git rev-list --parents -- $filter_subdir\n\nto rewrite commits in the first pass, before any ref rewriting.  If B\nis a non-merge commit, the only candidate is its parent.  If it is a\nmerge, there are two cases:\n\n- All sides of the merge bring the same subdirectory contents.  Then\n  rev-list already pruned away the merge in favour for just one of its\n  parents, so there is only one candidate.\n\n- Some merge sides, or the merge outcome, differ.  Then the merge is\n  not pruned and can be rewritten directly.\n\nSo it is always safe to use rev-list -1.\n\nSigned-off-by: Thomas Rast <trast@student.ethz.ch>\n---\n git-filter-branch.sh     |   27 +++++++++++----------------\n t/t7003-filter-branch.sh |    2 +-\n 2 files changed, 12 insertions(+), 17 deletions(-)\n\ndiff --git a/git-filter-branch.sh b/git-filter-branch.sh\nindex a324cf0..a140337 100755\n--- a/git-filter-branch.sh\n+++ b/git-filter-branch.sh\n@@ -317,24 +317,19 @@ done <../revs\n \n # In case of a subdirectory filter, it is possible that a specified head\n # is not in the set of rewritten commits, because it was pruned by the\n-# revision walker.  Fix it by mapping these heads to the next rewritten\n-# ancestor(s), i.e. the boundaries in the set of rewritten commits.\n+# revision walker.  Fix it by mapping these heads to the unique nearest\n+# ancestor that survived the pruning.\n \n-# NEEDSWORK: we should sort the unmapped refs topologically first\n-while read ref\n-do\n-\tsha1=$(git rev-parse \"$ref\"^0)\n-\ttest -f \"$workdir\"/../map/$sha1 && continue\n-\t# Assign the boundarie(s) in the set of rewritten commits\n-\t# as the replacement commit(s).\n-\t# (This would look a bit nicer if --not --stdin worked.)\n-\tfor p in $( (cd \"$workdir\"/../map; ls | sed \"s/^/^/\") |\n-\t\tgit rev-list $ref --boundary --stdin |\n-\t\tsed -n \"s/^-//p\")\n+if test \"$filter_subdir\"\n+then\n+\twhile read ref\n \tdo\n-\t\tmap $p >> \"$workdir\"/../map/$sha1\n-\tdone\n-done < \"$tempdir\"/heads\n+\t\tsha1=$(git rev-parse \"$ref\"^0)\n+\t\ttest -f \"$workdir\"/../map/$sha1 && continue\n+\t\tancestor=$(git rev-list -1 $ref -- \"$filter_subdir\")\n+\t\ttest \"$ancestor\" && echo $(map $ancestor) >> \"$workdir\"/../map/$sha1\n+\tdone < \"$tempdir\"/heads\n+fi\n \n # Finally update the refs\n \ndiff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh\nindex 4382baa..233254f 100755\n--- a/t/t7003-filter-branch.sh\n+++ b/t/t7003-filter-branch.sh\n@@ -101,7 +101,7 @@ test_expect_success 'filter subdirectory only' '\n \t\trefs/heads/sub refs/heads/sub-earlier\n '\n \n-test_expect_failure 'subdirectory filter result looks okay' '\n+test_expect_success 'subdirectory filter result looks okay' '\n \ttest 2 = $(git rev-list sub | wc -l) &&\n \tgit show sub:new &&\n \ttest_must_fail git show sub:subdir &&\n-- \n1.6.0.rc2.30.gb6bda\n"},{"id":"86913","messageId":"fb479cdb219a3ebbcd19cc864a50dd76c603b38a.1218529494.git.trast@student.ethz.ch","threadId":"14952","inReplyTo":"2c31171ea3181f9dfe7c96a2af98e1a5bcf1358a.1218529494.git.trast@student.ethz.ch","subject":"[PATCH 3/3] filter-branch: use --simplify-merges","fromName":"Thomas Rast","fromEmail":"trast@student.ethz.ch","sentAt":"2008-08-12T08:45:59Z","receivedAt":"2008-08-12T08:45:59Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Use rev-list --simplify-merges everywhere.  This changes the behaviour\nof --subdirectory-filter in cases such as\n\n  O -- A -\\\n   \\       \\\n    \\- B -- M\n\nwhere A and B bring the same changes to the subdirectory: It now keeps\nboth sides of the merge.  Previously, the history would have been\nsimplified to 'O -- A'.  Merges of unrelated side histories that never\ntouch the subdirectory are still removed.\n\nSigned-off-by: Thomas Rast <trast@student.ethz.ch>\n---\n git-filter-branch.sh |    7 ++++---\n 1 files changed, 4 insertions(+), 3 deletions(-)\n\ndiff --git a/git-filter-branch.sh b/git-filter-branch.sh\nindex a140337..2688254 100755\n--- a/git-filter-branch.sh\n+++ b/git-filter-branch.sh\n@@ -232,11 +232,11 @@ mkdir ../map || die \"Could not create map/ directory\"\n case \"$filter_subdir\" in\n \"\")\n \tgit rev-list --reverse --topo-order --default HEAD \\\n-\t\t--parents \"$@\"\n+\t\t--parents --simplify-merges \"$@\"\n \t;;\n *)\n \tgit rev-list --reverse --topo-order --default HEAD \\\n-\t\t--parents \"$@\" -- \"$filter_subdir\"\n+\t\t--parents --simplify-merges \"$@\" -- \"$filter_subdir\"\n esac > ../revs || die \"Could not get the commits\"\n commits=$(wc -l <../revs | tr -d \" \")\n \n@@ -326,7 +326,8 @@ then\n \tdo\n \t\tsha1=$(git rev-parse \"$ref\"^0)\n \t\ttest -f \"$workdir\"/../map/$sha1 && continue\n-\t\tancestor=$(git rev-list -1 $ref -- \"$filter_subdir\")\n+\t\tancestor=$(git rev-list --simplify-merges -1 \\\n+\t\t\t\t$ref -- \"$filter_subdir\")\n \t\ttest \"$ancestor\" && echo $(map $ancestor) >> \"$workdir\"/../map/$sha1\n \tdone < \"$tempdir\"/heads\n fi\n-- \n1.6.0.rc2.30.gb6bda\n"},{"id":"86920","messageId":"200808121411.06452.J.Wielemaker@uva.nl","threadId":"14952","inReplyTo":"cover.1218529494.git.trast@student.ethz.ch","subject":"Re: [PATCH 0/3] filter-branch --subdirectory-filter improvements","fromName":"Jan Wielemaker","fromEmail":"j.wielemaker@uva.nl","sentAt":"2008-08-12T12:11:06Z","receivedAt":"2008-08-12T12:11:06Z","isPatch":true,"sender":{"key":"j.wielemaker@uva.nl","avatar":null},"body":"On Tuesday 12 August 2008 10:45:56 am Thomas Rast wrote:\n> Junio C Hamano wrote:\n> > Anything parked in 'pu' is a fair game for replacement later, so please\n> > send a replacement series and tell me to drop the previous ones from\n> > 'pu'.\n>\n> So let's try this one.  The first two do not depend on\n> --simplify-merges.\n\nAnd I can confirm that (2) is a very important fix and (3) is a necessary\nstep to achieve what I believe --subdirectory-filter is meant for: extract\na directory from a big project and turn it into stand-alone project.  In\nthis case I want:\n\n\t% filter out dir X, creating X.git\n\t% git rm -r X\n\t% git submodule add <url> X\n\nAnd someone with a totally unrelated project adds X.git to his project.\nThat requires the history of X to become totally independent from the\noriginal project.\n\nThis works great with Thomas' patches.\n\n\tCheers --- Jan\n\nP.s.\tNote that this is a common problem for people moving from some\n\tunnamed ancient SCM system, either transferring a repository or\n\tsimply starting the wrong way due to historical brainwashing :-)\n\n> 1/3 is new, and extends the --subdirectory-filter test to prove the\n> existence of the bug in current filter-branch.  I hope it helps\n> explain the issue.\n>\n> 2/3 is the same as before[*] modulo changing the test to expect\n> success again.\n>\n> The third one does depend on --simplify-merges.\n>\n> 3/3 introduces --simplify-merges, which improves the history that\n> results from --subdirectory-filter.  It has absolutely nothing to do\n> with 2/3, except that it touches the same area of code.  (You could\n> s/rev-list/rev-list --simplify-merges/ in master:git-filter-branch.sh,\n> and get the improved history without the bugfix.)\n>\n> Sorry that I dispersed the patches and v2s randomly across the thread.\n>\n> - Thomas\n>\n> [*] http://kerneltrap.org/mailarchive/git/2008/8/8/2867244\n> \"[PATCH v2] filter-branch: fix ref rewriting with --subdirectory-filter\"\n>\n>\n> Thomas Rast (3):\n>   filter-branch: Extend test to show rewriting bug\n>   filter-branch: fix ref rewriting with --subdirectory-filter\n>   filter-branch: use --simplify-merges\n"}]}