git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v3 04/11] unpack-trees: introduce preserve_ignored to unpack_trees_options

From
Elijah Newren via GitGitGadget <gitgitgadget@gmail.com>
Date
Sep 27, 2021, 16:33 UTC
Message-ID
<f1a0700e598e52d6cdb507fe8a09c4fa9291c982.1632760428.git.gitgitgadget@gmail.com>
In-Reply-To
<pull.1036.v3.git.1632760428.gitgitgadget@gmail.com>
From: Elijah Newren <newren@gmail.com>
Currently, every caller of unpack_trees() that wants to ensure ignored
files are overwritten by default needs to:
   * allocate unpack_trees_options.dir
   * flip the DIR_SHOW_IGNORED flag in unpack_trees_options.dir->flags
   * call setup_standard_excludes
AND then after the call to unpack_trees() needs to
   * call dir_clear()
   * deallocate unpack_trees_options.dir
That's a fair amount of boilerplate, and every caller uses identical
code.  Make this easier by instead introducing a new boolean value where
the default value (0) does what we want so that new callers of
unpack_trees() automatically get the appropriate behavior.  And move all
the handling of unpack_trees_options.dir into unpack_trees() itself.

While preserve_ignored = 0 is the behavior we feel is the appropriate default, we defer fixing commands to use the appropriate default until a later commit. So, this commit introduces several locations where we manually set preserve_ignored=1. This makes it clear where code paths were previously preserving ignored files when they should not have been; a future commit will flip these to instead use a value of 0 to get the behavior we want.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 builtin/am.c        |  3 +++
 builtin/checkout.c  | 11 ++---------
 builtin/clone.c     |  2 ++
 builtin/merge.c     |  2 ++
 builtin/read-tree.c | 13 +++----------
 builtin/reset.c     |  2 ++
 builtin/stash.c     |  3 +++
 merge-ort.c         |  8 +-------
 merge-recursive.c   |  8 +-------
 merge.c             |  8 +-------
 reset.c             |  2 ++
 sequencer.c         |  2 ++
 unpack-trees.c      | 10 ++++++++++
 unpack-trees.h      |  1 +
 14 files changed, 35 insertions(+), 40 deletions(-)
diff --git a/builtin/am.c b/builtin/am.c
index e4a0ff9cd7c..1ee70692bc3 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -1918,6 +1918,9 @@ static int fast_forward_to(struct tree *head, struct tree *remote, int reset)
 	opts.update = 1;
 	opts.merge = 1;
 	opts.reset = reset;
