@matheusbsbs / matheus-bagliano / commits / 700e66d661

Fork 0

unpack-trees: let read-tree -u remove index entries outside sparse area

To avoid touching the worktree outside a sparse checkout, when the update flag is enabled unpack_trees() clears the CE_UPDATE and CE_REMOVE flags on entries that do not match the sparse pattern before actually committing any updates to the index file or worktree. The effect on the index was unintentional; sparse checkout was never meant to prevent index updates outside the area checked out. And the result is very confusing: for example, after a failed merge, currently "git reset --hard" does not reset the state completely but an additional "git reset --mixed" will. So stop clearing the CE_REMOVE flag. Instead, maintain a CE_WT_REMOVE flag to separately track whether a particular file removal should apply to the worktree in addition to the index or not. The CE_WT_REMOVE flag is used already to mark files that should be removed because of a narrowing checkout area. That usage will still apply; do not clear the CE_WT_REMOVE flag in that case (detectable because the CE_REMOVE flag is not set). This bug masked some other bugs illustrated by the test suite, which will be addressed by later patches. Reported-by: Frédéric Brière <fbriere@fbriere.net> Fixes: http://bugs.debian.org/583699 Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Nguyễn Thái Ngọc Duy committed Jul 31, 2010 at 13:14 UTC 700e66d66100fff952ffca439834a6deb1062d11

3 files changed +30 -13

cache.h

+1 -2

@@ -179,8 +179,7 @@ struct cache_entry {

179 #define CE_UNHASHED (0x200000)

180 #define CE_CONFLICTED (0x800000)

181

182 -/* Only remove in work directory, not index */

183 -#define CE_WT_REMOVE (0x400000)

182 +#define CE_WT_REMOVE (0x400000) /* remove in work directory */

183

184 #define CE_UNPACKED (0x1000000)

185

t/t1011-read-tree-sparse-checkout.sh

+10 -1

@@ -146,7 +146,7 @@ test_expect_success 'read-tree adds to worktree, absent case' '

146 test ! -f sub/added

147 '

148

149 -test_expect_success 'read-tree adds to worktree, dirty case' '

149 +test_expect_failure 'read-tree adds to worktree, dirty case' '

150 echo init.t >.git/info/sparse-checkout &&

151 git checkout -f removed &&

152 mkdir sub &&

@@ -167,4 +167,13 @@ test_expect_success 'index removal and worktree narrowing at the same time' '

167 test_cmp empty result

168 '

169

170 +test_expect_success 'read-tree --reset removes outside worktree' '

171 + >empty &&

172 + echo init.t >.git/info/sparse-checkout &&

173 + git checkout -f top &&

174 + git reset --hard removed &&

175 + git ls-files sub/added >result &&

176 + test_cmp empty result

177 +'

178 +

179 test_done

unpack-trees.c

+19 -10

@@ -53,6 +53,9 @@ static void add_entry(struct unpack_trees_options *o, struct cache_entry *ce,

54 clear |= CE_HASHED | CE_UNHASHED;

56 + if (set & CE_REMOVE)

57 + set |= CE_WT_REMOVE;

58 +

59 memcpy(new, ce, size);

60 new->next = NULL;

61 new->ce_flags = (new->ce_flags & ~clear) | set;

@@ -92,7 +95,7 @@ static int check_updates(struct unpack_trees_options *o)

95 if (o->update && o->verbose_update) {

96 for (total = cnt = 0; cnt < index->cache_nr; cnt++) {

97 struct cache_entry *ce = index->cache[cnt];

95 - if (ce->ce_flags & (CE_UPDATE | CE_REMOVE | CE_WT_REMOVE))

98 + if (ce->ce_flags & (CE_UPDATE | CE_WT_REMOVE))

99 total++;

100 }

101

@@ -112,12 +115,6 @@ static int check_updates(struct unpack_trees_options *o)

115 unlink_entry(ce);

116 continue;

117 }

115 -

116 - if (ce->ce_flags & CE_REMOVE) {

117 - display_progress(progress, ++cnt);

118 - if (o->update)

119 - unlink_entry(ce);

120 - }

118 }

119 remove_marked_cache_entries(&o->result);

120 remove_scheduled_dirs();

@@ -172,10 +169,22 @@ static int apply_sparse_checkout(struct cache_entry *ce, struct unpack_trees_opt

169 /*

170 * Merge strategies may set CE_UPDATE|CE_REMOVE outside checkout

171 * area as a result of ce_skip_worktree() shortcuts in

175 - * verify_absent() and verify_uptodate(). Clear them.

172 + * verify_absent() and verify_uptodate().

173 + * Make sure they don't modify worktree if they are already

174 + * outside checkout area

175 */

177 - if (was_skip_worktree && ce_skip_worktree(ce))

178 - ce->ce_flags &= ~(CE_UPDATE | CE_REMOVE);

176 + if (was_skip_worktree && ce_skip_worktree(ce)) {

177 + ce->ce_flags &= ~CE_UPDATE;

178 +

179 + /*

180 + * By default, when CE_REMOVE is on, CE_WT_REMOVE is also

181 + * on to get that file removed from both index and worktree.

182 + * If that file is already outside worktree area, don't

183 + * bother remove it.

184 + */

185 + if (ce->ce_flags & CE_REMOVE)

186 + ce->ce_flags &= ~CE_WT_REMOVE;

187 + }

188

189 if (!was_skip_worktree && ce_skip_worktree(ce)) {

190 /*