diff mbox series

[4/7] unpack-trees: do not mark a dirty path with SKIP_WORKTREE

Message ID baa4f23421b6bc245b790cdfc1fc64f5e064bca0.1584169893.git.gitgitgadget@gmail.com (mailing list archive)
State New, archived
Headers show
Series Sparse checkout improvements -- improved sparsity updating | expand

Commit Message

Linus Arver via GitGitGadget March 14, 2020, 7:11 a.m. UTC 
From: Elijah Newren <newren@gmail.com>

If a path is dirty, removing from the working tree risks losing data.
As such, we want to make sure any such path is not marked with
SKIP_WORKTREE.  While the current callers of this code detect this case
and re-populate with a previous set of sparsity patterns, we want to
allow some paths to be marked with SKIP_WORKTREE while others are left
unmarked without it being considered an error.  The reason this
shouldn't be considered an error is that SKIP_WORKTREE has always been
an advisory-only setting; merge and rebase for example were free to
materialize paths and clear the SKIP_WORKTREE bit in order to accomplish
their work even though they kept the SKIP_WORKTREE bit set for other
paths.  Leaving dirty working files in the working tree is thus a
natural extension of what we have already been doing.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 unpack-trees.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

Comments

Derrick Stolee March 15, 2020, 2:39 p.m. UTC | #1 
cOn 3/14/2020 3:11 AM, Elijah Newren via GitGitGadget wrote:
> From: Elijah Newren <newren@gmail.com>
> 
> If a path is dirty, removing from the working tree risks losing data.
> As such, we want to make sure any such path is not marked with
> SKIP_WORKTREE.  While the current callers of this code detect this case
> and re-populate with a previous set of sparsity patterns, we want to
> allow some paths to be marked with SKIP_WORKTREE while others are left
> unmarked without it being considered an error.  The reason this
> shouldn't be considered an error is that SKIP_WORKTREE has always been
> an advisory-only setting; merge and rebase for example were free to
> materialize paths and clear the SKIP_WORKTREE bit in order to accomplish
> their work even though they kept the SKIP_WORKTREE bit set for other
> paths.  Leaving dirty working files in the working tree is thus a
> natural extension of what we have already been doing.
> 
> Signed-off-by: Elijah Newren <newren@gmail.com>
> ---
>  unpack-trees.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/unpack-trees.c b/unpack-trees.c
> index 853d843b17a..8f10ac91ce1 100644
> --- a/unpack-trees.c
> +++ b/unpack-trees.c
> @@ -504,8 +504,11 @@ static int apply_sparse_checkout(struct index_state *istate,
>  		 * also stat info may have lost after merged_entry() so calling
>  		 * verify_uptodate() again may fail
>  		 */
> -		if (!(ce->ce_flags & CE_UPDATE) && verify_uptodate_sparse(ce, o))
> +		if (!(ce->ce_flags & CE_UPDATE) &&
> +		    verify_uptodate_sparse(ce, o)) {
> +			ce->ce_flags &= ~CE_SKIP_WORKTREE;
>  			return -1;
> +		}
>  		ce->ce_flags |= CE_WT_REMOVE;
>  		ce->ce_flags &= ~CE_UPDATE;
>  	}

I initially thought that you needed to update some
blocks in clear_ce_flags_dir() around cone mode where
we skip sections that are outside of the cone. However,
this seems to be working on the results after the
clear_ce_flags() has completed. Should work.

Thanks,
-Stolee
diff mbox series

Patch

diff --git a/unpack-trees.c b/unpack-trees.c
index 853d843b17a..8f10ac91ce1 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -504,8 +504,11 @@  static int apply_sparse_checkout(struct index_state *istate,
 		 * also stat info may have lost after merged_entry() so calling
 		 * verify_uptodate() again may fail
 		 */
-		if (!(ce->ce_flags & CE_UPDATE) && verify_uptodate_sparse(ce, o))
+		if (!(ce->ce_flags & CE_UPDATE) &&
+		    verify_uptodate_sparse(ce, o)) {
+			ce->ce_flags &= ~CE_SKIP_WORKTREE;
 			return -1;
+		}
 		ce->ce_flags |= CE_WT_REMOVE;
 		ce->ce_flags &= ~CE_UPDATE;
 	}