subtree: be stricter about validating flags
Don't silently ignore a flag that's invalid for a given subcommand. The user expected it to do something; we should tell the user that they are mistaken, instead of surprising the user. It could be argued that this change might break existing users. I'd argue that those existing users are already broken, and they just don't know it. Let them know that they're broken. 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
9a3e3ca2ba869f9fef9f8be390ed45457565ccd1
2 files changed
+175
-25
contrib/subtree/git-subtree.sh
+64
-25
@@ -44,17 +44,6 @@ squash merge subtree changes as a single commit
44
m,message= use the given message as the commit message for the merge commit
45
"
46
47
-arg_debug=
48
-arg_command=
49
-arg_prefix=
50
-arg_split_branch=
51
-arg_split_onto=
52
-arg_split_rejoin=
53
-arg_split_ignore_joins=
54
-arg_split_annotate=
55
-arg_addmerge_squash=
56
-arg_addmerge_message=
57
-
47
indent=0
48
49
# Usage: debug [MSG...]
@@ -106,10 +95,61 @@ main () {
95
then
96
set -- -h
97
fi
109
- eval "$(echo "$OPTS_SPEC" | git rev-parse --parseopt -- "$@" || echo exit $?)"
98
+ set_args="$(echo "$OPTS_SPEC" | git rev-parse --parseopt -- "$@" || echo exit $?)"
99
+ eval "$set_args"
100
. git-sh-setup
101
require_work_tree
102
103
+ # First figure out the command and whether we use --rejoin, so
104
+ # that we can provide more helpful validation when we do the
105
+ # "real" flag parsing.
106
+ arg_split_rejoin=
107
+ allow_split=
108
+ allow_addmerge=
109
+ while test $# -gt 0
110
+ do
111
+ opt="$1"
112
+ shift
113
+ case "$opt" in
114
+ --annotate|-b|-P|-m|--onto)
115
+ shift
116
+ ;;
117
+ --rejoin)
118
+ arg_split_rejoin=1
119
+ ;;
120
+ --no-rejoin)
121
+ arg_split_rejoin=
122
+ ;;
123
+ --)
124
+ break
125
+ ;;
126
+ esac
127
+ done
128
+ arg_command=$1
129
+ case "$arg_command" in
130
+ add|merge|pull)
131
+ allow_addmerge=1
132
+ ;;
133
+ split|push)
134
+ allow_split=1
135
+ allow_addmerge=$arg_split_rejoin
136
+ ;;
137
+ *)
138
+ die "Unknown command '$arg_command'"
139
+ ;;
140
+ esac
141
+ # Reset the arguments array for "real" flag parsing.
142
+ eval "$set_args"
143
+
144
+ # Begin "real" flag parsing.
145
+ arg_debug=
146
+ arg_prefix=
147
+ arg_split_branch=
148
+ arg_split_onto=
149
+ arg_split_ignore_joins=
150
+ arg_split_annotate=
151
+ arg_addmerge_squash=
152
+ arg_addmerge_message=
153
while test $# -gt 0
154
do
155
opt="$1"
@@ -123,13 +163,16 @@ main () {
163
arg_debug=1
164
;;
165
--annotate)
166
+ test -n "$allow_split" || die "The '$opt' flag does not make sense with 'git subtree $arg_command'."
167
arg_split_annotate="$1"
168
shift
169
;;
170
--no-annotate)
171
+ test -n "$allow_split" || die "The '$opt' flag does not make sense with 'git subtree $arg_command'."
172
arg_split_annotate=
173
;;
174
-b)
175
+ test -n "$allow_split" || die "The '$opt' flag does not make sense with 'git subtree $arg_command'."
176
arg_split_branch="$1"
177
shift
178
;;
@@ -138,6 +181,7 @@ main () {
181
shift
182
;;
183
-m)
184
+ test -n "$allow_addmerge" || die "The '$opt' flag does not make sense with 'git subtree $arg_command'."
185
arg_addmerge_message="$1"
186
shift
187
;;
@@ -145,28 +189,34 @@ main () {
189
arg_prefix=
190
;;
191
--onto)
192
+ test -n "$allow_split" || die "The '$opt' flag does not make sense with 'git subtree $arg_command'."
193
arg_split_onto="$1"
194
shift
195
;;
196
--no-onto)
197
+ test -n "$allow_split" || die "The '$opt' flag does not make sense with 'git subtree $arg_command'."
198
arg_split_onto=
199
;;
200
--rejoin)
155
- arg_split_rejoin=1
201
+ test -n "$allow_split" || die "The '$opt' flag does not make sense with 'git subtree $arg_command'."
202
;;
203
--no-rejoin)
158
- arg_split_rejoin=
204
+ test -n "$allow_split" || die "The '$opt' flag does not make sense with 'git subtree $arg_command'."
205
;;
206
--ignore-joins)
207
+ test -n "$allow_split" || die "The '$opt' flag does not make sense with 'git subtree $arg_command'."
208
arg_split_ignore_joins=1
209
;;
210
--no-ignore-joins)
211
+ test -n "$allow_split" || die "The '$opt' flag does not make sense with 'git subtree $arg_command'."
212
arg_split_ignore_joins=
213
;;
214
--squash)
215
+ test -n "$allow_addmerge" || die "The '$opt' flag does not make sense with 'git subtree $arg_command'."
216
arg_addmerge_squash=1
217
;;
218
--no-squash)
219
+ test -n "$allow_addmerge" || die "The '$opt' flag does not make sense with 'git subtree $arg_command'."
220
arg_addmerge_squash=
221
;;
222
--)
@@ -177,19 +227,8 @@ main () {
227
;;
228
esac
229
done
180
-
181
- arg_command="$1"
230
shift
231
184
- case "$arg_command" in
185
- add|merge|pull|split|push)
186
- :
187
- ;;
188
- *)
189
- die "Unknown command '$arg_command'"
190
- ;;
191
- esac
192
-
232
if test -z "$arg_prefix"
233
then
234
die "You must provide the --prefix option."
contrib/subtree/t/t7900-subtree.sh
+111
@@ -33,6 +33,12 @@ test_create_commit () (
33
git commit -m "$commit" || error "Could not commit"
34
)
35
36
+test_wrong_flag() {
37
+ test_must_fail "$@" >out 2>err &&
38
+ test_must_be_empty out &&
39
+ grep "flag does not make sense with" err
40
+}
41
+
42
last_commit_subject () {
43
git log --pretty=format:%s -1
44
}
@@ -72,6 +78,22 @@ test_expect_success 'no pull from non-existent subtree' '
78
)
79
'
80
81
+test_expect_success 'add rejects flags for split' '
82
+ subtree_test_create_repo "$test_count" &&
83
+ subtree_test_create_repo "$test_count/sub proj" &&
84
+ test_create_commit "$test_count" main1 &&
85
+ test_create_commit "$test_count/sub proj" sub1 &&
86
+ (
87
+ cd "$test_count" &&
88
+ git fetch ./"sub proj" HEAD &&
89
+ test_wrong_flag git subtree add --prefix="sub dir" --annotate=foo FETCH_HEAD &&
90
+ test_wrong_flag git subtree add --prefix="sub dir" --branch=foo FETCH_HEAD &&
91
+ test_wrong_flag git subtree add --prefix="sub dir" --ignore-joins FETCH_HEAD &&
92
+ test_wrong_flag git subtree add --prefix="sub dir" --onto=foo FETCH_HEAD &&
93
+ test_wrong_flag git subtree add --prefix="sub dir" --rejoin FETCH_HEAD
94
+ )
95
+'
96
+
97
test_expect_success 'add subproj as subtree into sub dir/ with --prefix' '
98
subtree_test_create_repo "$test_count" &&
99
subtree_test_create_repo "$test_count/sub proj" &&
@@ -128,6 +150,28 @@ test_expect_success 'add subproj as subtree into sub dir/ with --squash and --pr
150
# Tests for 'git subtree merge'
151
#
152
153
+test_expect_success 'merge rejects flags for split' '
154
+ subtree_test_create_repo "$test_count" &&
155
+ subtree_test_create_repo "$test_count/sub proj" &&
156
+ test_create_commit "$test_count" main1 &&
157
+ test_create_commit "$test_count/sub proj" sub1 &&
158
+ (
159
+ cd "$test_count" &&
160
+ git fetch ./"sub proj" HEAD &&
161
+ git subtree add --prefix="sub dir" FETCH_HEAD
162
+ ) &&
163
+ test_create_commit "$test_count/sub proj" sub2 &&
164
+ (
165
+ cd "$test_count" &&
166
+ git fetch ./"sub proj" HEAD &&
167
+ test_wrong_flag git subtree merge --prefix="sub dir" --annotate=foo FETCH_HEAD &&
168
+ test_wrong_flag git subtree merge --prefix="sub dir" --branch=foo FETCH_HEAD &&
169
+ test_wrong_flag git subtree merge --prefix="sub dir" --ignore-joins FETCH_HEAD &&
170
+ test_wrong_flag git subtree merge --prefix="sub dir" --onto=foo FETCH_HEAD &&
171
+ test_wrong_flag git subtree merge --prefix="sub dir" --rejoin FETCH_HEAD
172
+ )
173
+'
174
+
175
test_expect_success 'merge new subproj history into sub dir/ with --prefix' '
176
subtree_test_create_repo "$test_count" &&
177
subtree_test_create_repo "$test_count/sub proj" &&
@@ -262,6 +306,30 @@ test_expect_success 'split requires path given by option --prefix must exist' '
306
)
307
'
308
309
+test_expect_success 'split rejects flags for add' '
310
+ subtree_test_create_repo "$test_count" &&
311
+ subtree_test_create_repo "$test_count/sub proj" &&
312
+ test_create_commit "$test_count" main1 &&
313
+ test_create_commit "$test_count/sub proj" sub1 &&
314
+ (
315
+ cd "$test_count" &&
316
+ git fetch ./"sub proj" HEAD &&
317
+ git subtree add --prefix="sub dir" FETCH_HEAD
318
+ ) &&
319
+ test_create_commit "$test_count" "sub dir"/main-sub1 &&
320
+ test_create_commit "$test_count" main2 &&
321
+ test_create_commit "$test_count/sub proj" sub2 &&
322
+ test_create_commit "$test_count" "sub dir"/main-sub2 &&
323
+ (
324
+ cd "$test_count" &&
325
+ git fetch ./"sub proj" HEAD &&
326
+ git subtree merge --prefix="sub dir" FETCH_HEAD &&
327
+ split_hash=$(git subtree split --prefix="sub dir" --annotate="*") &&
328
+ test_wrong_flag git subtree split --prefix="sub dir" --squash &&
329
+ test_wrong_flag git subtree split --prefix="sub dir" --message=foo
330
+ )
331
+'
332
+
333
test_expect_success 'split sub dir/ with --rejoin' '
334
subtree_test_create_repo "$test_count" &&
335
subtree_test_create_repo "$test_count/sub proj" &&
@@ -542,6 +610,26 @@ test_expect_success 'pull basic operation' '
610
)
611
'
612
613
+test_expect_success 'pull rejects flags for split' '
614
+ subtree_test_create_repo "$test_count" &&
615
+ subtree_test_create_repo "$test_count/sub proj" &&
616
+ test_create_commit "$test_count" main1 &&
617
+ test_create_commit "$test_count/sub proj" sub1 &&
618
+ (
619
+ cd "$test_count" &&
620
+ git fetch ./"sub proj" HEAD &&
621
+ git subtree add --prefix="sub dir" FETCH_HEAD
622
+ ) &&
623
+ test_create_commit "$test_count/sub proj" sub2 &&
624
+ (
625
+ test_must_fail git subtree pull --prefix="sub dir" --annotate=foo ./"sub proj" HEAD &&
626
+ test_must_fail git subtree pull --prefix="sub dir" --branch=foo ./"sub proj" HEAD &&
627
+ test_must_fail git subtree pull --prefix="sub dir" --ignore-joins ./"sub proj" HEAD &&
628
+ test_must_fail git subtree pull --prefix="sub dir" --onto=foo ./"sub proj" HEAD &&
629
+ test_must_fail git subtree pull --prefix="sub dir" --rejoin ./"sub proj" HEAD
630
+ )
631
+'
632
+
633
#
634
# Tests for 'git subtree push'
635
#
@@ -584,6 +672,29 @@ test_expect_success 'push requires path given by option --prefix must exist' '
672
)
673
'
674
675
+test_expect_success 'push rejects flags for add' '
676
+ subtree_test_create_repo "$test_count" &&
677
+ subtree_test_create_repo "$test_count/sub proj" &&
678
+ test_create_commit "$test_count" main1 &&
679
+ test_create_commit "$test_count/sub proj" sub1 &&
680
+ (
681
+ cd "$test_count" &&
682
+ git fetch ./"sub proj" HEAD &&
683
+ git subtree add --prefix="sub dir" FETCH_HEAD
684
+ ) &&
685
+ test_create_commit "$test_count" "sub dir"/main-sub1 &&
686
+ test_create_commit "$test_count" main2 &&
687
+ test_create_commit "$test_count/sub proj" sub2 &&
688
+ test_create_commit "$test_count" "sub dir"/main-sub2 &&
689
+ (
690
+ cd "$test_count" &&
691
+ git fetch ./"sub proj" HEAD &&
692
+ git subtree merge --prefix="sub dir" FETCH_HEAD &&
693
+ test_wrong_flag git subtree split --prefix="sub dir" --squash ./"sub proj" from-mainline &&
694
+ test_wrong_flag git subtree split --prefix="sub dir" --message=foo ./"sub proj" from-mainline
695
+ )
696
+'
697
+
698
test_expect_success 'push basic operation' '
699
subtree_test_create_repo "$test_count" &&
700
subtree_test_create_repo "$test_count/sub proj" &&