@matheusbsbs / matheus-bagliano / commits / c81638541c
Fork 0

submodule add: show 'add --dry-run' stderr when aborting

Unless --force is specified, 'submodule add' checks if the destination path is ignored by calling 'git add --dry-run --ignore-missing', and, if that call fails, aborts with a custom "path is ignored" message (a slight variant of what 'git add' shows). Aborting early rather than letting the downstream 'git add' call fail is done so that the command exits before cloning into the destination path. However, in rare cases where the dry-run call fails for a reason other than the path being ignored---for example, due to a preexisting index.lock file---displaying the "ignored path" error message hides the real source of the failure. Instead of displaying the tailored "ignored path" message, let's report the standard error from the dry run to give the caller more accurate information about failures that are not due to an ignored path. For the ignored path case, this leads to the following change in the error message: The following [-path is-]{+paths are+} ignored by one of your .gitignore files: <destination path> Use -f if you really want to add [-it.-]{+them.+} The new phrasing is a bit awkward, because 'submodule add' is only dealing with one destination path. Alternatively, we could continue to use the tailored message when the exit code is 1 (the expected status for a failure due to an ignored path) and relay the standard error for all other non-zero exits. That, however, risks hiding the message of unrelated failures that share an exit code of 1, so it doesn't seem worth doing just to avoid a clunkier, but still clear, error message. Signed-off-by: Kyle Meyer <kyle@kyleam.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Kyle Meyer committed Jan 7, 2020 at 19:31 UTC c81638541cbbf5868e79037ca29b6e37e47461e5
2 files changed +21 -8
git-submodule.sh
+8 -6
@@ -241,13 +241,15 @@ cmd_add()
241 die "$(eval_gettext "'\$sm_path' does not have a commit checked out")"
242 fi
243
244 - if test -z "$force" &&
245 - ! git add --dry-run --ignore-missing --no-warn-embedded-repo "$sm_path" > /dev/null 2>&1
244 + if test -z "$force"
245 then
247 - eval_gettextln "The following path is ignored by one of your .gitignore files:
248 -\$sm_path
249 -Use -f if you really want to add it." >&2
250 - exit 1
246 + dryerr=$(git add --dry-run --ignore-missing --no-warn-embedded-repo "$sm_path" 2>&1 >/dev/null)
247 + res=$?
248 + if test $res -ne 0
249 + then
250 + echo >&2 "$dryerr"
251 + exit $res
252 + fi
253 fi
254
255 if test -n "$custom_name"
t/t7400-submodule-basic.sh
+13 -2
@@ -156,9 +156,9 @@ test_expect_success 'submodule add to .gitignored path fails' '
156 (
157 cd addtest-ignore &&
158 cat <<-\EOF >expect &&
159 - The following path is ignored by one of your .gitignore files:
159 + The following paths are ignored by one of your .gitignore files:
160 submod
161 - Use -f if you really want to add it.
161 + Use -f if you really want to add them.
162 EOF
163 # Does not use test_commit due to the ignore
164 echo "*" > .gitignore &&
@@ -191,6 +191,17 @@ test_expect_success 'submodule add to reconfigure existing submodule with --forc
191 )
192 '
193
194 +test_expect_success 'submodule add relays add --dry-run stderr' '
195 + test_when_finished "rm -rf addtest/.git/index.lock" &&
196 + (
197 + cd addtest &&
198 + : >.git/index.lock &&
199 + ! git submodule add "$submodurl" sub-while-locked 2>output.err &&
200 + test_i18ngrep "^fatal: .*index\.lock" output.err &&
201 + test_path_is_missing sub-while-locked
202 + )
203 +'
204 +
205 test_expect_success 'submodule add --branch' '
206 echo "refs/heads/initial" >expect-head &&
207 cat <<-\EOF >expect-heads &&