Re: [PATCH v2 09/18] unpack-trees: add a new update_sparsity() function

[Elijah Newren <newren@gmail.com>]

On Mon, Mar 23, 2020 at 11:02 AM Derrick Stolee <stolee@gmail.com> wrote:
>
> On 3/21/2020 2:00 PM, Elijah Newren via GitGitGadget wrote:

> > +     for (i = 0; i < o->src_index->cache_nr; i++) {
> > +             struct cache_entry *ce = o->src_index->cache[i];
> > +
> > +             if (apply_sparse_checkout(o->src_index, ce, o))
> > +                     ret = UPDATE_SPARSITY_WARNINGS;
> > +
> > +             if (!ce_skip_worktree(ce))
> > +                     empty_worktree = 0;
> > +
>
> nit: extra whitespace-only line

Will clean it up.

> > +     }
> > +
> > +     /*
> > +      * Sparse checkout is meant to narrow down checkout area
> > +      * but it does not make sense to narrow down to empty working
> > +      * tree. This is usually a mistake in sparse checkout rules.
> > +      * Do not allow users to do that.
> > +      */
> > +     if (o->src_index->cache_nr && empty_worktree) {
> > +             unpack_failed(o, "Sparse checkout leaves no entry on working directory");
> > +             ret = UPDATE_SPARSITY_INDEX_UPDATE_FAILURES;
> > +             goto done;
> > +     }
> > +
> > +skip_sparse_checkout:
> > +     if (check_updates(o, o->src_index))
> > +             ret = UPDATE_SPARSITY_WORKTREE_UPDATE_FAILURES;
> > +
> > +done:
> > +     display_error_msgs(o);
> > +     o->show_all_errors = old_show_all_errors;
> > +     if (free_pattern_list)
> > +             clear_pattern_list(&pl);
> > +     trace_performance_leave("update_sparsity");
> > +     return ret;
> > +}
> > +
> >  /* Here come the merge functions */
> >
> >  static int reject_merge(const struct cache_entry *ce,
> > diff --git a/unpack-trees.h b/unpack-trees.h
> > index d3516267f36..2c5d54cae9f 100644
> > --- a/unpack-trees.h
> > +++ b/unpack-trees.h
> > @@ -28,6 +28,13 @@ enum unpack_trees_error_types {
> >       NB_UNPACK_TREES_ERROR_TYPES
> >  };
> >
> > +enum update_sparsity_result {
> > +     UPDATE_SPARSITY_SUCCESS = 0,
> > +     UPDATE_SPARSITY_WARNINGS = 1,
> > +     UPDATE_SPARSITY_INDEX_UPDATE_FAILURES = -1,
> > +     UPDATE_SPARSITY_WORKTREE_UPDATE_FAILURES = -2
> > +};
> > +
>
> Is there a reason this isn't located just before
> update_sparsity()?

You mean move it to unpack-trees.c?  If I did that, the code in
sparse-checkout.c that uses two of these values would fail to compile.

> >  /*
> >   * Sets the list of user-friendly error messages to be used by the
> >   * command "cmd" (either merge or checkout), and show_all_errors to 1.
> > @@ -88,6 +95,8 @@ struct unpack_trees_options {
> >  int unpack_trees(unsigned n, struct tree_desc *t,
> >                struct unpack_trees_options *options);
> >
> > +int update_sparsity(struct unpack_trees_options *options);
> > +
>
> This appears to not use the enum as it should.

Whoops!  Will fix.  (Interesting that the compiler didn't flag any
kind of warning based on mismatch of declared function return types
for update_sparsity() in the .c and .h files...)

Thanks,
Elijah