@matheusbsbs / matheus-bagliano / commits / 105e8b014b

add: fail when adding an untracked sparse file

The add_files() method in builtin/add.c takes a set of untracked files that are being added by the input pathspec and inserts them into the index. If these files are outside of the sparse-checkout cone, then they gain the SKIP_WORKTREE bit at some point. However, this was not checked before inserting into the index, so these files are added even though we want to avoid modifying the index outside of the sparse-checkout cone. Add a check within add_files() for these files and write the advice about files outside of the sparse-checkout cone. This behavior change modifies some existing tests within t1092. These tests intended to document how a user could interact with the existing behavior in place. Many of these tests need to be marked as expecting failure. A future change will allow these tests to pass by adding a flag to 'git add' that allows users to modify index entries outside of the sparse-checkout cone. The 'submodule handling' test is intended to document what happens to directories that contain a submodule when the sparse index is enabled. It is not trying to say that users should be able to add submodules outside of the sparse-checkout cone, so that test can be modified to avoid that operation. Signed-off-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Derrick Stolee committed Sep 24, 2021 at 15:39 UTC 105e8b014b1d584174aca81081ee5428caea03cb

2 files changed +42 -9

builtin/add.c

+14

@@ -443,6 +443,7 @@ static void check_embedded_repo(const char *path)

443 static int add_files(struct dir_struct *dir, int flags)

444 {

445 int i, exit_status = 0;

446 + struct string_list matched_sparse_paths = STRING_LIST_INIT_NODUP;

447

448 if (dir->ignored_nr) {

449 fprintf(stderr, _(ignore_error));

@@ -456,6 +457,11 @@ static int add_files(struct dir_struct *dir, int flags)

457 }

458

459 for (i = 0; i < dir->nr; i++) {

460 + if (!path_in_sparse_checkout(dir->entries[i]->name, &the_index)) {

461 + string_list_append(&matched_sparse_paths,

462 + dir->entries[i]->name);

463 + continue;

464 + }

465 if (add_file_to_index(&the_index, dir->entries[i]->name, flags)) {

466 if (!ignore_add_errors)

467 die(_("adding files failed"));

@@ -464,6 +470,14 @@ static int add_files(struct dir_struct *dir, int flags)

470 check_embedded_repo(dir->entries[i]->name);

471 }

472 }

473 +

474 + if (matched_sparse_paths.nr) {

475 + advise_on_updating_sparse_paths(&matched_sparse_paths);

476 + exit_status = 1;

477 + }

478 +

479 + string_list_clear(&matched_sparse_paths, 0);

480 +

481 return exit_status;

482 }

483

t/t1092-sparse-checkout-compatibility.sh

+28 -9

@@ -301,8 +301,6 @@ test_expect_success 'add, commit, checkout' '

301 test_all_match git checkout -

302 '

303

304 -# NEEDSWORK: This documents current behavior, but is not a desirable

305 -# behavior (untracked files are handled differently than tracked).

304 test_expect_success 'add outside sparse cone' '

305 init_repos &&

306

@@ -312,7 +310,9 @@ test_expect_success 'add outside sparse cone' '

310 test_sparse_match test_must_fail git add folder1/a &&

311 grep "Disable or modify the sparsity rules" sparse-checkout-err &&

312 test_sparse_unstaged folder1/a &&

315 - test_sparse_match git add folder1/newfile

313 + test_sparse_match test_must_fail git add folder1/newfile &&

314 + grep "Disable or modify the sparsity rules" sparse-checkout-err &&

315 + test_sparse_unstaged folder1/newfile

316 '

317

318 test_expect_success 'commit including unstaged changes' '

@@ -343,7 +343,11 @@ test_expect_success 'commit including unstaged changes' '

343 test_all_match git status --porcelain=v2

344 '

345

346 -test_expect_success 'status/add: outside sparse cone' '

346 +# NEEDSWORK: Now that 'git add folder1/new' fails, the changes being

347 +# attempted here fail for the sparse-checkout and sparse-index repos.

348 +# We must enable a way for adding files outside the sparse-checkout

349 +# done, even if it is by an optional flag.

350 +test_expect_failure 'status/add: outside sparse cone' '

351 init_repos &&

352

353 # folder1 is at HEAD, but outside the sparse cone

@@ -368,10 +372,11 @@ test_expect_success 'status/add: outside sparse cone' '

