Re: [PATCH] Introduce "precious" file concept

From: Duy Nguyen <pclouds@gmail.com>
Date: Apr 10, 2019, 09:36 UTC
Message-ID: <CACsJy8Ct6A7a7FzzH7ZUv9R2+k=G6qhzZ8Yjq8GUDBdTr_QBQA@mail.gmail.com>
In-Reply-To: <xmqqh8b75s9l.fsf@gitster-ct.c.googlers.com>

On Tue, Apr 9, 2019 at 6:31 PM Junio C Hamano <gitster@pobox.com> wrote:

Show 26 quoted lines

> > @@ -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