@matheusbsbs / matheus-bagliano / commits / 681c637b4a

Fork 0

unpack-trees: failure to set SKIP_WORKTREE bits always just a warning

Setting and clearing of the SKIP_WORKTREE bit is not only done when users run 'sparse-checkout'; other commands such as 'checkout' also run through unpack_trees() which has logic for handling this special bit. As such, we need to consider how they handle special cases. A couple comparison points should help explain the rationale for changing how unpack_trees() handles these bits: Ignoring sparse checkouts for a moment, if you are switching branches and have dirty changes, it is only considered an error that will prevent the branch switching from being successful if the dirty file happens to be one of the paths with different contents. SKIP_WORKTREE has always been considered advisory; for example, if rebase or merge need or even want to materialize a path as part of their work, they have always been allowed to do so regardless of the SKIP_WORKTREE setting. This has been used for unmerged paths, but it was often used for paths it wasn't needed just because it made the code simpler. It was a best-effort consideration, and when it materialized paths contrary to the SKIP_WORKTREE setting, it was never required to even print a warning message. In the past if you trying to run e.g. 'git checkout' and: 1) you had a path that was materialized and had some dirty changes 2) the path was listed in $GITDIR/info/sparse-checkout 3) this path did not different between the current and target branches then despite the comparison points above, the inability to set SKIP_WORKTREE was treated as a *hard* error that would abort the checkout operation. This is completely inconsistent with how SKIP_WORKTREE is handled elsewhere, and rather annoying for users as leaving the paths materialized in the working copy (with a simple warning) should present no problem at all. Downgrade any errors from inability to toggle the SKIP_WORKTREE bit to a warning and allow the operations to continue. Reviewed-by: Derrick Stolee <dstolee@microsoft.com> Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Elijah Newren committed Mar 27, 2020 at 00:49 UTC 681c637b4ae1c46c09edda62a1aed6eaa69a2649

3 files changed +43 -21

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

+6 -5

@@ -233,18 +233,19 @@ test_expect_success 'read-tree --reset removes outside worktree' '

233 test_must_be_empty result

234 '

235

236 -test_expect_success 'print errors when failed to update worktree' '

236 +test_expect_success 'print warnings when some worktree updates disabled' '

237 echo sub >.git/info/sparse-checkout &&

238 git checkout -f init &&

239 mkdir sub &&

240 touch sub/added sub/addedtoo &&

241 - test_must_fail git checkout top 2>actual &&

241 + # Use -q to suppress "Previous HEAD position" and "Head is now at" msgs

242 + git checkout -q top 2>actual &&

243 cat >expected <<\EOF &&

243 -error: The following untracked working tree files would be overwritten by checkout:

244 +warning: The following paths were already present and thus not updated despite sparse patterns:

245 sub/added

246 sub/addedtoo

246 -Please move or remove them before you switch branches.

247 -Aborting

247 +

248 +After fixing the above paths, you may want to run `git sparse-checkout reapply`.

249 EOF

250 test_i18ncmp expected actual

251 '

t/t2018-checkout-branch.sh

+22

@@ -238,4 +238,26 @@ test_expect_success 'checkout -b after clone --no-checkout does a checkout of HE

238 test_path_is_file dest/a.t

239 '

240

241 +test_expect_success 'checkout -b to a new branch preserves mergeable changes despite sparse-checkout' '

242 + test_when_finished "

243 + git reset --hard &&

244 + git checkout branch1-scratch &&

245 + test_might_fail git branch -D branch3 &&

246 + git config core.sparseCheckout false &&

247 + rm .git/info/sparse-checkout" &&

248 +

249 + test_commit file2 &&

250 +

251 + echo stuff >>file1 &&

252 + echo file2 >.git/info/sparse-checkout &&

253 + git config core.sparseCheckout true &&

254 +

255 + CURHEAD=$(git rev-parse HEAD) &&

256 + do_checkout branch3 $CURHEAD &&

257 +

258 + echo file1 >expect &&

259 + git diff --name-only >actual &&

260 + test_cmp expect actual

261 +'

262 +

263 test_done

unpack-trees.c

+15 -16

@@ -1701,23 +1701,15 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options

1701 * correct CE_NEW_SKIP_WORKTREE

1702 */

1703 if (ce->ce_flags & CE_ADDED &&

1704 - verify_absent(ce, ERROR_WOULD_LOSE_UNTRACKED_OVERWRITTEN, o)) {

1705 - if (!o->show_all_errors)

1706 - goto return_failed;

1707 - ret = -1;

1708 - }

1704 + verify_absent(ce, WARNING_SPARSE_ORPHANED_NOT_OVERWRITTEN, o))

1705 + ret = 1;

1706 +

1707 + if (apply_sparse_checkout(&o->result, ce, o))

1708 + ret = 1;

1709

1710 - if (apply_sparse_checkout(&o->result, ce, o)) {

1711 - if (!o->show_all_errors)

1712 - goto return_failed;

1713 - ret = -1;

1714 - }

1710 if (!ce_skip_worktree(ce))

1711 empty_worktree = 0;

1717 -

1712 }

1719 - if (ret < 0)

1720 - goto return_failed;

1713 /*

1714 * Sparse checkout is meant to narrow down checkout area

1715 * but it does not make sense to narrow down to empty working

@@ -1728,6 +1720,15 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options

1720 ret = unpack_failed(o, "Sparse checkout leaves no entry on working directory");

1721 goto done;

1722 }

1723 + if (ret == 1) {

1724 + /*

1725 + * Inability to sparsify or de-sparsify individual

1726 + * paths is not an error, but just a warning.

1727 + */

1728 + if (o->show_all_errors)

1729 + display_warning_msgs(o);

1730 + ret = 0;

1731 + }

1732 }

1733

1734 ret = check_updates(o, &o->result) ? (-2) : 0;

@@ -1759,10 +1760,8 @@ done:

1760 return ret;

1761

1762 return_failed:

1762 - if (o->show_all_errors) {

1763 + if (o->show_all_errors)

1764 display_error_msgs(o);

1764 - display_warning_msgs(o);

1765 - }

1765 mark_all_ce_unused(o->src_index);

1766 ret = unpack_failed(o, NULL);

1767 if (o->exiting_early)