Re: [PATCH 2/6] Split unpack_trees 'reset' flag into two for untracked handling

On Sun, Sep 19, 2021 at 6:51 AM Ævar Arnfjörð Bjarmason
[off-list ref] wrote:

On Sat, Sep 18 2021, Elijah Newren via GitGitGadget wrote:

quoted

-     opts.reset = reset;
+     opts.reset_keep_untracked = reset;
      opts.fn = twoway_merge;
+     /* Setup opts.dir so that ignored files in the way get overwritten */
+     opts.dir = xcalloc(1, sizeof(*opts.dir));
+     opts.dir->flags |= DIR_SHOW_IGNORED;
+     setup_standard_excludes(opts.dir);

Is the "opts.dir" free'd later somehow?

No, much like other callsites that set this up (e.g.
builtin/checkout.c), there isn't a place that frees it.  In copying
how they worked, I also copied their bugs.  ;-)

I'm tempted to move this code into setup_unpack_trees_porcelain() and
then free it in clear_unpack_trees_porcelain()...though not all
callers make use of those functions.  Hmm...

quoted

      opts.head_idx = -1;
      opts.update = worktree;
      opts.skip_unmerged = !worktree;
-     opts.reset = 1;
+     if (o->force)
+             opts.reset_nuke_untracked = 1;
+     else
+             opts.reset_keep_untracked = 1;

In both cases opts.reset_keep_untracked is set to 1, I assume it's a
mistake

No, it only sets opts.reset_keep_untracked to 1 when o->force is
false.  If o->force is true, it instead sets opts.reset_nuke_untracked
to 1.  There's no mistake there.

quoted

      opts.merge = 1;
      opts.fn = oneway_merge;
      opts.verbose_update = o->show_progress;
      opts.src_index = &the_index;
      opts.dst_index = &the_index;
+     if (o->overwrite_ignore) {
+             opts.dir = xcalloc(1, sizeof(*opts.dir));

ditto potential leak.

quoted

+             opts.dir = xcalloc(1, sizeof(*opts.dir));
+             opts.dir->flags |= DIR_SHOW_IGNORED;
+             setup_standard_excludes(opts.dir);
+     }

ditto (also more omitted).

Yep.