Jeff King <peff@xxxxxxxx> writes:
> On Wed, Sep 12, 2012 at 11:32:22PM -0700, Junio C Hamano wrote:
>
>> "git repack" started giving the above warning, and I am guessing
>> that the recent 11e50b2 (attr: warn on inaccessible attribute files,
>> 2012-08-21) exposed a bug where we ask stat(2) not lstat(2) by
>> mistake before deciding to append .gitattributes to see if that
>> directory has a per-directory attributes file.
>
> Interesting. I don't get any such warning on repack. And RelNotes points
> to a file, so I'm not sure why stat() would make us think it was a dir.
Interesting. The command in question is
git-pack-objects --keep-true-parents --honor-pack-keep --non-empty \
--all --reflog --delta-base-offset </dev/null .junk-pack
And pack-objects.c::no_try_delta() is given "RelNotes/1.7.4.txt" as
a path (which is very strange), and is trying to see if "-delta" is
set for the path.
Three problems:
- "rev-list --object --all" does not produce "Relnotes/1.7.4.txt"
(it does have "Documentation/RelNotes/1.7.4.txt", of course).
Somebody in this callchain is screwing the name up.
- Even if the name were correct, we are looking at the path that
existed in the past. The value of checking the attributes file
in the working tree for "delta" attribute is dubious.
- This is done while traversing the commit list and enumerating
objects, so even if we have many incarnations of the same path in
different commits, the attr stack mechanism would only help
objects in the same directory in the same commit. Perhaps we
could do this after collecting all the blobs, check attributes
for each path only once (in a sorted order so that we can take
advantage of the attr stack), to reduce the cost of "delta"
attribute check.
In any case, because the directory that used to exist to house the
blob in it may no longer exist, giving the warning on ENOTDIR that
your 11e50b2 (attr: warn on inaccessible attribute files,
2012-08-21) is a wrong thing to do (assuming that checking the
current attribute setting for historical tree is a sensible thing to
do, that is).
I could check for ENOTDIR to squelch the warning, but I think your
patch uncovered a lot deeper issues.
diff --git i/attr.c w/attr.c
index f12c83f..056d702 100644
--- i/attr.c
+++ w/attr.c
@@ -353,7 +353,7 @@ static struct attr_stack *read_attr_from_file(const char *path, int macro_ok)
int lineno = 0;
if (!fp) {
- if (errno != ENOENT)
+ if (errno != ENOENT && errno != ENOTDIR)
warn_on_inaccessible(path);
return NULL;
}
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html