git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: git@vger.kernel.org
Cc: "Junio C Hamano" <gitster@pobox.com>,
	"Elijah Newren" <newren@gmail.com>,
	"Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>,
	"Martin Ågren" <martin.agren@gmail.com>,
	"Andrzej Hunt" <ajrhunt@google.com>, "Jeff King" <peff@peff.net>,
	"Fedor Biryukov" <fedor.birjukov@gmail.com>,
	"Philip Oakley" <philipoakley@iee.email>,
	"Phillip Wood" <phillip.wood123@gmail.com>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Subject: [RFC PATCH v4 00/10] Fix various issues around removal of untracked files/directories
Date: Mon,  4 Oct 2021 03:11:49 +0200	[thread overview]
Message-ID: <RFC-cover-v4-00.10-00000000000-20211004T004902Z-avarab@gmail.com> (raw)
In-Reply-To: <pull.1036.v3.git.1632760428.gitgitgadget@gmail.com>

This is an RFC proposed v4 of Elijah's en/removing-untracked-fixes
series[1] based on top of my memory leak fixes in the "unpack-trees" &
"dir" APIs[2].

As noted in [2] Elijah and I have been having a back & forth about the
approach his series takes to fixing memory leaks in those APIs. I
think submitting working code is more productive than continuing that
point-by-point discussion, so here we are.

I've avoided making any changes to this series except those narrowly
required to rebase it on top of mine, and to those parts of Elijah's
commit messages that became outdated as a result. In particular
3/10[3]'s is significantly changed, as much of its commit message
dicusses complexities that have gone away due to my preceding
series[2].

The "make dir an internal-only struct" has been replaced by a commit
that renames that struct member from "dir" to "private_dir". I think
even that is unnecessary as argued in [4], but I think the judgement
that something must be done to address that is Elijah's design
decision, so I did my best to retain it.

I did drop the dynamic allocation & it being a pointer, since with my
preceding [2] and subsequent unsubmitted memory leak fixes I've got on
top having it be embedded in "struct unpack_trees_options" makes
things easier to manage.

Havingn read through all this code quite thoroughly at this point I do
have other comments on it, but I'll reserve those until we've found
out what direction we're going forward with vis-a-vis what this will
be based on top of.

I'm (obviously) hoping for an answer of either on top of my series[2],
or alternatively that Elijah's series can stick to introducing the
"preserve_ignored" flag, but not change how the memory
management/name/type of the embedded "dir" happens (and we could thus
proceed in parallel).

But I'll hold off on any such general comments until we've got a way
forward with this, since if I start commenting inline on patches in
Elijah's v3, or this RFC-v4 on something unrelated to this proposed
re-arrangement that'll likely just confuse things, particularly as
some of those comments would be different depending on the base of his
v3 v.s. my series[2] in this RFC v4.

1. https://lore.kernel.org/git/pull.1036.v3.git.1632760428.gitgitgadget@gmail.com
2. https://lore.kernel.org/git/cover-00.10-00000000000-20211004T002226Z-avarab@gmail.com/
3. https://lore.kernel.org/git/RFC-patch-v4-03.10-739e9b871c4-20211004T004902Z-avarab@gmail.com
4. https://lore.kernel.org/git/87k0ivpzfx.fsf@evledraar.gmail.com

