From: James Limbouris <james@xxxxxxxxxxxxxxxxx>
315a84f9aa0 (subtree: use commits before rejoins for splits, 2018-09-28)
changed the signature of check_parents from 'check_parents [REV...]'
to 'check_parents PARENTS_EXPR INDENT'. In other words the variable list
of parent revisions became a list embedded in a string. However it
neglected to unpack the list again before sending it to cache_miss,
leading to incorrect calls whenever more than one parent was present.
This is the case whenever a merge commit is processed, with the end
result being a loss of performance from unecessary rechecks.
The indent parameter was subsequently removed in e9525a8a029 (subtree:
have $indent actually affect indentation, 2021-04-27), but the argument
handling bug remained.
For consistency, take multiple arguments in check_parents,
and pass all of them to cache_miss separately.
Signed-off-by: James Limbouris <james@xxxxxxxxxxxxxxxxx>
---
subtree: fix argument handling in check_parents
> I saw that you sent a v3, but did not see any of this information
> (which took a good while to assemble, as you might have guessed) in
> the commit message. However, I think that message would make for the
> best home for this information.
Sorry Dscho - it wasn't 100% clear to me which details were required.
I've rerolled and tried again. Also sorry if I'm not replying to the
mail correctly - I'm not actually subscribed to the list, and this seems
like the only easy way to get text onto it through gitgitgadget without
fighting Outlook.
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1086%2Fjamesl-dm%2Fmaint-v4
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1086/jamesl-dm/maint-v4
Pull-Request: https://github.com/gitgitgadget/git/pull/1086
Range-diff vs v3:
1: f734ca9d276 ! 1: cdc5295a7ac subtree: fix argument handling in check_parents
@@ Metadata
## Commit message ##
subtree: fix argument handling in check_parents
- check_parents was taking all of its arguments as a single string,
- and erroneously passing them to cache_miss as a single string. For
- commits with a single parent this would succeed, but whenever a merge
- commit was processed, cache_miss would be passed "parent1 parent2"
- instead of "parent1" "parent2" and fail, leading to unecessary rechecks
- of the parent commits.
+ 315a84f9aa0 (subtree: use commits before rejoins for splits, 2018-09-28)
+ changed the signature of check_parents from 'check_parents [REV...]'
+ to 'check_parents PARENTS_EXPR INDENT'. In other words the variable list
+ of parent revisions became a list embedded in a string. However it
+ neglected to unpack the list again before sending it to cache_miss,
+ leading to incorrect calls whenever more than one parent was present.
+ This is the case whenever a merge commit is processed, with the end
+ result being a loss of performance from unecessary rechecks.
+
+ The indent parameter was subsequently removed in e9525a8a029 (subtree:
+ have $indent actually affect indentation, 2021-04-27), but the argument
+ handling bug remained.
For consistency, take multiple arguments in check_parents,
and pass all of them to cache_miss separately.
contrib/subtree/git-subtree.sh | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
index 7f767b5c38f..71f1fd94bde 100755
--- a/contrib/subtree/git-subtree.sh
+++ b/contrib/subtree/git-subtree.sh
@@ -296,10 +296,9 @@ cache_miss () {
done
}
-# Usage: check_parents PARENTS_EXPR
+# Usage: check_parents [REVS...]
check_parents () {
- assert test $# = 1
- missed=$(cache_miss "$1") || exit $?
+ missed=$(cache_miss "$@") || exit $?
local indent=$(($indent + 1))
for miss in $missed
do
@@ -753,7 +752,7 @@ process_split_commit () {
fi
createcount=$(($createcount + 1))
debug "parents: $parents"
- check_parents "$parents"
+ check_parents $parents
newparents=$(cache_get $parents) || exit $?
debug "newparents: $newparents"
base-commit: e9d7761bb94f20acc98824275e317fa82436c25d
--
gitgitgadget