+	if (!reset)
+		/* FIXME: Default should be to remove ignored files */
+		opts.preserve_ignored = 1;
 	opts.fn = twoway_merge;
 	init_tree_desc(&t[0], head->buffer, head->size);
 	init_tree_desc(&t[1], remote->buffer, remote->size);
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 5335435d616..5e7957dd068 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -648,6 +648,7 @@ static int reset_tree(struct tree *tree, const struct checkout_opts *o,
 	opts.skip_unmerged = !worktree;
 	opts.reset = 1;
 	opts.merge = 1;
+	opts.preserve_ignored = 0;
 	opts.fn = oneway_merge;
 	opts.verbose_update = o->show_progress;
 	opts.src_index = &the_index;
@@ -746,11 +747,7 @@ static int merge_working_tree(const struct checkout_opts *opts,
 				       new_branch_info->commit ?
 				       &new_branch_info->commit->object.oid :
 				       &new_branch_info->oid, NULL);
-		if (opts->overwrite_ignore) {
-			topts.dir = xcalloc(1, sizeof(*topts.dir));
-			topts.dir->flags |= DIR_SHOW_IGNORED;
-			setup_standard_excludes(topts.dir);
-		}
+		topts.preserve_ignored = !opts->overwrite_ignore;
 		tree = parse_tree_indirect(old_branch_info->commit ?
 					   &old_branch_info->commit->object.oid :
 					   the_hash_algo->empty_tree);
@@ -760,10 +757,6 @@ static int merge_working_tree(const struct checkout_opts *opts,
 		init_tree_desc(&trees[1], tree->buffer, tree->size);
 
 		ret = unpack_trees(2, trees, &topts);
-		if (topts.dir) {
-			dir_clear(topts.dir);
-			FREE_AND_NULL(topts.dir);
-		}
 		clear_unpack_trees_porcelain(&topts);
 		if (ret == -1) {
 			/*
diff --git a/builtin/clone.c b/builtin/clone.c
index ff1d3d447a3..be1c3840d62 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -687,6 +687,8 @@ static int checkout(int submodule_progress)
 	opts.update = 1;
 	opts.merge = 1;
 	opts.clone = 1;
+	/* FIXME: Default should be to remove ignored files */
+	opts.preserve_ignored = 1;
 	opts.fn = oneway_merge;
 	opts.verbose_update = (option_verbosity >= 0);
 	opts.src_index = &the_index;
diff --git a/builtin/merge.c b/builtin/merge.c
index 3fbdacc7db4..1e5fff095fc 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -680,6 +680,8 @@ static int read_tree_trivial(struct object_id *common, struct object_id *head,
 	opts.verbose_update = 1;
 	opts.trivial_merges_only = 1;
 	opts.merge = 1;
+	/* FIXME: Default should be to remove ignored files */
+	opts.preserve_ignored = 1;
 	trees[nr_trees] = parse_tree_indirect(common);
 	if (!trees[nr_trees++])
 		return -1;
diff --git a/builtin/read-tree.c b/builtin/read-tree.c
index 73cb957a69b..443d206eca6 100644
--- a/builtin/read-tree.c
+++ b/builtin/read-tree.c
@@ -201,11 +201,9 @@ int cmd_read_tree(int argc, const char **argv, const char *cmd_prefix)
 	if ((opts.update || opts.index_only) && !opts.merge)
 		die("%s is meaningless without -m, --reset, or --prefix",
 		    opts.update ? "-u" : "-i");
-	if (opts.update && !opts.reset) {
-		CALLOC_ARRAY(opts.dir, 1);
-		opts.dir->flags |= DIR_SHOW_IGNORED;
-		setup_standard_excludes(opts.dir);
-	}
+	if (opts.update && !opts.reset)
+		opts.preserve_ignored = 0;
+	/* otherwise, opts.preserve_ignored is irrelevant */
 	if (opts.merge && !opts.index_only)
 		setup_work_tree();
 
@@ -245,11 +243,6 @@ int cmd_read_tree(int argc, const char **argv, const char *cmd_prefix)
 	if (unpack_trees(nr_trees, t, &opts))
 		return 128;
 
-	if (opts.dir) {
-		dir_clear(opts.dir);
-		FREE_AND_NULL(opts.dir);
-	}
-
 	if (opts.debug_unpack || opts.dry_run)
 		return 0; /* do not write the index out */
 
diff --git a/builtin/reset.c b/builtin/reset.c
index 51c9e2f43ff..7f38656f018 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -67,6 +67,8 @@ static int reset_index(const char *ref, const struct object_id *oid, int reset_t
 	case KEEP:
 	case MERGE:
 		opts.update = 1;
+		/* FIXME: Default should be to remove ignored files */
+		opts.preserve_ignored = 1;
 		break;
 	case HARD:
 		opts.update = 1;
diff --git a/builtin/stash.c b/builtin/stash.c
index 8f42360ca91..88287b890d5 100644
--- a/builtin/stash.c
+++ b/builtin/stash.c
@@ -258,6 +258,9 @@ static int reset_tree(struct object_id *i_tree, int update, int reset)
 	opts.merge = 1;
 	opts.reset = reset;
 	opts.update = update;
+	if (update && !reset)
+		/* FIXME: Default should be to remove ignored files */
+		opts.preserve_ignored = 1;
 	opts.fn = oneway_merge;
 
 	if (unpack_trees(nr_trees, t, &opts))
diff --git a/merge-ort.c b/merge-ort.c
index 35aa979c3a4..0d64ec716bd 100644
--- a/merge-ort.c
+++ b/merge-ort.c
@@ -4045,11 +4045,7 @@ static int checkout(struct merge_options *opt,
 	unpack_opts.quiet = 0; /* FIXME: sequencer might want quiet? */
 	unpack_opts.verbose_update = (opt->verbosity > 2);
 	unpack_opts.fn = twoway_merge;
-	if (1/* FIXME: opts->overwrite_ignore*/) {
-		CALLOC_ARRAY(unpack_opts.dir, 1);
-		unpack_opts.dir->flags |= DIR_SHOW_IGNORED;
-		setup_standard_excludes(unpack_opts.dir);
-	}
+	unpack_opts.preserve_ignored = 0; /* FIXME: !opts->overwrite_ignore*/
 	parse_tree(prev);
 	init_tree_desc(&trees[0], prev->buffer, prev->size);
 	parse_tree(next);
@@ -4057,8 +4053,6 @@ static int checkout(struct merge_options *opt,
 
 	ret = unpack_trees(2, trees, &unpack_opts);
 	clear_unpack_trees_porcelain(&unpack_opts);
-	dir_clear(unpack_opts.dir);
-	FREE_AND_NULL(unpack_opts.dir);
 	return ret;
 }
 
diff --git a/merge-recursive.c b/merge-recursive.c
index 233d9f686ad..2be3f5d4044 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -411,9 +411,7 @@ static int unpack_trees_start(struct merge_options *opt,
 	else {
 		opt->priv->unpack_opts.update = 1;
 		/* FIXME: should only do this if !overwrite_ignore */
-		CALLOC_ARRAY(opt->priv->unpack_opts.dir, 1);
-		opt->priv->unpack_opts.dir->flags |= DIR_SHOW_IGNORED;
-		setup_standard_excludes(opt->priv->unpack_opts.dir);
+		opt->priv->unpack_opts.preserve_ignored = 0;
 	}
 	opt->priv->unpack_opts.merge = 1;
 	opt->priv->unpack_opts.head_idx = 2;
@@ -428,10 +426,6 @@ static int unpack_trees_start(struct merge_options *opt,
 	init_tree_desc_from_tree(t+2, merge);
 
 	rc = unpack_trees(3, t, &opt->priv->unpack_opts);
-	if (opt->priv->unpack_opts.dir) {
-		dir_clear(opt->priv->unpack_opts.dir);
-		FREE_AND_NULL(opt->priv->unpack_opts.dir);
-	}
 	cache_tree_free(&opt->repo->index->cache_tree);
 
 	/*
diff --git a/merge.c b/merge.c
index 6e736881d90..2382ff66d35 100644
--- a/merge.c
+++ b/merge.c
@@ -53,7 +53,6 @@ int checkout_fast_forward(struct repository *r,
 	struct unpack_trees_options opts;
 	struct tree_desc t[MAX_UNPACK_TREES];
 	int i, nr_trees = 0;
-	struct dir_struct dir = DIR_INIT;
 	struct lock_file lock_file = LOCK_INIT;
 
 	refresh_index(r->index, REFRESH_QUIET, NULL, NULL, NULL);
@@ -80,11 +79,7 @@ int checkout_fast_forward(struct repository *r,
 	}
 
 	memset(&opts, 0, sizeof(opts));
-	if (overwrite_ignore) {
-		dir.flags |= DIR_SHOW_IGNORED;
-		setup_standard_excludes(&dir);
-		opts.dir = &dir;
-	}
+	opts.preserve_ignored = !overwrite_ignore;
 
 	opts.head_idx = 1;
 	opts.src_index = r->index;
@@ -101,7 +96,6 @@ int checkout_fast_forward(struct repository *r,
 		clear_unpack_trees_porcelain(&opts);
 		return -1;
 	}
-	dir_clear(&dir);
 	clear_unpack_trees_porcelain(&opts);
 
 	if (write_locked_index(r->index, &lock_file, COMMIT_LOCK))
diff --git a/reset.c b/reset.c
index 79310ae071b..41b3e2d88de 100644
--- a/reset.c
+++ b/reset.c
@@ -56,6 +56,8 @@ int reset_head(struct repository *r, struct object_id *oid, const char *action,
 	unpack_tree_opts.fn = reset_hard ? oneway_merge : twoway_merge;
 	unpack_tree_opts.update = 1;
 	unpack_tree_opts.merge = 1;
+	/* FIXME: Default should be to remove ignored files */
+	unpack_tree_opts.preserve_ignored = 1;
 	init_checkout_metadata(&unpack_tree_opts.meta, switch_to_branch, oid, NULL);
 	if (!detach_head)
 		unpack_tree_opts.reset = 1;
diff --git a/sequencer.c b/sequencer.c
index 614d56f5e21..098566c68d9 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -3699,6 +3699,8 @@ static int do_reset(struct repository *r,
 	unpack_tree_opts.fn = oneway_merge;
 	unpack_tree_opts.merge = 1;
 	unpack_tree_opts.update = 1;
+	/* FIXME: Default should be to remove ignored files */
+	unpack_tree_opts.preserve_ignored = 1;
 	init_checkout_metadata(&unpack_tree_opts.meta, name, &oid, NULL);
 
 	if (repo_read_index_unmerged(r)) {
diff --git a/unpack-trees.c b/unpack-trees.c
index 8ea0a542da8..1e4eae1dc7d 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -1707,6 +1707,12 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
 		ensure_full_index(o->dst_index);
 	}
 
+	if (!o->preserve_ignored) {
+		CALLOC_ARRAY(o->dir, 1);
+		o->dir->flags |= DIR_SHOW_IGNORED;
+		setup_standard_excludes(o->dir);
+	}
+
 	if (!core_apply_sparse_checkout || !o->update)
 		o->skip_sparse_checkout = 1;
 	if (!o->skip_sparse_checkout && !o->pl) {
@@ -1868,6 +1874,10 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
 done:
 	if (free_pattern_list)
 		clear_pattern_list(&pl);
+	if (o->dir) {
+		dir_clear(o->dir);
+		FREE_AND_NULL(o->dir);
+	}
 	trace2_region_leave("unpack_trees", "unpack_trees", the_repository);
 	trace_performance_leave("unpack_trees");
 	return ret;
diff --git a/unpack-trees.h b/unpack-trees.h
index 2d88b19dca7..f98cfd49d7b 100644
--- a/unpack-trees.h
+++ b/unpack-trees.h
@@ -49,6 +49,7 @@ struct unpack_trees_options {
 	unsigned int reset,
 		     merge,
 		     update,
+		     preserve_ignored,
 		     clone,
 		     index_only,
 		     nontrivial_merge,

-- 
gitgitgadget
Previous: Elijah Newren via GitGitGadgetNext: Ævar Arnfjörð Bjarmason
Message 42 of 82 in “Fix various issues around removal of untracked files/directories”
  1. 0/6 Fix various issues around removal of untracked files/directoriesElijah Newren via GitGitGadget, Sep 18, 2021
  2. 1/6 t2500: add various tests for nuking untracked filesElijah Newren via GitGitGadget, Sep 18, 2021
  3. Ævar Arnfjörð BjarmasonSep 19, 2021
  4. Elijah NewrenSep 20, 2021
  5. 4/6 unpack-trees: avoid nuking untracked dir in way of locally deleted fileElijah Newren via GitGitGadget, Sep 18, 2021
  6. Ævar Arnfjörð BjarmasonSep 19, 2021
  7. Elijah NewrenSep 20, 2021
  8. 3/6 unpack-trees: avoid nuking untracked dir in way of unmerged fileElijah Newren via GitGitGadget, Sep 18, 2021
  9. 2/6 Split unpack_trees 'reset' flag into two for untracked handlingElijah Newren via GitGitGadget, Sep 18, 2021
  10. Ævar Arnfjörð BjarmasonSep 19, 2021
  11. Elijah NewrenSep 20, 2021
  12. Phillip WoodSep 20, 2021
  13. Elijah NewrenSep 20, 2021
  14. Phillip WoodSep 20, 2021
  15. Elijah NewrenSep 24, 2021
  16. 6/6 Documentation: call out commands that nuke untracked files/directoriesElijah Newren via GitGitGadget, Sep 18, 2021
  17. Philip OakleySep 19, 2021
  18. Philip OakleySep 19, 2021
  19. Elijah NewrenSep 20, 2021
  20. 5/6 Comment important codepaths regarding nuking untracked files/dirsElijah Newren via GitGitGadget, Sep 18, 2021
  21. Luke DiamandSep 24, 2021
  22. Elijah NewrenSep 24, 2021
  23. 0/6 Fix various issues around removal of untracked files/directoriesElijah Newren via GitGitGadget, Sep 24, 2021
  24. 2/6 Change unpack_trees' 'reset' flag into an enumElijah Newren via GitGitGadget, Sep 24, 2021
  25. Junio C HamanoSep 24, 2021
  26. Elijah NewrenSep 26, 2021
  27. 1/6 t2500: add various tests for nuking untracked filesElijah Newren via GitGitGadget, Sep 24, 2021
  28. 3/6 unpack-trees: avoid nuking untracked dir in way of unmerged fileElijah Newren via GitGitGadget, Sep 24, 2021
  29. 4/6 unpack-trees: avoid nuking untracked dir in way of locally deleted fileElijah Newren via GitGitGadget, Sep 24, 2021
  30. 5/6 Comment important codepaths regarding nuking untracked files/dirsElijah Newren via GitGitGadget, Sep 24, 2021
  31. Eric SunshineSep 24, 2021
  32. Elijah NewrenSep 26, 2021
  33. 6/6 Documentation: call out commands that nuke untracked files/directoriesElijah Newren via GitGitGadget, Sep 24, 2021
  34. 00/11 Fix various issues around removal of untracked files/directoriesElijah Newren via GitGitGadget, Sep 27, 2021
  35. 02/11 checkout, read-tree: fix leak of unpack_trees_options.dirElijah Newren via GitGitGadget, Sep 27, 2021
  36. 03/11 read-tree, merge-recursive: overwrite ignored files by defaultElijah Newren via GitGitGadget, Sep 27, 2021
  37. Jack O'ConnorDec 13, 2021
  38. Elijah NewrenDec 13, 2021
  39. 01/11 t2500: add various tests for nuking untracked filesElijah Newren via GitGitGadget, Sep 27, 2021
  40. 06/11 Remove ignored files by default when they are in the wayElijah Newren via GitGitGadget, Sep 27, 2021
  41. 05/11 unpack-trees: make dir an internal-only structElijah Newren via GitGitGadget, Sep 27, 2021
  42. 04/11 unpack-trees: introduce preserve_ignored to unpack_trees_optionsElijah Newren via GitGitGadget, Sep 27, 2021
  43. Ævar Arnfjörð BjarmasonSep 29, 2021
  44. Elijah NewrenSep 29, 2021
  45. Ævar Arnfjörð BjarmasonSep 29, 2021
  46. Elijah NewrenSep 30, 2021
  47. Ævar Arnfjörð BjarmasonSep 30, 2021
  48. Elijah NewrenOct 1, 2021
  49. Ævar Arnfjörð BjarmasonOct 1, 2021
  50. Ævar Arnfjörð BjarmasonOct 1, 2021
  51. Elijah NewrenOct 1, 2021
  52. Ævar Arnfjörð BjarmasonOct 2, 2021
  53. Ævar Arnfjörð BjarmasonOct 3, 2021
  54. Elijah NewrenOct 4, 2021
  55. Elijah NewrenOct 4, 2021
  56. Ævar Arnfjörð BjarmasonOct 4, 2021
  57. Elijah NewrenOct 4, 2021
  58. 07/11 Change unpack_trees' 'reset' flag into an enumElijah Newren via GitGitGadget, Sep 27, 2021
  59. 08/11 unpack-trees: avoid nuking untracked dir in way of unmerged fileElijah Newren via GitGitGadget, Sep 27, 2021
  60. 09/11 unpack-trees: avoid nuking untracked dir in way of locally deleted fileElijah Newren via GitGitGadget, Sep 27, 2021
  61. 10/11 Comment important codepaths regarding nuking untracked files/dirsElijah Newren via GitGitGadget, Sep 27, 2021
  62. 11/11 Documentation: call out commands that nuke untracked files/directoriesElijah Newren via GitGitGadget, Sep 27, 2021
  63. Junio C HamanoSep 27, 2021
  64. Elijah NewrenSep 27, 2021
  65. Elijah NewrenSep 27, 2021
  66. Phillip WoodSep 30, 2021
  67. 00/10 Fix various issues around removal of untracked files/directoriesÆvar Arnfjörð Bjarmason, Oct 4, 2021
  68. 01/10 t2500: add various tests for nuking untracked filesÆvar Arnfjörð Bjarmason, Oct 4, 2021
  69. 02/10 read-tree, merge-recursive: overwrite ignored files by defaultÆvar Arnfjörð Bjarmason, Oct 4, 2021
  70. 03/10 unpack-trees: introduce preserve_ignored to unpack_trees_optionsÆvar Arnfjörð Bjarmason, Oct 4, 2021
  71. 04/10 unpack-trees: rename "dir" to "private_dir"Ævar Arnfjörð Bjarmason, Oct 4, 2021
  72. 05/10 Remove ignored files by default when they are in the wayÆvar Arnfjörð Bjarmason, Oct 4, 2021
  73. 06/10 Change unpack_trees' 'reset' flag into an enumÆvar Arnfjörð Bjarmason, Oct 4, 2021
  74. 07/10 unpack-trees: avoid nuking untracked dir in way of unmerged fileÆvar Arnfjörð Bjarmason, Oct 4, 2021
  75. 08/10 unpack-trees: avoid nuking untracked dir in way of locally deleted fileÆvar Arnfjörð Bjarmason, Oct 4, 2021
  76. 09/10 Comment important codepaths regarding nuking untracked files/dirsÆvar Arnfjörð Bjarmason, Oct 4, 2021
  77. 10/10 Documentation: call out commands that nuke untracked files/directoriesÆvar Arnfjörð Bjarmason, Oct 4, 2021
  78. Elijah NewrenOct 4, 2021
  79. Ævar Arnfjörð BjarmasonOct 4, 2021
  80. Elijah NewrenOct 5, 2021
  81. Junio C HamanoOct 4, 2021
  82. Elijah NewrenOct 1, 2021

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.