{"thread":{"id":"49452","subject":"[PATCH 3/4] subtree: use commits before rejoins for splits","startedAt":"2018-09-28T18:35:54Z","lastAt":"2018-10-12T07:35:23Z","messageCount":11,"participants":["Strain, Roger L","Roger Strain","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"359231","messageId":"20180928183540.48968-4-roger.strain@swri.org","threadId":"49452","inReplyTo":"20180928183540.48968-1-roger.strain@swri.org","subject":"[PATCH 3/4] subtree: use commits before rejoins for splits","fromName":"Strain, Roger L","fromEmail":"roger.strain@swri.org","sentAt":"2018-09-28T18:35:39Z","receivedAt":"2018-09-28T18:35:54Z","isPatch":true,"sender":{"key":"roger.strain@swri.org","avatar":"https://avatars.githubusercontent.com/u/52041877?v=4"},"body":"Adds recursive evaluation of parent commits which were not part of the\ninitial commit list when performing a split.\n\nSplit expects all relevant commits to be reachable from the target commit\nbut not reachable from any previous rejoins. However, a branch could be\nbased on a commit prior to a rejoin, then later merged back into the\ncurrent code. In this case, a parent to the commit will not be present in\nthe initial list of commits, trigging an \"incorrect order\" warning.\n\nPrevious behavior was to consider that commit to have no parent, creating\nan original commit containing all subtree content. This commit is not\npresent in an existing subtree commit graph, changing commit hashes and\nmaking pushing to a subtree repo impossible.\n\nNew behavior will recursively check these unexpected parent commits to\ntrack them back to either an earlier rejoin, or a true original commit.\nThe generated synthetic commits will properly match previously-generated\ncommits, allowing successful pushing to a prior subtree repo.\n\nSigned-off-by: Strain, Roger L <roger.strain@swri.org>\n---\n contrib/subtree/git-subtree.sh | 26 ++++++++++++++++++++------\n 1 file changed, 20 insertions(+), 6 deletions(-)\n\ndiff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh\nindex d8861f306..23dd04cbe 100755\n--- a/contrib/subtree/git-subtree.sh\n+++ b/contrib/subtree/git-subtree.sh\n@@ -231,12 +231,14 @@ cache_miss () {\n }\n \n check_parents () {\n-\tmissed=$(cache_miss \"$@\")\n+\tmissed=$(cache_miss \"$1\")\n+\tlocal indent=$(($2 + 1))\n \tfor miss in $missed\n \tdo\n \t\tif ! test -r \"$cachedir/notree/$miss\"\n \t\tthen\n \t\t\tdebug \"  incorrect order: $miss\"\n+\t\t\tprocess_split_commit \"$miss\" \"\" \"$indent\"\n \t\tfi\n \tdone\n }\n@@ -606,8 +608,20 @@ ensure_valid_ref_format () {\n process_split_commit () {\n \tlocal rev=\"$1\"\n \tlocal parents=\"$2\"\n-\trevcount=$(($revcount + 1))\n-\tprogress \"$revcount/$revmax ($createcount)\"\n+\tlocal indent=$3\n+\n+\tif test $indent -eq 0\n+\tthen\n+\t\trevcount=$(($revcount + 1))\n+\telse\n+\t\t# processing commit without normal parent information;\n+\t\t# fetch from repo\n+\t\tparents=$(git show -s --pretty=%P \"$rev\")\n+\t\textracount=$(($extracount + 1))\n+\tfi\n+\n+\tprogress \"$revcount/$revmax ($createcount) [$extracount]\"\n+\n \tdebug \"Processing commit: $rev\"\n \texists=$(cache_get \"$rev\")\n \tif test -n \"$exists\"\n@@ -617,14 +631,13 @@ process_split_commit () {\n \tfi\n \tcreatecount=$(($createcount + 1))\n \tdebug \"  parents: $parents\"\n+\tcheck_parents \"$parents\" \"$indent\"\n \tnewparents=$(cache_get $parents)\n \tdebug \"  newparents: $newparents\"\n \n \ttree=$(subtree_for_commit \"$rev\" \"$dir\")\n \tdebug \"  tree is: $tree\"\n \n-\tcheck_parents $parents\n-\n \t# ugly.  is there no better way to tell if this is a subtree\n \t# vs. a mainline commit?  Does it matter?\n \tif test -z \"$tree\"\n@@ -744,10 +757,11 @@ cmd_split () {\n \trevmax=$(eval \"$grl\" | wc -l)\n \trevcount=0\n \tcreatecount=0\n+\textracount=0\n \teval \"$grl\" |\n \twhile read rev parents\n \tdo\n-\t\tprocess_split_commit \"$rev\" \"$parents\"\n+\t\tprocess_split_commit \"$rev\" \"$parents\" 0\n \tdone || exit $?\n \n \tlatest_new=$(cache_get latest_new)\n-- \n2.19.0.windows.1\n\n"},{"id":"359232","messageId":"20180928183540.48968-3-roger.strain@swri.org","threadId":"49452","inReplyTo":"20180928183540.48968-1-roger.strain@swri.org","subject":"[PATCH 2/4] subtree: make --ignore-joins pay attention to adds","fromName":"Strain, Roger L","fromEmail":"roger.strain@swri.org","sentAt":"2018-09-28T18:35:38Z","receivedAt":"2018-09-28T18:35:57Z","isPatch":true,"sender":{"key":"roger.strain@swri.org","avatar":"https://avatars.githubusercontent.com/u/52041877?v=4"},"body":"Changes the behavior of --ignore-joins to always consider a subtree add\ncommit, and ignore only splits and squashes.\n\nThe --ignore-joins option is documented to ignore prior --rejoin commits.\nHowever, it additionally ignored subtree add commits generated when a\nsubtree was initially added to a repo.\n\nDue to the logic which determines whether a commit is a mainline commit\nor a subtree commit (namely, the presence or absence of content in the\nsubtree prefix) this causes commits before the initial add to appear to\nbe part of the subtree. An --ignore-joins split would therefore consider\nthose commits part of the subtree history and include them at the\nbeginning of the synthetic history, causing the resulting hashes to be\nincorrect for all later commits.\n\nSigned-off-by: Strain, Roger L <roger.strain@swri.org>\n---\n contrib/subtree/git-subtree.sh | 14 +++++++-------\n 1 file changed, 7 insertions(+), 7 deletions(-)\n\ndiff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh\nindex 2cd7b345b..d8861f306 100755\n--- a/contrib/subtree/git-subtree.sh\n+++ b/contrib/subtree/git-subtree.sh\n@@ -340,7 +340,12 @@ find_existing_splits () {\n \trevs=\"$2\"\n \tmain=\n \tsub=\n-\tgit log --grep=\"^git-subtree-dir: $dir/*\\$\" \\\n+\tlocal grep_format=\"^git-subtree-dir: $dir/*\\$\"\n+\tif test -n \"$ignore_joins\"\n+\tthen\n+\t\tgrep_format=\"^Add '$dir/' from commit '\"\n+\tfi\n+\tgit log --grep=\"$grep_format\" \\\n \t\t--no-show-signature --pretty=format:'START %H%n%s%n%n%b%nEND%n' $revs |\n \twhile read a b junk\n \tdo\n@@ -730,12 +735,7 @@ cmd_split () {\n \t\tdone\n \tfi\n \n-\tif test -n \"$ignore_joins\"\n-\tthen\n-\t\tunrevs=\n-\telse\n-\t\tunrevs=\"$(find_existing_splits \"$dir\" \"$revs\")\"\n-\tfi\n+\tunrevs=\"$(find_existing_splits \"$dir\" \"$revs\")\"\n \n \t# We can't restrict rev-list to only $dir here, because some of our\n \t# parents have the $dir contents the root, and those won't match.\n-- \n2.19.0.windows.1\n\n"},{"id":"359233","messageId":"20180928183540.48968-2-roger.strain@swri.org","threadId":"49452","inReplyTo":"20180928183540.48968-1-roger.strain@swri.org","subject":"[PATCH 1/4] subtree: refactor split of a commit into standalone method","fromName":"Strain, Roger L","fromEmail":"roger.strain@swri.org","sentAt":"2018-09-28T18:35:37Z","receivedAt":"2018-09-28T18:36:04Z","isPatch":true,"sender":{"key":"roger.strain@swri.org","avatar":"https://avatars.githubusercontent.com/u/52041877?v=4"},"body":"In a particularly complex repo, subtree split was not creating\ncompatible splits for pushing back to a separate repo. Addressing\none of the issues requires recursive handling of parent commits\nthat were not initially considered by the algorithm. This commit\nmakes no functional changes, but relocates the code to be called\nrecursively into a new method to simply comparisons of later\ncommits.\n\nSigned-off-by: Strain, Roger L <roger.strain@swri.org>\n---\n contrib/subtree/git-subtree.sh | 78 ++++++++++++++++++----------------\n 1 file changed, 42 insertions(+), 36 deletions(-)\n\ndiff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh\nindex d3f39a862..2cd7b345b 100755\n--- a/contrib/subtree/git-subtree.sh\n+++ b/contrib/subtree/git-subtree.sh\n@@ -598,6 +598,47 @@ ensure_valid_ref_format () {\n \t\tdie \"'$1' does not look like a ref\"\n }\n \n+process_split_commit () {\n+\tlocal rev=\"$1\"\n+\tlocal parents=\"$2\"\n+\trevcount=$(($revcount + 1))\n+\tprogress \"$revcount/$revmax ($createcount)\"\n+\tdebug \"Processing commit: $rev\"\n+\texists=$(cache_get \"$rev\")\n+\tif test -n \"$exists\"\n+\tthen\n+\t\tdebug \"  prior: $exists\"\n+\t\treturn\n+\tfi\n+\tcreatecount=$(($createcount + 1))\n+\tdebug \"  parents: $parents\"\n+\tnewparents=$(cache_get $parents)\n+\tdebug \"  newparents: $newparents\"\n+\n+\ttree=$(subtree_for_commit \"$rev\" \"$dir\")\n+\tdebug \"  tree is: $tree\"\n+\n+\tcheck_parents $parents\n+\n+\t# ugly.  is there no better way to tell if this is a subtree\n+\t# vs. a mainline commit?  Does it matter?\n+\tif test -z \"$tree\"\n+\tthen\n+\t\tset_notree \"$rev\"\n+\t\tif test -n \"$newparents\"\n+\t\tthen\n+\t\t\tcache_set \"$rev\" \"$rev\"\n+\t\tfi\n+\t\treturn\n+\tfi\n+\n+\tnewrev=$(copy_or_skip \"$rev\" \"$tree\" \"$newparents\") || exit $?\n+\tdebug \"  newrev is: $newrev\"\n+\tcache_set \"$rev\" \"$newrev\"\n+\tcache_set latest_new \"$newrev\"\n+\tcache_set latest_old \"$rev\"\n+}\n+\n cmd_add () {\n \tif test -e \"$dir\"\n \tthen\n@@ -706,42 +747,7 @@ cmd_split () {\n \teval \"$grl\" |\n \twhile read rev parents\n \tdo\n-\t\trevcount=$(($revcount + 1))\n-\t\tprogress \"$revcount/$revmax ($createcount)\"\n-\t\tdebug \"Processing commit: $rev\"\n-\t\texists=$(cache_get \"$rev\")\n-\t\tif test -n \"$exists\"\n-\t\tthen\n-\t\t\tdebug \"  prior: $exists\"\n-\t\t\tcontinue\n-\t\tfi\n-\t\tcreatecount=$(($createcount + 1))\n-\t\tdebug \"  parents: $parents\"\n-\t\tnewparents=$(cache_get $parents)\n-\t\tdebug \"  newparents: $newparents\"\n-\n-\t\ttree=$(subtree_for_commit \"$rev\" \"$dir\")\n-\t\tdebug \"  tree is: $tree\"\n-\n-\t\tcheck_parents $parents\n-\n-\t\t# ugly.  is there no better way to tell if this is a subtree\n-\t\t# vs. a mainline commit?  Does it matter?\n-\t\tif test -z \"$tree\"\n-\t\tthen\n-\t\t\tset_notree \"$rev\"\n-\t\t\tif test -n \"$newparents\"\n-\t\t\tthen\n-\t\t\t\tcache_set \"$rev\" \"$rev\"\n-\t\t\tfi\n-\t\t\tcontinue\n-\t\tfi\n-\n-\t\tnewrev=$(copy_or_skip \"$rev\" \"$tree\" \"$newparents\") || exit $?\n-\t\tdebug \"  newrev is: $newrev\"\n-\t\tcache_set \"$rev\" \"$newrev\"\n-\t\tcache_set latest_new \"$newrev\"\n-\t\tcache_set latest_old \"$rev\"\n+\t\tprocess_split_commit \"$rev\" \"$parents\"\n \tdone || exit $?\n \n \tlatest_new=$(cache_get latest_new)\n-- \n2.19.0.windows.1\n\n"},{"id":"359234","messageId":"20180928183540.48968-5-roger.strain@swri.org","threadId":"49452","inReplyTo":"20180928183540.48968-1-roger.strain@swri.org","subject":"[PATCH 4/4] subtree: improve decision on merges kept in split","fromName":"Strain, Roger L","fromEmail":"roger.strain@swri.org","sentAt":"2018-09-28T18:35:40Z","receivedAt":"2018-09-28T18:36:04Z","isPatch":true,"sender":{"key":"roger.strain@swri.org","avatar":"https://avatars.githubusercontent.com/u/52041877?v=4"},"body":"When multiple identical parents are detected for a commit being considered\nfor copying, explicitly check whether one is the common merge base between\nthe commits. If so, the other commit can be used as the identical parent;\nif not, a merge must be performed to maintain history.\n\nIn some situations two parents of a merge commit may appear to both have\nidentical subtree content with each other and the current commit. However,\nthose parents can potentially come from different commit graphs.\n\nPrevious behavior would simply select one of the identical parents to\nserve as the replacement for this commit, based on the order in which they\nwere processed.\n\nNew behavior compares the merge base between the commits to determine if\na new merge commit is necessary to maintain history despite the identical\ncontent.\n\nSigned-off-by: Strain, Roger L <roger.strain@swri.org>\n---\n contrib/subtree/git-subtree.sh | 21 +++++++++++++++++++--\n 1 file changed, 19 insertions(+), 2 deletions(-)\n\ndiff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh\nindex 23dd04cbe..1c157dbd9 100755\n--- a/contrib/subtree/git-subtree.sh\n+++ b/contrib/subtree/git-subtree.sh\n@@ -541,6 +541,7 @@ copy_or_skip () {\n \tnonidentical=\n \tp=\n \tgotparents=\n+\tcopycommit=\n \tfor parent in $newparents\n \tdo\n \t\tptree=$(toptree_for_commit $parent) || exit $?\n@@ -548,7 +549,24 @@ copy_or_skip () {\n \t\tif test \"$ptree\" = \"$tree\"\n \t\tthen\n \t\t\t# an identical parent could be used in place of this rev.\n-\t\t\tidentical=\"$parent\"\n+\t\t\tif test -n \"$identical\"\n+\t\t\tthen\n+\t\t\t\t# if a previous identical parent was found, check whether\n+\t\t\t\t# one is already an ancestor of the other\n+\t\t\t\tmergebase=$(git merge-base $identical $parent)\n+\t\t\t\tif test \"$identical\" = \"$mergebase\"\n+\t\t\t\tthen\n+\t\t\t\t\t# current identical commit is an ancestor of parent\n+\t\t\t\t\tidentical=\"$parent\"\n+\t\t\t\telif test \"$parent\" != \"$mergebase\"\n+\t\t\t\tthen\n+\t\t\t\t\t# no common history; commit must be copied\n+\t\t\t\t\tcopycommit=1\n+\t\t\t\tfi\n+\t\t\telse\n+\t\t\t\t# first identical parent detected\n+\t\t\t\tidentical=\"$parent\"\n+\t\t\tfi\n \t\telse\n \t\t\tnonidentical=\"$parent\"\n \t\tfi\n@@ -571,7 +589,6 @@ copy_or_skip () {\n \t\tfi\n \tdone\n \n-\tcopycommit=\n \tif test -n \"$identical\" && test -n \"$nonidentical\"\n \tthen\n \t\textras=$(git rev-list --count $identical..$nonidentical)\n-- \n2.19.0.windows.1\n\n"},{"id":"359237","messageId":"20180928183540.48968-1-roger.strain@swri.org","threadId":"49452","inReplyTo":null,"subject":"[PATCH 0/4] Multiple subtree split fixes regarding complex repos","fromName":"Strain, Roger L","fromEmail":"roger.strain@swri.org","sentAt":"2018-09-28T18:56:39Z","receivedAt":"2018-09-28T18:56:49Z","isPatch":true,"sender":{"key":"roger.strain@swri.org","avatar":"https://avatars.githubusercontent.com/u/52041877?v=4"},"body":"We recently (about eight months ago) transitioned to git source control systems for several very large, very complex systems. We brought over several active versions requiring maintenance updates, and also set up several subtree repos to manage code shared between the systems. Recently, we attempted to push updates back to those subtrees and encountered errors. I believe I have identified and corrected the errors we found in our repos, and would like to contribute those fixes back.\n\nCommands to demonstrate both failures using the current version of the subtree script are here:\nhttps://gist.github.com/FoxFireX/1b794384612b7fd5e7cd157cff96269e\n\nShort summary of three problems involved:\n1. Split using rejoins fails in some cases where a commit has a parent which was a parent commit further upstream from a rejoin, causing a new initial commit to be created, which is not related to the original subtree commits.\n2. Split using rejoins fails to generate a merge commit which may have triaged the previous problem, but instead elected to use only the parent which is not connected to the original subtree commits. (This may occur when the commit and both parents all share the same subtree hash.)\n3. Split ignoring joins also ignores the original add commit, which causes content prior to the add to be considered part of the subtree graph, changing the commit hashes so it is not connected to the original subtree commits.\n\nThe following commits address each problem individually, along with a single commit that makes no functional change but performs a small refactor of the existing code. Hopefully that will make reviewing it a simpler task. This is my first attempt at submitting a patch back, so apologies if I've made any errors in the process.\n\nStrain, Roger L (4):\n  subtree: refactor split of a commit into standalone method\n  subtree: make --ignore-joins pay attention to adds\n  subtree: use commits before rejoins for splits\n  subtree: improve decision on merges kept in split\n\n contrib/subtree/git-subtree.sh | 129 +++++++++++++++++++++------------\n 1 file changed, 83 insertions(+), 46 deletions(-)\n\n-- \n2.19.0.windows.1\n\n"},{"id":"360203","messageId":"20181011194605.19518-5-rstrain@swri.org","threadId":"49452","inReplyTo":"20180928183540.48968-1-roger.strain@swri.org","subject":"[PATCH v2 4/4] subtree: improve decision on merges kept in split","fromName":"Roger Strain","fromEmail":"rstrain@swri.org","sentAt":"2018-10-11T19:46:05Z","receivedAt":"2018-10-11T20:01:25Z","isPatch":true,"sender":{"key":"rstrain@swri.org","avatar":"https://gravatar.com/avatar/8c08dc63d400a738e756b1d45fb5d0562bd397477bde48a14c012c144150090b?d=mp&s=160"},"body":"From: \"Strain, Roger L\" <roger.strain@swri.org>\n\nWhen multiple identical parents are detected for a commit being considered\nfor copying, explicitly check whether one is the common merge base between\nthe commits. If so, the other commit can be used as the identical parent;\nif not, a merge must be performed to maintain history.\n\nIn some situations two parents of a merge commit may appear to both have\nidentical subtree content with each other and the current commit. However,\nthose parents can potentially come from different commit graphs.\n\nPrevious behavior would simply select one of the identical parents to\nserve as the replacement for this commit, based on the order in which they\nwere processed.\n\nNew behavior compares the merge base between the commits to determine if\na new merge commit is necessary to maintain history despite the identical\ncontent.\n\nSigned-off-by: Strain, Roger L <roger.strain@swri.org>\n---\n contrib/subtree/git-subtree.sh | 21 +++++++++++++++++++--\n 1 file changed, 19 insertions(+), 2 deletions(-)\n\ndiff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh\nindex eef4199ae..7dd643998 100755\n--- a/contrib/subtree/git-subtree.sh\n+++ b/contrib/subtree/git-subtree.sh\n@@ -541,6 +541,7 @@ copy_or_skip () {\n \tnonidentical=\n \tp=\n \tgotparents=\n+\tcopycommit=\n \tfor parent in $newparents\n \tdo\n \t\tptree=$(toptree_for_commit $parent) || exit $?\n@@ -548,7 +549,24 @@ copy_or_skip () {\n \t\tif test \"$ptree\" = \"$tree\"\n \t\tthen\n \t\t\t# an identical parent could be used in place of this rev.\n-\t\t\tidentical=\"$parent\"\n+\t\t\tif test -n \"$identical\"\n+\t\t\tthen\n+\t\t\t\t# if a previous identical parent was found, check whether\n+\t\t\t\t# one is already an ancestor of the other\n+\t\t\t\tmergebase=$(git merge-base $identical $parent)\n+\t\t\t\tif test \"$identical\" = \"$mergebase\"\n+\t\t\t\tthen\n+\t\t\t\t\t# current identical commit is an ancestor of parent\n+\t\t\t\t\tidentical=\"$parent\"\n+\t\t\t\telif test \"$parent\" != \"$mergebase\"\n+\t\t\t\tthen\n+\t\t\t\t\t# no common history; commit must be copied\n+\t\t\t\t\tcopycommit=1\n+\t\t\t\tfi\n+\t\t\telse\n+\t\t\t\t# first identical parent detected\n+\t\t\t\tidentical=\"$parent\"\n+\t\t\tfi\n \t\telse\n \t\t\tnonidentical=\"$parent\"\n \t\tfi\n@@ -571,7 +589,6 @@ copy_or_skip () {\n \t\tfi\n \tdone\n \n-\tcopycommit=\n \tif test -n \"$identical\" && test -n \"$nonidentical\"\n \tthen\n \t\textras=$(git rev-list --count $identical..$nonidentical)\n-- \n2.19.1\n\n"},{"id":"360204","messageId":"20181011194605.19518-1-rstrain@swri.org","threadId":"49452","inReplyTo":"20180928183540.48968-1-roger.strain@swri.org","subject":"[PATCH v2 0/4] Multiple subtree split fixes regarding complex repos","fromName":"Roger Strain","fromEmail":"rstrain@swri.org","sentAt":"2018-10-11T19:46:01Z","receivedAt":"2018-10-11T20:05:25Z","isPatch":true,"sender":{"key":"rstrain@swri.org","avatar":"https://gravatar.com/avatar/8c08dc63d400a738e756b1d45fb5d0562bd397477bde48a14c012c144150090b?d=mp&s=160"},"body":"After doing some testing at scale, determined that one call was taking too long; replaced that with an alternate call which returns the same data significantly faster.\n\nAlso, if anyone has any other feedback on these I'd really love to hear it. It's working better for us (as in, it actually generates a compatible tree version to version) but still isn't perfect, and I'm not sure perfect is achievable, but want to make sure this doesn't things for anyone else.\n\nChanges since v1:\ndiff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh\nindex 1c157dbd9..7dd643998 100755\n--- a/contrib/subtree/git-subtree.sh\n+++ b/contrib/subtree/git-subtree.sh\n@@ -633,7 +633,7 @@ process_split_commit () {\n        else\n                # processing commit without normal parent information;\n                # fetch from repo\n-               parents=$(git show -s --pretty=%P \"$rev\")\n+               parents=$(git log --pretty=%P -n 1 \"$rev\")\n                extracount=$(($extracount + 1))\n        fi\n\nStrain, Roger L (4):\n  subtree: refactor split of a commit into standalone method\n  subtree: make --ignore-joins pay attention to adds\n  subtree: use commits before rejoins for splits\n  subtree: improve decision on merges kept in split\n\n contrib/subtree/git-subtree.sh | 129 +++++++++++++++++++++------------\n 1 file changed, 83 insertions(+), 46 deletions(-)\n\n-- \n2.19.1\n\n"},{"id":"360205","messageId":"20181011194605.19518-4-rstrain@swri.org","threadId":"49452","inReplyTo":"20180928183540.48968-1-roger.strain@swri.org","subject":"[PATCH v2 3/4] subtree: use commits before rejoins for splits","fromName":"Roger Strain","fromEmail":"rstrain@swri.org","sentAt":"2018-10-11T19:46:04Z","receivedAt":"2018-10-11T20:05:25Z","isPatch":true,"sender":{"key":"rstrain@swri.org","avatar":"https://gravatar.com/avatar/8c08dc63d400a738e756b1d45fb5d0562bd397477bde48a14c012c144150090b?d=mp&s=160"},"body":"From: \"Strain, Roger L\" <roger.strain@swri.org>\n\nAdds recursive evaluation of parent commits which were not part of the\ninitial commit list when performing a split.\n\nSplit expects all relevant commits to be reachable from the target commit\nbut not reachable from any previous rejoins. However, a branch could be\nbased on a commit prior to a rejoin, then later merged back into the\ncurrent code. In this case, a parent to the commit will not be present in\nthe initial list of commits, trigging an \"incorrect order\" warning.\n\nPrevious behavior was to consider that commit to have no parent, creating\nan original commit containing all subtree content. This commit is not\npresent in an existing subtree commit graph, changing commit hashes and\nmaking pushing to a subtree repo impossible.\n\nNew behavior will recursively check these unexpected parent commits to\ntrack them back to either an earlier rejoin, or a true original commit.\nThe generated synthetic commits will properly match previously-generated\ncommits, allowing successful pushing to a prior subtree repo.\n\nSigned-off-by: Strain, Roger L <roger.strain@swri.org>\n---\n contrib/subtree/git-subtree.sh | 26 ++++++++++++++++++++------\n 1 file changed, 20 insertions(+), 6 deletions(-)\n\ndiff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh\nindex d8861f306..eef4199ae 100755\n--- a/contrib/subtree/git-subtree.sh\n+++ b/contrib/subtree/git-subtree.sh\n@@ -231,12 +231,14 @@ cache_miss () {\n }\n \n check_parents () {\n-\tmissed=$(cache_miss \"$@\")\n+\tmissed=$(cache_miss \"$1\")\n+\tlocal indent=$(($2 + 1))\n \tfor miss in $missed\n \tdo\n \t\tif ! test -r \"$cachedir/notree/$miss\"\n \t\tthen\n \t\t\tdebug \"  incorrect order: $miss\"\n+\t\t\tprocess_split_commit \"$miss\" \"\" \"$indent\"\n \t\tfi\n \tdone\n }\n@@ -606,8 +608,20 @@ ensure_valid_ref_format () {\n process_split_commit () {\n \tlocal rev=\"$1\"\n \tlocal parents=\"$2\"\n-\trevcount=$(($revcount + 1))\n-\tprogress \"$revcount/$revmax ($createcount)\"\n+\tlocal indent=$3\n+\n+\tif test $indent -eq 0\n+\tthen\n+\t\trevcount=$(($revcount + 1))\n+\telse\n+\t\t# processing commit without normal parent information;\n+\t\t# fetch from repo\n+\t\tparents=$(git log --pretty=%P -n 1 \"$rev\")\n+\t\textracount=$(($extracount + 1))\n+\tfi\n+\n+\tprogress \"$revcount/$revmax ($createcount) [$extracount]\"\n+\n \tdebug \"Processing commit: $rev\"\n \texists=$(cache_get \"$rev\")\n \tif test -n \"$exists\"\n@@ -617,14 +631,13 @@ process_split_commit () {\n \tfi\n \tcreatecount=$(($createcount + 1))\n \tdebug \"  parents: $parents\"\n+\tcheck_parents \"$parents\" \"$indent\"\n \tnewparents=$(cache_get $parents)\n \tdebug \"  newparents: $newparents\"\n \n \ttree=$(subtree_for_commit \"$rev\" \"$dir\")\n \tdebug \"  tree is: $tree\"\n \n-\tcheck_parents $parents\n-\n \t# ugly.  is there no better way to tell if this is a subtree\n \t# vs. a mainline commit?  Does it matter?\n \tif test -z \"$tree\"\n@@ -744,10 +757,11 @@ cmd_split () {\n \trevmax=$(eval \"$grl\" | wc -l)\n \trevcount=0\n \tcreatecount=0\n+\textracount=0\n \teval \"$grl\" |\n \twhile read rev parents\n \tdo\n-\t\tprocess_split_commit \"$rev\" \"$parents\"\n+\t\tprocess_split_commit \"$rev\" \"$parents\" 0\n \tdone || exit $?\n \n \tlatest_new=$(cache_get latest_new)\n-- \n2.19.1\n\n"},{"id":"360206","messageId":"20181011194605.19518-3-rstrain@swri.org","threadId":"49452","inReplyTo":"20180928183540.48968-1-roger.strain@swri.org","subject":"[PATCH v2 2/4] subtree: make --ignore-joins pay attention to adds","fromName":"Roger Strain","fromEmail":"rstrain@swri.org","sentAt":"2018-10-11T19:46:03Z","receivedAt":"2018-10-11T20:05:26Z","isPatch":true,"sender":{"key":"rstrain@swri.org","avatar":"https://gravatar.com/avatar/8c08dc63d400a738e756b1d45fb5d0562bd397477bde48a14c012c144150090b?d=mp&s=160"},"body":"From: \"Strain, Roger L\" <roger.strain@swri.org>\n\nChanges the behavior of --ignore-joins to always consider a subtree add\ncommit, and ignore only splits and squashes.\n\nThe --ignore-joins option is documented to ignore prior --rejoin commits.\nHowever, it additionally ignored subtree add commits generated when a\nsubtree was initially added to a repo.\n\nDue to the logic which determines whether a commit is a mainline commit\nor a subtree commit (namely, the presence or absence of content in the\nsubtree prefix) this causes commits before the initial add to appear to\nbe part of the subtree. An --ignore-joins split would therefore consider\nthose commits part of the subtree history and include them at the\nbeginning of the synthetic history, causing the resulting hashes to be\nincorrect for all later commits.\n\nSigned-off-by: Strain, Roger L <roger.strain@swri.org>\n---\n contrib/subtree/git-subtree.sh | 14 +++++++-------\n 1 file changed, 7 insertions(+), 7 deletions(-)\n\ndiff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh\nindex 2cd7b345b..d8861f306 100755\n--- a/contrib/subtree/git-subtree.sh\n+++ b/contrib/subtree/git-subtree.sh\n@@ -340,7 +340,12 @@ find_existing_splits () {\n \trevs=\"$2\"\n \tmain=\n \tsub=\n-\tgit log --grep=\"^git-subtree-dir: $dir/*\\$\" \\\n+\tlocal grep_format=\"^git-subtree-dir: $dir/*\\$\"\n+\tif test -n \"$ignore_joins\"\n+\tthen\n+\t\tgrep_format=\"^Add '$dir/' from commit '\"\n+\tfi\n+\tgit log --grep=\"$grep_format\" \\\n \t\t--no-show-signature --pretty=format:'START %H%n%s%n%n%b%nEND%n' $revs |\n \twhile read a b junk\n \tdo\n@@ -730,12 +735,7 @@ cmd_split () {\n \t\tdone\n \tfi\n \n-\tif test -n \"$ignore_joins\"\n-\tthen\n-\t\tunrevs=\n-\telse\n-\t\tunrevs=\"$(find_existing_splits \"$dir\" \"$revs\")\"\n-\tfi\n+\tunrevs=\"$(find_existing_splits \"$dir\" \"$revs\")\"\n \n \t# We can't restrict rev-list to only $dir here, because some of our\n \t# parents have the $dir contents the root, and those won't match.\n-- \n2.19.1\n\n"},{"id":"360207","messageId":"20181011194605.19518-2-rstrain@swri.org","threadId":"49452","inReplyTo":"20180928183540.48968-1-roger.strain@swri.org","subject":"[PATCH v2 1/4] subtree: refactor split of a commit into standalone method","fromName":"Roger Strain","fromEmail":"rstrain@swri.org","sentAt":"2018-10-11T19:46:02Z","receivedAt":"2018-10-11T20:05:27Z","isPatch":true,"sender":{"key":"rstrain@swri.org","avatar":"https://gravatar.com/avatar/8c08dc63d400a738e756b1d45fb5d0562bd397477bde48a14c012c144150090b?d=mp&s=160"},"body":"From: \"Strain, Roger L\" <roger.strain@swri.org>\n\nIn a particularly complex repo, subtree split was not creating\ncompatible splits for pushing back to a separate repo. Addressing\none of the issues requires recursive handling of parent commits\nthat were not initially considered by the algorithm. This commit\nmakes no functional changes, but relocates the code to be called\nrecursively into a new method to simply comparisons of later\ncommits.\n\nSigned-off-by: Strain, Roger L <roger.strain@swri.org>\n---\n contrib/subtree/git-subtree.sh | 78 ++++++++++++++++++----------------\n 1 file changed, 42 insertions(+), 36 deletions(-)\n\ndiff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh\nindex d3f39a862..2cd7b345b 100755\n--- a/contrib/subtree/git-subtree.sh\n+++ b/contrib/subtree/git-subtree.sh\n@@ -598,6 +598,47 @@ ensure_valid_ref_format () {\n \t\tdie \"'$1' does not look like a ref\"\n }\n \n+process_split_commit () {\n+\tlocal rev=\"$1\"\n+\tlocal parents=\"$2\"\n+\trevcount=$(($revcount + 1))\n+\tprogress \"$revcount/$revmax ($createcount)\"\n+\tdebug \"Processing commit: $rev\"\n+\texists=$(cache_get \"$rev\")\n+\tif test -n \"$exists\"\n+\tthen\n+\t\tdebug \"  prior: $exists\"\n+\t\treturn\n+\tfi\n+\tcreatecount=$(($createcount + 1))\n+\tdebug \"  parents: $parents\"\n+\tnewparents=$(cache_get $parents)\n+\tdebug \"  newparents: $newparents\"\n+\n+\ttree=$(subtree_for_commit \"$rev\" \"$dir\")\n+\tdebug \"  tree is: $tree\"\n+\n+\tcheck_parents $parents\n+\n+\t# ugly.  is there no better way to tell if this is a subtree\n+\t# vs. a mainline commit?  Does it matter?\n+\tif test -z \"$tree\"\n+\tthen\n+\t\tset_notree \"$rev\"\n+\t\tif test -n \"$newparents\"\n+\t\tthen\n+\t\t\tcache_set \"$rev\" \"$rev\"\n+\t\tfi\n+\t\treturn\n+\tfi\n+\n+\tnewrev=$(copy_or_skip \"$rev\" \"$tree\" \"$newparents\") || exit $?\n+\tdebug \"  newrev is: $newrev\"\n+\tcache_set \"$rev\" \"$newrev\"\n+\tcache_set latest_new \"$newrev\"\n+\tcache_set latest_old \"$rev\"\n+}\n+\n cmd_add () {\n \tif test -e \"$dir\"\n \tthen\n@@ -706,42 +747,7 @@ cmd_split () {\n \teval \"$grl\" |\n \twhile read rev parents\n \tdo\n-\t\trevcount=$(($revcount + 1))\n-\t\tprogress \"$revcount/$revmax ($createcount)\"\n-\t\tdebug \"Processing commit: $rev\"\n-\t\texists=$(cache_get \"$rev\")\n-\t\tif test -n \"$exists\"\n-\t\tthen\n-\t\t\tdebug \"  prior: $exists\"\n-\t\t\tcontinue\n-\t\tfi\n-\t\tcreatecount=$(($createcount + 1))\n-\t\tdebug \"  parents: $parents\"\n-\t\tnewparents=$(cache_get $parents)\n-\t\tdebug \"  newparents: $newparents\"\n-\n-\t\ttree=$(subtree_for_commit \"$rev\" \"$dir\")\n-\t\tdebug \"  tree is: $tree\"\n-\n-\t\tcheck_parents $parents\n-\n-\t\t# ugly.  is there no better way to tell if this is a subtree\n-\t\t# vs. a mainline commit?  Does it matter?\n-\t\tif test -z \"$tree\"\n-\t\tthen\n-\t\t\tset_notree \"$rev\"\n-\t\t\tif test -n \"$newparents\"\n-\t\t\tthen\n-\t\t\t\tcache_set \"$rev\" \"$rev\"\n-\t\t\tfi\n-\t\t\tcontinue\n-\t\tfi\n-\n-\t\tnewrev=$(copy_or_skip \"$rev\" \"$tree\" \"$newparents\") || exit $?\n-\t\tdebug \"  newrev is: $newrev\"\n-\t\tcache_set \"$rev\" \"$newrev\"\n-\t\tcache_set latest_new \"$newrev\"\n-\t\tcache_set latest_old \"$rev\"\n+\t\tprocess_split_commit \"$rev\" \"$parents\"\n \tdone || exit $?\n \n \tlatest_new=$(cache_get latest_new)\n-- \n2.19.1\n\n"},{"id":"360278","messageId":"xmqqk1mnbosr.fsf@gitster-ct.c.googlers.com","threadId":"49452","inReplyTo":"20181011194605.19518-1-rstrain@swri.org","subject":"Re: [PATCH v2 0/4] Multiple subtree split fixes regarding complex repos","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-10-12T07:35:16Z","receivedAt":"2018-10-12T07:35:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Roger Strain <rstrain@swri.org> writes:\n\n> After doing some testing at scale, determined that one call was\n> taking too long; replaced that with an alternate call which\n> returns the same data significantly faster.\n\nCurious where the time goes.  Do you know?\n\n> Also, if anyone has any other feedback on these I'd really love to\n> hear it. It's working better for us (as in, it actually generates\n\nThe previous one is already in 'next'; please make it incremental\nwith explanation as to why \"show -s\" is worse than \"log -1\" (but see\nbelow).\n\n>                 # processing commit without normal parent information;\n>                 # fetch from repo\n> -               parents=$(git show -s --pretty=%P \"$rev\")\n> +               parents=$(git log --pretty=%P -n 1 \"$rev\")\n\nIf you want to learn the parents of a given commit:\n\n\t$ git help revisions\n\nsays\n\n       <rev>^@, e.g. HEAD^@\n           A suffix ^ followed by an at sign is the same as listing all parents of <rev>\n           (meaning, include anything reachable from its parents, but not the commit\n           itself).\n\nso\n\n\t\tparents=$(git rev-parse \"$rev^@\")\n\nought to be the most efficient way to do this, I suspect.\n"}]}