All of lore.kernel.org
 help / color / mirror / Atom feed
From: Duy Nguyen <pclouds@gmail.com>
To: David Turner <dturner@twopensource.com>
Cc: git@vger.kernel.org, Anatole Shaw <git-devel@omni.poc.net>,
	Junio C Hamano <gitster@pobox.com>
Subject: Re: [PATCH v2] unpack-trees: don't update files with CE_WT_REMOVE set
Date: Sat, 18 Jul 2015 15:37:19 +0700	[thread overview]
Message-ID: <20150718083719.GA19676@lanh> (raw)
In-Reply-To: <1437167967-5933-1-git-send-email-dturner@twopensource.com>

On Fri, Jul 17, 2015 at 05:19:27PM -0400, David Turner wrote:
> Don't update files in the worktree from cache entries which are
> flagged with CE_WT_REMOVE.
> 
> When a user does a sparse checkout, git removes files that are marked
> with CE_WT_REMOVE (because they are out-of-scope for the sparse
> checkout). If those files are also marked CE_UPDATE (for instance,
> because they differ in the branch that is being checked out and the
> outgoing branch), git would previously recreate them.  This patch
> prevents them from being recreated.
> 
> These erroneously-created files would also interfere with merges,
> causing pre-merge revisions of out-of-scope files to appear in the
> worktree.

Thank you both for catching this. Just a small suggestion. Perhaps we
should do this instead. apply_sparse_checkout() is the function where
all "action" manipulation (add, delete, update files..) for sparse
checkout occurs and it should not ask to delete and update both at the
same time.

-- 8< --
diff --git a/unpack-trees.c b/unpack-trees.c
index 2927660..d6cf849 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -224,6 +224,9 @@ static int check_updates(struct unpack_trees_options *o)
 		struct cache_entry *ce = index->cache[i];
 
 		if (ce->ce_flags & CE_UPDATE) {
+			if (ce->ce_flags & CE_WT_REMOVE)
+				die("BUG: both update and delete flags are set on %s",
+				    ce->name);
 			display_progress(progress, ++cnt);
 			ce->ce_flags &= ~CE_UPDATE;
 			if (o->update && !o->dry_run) {
@@ -293,6 +296,7 @@ static int apply_sparse_checkout(struct index_state *istate,
 		if (!(ce->ce_flags & CE_UPDATE) && verify_uptodate_sparse(ce, o))
 			return -1;
 		ce->ce_flags |= CE_WT_REMOVE;
+		ce->ce_flags &= ~CE_UPDATE;
 	}
 	if (was_skip_worktree && !ce_skip_worktree(ce)) {
 		if (verify_absent_sparse(ce, ERROR_WOULD_LOSE_UNTRACKED_OVERWRITTEN, o))
-- 8< --

--
Duy

  parent reply	other threads:[~2015-07-18  8:36 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-07-17 19:48 [PATCH] unpack-trees: don't update files flagged for deletion David Turner
2015-07-17 20:05 ` Junio C Hamano
2015-07-17 21:19   ` [PATCH v2] unpack-trees: don't update files with CE_WT_REMOVE set David Turner
2015-07-17 21:34     ` Junio C Hamano
2015-07-18  8:37     ` Duy Nguyen [this message]
2015-07-18 21:20       ` Junio C Hamano
2015-07-21 20:52         ` Junio C Hamano
2015-07-20 17:40       ` David Turner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150718083719.GA19676@lanh \
    --to=pclouds@gmail.com \
    --cc=dturner@twopensource.com \
    --cc=git-devel@omni.poc.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.