Elijah Newren (10):
  t2500: add various tests for nuking untracked files
  read-tree, merge-recursive: overwrite ignored files by default
  unpack-trees: introduce preserve_ignored to unpack_trees_options
  unpack-trees: rename "dir" to "private_dir"
  Remove ignored files by default when they are in the way
  Change unpack_trees' 'reset' flag into an enum
  unpack-trees: avoid nuking untracked dir in way of unmerged file
  unpack-trees: avoid nuking untracked dir in way of locally deleted
    file
  Comment important codepaths regarding nuking untracked files/dirs
  Documentation: call out commands that nuke untracked files/directories

 Documentation/git-checkout.txt   |   5 +-
 Documentation/git-read-tree.txt  |  23 +--
 Documentation/git-reset.txt      |   3 +-
 builtin/am.c                     |   3 +-
 builtin/checkout.c               |   9 +-
 builtin/clone.c                  |   1 +
 builtin/merge.c                  |   1 +
 builtin/read-tree.c              |  23 ++-
 builtin/reset.c                  |  10 +-
 builtin/stash.c                  |   5 +-
 builtin/submodule--helper.c      |   4 +
 contrib/rerere-train.sh          |   2 +-
 merge-ort.c                      |   5 +-
 merge-recursive.c                |   5 +-
 merge.c                          |   6 +-
 reset.c                          |   3 +-
 sequencer.c                      |   1 +
 submodule.c                      |   1 +
 t/t1013-read-tree-submodule.sh   |   1 -
 t/t2500-untracked-overwriting.sh | 244 +++++++++++++++++++++++++++++++
 t/t7112-reset-submodule.sh       |   1 -
 unpack-trees.c                   |  59 +++++++-
 unpack-trees.h                   |  16 +-
 23 files changed, 362 insertions(+), 69 deletions(-)
 create mode 100755 t/t2500-untracked-overwriting.sh

