[RFC PATCH] unpack-trees: add check_worktree flag to enable dry-run functionality
- From
Jens Lehmann <jens.lehmann@web.de>
- Date
- May 19, 2011, 17:47 UTC
- Message-ID
- <4DD557AF.4030700@web.de>
- In-Reply-To
- <7vpqnheygi.fsf@alter.siamese.dyndns.org>
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 <Jens.Lehmann@web.de> --- Am 18.05.2011 00:47, schrieb Junio C Hamano:
Show 11 quoted lines
> Jens Lehmann <Jens.Lehmann@web.de> 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?
Show 11 quoted lines
>> @@ -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