Re: [PATCH 02/11] dir: add a usage note to exclude_per_dir

>
> Elijah Newren <newren@gmail.com> writes:
> > On Fri, Feb 24, 2023 at 2:31 PM Jonathan Tan <jonathantanmy@google.com> wrote:
> > >
> > > "Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com> writes:
> > > > diff --git a/dir.h b/dir.h
> > > > index 33fd848fc8d..2196e12630c 100644
> > > > --- a/dir.h
> > > > +++ b/dir.h
> > > > @@ -295,8 +295,12 @@ struct dir_struct {
> > > >       struct untracked_cache *untracked;
> > > >
> > > >       /**
> > > > -      * The name of the file to be read in each directory for excluded files
> > > > -      * (typically `.gitignore`).
> > > > +      * Deprecated: ls-files is the only allowed caller; all other callers
> > > > +      * should leave this as NULL; it pre-dated the
> > > > +      * setup_standard_excludes() mechanism that replaces this.
> > > > +      *
> > > > +      * This field tracks the name of the file to be read in each directory
> > > > +      * for excluded files (typically `.gitignore`).
> > > >        */
> > > >       const char *exclude_per_dir;
> > >
> > > I'm not sure what is meant by "allowed caller", but I wouldn't have
> > > expected this to also mean that unpack-trees would need to know to
> > > propagate this from o->internal.dir to d in verify_clean_subdirectory.
> >
> > Are you confusing fields that are internal to dir, with fields that
> > are internal to unpack-trees?
> >
> > This series does not make exclude_per_dir an internal field within dir_struct.
>
> Agreed, but the comment says that ls-files is the only allowed caller,
> and I would have expected that non-"allowed callers" would not need to
> write to exclude_per_dir. But in unpack-trees.c:
>
>   2346          if (o->internal.dir)
>   2347                  d.exclude_per_dir = o->internal.dir->exclude_per_dir;
>
> Both "d" and "o->internal.dir" are of type "struct dir_struct" (well,
> one is not a pointer and one is). I would not have expected such non-
> ls-files code to read or write from this field. (But if unpack-trees
> is considered part of ls-files and/or copying the same field to another
> struct is not considered "calling", then this patch is fine, I guess.)