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>

1 files changed, 15 insertions, 16 deletions

diff --git a/unpack-trees.c b/unpack-trees.c
index dec044339d..b43f3e775a 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -1701,23 +1701,15 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
 			 * correct CE_NEW_SKIP_WORKTREE
 			 */
 			if (ce->ce_flags & CE_ADDED &&
-			    verify_absent(ce, ERROR_WOULD_LOSE_UNTRACKED_OVERWRITTEN, o)) {
-				if (!o->show_all_errors)
-					goto return_failed;
-				ret = -1;
-			}
+			    verify_absent(ce, WARNING_SPARSE_ORPHANED_NOT_OVERWRITTEN, o))
+				ret = 1;
+
+			if (apply_sparse_checkout(&o->result, ce, o))
+				ret = 1;
 
-			if (apply_sparse_checkout(&o->result, ce, o)) {
-				if (!o->show_all_errors)
-					goto return_failed;
-				ret = -1;
-			}
 			if (!ce_skip_worktree(ce))
 				empty_worktree = 0;
-
 		}
-		if (ret < 0)
-			goto return_failed;
 		/*
 		 * Sparse checkout is meant to narrow down checkout area
 		 * 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
 			ret = unpack_failed(o, "Sparse checkout leaves no entry on working directory");
 			goto done;
 		}
+		if (ret == 1) {
+			/*
+			 * Inability to sparsify or de-sparsify individual
+			 * paths is not an error, but just a warning.
+			 */
+			if (o->show_all_errors)
+				display_warning_msgs(o);
+			ret = 0;
+		}
 	}
 
 	ret = check_updates(o, &o->result) ? (-2) : 0;
@@ -1759,10 +1760,8 @@ done:
 	return ret;
 
 return_failed:
-	if (o->show_all_errors) {
+	if (o->show_all_errors)
 		display_error_msgs(o);
-		display_warning_msgs(o);
-	}
 	mark_all_ce_unused(o->src_index);
 	ret = unpack_failed(o, NULL);
 	if (o->exiting_early)