From: Elijah Newren <newren@xxxxxxxxx> sparse-checkout's purpose is to update the working tree to have it reflect a subset of the tracked files. As such, it shouldn't be switching branches, making commits, downloading or uploading data, or staging or unstaging changes. Other than updating the worktree, the only thing sparse-checkout should touch is the SKIP_WORKTREE bit of the index. In particular, this sets up a nice invariant: running sparse-checkout will never change the status of any file in `git status` (reflecting the fact that we only set the SKIP_WORKTREE bit if the file is safe to delete, i.e. if the file is unmodified). Traditionally, we did a _really_ bad job with this goal. The predecessor to sparse-checkout involved manual editing of .git/info/sparse-checkout and running `git read-tree -mu HEAD`. That command would stage and unstage changes and overwrite dirty changes in the working tree. The initial implementation of the sparse-checkout command was no better; it simply invoked `git read-tree -mu HEAD` as a subprocess and had the same caveats, though this issue came up repeatedly in review comments and workarounds for the problems were put in place before the feature was merged[1, 2, 3, 4, 5, 6; especially see 4 & 6]. [1] https://lore.kernel.org/git/CABPp-BFT9A5n=_bx5LsjCvbogqwSjiwgr5amcjgbU1iAk4KLJg@xxxxxxxxxxxxxx/ [2] https://lore.kernel.org/git/CABPp-BEmwSwg4tgJg6nVG8a3Hpn_g-=ZjApZF4EiJO+qVgu4uw@xxxxxxxxxxxxxx/ [3] https://lore.kernel.org/git/CABPp-BFV7TA0qwZCQpHCqx9N+JifyRyuBQ-pZ_oGfe-NOgyh7A@xxxxxxxxxxxxxx/ [4] https://lore.kernel.org/git/CABPp-BHYCCD+Vx5fq35jH82eHc1-P53Lz_aGNpHJNcx9kg2K-A@xxxxxxxxxxxxxx/ [5] https://lore.kernel.org/git/CABPp-BF+JWYZfDqp2Tn4AEKVp4b0YMA=Mbz4Nz62D-gGgiduYQ@xxxxxxxxxxxxxx/ [6] https://lore.kernel.org/git/20191121163706.GV23183@xxxxxxxxxx/ However, these workarounds, in addition to disabling the feature in a number of important cases, also missed one special case. I'll get back to it later. In the 2.27.0 cycle, the disabling of the feature was lifted by finally replacing the internal equivalent of `git read-tree -mu HEAD` with something that did what we wanted: the new update_sparsity() function in unpack-trees.c that only ever updates SKIP_WORKTREE bits in the index and updates the working tree to match. This new function handles all the cases that were problematic for the old implementation, except that it breaks the same special case that avoided the workarounds of the old implementation, but broke it in a different way. So...that brings us to the special case: a git clone performed with --no-checkout. As per the meaning of the flag, --no-checkout does not check out any branch, with the implication that you aren't on one and need to switch to one after the clone. Implementationally, HEAD is still set (so in some sense you are partially on a branch), but * the index is "unborn" (non-existent) * there are no files in the working tree (other than .git/) * the next time git switch (or git checkout) is run it will run unpack_trees with `initial_checkout` flag set to true. It is not until you run, e.g. `git switch <somebranch>` that the index will be written and files in the working tree populated. With this special --no-checkout case, the traditional `read-tree -mu HEAD` behavior would have done the equivalent of acting like checkout -- switch to the default branch (HEAD), write out an index that matches HEAD, and update the working tree to match. This special case slipped through the avoid-making-changes checks in the original sparse-checkout command and thus continued there. After update_sparsity() was introduced and used (see commit f56f31af03 ("sparse-checkout: use new update_sparsity() function", 2020-03-27)), the behavior for the --no-checkout case changed: Due to git's auto-vivification of an empty in-memory index (see do_read_index() and note that `must_exist` is false), and due to sparse-checkout's update_working_directory() code to always write out the index after it was done, we got a new bug. That made it so that sparse-checkout would switch the repository from a clone with an "unborn" index (i.e. still needing an initial_checkout), to one that had a recorded index with no entries. Thus, instead of all the files appearing deleted in `git status` being known to git as a special artifact of not yet being on a branch, our recording of an empty index made it suddenly look to git as though it was definitely on a branch with ALL files staged for deletion! A subsequent checkout or switch then had to contend with the fact that it wasn't on an initial_checkout but had a bunch of staged deletions. Make sure that sparse-checkout changes nothing in the index other than the SKIP_WORKTREE bit; in particular, when the index is unborn we do not have any branch checked out so there is no sparsification or de-sparsification work to do. Simply return from update_working_directory() early. Signed-off-by: Elijah Newren <newren@xxxxxxxxx> --- sparse-checkout: avoid staging deletions of all files Changes since v1: * Fixed the ls command for portability, using the construct Junio suggested Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-801%2Fnewren%2Fsparse-checkout-and-unborn-index-v2 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-801/newren/sparse-checkout-and-unborn-index-v2 Pull-Request: https://github.com/git/git/pull/801 Range-diff vs v1: 1: 4c8b512506e ! 1: 8be6550d11b sparse-checkout: avoid staging deletions of all files @@ t/t1091-sparse-checkout-builtin.sh: test_expect_success 'clone --sparse' ' + git clone --no-checkout "file://$(pwd)/repo" clone_no_checkout && + git -C clone_no_checkout sparse-checkout init --cone && + git -C clone_no_checkout sparse-checkout set folder1 && ++ + git -C clone_no_checkout sparse-checkout list >actual && + cat >expect <<-\EOF && + folder1 + EOF + test_cmp expect actual && -+ ls clone_no_checkout >actual && ++ ++ # nothing checked out, expect "No such file or directory" ++ ! ls clone_no_checkout/* >actual && + test_must_be_empty actual && + test_path_is_missing clone_no_checkout/.git/index && + builtin/sparse-checkout.c | 4 ++++ t/t1091-sparse-checkout-builtin.sh | 22 ++++++++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c index 95d08824172..595463be68e 100644 --- a/builtin/sparse-checkout.c +++ b/builtin/sparse-checkout.c @@ -99,6 +99,10 @@ static int update_working_directory(struct pattern_list *pl) struct lock_file lock_file = LOCK_INIT; struct repository *r = the_repository; + /* If no branch has been checked out, there are no updates to make. */ + if (is_index_unborn(r->index)) + return UPDATE_SPARSITY_SUCCESS; + memset(&o, 0, sizeof(o)); o.verbose_update = isatty(2); o.update = 1; diff --git a/t/t1091-sparse-checkout-builtin.sh b/t/t1091-sparse-checkout-builtin.sh index 88cdde255cd..7cd45fc1394 100755 --- a/t/t1091-sparse-checkout-builtin.sh +++ b/t/t1091-sparse-checkout-builtin.sh @@ -100,6 +100,28 @@ test_expect_success 'clone --sparse' ' check_files clone a ' +test_expect_success 'interaction with clone --no-checkout (unborn index)' ' + git clone --no-checkout "file://$(pwd)/repo" clone_no_checkout && + git -C clone_no_checkout sparse-checkout init --cone && + git -C clone_no_checkout sparse-checkout set folder1 && + + git -C clone_no_checkout sparse-checkout list >actual && + cat >expect <<-\EOF && + folder1 + EOF + test_cmp expect actual && + + # nothing checked out, expect "No such file or directory" + ! ls clone_no_checkout/* >actual && + test_must_be_empty actual && + test_path_is_missing clone_no_checkout/.git/index && + + # No branch is checked out until we manually switch to one + git -C clone_no_checkout switch master && + test_path_is_file clone_no_checkout/.git/index && + check_files clone_no_checkout a folder1 +' + test_expect_success 'set enables config' ' git init empty-config && ( base-commit: 20514004ddf1a3528de8933bc32f284e175e1012 -- gitgitgadget
