Elijah Newren <newren@xxxxxxxxx> writes: > On Fri, Feb 24, 2023 at 2:31 PM Jonathan Tan <jonathantanmy@xxxxxxxxxx> wrote: > > > > "Elijah Newren via GitGitGadget" <gitgitgadget@xxxxxxxxx> 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.)