Re: [PATCH] Introduce "precious" file concept

[Duy Nguyen <pclouds@gmail.com>]

On Tue, Apr 9, 2019 at 6:31 PM Junio C Hamano <gitster@pobox.com> wrote:
> > @@ -193,9 +203,16 @@ static int remove_dirs(struct strbuf *path, const char *prefix, int force_flag,
> >
> >               strbuf_setlen(path, len);
> >               strbuf_addstr(path, e->d_name);
> > -             if (lstat(path->buf, &st))
> > +             if (lstat(path->buf, &st)) {
> >                       ; /* fall thru */
> > -             else if (S_ISDIR(st.st_mode)) {
> > +             } else if ((!prefix && skip_precious_file(&the_index, path->buf)) ||
> > +                        (prefix && skip_prefix(path->buf, prefix, &rel_path) &&
> > +                         skip_precious_file(&the_index, rel_path))) {
> > +                     quote_path_relative(path->buf, prefix, &quoted);
> > +                     printf(dry_run ? _(msg_would_skip_precious) : _(msg_skip_precious), quoted.buf);
> > +                     *dir_gone = 0;
> > +                     continue;
>
> An attribute is given to something that can be tracked, and a
> directory would not get an attribute, because Git does not track
> directories (there is a reason why skip_precious_file() takes
> &the_index that is passed down the callchain to git_check_attr()).
>
> Triggering this logic before excluding S_ISDIR(st.st_mode) feels
> iffy.
>
> But let's assume that being able to say "this directory and anything
> (recursively) inside are precious" is a good idea and read on.

Hm... we do allow to set attributes on directories even though we
don't track them. I was under the impression that an attribute set on
a directory will be propagated to all files inside anyway, so this is
more of an (mis?-)optimization.

But gitattributes.txt explicitly says that it's wrong. One would need
to do "path/** precious" to achieve the same thing.

So yeah maybe doing this before S_ISDIR() is wrong. The definition of
`precious` also only says "... is set on _files_". Maybe best to
ignore attributes on directories? At least it looks like that's how
all other attributes do.
-- 
Duy