On 06/12/2012 00:35, Junio C Hamano wrote:
"Jean-Noël AVILA" <jn.av...@free.fr> writes:

-static void prepare_attr_stack(const char *path)
+static void prepare_attr_stack(const char *path, unsigned mode)
        struct attr_stack *elem, *info;
        int dirlen, len;
@@ -645,28 +645,43 @@ static void prepare_attr_stack(const char *path)

The new "mode" parameter does not seem to be used in this function
at all.

  static int path_matches(const char *pathname, int pathlen,
-                       const char *pattern,
+                       const unsigned mode, char *pattern,
                        const char *base, int baselen)
-       if (!strchr(pattern, '/')) {
+       size_t len;
+       char buf[PATH_MAX];
+       char * lpattern = buf;
+       len = strlen(pattern);
+       if (PATH_MAX <= len)
+               return 0;
+       strncpy(buf,pattern,len);
+       buf[len] ='\0';
+       if (len && lpattern[len - 1] == '/') {
+               if (S_ISDIR(mode))
+                       lpattern[len - 1] = '\0';
+               else
+                       return 0;
+       }
+       if (!strchr(lpattern, '/')) {
                /* match basename */
                const char *basename = strrchr(pathname, '/');
                basename = basename ? basename + 1 : pathname;
-               return (fnmatch_icase(pattern, basename, 0) == 0);
+               return (fnmatch_icase(lpattern, basename, 0) == 0);
         * match with FNM_PATHNAME; the pattern has base implicitly
         * in front of it.
-       if (*pattern == '/')
-               pattern++;
+       if (*lpattern == '/')
+               lpattern++;
        if (pathlen < baselen ||
            (baselen && pathname[baselen] != '/') ||
            strncmp(pathname, base, baselen))
                return 0;
        if (baselen != 0)
-       return fnmatch_icase(pattern, pathname + baselen, FNM_PATHNAME) == 0;
+       return fnmatch_icase(lpattern, pathname + baselen, FNM_PATHNAME) == 0;
It appears to me that you are forcing the caller to tell this
function if the path is a directory, but in the attribute system,
the caller does not necessarily know if the path it is passing is
meant to be a directory or a regular file.  "check-attr" is meant to
be usable against a path that does not even exist on the working
tree, so using stat() or lstat() in it is not a solution.  In other
words, it is unfair (read: unworkable) to force it to append a
trailing slash after the path it calls this function with.

Thank you for your comments. Changing the whole attr.h interface header
is definitely not a good option, but at some point, we may need more information
on the path to be able to match a path pattern against it.

If you are interested in export-subst, all is not lost, though:

        $ git init
         $ mkdir a
         $ >a/b
         $ echo a export-ignore >.gitattributes
         $ git add a/b .gitattributes
        $ git commit -m initial
         $ git archive HEAD | tar tf -
         $ exit

You could change the "echo" to

        $ echo "a/*" export-ignore >.gitattributes

as well, but it seems to create an useless empty directory "a/" in
the output, which I think is an unrelated bug in "git archive".

This is quite different from the pattern matching documented for gitignore.


 $ mkdir -p not-ignored-dir/ignored-dir
 $ echo test >not-ignored-dir/ignored-dir/ignored
 $ echo 'ignored-dir/*' >.gitattributes
 $ git add not-ignored-dir .gitattributes
 $ git commit -m '.'
 $ git archive HEAD | tar tf -

This patch seems to be based on a stale codebase.
Sorry. I thought the patch submissions had to be based on the 'maint' branch.

   I do not think
I'd be opposed to change the sementics to allow the callers that
know that a path is a directory to optionally pass mode parameter by
ending the pathname with slash (in other words, have "git
check-attr" ask about a directory 'a' by saying "git check-attr
export-subst a/", and lose the "mode" argument in this patch), or
keep the "mode" parameter and instead allow "git check-attr" to ask
about a directory that does not exist in the working tree by a more
explicit "git check-attr --directory export-ignore a" or something.
Such an enhancement should be done on top of the current codebase.

OK. I like the idea of proposing a path ending with '/' when it is meant to be directory. This would not change the interface attr.h . I will rework with this idea.

Thank you.

To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to