Hi James, 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: On Fri, 3 Dec 2021, Johannes Schindelin wrote: > On Wed, 1 Dec 2021, James Limbouris via GitGitGadget wrote: > > > From: James Limbouris <james@xxxxxxxxxxxxxxxxx> > > > > check_parents was taking all of its arguments as a single string, > > and erroneously passing them to cache_miss as a single string. > > cache_miss would then fail, and the spurious cache misses it produced > > would hurt performance. > > > > 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 > > > > Hello git developers. Please consider this small patch that fixes a bug > > introduced during a coding style cleanup of the subtree command. Changes > > to the argument handling were causing check_parents to fail when more > > than one parent was supplied, which led to a small loss of performance. > > When I look through the commit history of `git-subtree.sh`, I see that the > change was introduced in 315a84f9aa0 (subtree: use commits before rejoins > for splits, 2018-09-28) (which was not really a coding style cleanup). > > The change was actually not done right, if I read the commit correctly, > because it added a new parameter _to the end_, even if the > `check_parents()` function took an arbitrary number of parameters already. > And indeed, it changed the `"$@"` into a "$1", pretending that only one > parent would be passed. > > Now, I do not really understand under what circumstances multiple parents > could be passed to `check_parents()`, but I think that it does not matter > whether you use `--format=%P` or `^@` (the former was changed to the > latter in 19ad68d95d6 (subtree: performance improvement for finding > unexpected parent commits, 2018-10-12)), you can always get an arbitrary > number of parents that way. > > The natural thing, now, would be to move the added `indent` parameter to > the front of the parameter list, but I see that there was some cleanup in > e9525a8a029 (subtree: have $indent actually affect indentation, > 2021-04-27) which _removed_ that `indent` parameter. Thanks, Dscho
