Subject: [RFC PATCH] unpack-trees: add check_worktree flag to enable dry-run functionality

From: Jens Lehmann <[email-masked]>
Date: Thu, 19 May 2011 19:47:27 +0200

To enable the users of unpack_trees() to check if a work tree update would
succeed without actually touching the work tree the new check_worktree
flag is added to struct unpack_trees_options. This is necessary because
when update is set to 0 testing for absent new files and sparse handling
were disabled. This is fine when e.g. only the index should be updated but
is not sufficient when the user wants to check if a work tree update would
succeed.

Now users can set update to 0 and check_worktree to 1 to see if an
unpack_tree() with update=1 run would succeed. The only class of problems
that are not detected at the moment are file system conditions like ENOSPC
or missing permissions.

Signed-off-by: Jens Lehmann <[email-masked]>
---
Am 18.05.2011 00:47, schrieb Junio C Hamano:
> Jens Lehmann <[email-masked]> writes:
>> The only exceptions where I could not make that replacement without
>> breaking a test were:
>>
>> * t1001:381 (#25) new file conflicts with untracked directory
>>   This should have been detected by verify_absent_1(), but that function
>>   is left early when opts->update is 0. Will look into that.
> 
> Thanks. This is the kind of corner case I was worried about when I said
> "we might have a case where read-tree -m succeeds but read-tree -m -u
> doesn't". Fixing it would mean that ...

Maybe something like this patch?

>> @@ -183,6 +184,8 @@ int cmd_read_tree(int argc, const char **argv, const char *unused_prefix)
>>  		die("--exclude-per-directory is meaningless unless -u");
>>  	if (opts.merge && !opts.index_only)
>>  		setup_work_tree();
>> +	if (opts.update && dry_run)
>> +		opts.update = 0;
>>
>>  	if (opts.merge) {
>>  		if (stage < 2)
> 
> ... this hunk must go, right?

But this is the "don't update the work tree when -n is used together
with -u" part, so it is needed, no? With this patch applied first and
opts.check_worktree set to 1 inside that if() added there all tests
succeed.

> Now at this point wouldn't it make more sense to move these helpers to
> lib-read-tree.sh or something and .-source it from all nine tests?

Sure, will do in the next round.

 unpack-trees.c |    4 ++--
 unpack-trees.h |    3 ++-
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/unpack-trees.c b/unpack-trees.c
index 500ebcf..b6dca8f 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -1001,7 +1001,7 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
 	state.refresh_cache = 1;

 	memset(&el, 0, sizeof(el));
-	if (!core_apply_sparse_checkout || !o->update)
+	if (!core_apply_sparse_checkout || (!o->update && !o->check_worktree))
 		o->skip_sparse_checkout = 1;
 	if (!o->skip_sparse_checkout) {
 		if (add_excludes_from_file_to_list(git_path("info/sparse-checkout"), "", 0, NULL, &el, 0) < 0)
@@ -1368,7 +1368,7 @@ static int verify_absent_1(struct cache_entry *ce,
 	int len;
 	struct stat st;

-	if (o->index_only || o->reset || !o->update)
+	if (o->index_only || o->reset || (!o->update && !o->check_worktree))
 		return 0;

 	len = check_leading_path(ce->name, ce_namelen(ce));
diff --git a/unpack-trees.h b/unpack-trees.h
index cd11a08..402bbdb 100644
--- a/unpack-trees.h
+++ b/unpack-trees.h
@@ -46,7 +46,8 @@ struct unpack_trees_options {
 		     debug_unpack,
 		     skip_sparse_checkout,
 		     gently,
-		     show_all_errors;
+		     show_all_errors,
+		     check_worktree; /* check work tree contents even if !update */
 	const char *prefix;
 	int cache_bottom;
 	struct dir_struct *dir;
-- 
1.7.5.1.340.g5ecaf