subtree: add comments and sanity checks

For each function in subtree, add a usage comment saying what the arguments are, and add an `assert` checking the number of arguments. In figuring out each thing's arguments in order to write those comments and assertions, it turns out that find_existing_splits is written as if it takes multiple 'revs', but it is in fact only ever passed a single 'rev': unrevs="$(find_existing_splits "$dir" "$rev")" || exit $? So go ahead and codify that by documenting and asserting that it takes exactly two arguments, one dir and one rev. Signed-off-by: Luke Shumaker <lukeshu@datawire.io> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Luke Shumaker committed Apr 27, 2021 at 15:17 UTC 5cdae0f6fd76bd1c12304240a17a5a12632fbb12
1 file changed +61 -3
contrib/subtree/git-subtree.sh
+61 -3
@@ -55,6 +55,7 @@ arg_split_annotate=
55 arg_addmerge_squash=
56 arg_addmerge_message=
57
58 +# Usage: debug [MSG...]
59 debug () {
60 if test -n "$arg_debug"
61 then
@@ -62,6 +63,7 @@ debug () {
63 fi
64 }
65
66 +# Usage: progress [MSG...]
67 progress () {
68 if test -z "$GIT_QUIET"
69 then
@@ -69,6 +71,7 @@ progress () {
71 fi
72 }
73
74 +# Usage: assert CMD...
75 assert () {
76 if ! "$@"
77 then
@@ -192,7 +195,9 @@ main () {
195 "cmd_$arg_command" "$@"
196 }
197
198 +# Usage: cache_setup
199 cache_setup () {
200 + assert test $# = 0
201 cachedir="$GIT_DIR/subtree-cache/$$"
202 rm -rf "$cachedir" ||
203 die "Can't delete old cachedir: $cachedir"
@@ -203,6 +208,7 @@ cache_setup () {
208 debug "Using cachedir: $cachedir" >&2
209 }
210
211 +# Usage: cache_get [REVS...]
212 cache_get () {
213 for oldrev in "$@"
214 do
@@ -214,6 +220,7 @@ cache_get () {
220 done
221 }
222
223 +# Usage: cache_miss [REVS...]
224 cache_miss () {
225 for oldrev in "$@"
226 do
@@ -224,7 +231,9 @@ cache_miss () {
231 done
232 }
233
234 +# Usage: check_parents PARENTS_EXPR INDENT
235 check_parents () {
236 + assert test $# = 2
237 missed=$(cache_miss "$1") || exit $?
238 local indent=$(($2 + 1))
239 for miss in $missed
@@ -237,11 +246,15 @@ check_parents () {
246 done
247 }
248
249 +# Usage: set_notree REV
250 set_notree () {
251 + assert test $# = 1
252 echo "1" > "$cachedir/notree/$1"
253 }
254
255 +# Usage: cache_set OLDREV NEWREV
256 cache_set () {
257 + assert test $# = 2
258 oldrev="$1"
259 newrev="$2"
260 if test "$oldrev" != "latest_old" &&
@@ -253,7 +266,9 @@ cache_set () {
266 echo "$newrev" >"$cachedir/$oldrev"
267 }
268
269 +# Usage: rev_exists REV
270 rev_exists () {
271 + assert test $# = 1
272 if git rev-parse "$1" >/dev/null 2>&1
273 then
274 return 0
@@ -262,17 +277,22 @@ rev_exists () {
277 fi
278 }
279
265 -# if a commit doesn't have a parent, this might not work. But we only want
280 +# Usage: try_remove_previous REV
281 +#
282 +# If a commit doesn't have a parent, this might not work. But we only want
283 # to remove the parent from the rev-list, and since it doesn't exist, it won't
284 # be there anyway, so do nothing in that case.
285 try_remove_previous () {
286 + assert test $# = 1
287 if rev_exists "$1^"
288 then
289 echo "^$1^"
290 fi
291 }
292
293 +# Usage: find_latest_squash DIR
294 find_latest_squash () {
295 + assert test $# = 1
296 debug "Looking for latest squash ($dir)..."
297 dir="$1"
298 sq=
@@ -316,10 +336,12 @@ find_latest_squash () {
336 done || exit $?
337 }
338
339 +# Usage: find_existing_splits DIR REV
340 find_existing_splits () {
341 + assert test $# = 2
342 debug "Looking for prior splits..."
343 dir="$1"
322 - revs="$2"
344 + rev="$2"
345 main=
346 sub=
347 local grep_format="^git-subtree-dir: $dir/*\$"
@@ -328,7 +350,7 @@ find_existing_splits () {
350 grep_format="^Add '$dir/' from commit '"
351 fi
352 git log --grep="$grep_format" \
331 - --no-show-signature --pretty=format:'START %H%n%s%n%n%b%nEND%n' $revs |
353 + --no-show-signature --pretty=format:'START %H%n%s%n%n%b%nEND%n' "$rev" |
354 while read a b junk
355 do
356 case "$a" in
@@ -365,7 +387,9 @@ find_existing_splits () {
387 done || exit $?
388 }
389
390 +# Usage: copy_commit REV TREE FLAGS_STR
391 copy_commit () {
392 + assert test $# = 3
393 # We're going to set some environment vars here, so
394 # do it in a subshell to get rid of them safely later
395 debug copy_commit "{$1}" "{$2}" "{$3}"
@@ -391,7 +415,9 @@ copy_commit () {
415 ) || die "Can't copy commit $1"
416 }
417
418 +# Usage: add_msg DIR LATEST_OLD LATEST_NEW
419 add_msg () {
420 + assert test $# = 3
421 dir="$1"
422 latest_old="$2"
423 latest_new="$3"
@@ -410,7 +436,9 @@ add_msg () {
436 EOF
437 }
438
439 +# Usage: add_squashed_msg REV DIR
440 add_squashed_msg () {
441 + assert test $# = 2
442 if test -n "$arg_addmerge_message"
443 then
444 echo "$arg_addmerge_message"
@@ -419,7 +447,9 @@ add_squashed_msg () {
447 fi
448 }
449
450 +# Usage: rejoin_msg DIR LATEST_OLD LATEST_NEW
451 rejoin_msg () {
452 + assert test $# = 3
453 dir="$1"
454 latest_old="$2"
455 latest_new="$3"
@@ -438,7 +468,9 @@ rejoin_msg () {
468 EOF
469 }
470
471 +# Usage: squash_msg DIR OLD_SUBTREE_COMMIT NEW_SUBTREE_COMMIT
472 squash_msg () {
473 + assert test $# = 3
474 dir="$1"
475 oldsub="$2"
476 newsub="$3"
@@ -460,12 +492,16 @@ squash_msg () {
492 echo "git-subtree-split: $newsub"
493 }
494
495 +# Usage: toptree_for_commit COMMIT
496 toptree_for_commit () {
497 + assert test $# = 1
498 commit="$1"
499 git rev-parse --verify "$commit^{tree}" || exit $?
500 }
501
502 +# Usage: subtree_for_commit COMMIT DIR
503 subtree_for_commit () {
504 + assert test $# = 2
505 commit="$1"
506 dir="$2"
507 git ls-tree "$commit" -- "$dir" |
@@ -479,7 +515,9 @@ subtree_for_commit () {
515 done || exit $?
516 }
517
518 +# Usage: tree_changed TREE [PARENTS...]
519 tree_changed () {
520 + assert test $# -gt 0
521 tree=$1
522 shift
523 if test $# -ne 1
@@ -496,7 +534,9 @@ tree_changed () {
534 fi
535 }
536
537 +# Usage: new_squash_commit OLD_SQUASHED_COMMIT OLD_NONSQUASHED_COMMIT NEW_NONSQUASHED_COMMIT
538 new_squash_commit () {
539 + assert test $# = 3
540 old="$1"
541 oldsub="$2"
542 newsub="$3"
@@ -511,7 +551,9 @@ new_squash_commit () {
551 fi
552 }
553
554 +# Usage: copy_or_skip REV TREE NEWPARENTS
555 copy_or_skip () {
556 + assert test $# = 3
557 rev="$1"
558 tree="$2"
559 newparents="$3"
@@ -586,7 +628,9 @@ copy_or_skip () {
628 fi
629 }
630
631 +# Usage: ensure_clean
632 ensure_clean () {
633 + assert test $# = 0
634 if ! git diff-index HEAD --exit-code --quiet 2>&1
635 then
636 die "Working tree has modifications. Cannot add."
@@ -597,12 +641,16 @@ ensure_clean () {
641 fi
642 }
643
644 +# Usage: ensure_valid_ref_format REF
645 ensure_valid_ref_format () {
646 + assert test $# = 1
647 git check-ref-format "refs/heads/$1" ||
648 die "'$1' does not look like a ref"
649 }
650
651 +# Usage: process_split_commit REV PARENTS INDENT
652 process_split_commit () {
653 + assert test $# = 3
654 local rev="$1"
655 local parents="$2"
656 local indent=$3
@@ -654,6 +702,8 @@ process_split_commit () {
702 cache_set latest_old "$rev"
703 }
704
705 +# Usage: cmd_add REV
706 +# Or: cmd_add REPOSITORY REF
707 cmd_add () {
708
709 ensure_clean
@@ -681,7 +731,9 @@ cmd_add () {
731 fi
732 }
733
734 +# Usage: cmd_add_repository REPOSITORY REFSPEC
735 cmd_add_repository () {
736 + assert test $# = 2
737 echo "git fetch" "$@"
738 repository=$1
739 refspec=$2
@@ -689,9 +741,11 @@ cmd_add_repository () {
741 cmd_add_commit FETCH_HEAD
742 }
743
744 +# Usage: cmd_add_commit REV
745 cmd_add_commit () {
746 # The rev has already been validated by cmd_add(), we just
747 # need to normalize it.
748 + assert test $# = 1
749 rev=$(git rev-parse --verify "$1^{commit}") || exit $?
750
751 debug "Adding $dir as '$rev'..."
@@ -722,6 +776,7 @@ cmd_add_commit () {
776 say >&2 "Added dir '$dir'"
777 }
778
779 +# Usage: cmd_split [REV]
780 cmd_split () {
781 if test $# -eq 0
782 then
@@ -801,6 +856,7 @@ cmd_split () {
856 exit 0
857 }
858
859 +# Usage: cmd_merge REV
860 cmd_merge () {
861 test $# -eq 1 ||
862 die "You must provide exactly one revision. Got: '$*'"
@@ -837,6 +893,7 @@ cmd_merge () {
893 fi
894 }
895
896 +# Usage: cmd_pull REPOSITORY REMOTEREF
897 cmd_pull () {
898 if test $# -ne 2
899 then
@@ -848,6 +905,7 @@ cmd_pull () {
905 cmd_merge FETCH_HEAD
906 }
907
908 +# Usage: cmd_push REPOSITORY REMOTEREF
909 cmd_push () {
910 if test $# -ne 2
911 then