Denton Liu <liu.denton@xxxxxxxxx> writes:
> @@ -185,6 +191,7 @@ if test "$command" != "pull" &&
> then
> revs=$(git rev-parse $default --revs-only "$@") || exit $?
> dirs=$(git rev-parse --no-revs --no-flags "$@") || exit $?
> + ensure_single_rev $revs
This applies to anything other than pull, add and push, so certainly
'split' is covered here.
> @@ -716,9 +723,8 @@ cmd_add_repository () {
> }
>
> cmd_add_commit () {
> - revs=$(git rev-parse $default --revs-only "$@") || exit $?
> - set -- $revs
> - rev="$1"
> + rev=$(git rev-parse $default --revs-only "$@") || exit $?
> + ensure_single_rev $rev
There are two callers of this helper. cmd_add passes "$@" but it
does so only after making sure there is only one argument that is a
commit, so this conversion is not incorrect.
I am not sure if the other caller is OK, though. cmd_add_repository
can get more than one revs, and uses the first one as $rev to read
the tree from, expecting that this helper to ignore other ones that
are emitted from 'git rev-parse --revs-only "$@"'.
For that matter, one of the early things cmd_split does is to call
the find_existing_splits helper with $revs, and it seems to be
prepared to be red multiple $revs (it is passed to "git log", so I
would expect that incoming $revs is allowed to specify bottom to
limit the traversal, e.g. "git log maint..master"). The addition of
"ensure_single_rev" we saw in an earlier hunk near ll.191 makes such
call impossible. I am not a user of subtree, so I do not know if
it is a good change (i.e. making something nonsensical impossible to
do is good, making something useful impossible to do is bad).
> @@ -817,16 +823,10 @@ cmd_split () {
> }
>
> cmd_merge () {
> - revs=$(git rev-parse $default --revs-only "$@") || exit $?
> + rev=$(git rev-parse $default --revs-only "$@") || exit $?
> + ensure_single_rev $rev
> ensure_clean
>
> - set -- $revs
> - if test $# -ne 1
> - then
> - die "You must provide exactly one revision. Got: '$revs'"
> - fi
> - rev="$1"
> -
This one already was insisting on a single version, so it clearly is
a correct no-op conversion, but wouldn't this have been already
caught upfront where anything other than pull, add and push are
handled? I do understand if the new call to ensure_single is made
to the other caller of cmd_merge in cmd_pull, though.
> if test -n "$squash"
> then
> first_split="$(find_latest_squash "$dir")"
In any case, I do not use subtree, and the last time I looked at
this script is a long time ago, so take all of the above with a
large grain of salt.
Thanks.