372 test_sparse_match test_must_fail git add --refresh folder1/a &&

373 grep "Disable or modify the sparsity rules" sparse-checkout-err &&

374 test_sparse_unstaged folder1/a &&

375 + test_sparse_match test_must_fail git add folder1/new &&

376 + grep "Disable or modify the sparsity rules" sparse-checkout-err &&

377 + test_sparse_unstaged folder1/new &&

378

372 - # NEEDSWORK: Adding a newly-tracked file outside the cone succeeds

373 - test_sparse_match git add folder1/new &&

374 -

379 + # NEEDSWORK: behavior begins to deviate here.

380 test_all_match git add . &&

381 test_all_match git status --porcelain=v2 &&

382 test_all_match git commit -m folder1/new &&

@@ -527,7 +532,7 @@ test_expect_success 'merge, cherry-pick, and rebase' '

532 # Right now, users might be using this flow to work through conflicts,

533 # so any solution should present advice to users who try this sequence

534 # of commands to follow whatever new method we create.

530 -test_expect_success 'merge with conflict outside cone' '

535 +test_expect_failure 'merge with conflict outside cone' '

536 init_repos &&

537

538 test_all_match git checkout -b merge-tip merge-left &&

@@ -541,12 +546,18 @@ test_expect_success 'merge with conflict outside cone' '

546 test_all_match git status --porcelain=v2 &&

547

548 # 2. Add the file with conflict markers

549 + # NEEDSWORK: Even though the merge conflict removed the

550 + # SKIP_WORKTREE bit from the index entry for folder1/a, we should

551 + # warn that this is a problematic add.

552 test_all_match git add folder1/a &&

553 test_all_match git status --porcelain=v2 &&

554

555 # 3. Rename the file to another sparse filename and

556 # accept conflict markers as resolved content.

557 run_on_all mv folder2/a folder2/z &&

558 + # NEEDSWORK: This mode now fails, because folder2/z is

559 + # outside of the sparse-checkout cone and does not match an

560 + # existing index entry with the SKIP_WORKTREE bit cleared.

561 test_all_match git add folder2 &&

562 test_all_match git status --porcelain=v2 &&

563

@@ -555,7 +566,7 @@ test_expect_success 'merge with conflict outside cone' '

566 test_all_match git rev-parse HEAD^{tree}

567 '

568

558 -test_expect_success 'cherry-pick/rebase with conflict outside cone' '

569 +test_expect_failure 'cherry-pick/rebase with conflict outside cone' '

570 init_repos &&

571

572 for OPERATION in cherry-pick rebase

@@ -572,11 +583,17 @@ test_expect_success 'cherry-pick/rebase with conflict outside cone' '

583 test_all_match git status --porcelain=v2 &&

584

585 # 2. Add the file with conflict markers

586 + # NEEDSWORK: Even though the merge conflict removed the

587 + # SKIP_WORKTREE bit from the index entry for folder1/a, we should

588 + # warn that this is a problematic add.

589 test_all_match git add folder1/a &&

590 test_all_match git status --porcelain=v2 &&

591

592 # 3. Rename the file to another sparse filename and

593 # accept conflict markers as resolved content.

594 + # NEEDSWORK: This mode now fails, because folder2/z is

595 + # outside of the sparse-checkout cone and does not match an

596 + # existing index entry with the SKIP_WORKTREE bit cleared.

597 run_on_all mv folder2/a folder2/z &&

598 test_all_match git add folder2 &&

599 test_all_match git status --porcelain=v2 &&

@@ -654,6 +671,7 @@ test_expect_success 'clean' '

671 test_expect_success 'submodule handling' '

672 init_repos &&

673

674 + test_sparse_match git sparse-checkout add modules &&

675 test_all_match mkdir modules &&

676 test_all_match touch modules/a &&

677 test_all_match git add modules &&

@@ -663,6 +681,7 @@ test_expect_success 'submodule handling' '

681 test_all_match git commit -m "add submodule" &&

682

683 # having a submodule prevents "modules" from collapse

684 + test_sparse_match git sparse-checkout set deep/deeper1 &&

685 test-tool -C sparse-index read-cache --table >cache &&

686 grep "100644 blob .* modules/a" cache &&

687 grep "160000 commit $(git -C initial-repo rev-parse HEAD) modules/sub" cache