@matheusbsbs / matheus-bagliano / commits / 9ebd7fe158

add: propagate --chmod errors to exit status

If `add` encounters an error while applying the --chmod changes, it prints a message to stderr, but exits with a success code. This might have been an oversight, as the command does exit with a non-zero code in other situations where it cannot (or refuses to) update all of the requested paths (e.g. when some of the given paths are ignored). So make the exit behavior more consistent by also propagating --chmod errors to the exit status. Note: the test "all statuses changed in folder if . is given" uses paths added by previous test cases, some of which might be symbolic links. Because `git add --chmod` will now fail with such paths, this test would depend on whether all the previous tests were executed, or only some of them. Avoid that by running the test on a fresh repo with only regular files. Signed-off-by: Matheus Tavares <matheus.bernardino@usp.br> Reviewed-by: Taylor Blau <me@ttaylorr.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Matheus Tavares committed Feb 22, 2021 at 22:10 UTC 9ebd7fe15869910c81c42290bd80296664597dec

2 files changed +28 -10

builtin/add.c

+6 -4

@@ -38,9 +38,9 @@ struct update_callback_data {

38 int add_errors;

39 };

41 -static void chmod_pathspec(struct pathspec *pathspec, char flip, int show_only)

41 +static int chmod_pathspec(struct pathspec *pathspec, char flip, int show_only)

42 {

43 - int i;

43 + int i, ret = 0;

45 for (i = 0; i < active_nr; i++) {

46 struct cache_entry *ce = active_cache[i];

@@ -55,8 +55,10 @@ static void chmod_pathspec(struct pathspec *pathspec, char flip, int show_only)

55 err = S_ISREG(ce->ce_mode) ? 0 : -1;

57 if (err < 0)

58 - error(_("cannot chmod %cx '%s'"), flip, ce->name);

58 + ret = error(_("cannot chmod %cx '%s'"), flip, ce->name);

59 }

60 +

61 + return ret;

62 }

64 static int fix_unmerged_status(struct diff_filepair *p,

@@ -615,7 +617,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)

617 exit_status |= add_files(&dir, flags);

618

619 if (chmod_arg && pathspec.nr)

618 - chmod_pathspec(&pathspec, chmod_arg[0], show_only);

620 + exit_status |= chmod_pathspec(&pathspec, chmod_arg[0], show_only);

621 unplug_bulk_checkin();

622

623 finish:

t/t3700-add.sh

+22 -6

@@ -386,6 +386,16 @@ test_expect_success POSIXPERM 'git add --chmod=[+-]x does not change the working

386 ! test -x foo4

387 '

388

389 +test_expect_success 'git add --chmod fails with non regular files (but updates the other paths)' '

390 + git reset --hard &&

391 + test_ln_s_add foo foo3 &&

392 + touch foo4 &&

393 + test_must_fail git add --chmod=+x foo3 foo4 2>stderr &&

394 + test_i18ngrep "cannot chmod +x .foo3." stderr &&

395 + test_mode_in_index 120000 foo3 &&

396 + test_mode_in_index 100755 foo4

397 +'

398 +

399 test_expect_success 'git add --chmod honors --dry-run' '

400 git reset --hard &&

401 echo foo >foo4 &&

@@ -397,7 +407,7 @@ test_expect_success 'git add --chmod honors --dry-run' '

407 test_expect_success 'git add --chmod --dry-run reports error for non regular files' '

408 git reset --hard &&

409 test_ln_s_add foo foo4 &&

400 - git add --chmod=+x --dry-run foo4 2>stderr &&

410 + test_must_fail git add --chmod=+x --dry-run foo4 2>stderr &&

411 test_i18ngrep "cannot chmod +x .foo4." stderr

412 '

413

@@ -429,11 +439,17 @@ test_expect_success 'no file status change if no pathspec is given in subdir' '

439 '

440

441 test_expect_success 'all statuses changed in folder if . is given' '

432 - rm -fr empty &&

433 - git add --chmod=+x . &&

434 - test $(git ls-files --stage | grep ^100644 | wc -l) -eq 0 &&

435 - git add --chmod=-x . &&

436 - test $(git ls-files --stage | grep ^100755 | wc -l) -eq 0

442 + git init repo &&

443 + (

444 + cd repo &&

445 + mkdir -p sub/dir &&

446 + touch x y z sub/a sub/dir/b &&

447 + git add -A &&

448 + git add --chmod=+x . &&

449 + test $(git ls-files --stage | grep ^100644 | wc -l) -eq 0 &&

450 + git add --chmod=-x . &&

451 + test $(git ls-files --stage | grep ^100755 | wc -l) -eq 0

452 + )

453 '

454

455 test_expect_success CASE_INSENSITIVE_FS 'path is case-insensitive' '