Range-diff against v3:
 1:  66270ffc74e !  1:  3a3203beee6 t2500: add various tests for nuking untracked files
    @@ Commit message
         removing untracked files and directories.
     
         Signed-off-by: Elijah Newren <newren@gmail.com>
    +    Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
     
      ## t/t2500-untracked-overwriting.sh (new) ##
     @@
 2:  0c74285b253 <  -:  ----------- checkout, read-tree: fix leak of unpack_trees_options.dir
 3:  2501a0c552a !  2:  8e5f4006604 read-tree, merge-recursive: overwrite ignored files by default
    @@ Commit message
         The read-tree changes happen to fix a bug in t1013.
     
         Signed-off-by: Elijah Newren <newren@gmail.com>
    +    Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
     
      ## Documentation/git-read-tree.txt ##
     @@ Documentation/git-read-tree.txt: SYNOPSIS
    @@ builtin/read-tree.c: static int list_tree(struct object_id *oid)
      	NULL
      };
      
    -@@ builtin/read-tree.c: static int index_output_cb(const struct option *opt, const char *arg,
    - static int exclude_per_directory_cb(const struct option *opt, const char *arg,
    - 				    int unset)
    - {
    --	struct dir_struct *dir;
    - 	struct unpack_trees_options *opts;
    - 
    - 	BUG_ON_OPT_NEG(unset);
    +@@ builtin/read-tree.c: static int exclude_per_directory_cb(const struct option *opt, const char *arg,
      
      	opts = (struct unpack_trees_options *)opt->value;
      
    --	if (opts->dir)
    +-	if (opts->dir.exclude_per_dir)
     -		die("more than one --exclude-per-directory given.");
     -
    --	dir = xcalloc(1, sizeof(*opts->dir));
    --	dir->flags |= DIR_SHOW_IGNORED;
    --	dir->exclude_per_dir = arg;
    --	opts->dir = dir;
    +-	opts->dir.flags |= DIR_SHOW_IGNORED;
    +-	opts->dir.exclude_per_dir = arg;
     -	/* We do not need to nor want to do read-directory
     -	 * here; we are merely interested in reusing the
     -	 * per directory ignore stack mechanism.
    @@ builtin/read-tree.c: int cmd_read_tree(int argc, const char **argv, const char *
      	if ((opts.update || opts.index_only) && !opts.merge)
      		die("%s is meaningless without -m, --reset, or --prefix",
      		    opts.update ? "-u" : "-i");
    --	if ((opts.dir && !opts.update))
    +-	if ((opts.dir.exclude_per_dir && !opts.update))
     -		die("--exclude-per-directory is meaningless unless -u");
     +	if (opts.update && !opts.reset) {
    -+		CALLOC_ARRAY(opts.dir, 1);
    -+		opts.dir->flags |= DIR_SHOW_IGNORED;
    -+		setup_standard_excludes(opts.dir);
    ++		opts.dir.flags |= DIR_SHOW_IGNORED;
    ++		setup_standard_excludes(&opts.dir);
     +	}
      	if (opts.merge && !opts.index_only)
      		setup_work_tree();
    @@ builtin/read-tree.c: int cmd_read_tree(int argc, const char **argv, const char *
     
      ## merge-recursive.c ##
     @@ merge-recursive.c: static int unpack_trees_start(struct merge_options *opt,
    - 	memset(&opt->priv->unpack_opts, 0, sizeof(opt->priv->unpack_opts));
    + 	unpack_trees_options_init(&opt->priv->unpack_opts);
      	if (opt->priv->call_depth)
      		opt->priv->unpack_opts.index_only = 1;
     -	else
     +	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.dir.flags |= DIR_SHOW_IGNORED;
    ++		setup_standard_excludes(&opt->priv->unpack_opts.dir);
     +	}
      	opt->priv->unpack_opts.merge = 1;
      	opt->priv->unpack_opts.head_idx = 2;
      	opt->priv->unpack_opts.fn = threeway_merge;
    -@@ merge-recursive.c: 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);
    - 
    - 	/*
     
      ## t/t1013-read-tree-submodule.sh ##
     @@ t/t1013-read-tree-submodule.sh: test_description='read-tree can handle submodules'
 4:  f1a0700e598 !  3:  739e9b871c4 unpack-trees: introduce preserve_ignored to unpack_trees_options
    @@ Commit message
     
         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
    +
    +       * flip the DIR_SHOW_IGNORED flag in unpack_trees_options.dir.flags
    +       * call setup_standard_excludes(&unpack_trees_options.dir)
    +
    +    Avoid that boilerplate by 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.
    @@ Commit message
         behavior we want.
     
         Signed-off-by: Elijah Newren <newren@gmail.com>
    +    Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
     
      ## builtin/am.c ##
     @@ builtin/am.c: static int fast_forward_to(struct tree *head, struct tree *remote, int reset)
    @@ builtin/checkout.c: static int merge_working_tree(const struct checkout_opts *op
      				       &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.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);
    -@@ builtin/checkout.c: 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) {
    - 			/*
     
      ## builtin/clone.c ##
     @@ builtin/clone.c: static int checkout(int submodule_progress)
    @@ builtin/read-tree.c: int cmd_read_tree(int argc, const char **argv, const char *
      		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);
    +-		opts.dir.flags |= DIR_SHOW_IGNORED;
    +-		setup_standard_excludes(&opts.dir);
     -	}
     +	if (opts.update && !opts.reset)
     +		opts.preserve_ignored = 0;
    @@ builtin/read-tree.c: int cmd_read_tree(int argc, const char **argv, const char *
      	if (opts.merge && !opts.index_only)
      		setup_work_tree();
      
    -@@ builtin/read-tree.c: 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 */
    - 
     
      ## builtin/reset.c ##
     @@ builtin/reset.c: static int reset_index(const char *ref, const struct object_id *oid, int reset_t
    @@ builtin/stash.c: static int reset_tree(struct object_id *i_tree, int update, int
     +		opts.preserve_ignored = 1;
      	opts.fn = oneway_merge;
      
    - 	if (unpack_trees(nr_trees, t, &opts))
    + 	if (unpack_trees(nr_trees, t, &opts)) {
     
      ## merge-ort.c ##
     @@ merge-ort.c: static int checkout(struct merge_options *opt,
    @@ merge-ort.c: static int checkout(struct merge_options *opt,
      	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.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);
    -@@ merge-ort.c: 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;
    - }
    - 
     
      ## merge-recursive.c ##
     @@ merge-recursive.c: static int unpack_trees_start(struct merge_options *opt,
    + 		opt->priv->unpack_opts.index_only = 1;
      	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.dir.flags |= DIR_SHOW_IGNORED;
    +-		setup_standard_excludes(&opt->priv->unpack_opts.dir);
    ++		/* FIXME: should only do this if !overwrite_ignore */
     +		opt->priv->unpack_opts.preserve_ignored = 0;
      	}
      	opt->priv->unpack_opts.merge = 1;
      	opt->priv->unpack_opts.head_idx = 2;
    -@@ merge-recursive.c: 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);
    - 
    - 	/*
     
      ## merge.c ##
     @@ merge.c: 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);
    -@@ merge.c: int checkout_fast_forward(struct repository *r,
    + 		init_tree_desc(t+i, trees[i]->buffer, trees[i]->size);
      	}
      
    - 	memset(&opts, 0, sizeof(opts));
     -	if (overwrite_ignore) {
    --		dir.flags |= DIR_SHOW_IGNORED;
    --		setup_standard_excludes(&dir);
    --		opts.dir = &dir;
    +-		opts.dir.flags |= DIR_SHOW_IGNORED;
    +-		setup_standard_excludes(&opts.dir);
     -	}
    +-
     +	opts.preserve_ignored = !overwrite_ignore;
    - 
      	opts.head_idx = 1;
      	opts.src_index = r->index;
    -@@ merge.c: 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))
    + 	opts.dst_index = r->index;
     
      ## reset.c ##
     @@ reset.c: int reset_head(struct repository *r, struct object_id *oid, const char *action,
    @@ unpack-trees.c: int unpack_trees(unsigned len, struct tree_desc *t, struct unpac
      	}
      
     +	if (!o->preserve_ignored) {
    -+		CALLOC_ARRAY(o->dir, 1);
    -+		o->dir->flags |= DIR_SHOW_IGNORED;
    -+		setup_standard_excludes(o->dir);
    ++		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) {
    -@@ unpack-trees.c: 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;
     
      ## unpack-trees.h ##
     @@ unpack-trees.h: struct unpack_trees_options {
 5:  0d119142778 !  4:  296c1e03673 unpack-trees: make dir an internal-only struct
    @@ Metadata
     Author: Elijah Newren <newren@gmail.com>
     
      ## Commit message ##
    -    unpack-trees: make dir an internal-only struct
    +    unpack-trees: rename "dir" to "private_dir"
     
    -    Avoid accidental misuse or confusion over ownership by clearly making
    -    unpack_trees_options.dir an internal-only variable.
    +    Until the introduction of the "preserve_ignored" flag in the preceding
    +    commit callers who wanted its behavior needed to adjust "dir.flags"
    +    and call setup_standard_excludes() themselves.
    +
    +    Now that we have no external users of "dir" anymore let's rename it to
    +    "private_dir" and add a comment indicating that we'd like it not to be
    +    messed with by external callers. This should avoid avoid accidental
    +    misuse or confusion over its ownership.
     
         Signed-off-by: Elijah Newren <newren@gmail.com>
    +    Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
     
      ## unpack-trees.c ##
    -@@ unpack-trees.c: int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
    - 	static struct cache_entry *dfc;
    - 	struct pattern_list pl;
    - 	int free_pattern_list = 0;
    -+	struct dir_struct dir = DIR_INIT;
    +@@ unpack-trees.c: void unpack_trees_options_init(struct unpack_trees_options *o)
    + void unpack_trees_options_release(struct unpack_trees_options *opts)
    + {
    + 	strvec_clear(&opts->msgs_to_free);
    +-	dir_clear(&opts->dir);
    ++	dir_clear(&opts->private_dir);
    + }
      
    - 	if (len > MAX_UNPACK_TREES)
    - 		die("unpack_trees takes at most %d trees", MAX_UNPACK_TREES);
    -+	if (o->dir)
    -+		BUG("o->dir is for internal use only");
    - 
    - 	trace_performance_enter();
    - 	trace2_region_enter("unpack_trees", "unpack_trees", the_repository);
    + static int do_add_entry(struct unpack_trees_options *o, struct cache_entry *ce,
     @@ unpack-trees.c: int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
      	}
      
      	if (!o->preserve_ignored) {
    --		CALLOC_ARRAY(o->dir, 1);
    -+		o->dir = &dir;
    - 		o->dir->flags |= DIR_SHOW_IGNORED;
    - 		setup_standard_excludes(o->dir);
    - 	}
    -@@ unpack-trees.c: int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
    - 		clear_pattern_list(&pl);
    - 	if (o->dir) {
    - 		dir_clear(o->dir);
    --		FREE_AND_NULL(o->dir);
    -+		o->dir = NULL;
    +-		o->dir.flags |= DIR_SHOW_IGNORED;
    +-		setup_standard_excludes(&o->dir);
    ++		o->private_dir.flags |= DIR_SHOW_IGNORED;
    ++		setup_standard_excludes(&o->private_dir);
      	}
    - 	trace2_region_leave("unpack_trees", "unpack_trees", the_repository);
    - 	trace_performance_leave("unpack_trees");
    + 
    + 	if (!core_apply_sparse_checkout || !o->update)
    +@@ unpack-trees.c: static int verify_clean_subdirectory(const struct cache_entry *ce,
    + 	 */
    + 	pathbuf = xstrfmt("%.*s/", namelen, ce->name);
    + 
    +-	d.exclude_per_dir = o->dir.exclude_per_dir;
    ++	d.exclude_per_dir = o->private_dir.exclude_per_dir;
    + 	i = read_directory(&d, o->src_index, pathbuf, namelen+1, NULL);
    + 	dir_clear(&d);
    + 	free(pathbuf);
    +@@ unpack-trees.c: static int check_ok_to_remove(const char *name, int len, int dtype,
    + 	if (ignore_case && icase_exists(o, name, len, st))
    + 		return 0;
    + 
    +-	if (is_excluded(&o->dir, o->src_index, name, &dtype))
    ++	if (is_excluded(&o->private_dir, o->src_index, name, &dtype))
    + 		/*
    + 		 * ce->name is explicitly excluded, so it is Ok to
    + 		 * overwrite it.
     
      ## unpack-trees.h ##
     @@ unpack-trees.h: struct unpack_trees_options {
      		     dry_run;
      	const char *prefix;
      	int cache_bottom;
    --	struct dir_struct *dir;
    +-	struct dir_struct dir;
    ++	struct dir_struct private_dir; /* for internal use only */
      	struct pathspec *pathspec;
      	merge_fn_t fn;
      	const char *msgs[NB_UNPACK_TREES_WARNING_TYPES];
     @@ unpack-trees.h: struct unpack_trees_options {
    - 	struct index_state result;
      
    - 	struct pattern_list *pl; /* for internal use */
    -+	struct dir_struct *dir; /* for internal use only */
    - 	struct checkout_metadata meta;
    - };
    + #define UNPACK_TREES_OPTIONS_INIT { \
    + 	.msgs_to_free = STRVEC_INIT, \
    +-	.dir = DIR_INIT, \
    ++	.private_dir = DIR_INIT, \
    + }
    + void unpack_trees_options_init(struct unpack_trees_options *o);
      
 6:  b7fe354efff !  5:  27496506430 Remove ignored files by default when they are in the way
    @@ Commit message
         Incidentally, this fixes a test failure in t7112.
     
         Signed-off-by: Elijah Newren <newren@gmail.com>
    +    Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
     
      ## builtin/am.c ##
     @@ builtin/am.c: static int fast_forward_to(struct tree *head, struct tree *remote, int reset)
    @@ builtin/stash.c: static int reset_tree(struct object_id *i_tree, int update, int
     +		opts.preserve_ignored = 0; /* FIXME: !overwrite_ignore */
      	opts.fn = oneway_merge;
      
    - 	if (unpack_trees(nr_trees, t, &opts))
    + 	if (unpack_trees(nr_trees, t, &opts)) {
     
      ## merge-ort.c ##
     @@ merge-ort.c: static int checkout(struct merge_options *opt,
 7:  9eb20121fc3 !  6:  7b539a120b9 Change unpack_trees' 'reset' flag into an enum
    @@ Commit message
         [1] https://lore.kernel.org/git/15dad590-087e-5a48-9238-5d2826950506@gmail.com/
     
         Signed-off-by: Elijah Newren <newren@gmail.com>
    +    Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
     
      ## builtin/am.c ##
     @@ builtin/am.c: static int fast_forward_to(struct tree *head, struct tree *remote, int reset)
    @@ t/t2500-untracked-overwriting.sh: test_expect_failure 'git rebase --abort and un
     
      ## unpack-trees.c ##
     @@ unpack-trees.c: int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
    + 	struct pattern_list pl;
      	int free_pattern_list = 0;
    - 	struct dir_struct dir = DIR_INIT;
      
     +	if (o->reset == UNPACK_RESET_INVALID)
     +		BUG("o->reset had a value of 1; should be UNPACK_TREES_*_UNTRACKED");
     +
      	if (len > MAX_UNPACK_TREES)
      		die("unpack_trees takes at most %d trees", MAX_UNPACK_TREES);
    - 	if (o->dir)
    + 
     @@ unpack-trees.c: int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
      		ensure_full_index(o->dst_index);
      	}
    @@ unpack-trees.c: int unpack_trees(unsigned len, struct tree_desc *t, struct unpac
     +		BUG("UNPACK_RESET_OVERWRITE_UNTRACKED incompatible with preserved ignored files");
     +
      	if (!o->preserve_ignored) {
    - 		o->dir = &dir;
    - 		o->dir->flags |= DIR_SHOW_IGNORED;
    + 		o->private_dir.flags |= DIR_SHOW_IGNORED;
    + 		setup_standard_excludes(&o->private_dir);
     @@ unpack-trees.c: static int verify_absent_1(const struct cache_entry *ce,
      	int len;
      	struct stat st;
    @@ unpack-trees.c: static int verify_absent_1(const struct cache_entry *ce,
      ## unpack-trees.h ##
     @@ unpack-trees.h: void setup_unpack_trees_porcelain(struct unpack_trees_options *opts,
       */
    - void clear_unpack_trees_porcelain(struct unpack_trees_options *opts);
    + void unpack_trees_options_release(struct unpack_trees_options *opts);
      
     +enum unpack_trees_reset_type {
     +	UNPACK_RESET_NONE = 0,    /* traditional "false" value; still valid */
    @@ unpack-trees.h: struct unpack_trees_options {
     +	enum unpack_trees_reset_type reset;
      	const char *prefix;
      	int cache_bottom;
    - 	struct pathspec *pathspec;
    + 	struct dir_struct private_dir; /* for internal use only */
 8:  e4c42d43b09 !  7:  b6769f629ae unpack-trees: avoid nuking untracked dir in way of unmerged file
    @@ Commit message
         unpack-trees: avoid nuking untracked dir in way of unmerged file
     
         Signed-off-by: Elijah Newren <newren@gmail.com>
    +    Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
     
      ## t/t2500-untracked-overwriting.sh ##
     @@ t/t2500-untracked-overwriting.sh: test_expect_failure 'git stash and untracked files' '
 9:  1a770681704 !  8:  10a7cbf049e unpack-trees: avoid nuking untracked dir in way of locally deleted file
    @@ Commit message
         unpack-trees: avoid nuking untracked dir in way of locally deleted file
     
         Signed-off-by: Elijah Newren <newren@gmail.com>
    +    Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
     
      ## t/t2500-untracked-overwriting.sh ##
     @@ t/t2500-untracked-overwriting.sh: test_expect_success 'git am --abort and untracked dir vs. unmerged file' '
10:  6b42a80bf3d !  9:  b2f27d961a9 Comment important codepaths regarding nuking untracked files/dirs
    @@ Commit message
         running in an empty directory.
     
         Signed-off-by: Elijah Newren <newren@gmail.com>
    +    Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
     
      ## builtin/stash.c ##
     @@ builtin/stash.c: static int do_push_stash(const struct pathspec *ps, const char *stash_msg, int q
11:  de416f887d7 ! 10:  e88f81baa50 Documentation: call out commands that nuke untracked files/directories
    @@ Commit message
         these cases.
     
         Signed-off-by: Elijah Newren <newren@gmail.com>
    +    Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
     
      ## Documentation/git-checkout.txt ##
     @@ Documentation/git-checkout.txt: OPTIONS
-- 
2.33.0.1404.g83021034c5d


  parent reply	other threads:[~2021-10-04  1:12 UTC|newest]

Thread overview: 82+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-18 23:15 [PATCH 0/6] Fix various issues around removal of untracked files/directories Elijah Newren via GitGitGadget
2021-09-18 23:15 ` [PATCH 1/6] t2500: add various tests for nuking untracked files Elijah Newren via GitGitGadget
2021-09-19 13:44   ` Ævar Arnfjörð Bjarmason
2021-09-20 14:48     ` Elijah Newren
2021-09-18 23:15 ` [PATCH 2/6] Split unpack_trees 'reset' flag into two for untracked handling Elijah Newren via GitGitGadget
2021-09-19 13:48   ` Ævar Arnfjörð Bjarmason
2021-09-20 15:20     ` Elijah Newren
2021-09-20 10:19   ` Phillip Wood
2021-09-20 16:05     ` Elijah Newren
2021-09-20 18:11       ` Phillip Wood
2021-09-24  2:27         ` Elijah Newren
2021-09-18 23:15 ` [PATCH 3/6] unpack-trees: avoid nuking untracked dir in way of unmerged file Elijah Newren via GitGitGadget
2021-09-18 23:15 ` [PATCH 4/6] unpack-trees: avoid nuking untracked dir in way of locally deleted file Elijah Newren via GitGitGadget
2021-09-19 13:52   ` Ævar Arnfjörð Bjarmason
2021-09-20 16:12     ` Elijah Newren
2021-09-18 23:15 ` [PATCH 5/6] Comment important codepaths regarding nuking untracked files/dirs Elijah Newren via GitGitGadget
2021-09-24 11:47   ` Luke Diamand
2021-09-24 13:41     ` Elijah Newren
2021-09-18 23:15 ` [PATCH 6/6] Documentation: call out commands that nuke untracked files/directories Elijah Newren via GitGitGadget
2021-09-19 10:52   ` Philip Oakley
2021-09-19 13:36     ` Philip Oakley
2021-09-20 16:29       ` Elijah Newren
2021-09-24  6:37 ` [PATCH v2 0/6] Fix various issues around removal of " Elijah Newren via GitGitGadget
2021-09-24  6:37   ` [PATCH v2 1/6] t2500: add various tests for nuking untracked files Elijah Newren via GitGitGadget
2021-09-24  6:37   ` [PATCH v2 2/6] Change unpack_trees' 'reset' flag into an enum Elijah Newren via GitGitGadget
2021-09-24 17:35     ` Junio C Hamano
2021-09-26  6:50       ` Elijah Newren
2021-09-24  6:37   ` [PATCH v2 3/6] unpack-trees: avoid nuking untracked dir in way of unmerged file Elijah Newren via GitGitGadget
2021-09-24  6:37   ` [PATCH v2 4/6] unpack-trees: avoid nuking untracked dir in way of locally deleted file Elijah Newren via GitGitGadget
2021-09-24  6:37   ` [PATCH v2 5/6] Comment important codepaths regarding nuking untracked files/dirs Elijah Newren via GitGitGadget
2021-09-24 17:50     ` Eric Sunshine
2021-09-26  6:35       ` Elijah Newren
2021-09-24  6:37   ` [PATCH v2 6/6] Documentation: call out commands that nuke untracked files/directories Elijah Newren via GitGitGadget
2021-09-27 16:33   ` [PATCH v3 00/11] Fix various issues around removal of " Elijah Newren via GitGitGadget
2021-09-27 16:33     ` [PATCH v3 01/11] t2500: add various tests for nuking untracked files Elijah Newren via GitGitGadget
2021-09-27 16:33     ` [PATCH v3 02/11] checkout, read-tree: fix leak of unpack_trees_options.dir Elijah Newren via GitGitGadget
2021-09-27 16:33     ` [PATCH v3 03/11] read-tree, merge-recursive: overwrite ignored files by default Elijah Newren via GitGitGadget
2021-12-13 17:12       ` Jack O'Connor
2021-12-13 20:10         ` Elijah Newren
2021-09-27 16:33     ` [PATCH v3 04/11] unpack-trees: introduce preserve_ignored to unpack_trees_options Elijah Newren via GitGitGadget
2021-09-29  9:22       ` Ævar Arnfjörð Bjarmason
2021-09-29 15:35         ` Elijah Newren
2021-09-29 18:30           ` Ævar Arnfjörð Bjarmason
2021-09-30  4:25             ` Elijah Newren
2021-09-30 14:04               ` Ævar Arnfjörð Bjarmason
2021-10-01  1:53                 ` Elijah Newren
2021-10-01  8:15                   ` Ævar Arnfjörð Bjarmason
2021-10-01  9:53                     ` Ævar Arnfjörð Bjarmason
2021-10-01 18:50                     ` Elijah Newren
2021-10-02  8:44                       ` Ævar Arnfjörð Bjarmason
2021-10-03 22:21                         ` Ævar Arnfjörð Bjarmason
2021-10-04 13:45                           ` Elijah Newren
2021-10-04 13:45                         ` Elijah Newren
2021-10-04 14:07                           ` Ævar Arnfjörð Bjarmason
2021-10-04 14:57                             ` Elijah Newren
2021-09-27 16:33     ` [PATCH v3 05/11] unpack-trees: make dir an internal-only struct Elijah Newren via GitGitGadget
2021-09-27 16:33     ` [PATCH v3 06/11] Remove ignored files by default when they are in the way Elijah Newren via GitGitGadget
2021-09-27 16:33     ` [PATCH v3 07/11] Change unpack_trees' 'reset' flag into an enum Elijah Newren via GitGitGadget
2021-09-27 16:33     ` [PATCH v3 08/11] unpack-trees: avoid nuking untracked dir in way of unmerged file Elijah Newren via GitGitGadget
2021-09-27 16:33     ` [PATCH v3 09/11] unpack-trees: avoid nuking untracked dir in way of locally deleted file Elijah Newren via GitGitGadget
2021-09-27 16:33     ` [PATCH v3 10/11] Comment important codepaths regarding nuking untracked files/dirs Elijah Newren via GitGitGadget
2021-09-27 16:33     ` [PATCH v3 11/11] Documentation: call out commands that nuke untracked files/directories Elijah Newren via GitGitGadget
2021-09-27 20:36     ` [PATCH v3 00/11] Fix various issues around removal of " Junio C Hamano
2021-09-27 20:41       ` Elijah Newren
2021-09-27 21:31         ` Elijah Newren
2021-09-30 14:00     ` Phillip Wood
     [not found]     ` <aaa8ea3b-0902-f9e6-c1a4-0ca2b1b2f57b@gmail.com>
2021-10-01  2:08       ` Elijah Newren
2021-10-04  1:11     ` Ævar Arnfjörð Bjarmason [this message]
2021-10-04  1:11       ` [RFC PATCH v4 01/10] t2500: add various tests for nuking untracked files Ævar Arnfjörð Bjarmason
2021-10-04  1:11       ` [RFC PATCH v4 02/10] read-tree, merge-recursive: overwrite ignored files by default Ævar Arnfjörð Bjarmason
2021-10-04  1:11       ` [RFC PATCH v4 03/10] unpack-trees: introduce preserve_ignored to unpack_trees_options Ævar Arnfjörð Bjarmason
2021-10-04  1:11       ` [RFC PATCH v4 04/10] unpack-trees: rename "dir" to "private_dir" Ævar Arnfjörð Bjarmason
2021-10-04  1:11       ` [RFC PATCH v4 05/10] Remove ignored files by default when they are in the way Ævar Arnfjörð Bjarmason
2021-10-04  1:11       ` [RFC PATCH v4 06/10] Change unpack_trees' 'reset' flag into an enum Ævar Arnfjörð Bjarmason
2021-10-04  1:11       ` [RFC PATCH v4 07/10] unpack-trees: avoid nuking untracked dir in way of unmerged file Ævar Arnfjörð Bjarmason
2021-10-04  1:11       ` [RFC PATCH v4 08/10] unpack-trees: avoid nuking untracked dir in way of locally deleted file Ævar Arnfjörð Bjarmason
2021-10-04  1:11       ` [RFC PATCH v4 09/10] Comment important codepaths regarding nuking untracked files/dirs Ævar Arnfjörð Bjarmason
2021-10-04  1:11       ` [RFC PATCH v4 10/10] Documentation: call out commands that nuke untracked files/directories Ævar Arnfjörð Bjarmason
2021-10-04 14:38       ` [RFC PATCH v4 00/10] Fix various issues around removal of " Elijah Newren
2021-10-04 16:08         ` Ævar Arnfjörð Bjarmason
2021-10-05  7:40           ` Elijah Newren
2021-10-04 18:17         ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=RFC-cover-v4-00.10-00000000000-20211004T004902Z-avarab@gmail.com \
    --to=avarab@gmail.com \
    --cc=ajrhunt@google.com \
    --cc=fedor.birjukov@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=martin.agren@gmail.com \
    --cc=newren@gmail.com \
    --cc=pclouds@gmail.com \
    --cc=peff@peff.net \
    --cc=philipoakley@iee.email \
    --cc=phillip.wood123@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).