Subject: Re: make: Dir_FindFile still broken
To: Christos Zoulas <christos@zoulas.com>
From: Simon J. Gerraty <sjg@crufty.net>
List: tech-toolchain
Date: 11/13/2002 17:37:57
>>Do you mean when say cat.c was first found as ../../src/bin/cat/cat.c ?
>>It does, but then ../../src/bin/cat/cat.c ends up in .depend (a side effect
>>of using gcc's -MD to automatically update dependencies), so on
>>the next build make looks for ../../src/bin/cat/cat.c instead of cat.c
>>and absent this change or some other fix, will find 
>>../../src/bin/cat/../../src/bin/cat/cat.c and so on and so forth.
>>
>>I guess the real root cause of this is the fact that when you
>>.PATH:
>>to clear .PATH, the original 'dot' which has an absolute path is left
>>which requires a releative path to .CURDIR to be added as well as .DOTLAST
>>to get the desired behaviour.

>But this is what it is supposed to do... I don't really want to do
>the extra stat() because then we cheat and we look at `.' when we
>are supposed to only look at it last; also we need to make sure

Yes, but all this lookup code currently special cases '.' and 
paths that start with '/', paths that start with '../' are really 
not handled correctly at all.

>that the two paths resolve to the same file, and that will require
>2 stats(). One way is to use size_t len = strlen(path); strncmp(path,
>file, len) && file[len] == '/' but that does not feel right. Another

That (absent the last check for '/') is what I'm doing, adding the check for 
'/' is a good idea.  In the case of a hit there is no 2nd stat call 
the goto skips by it ;-)

>way is to parse the /../ and collapse them. Again this is ucky and
>does not work in all cases.

And would defeat the purpose of putting a relative path in .PATH.

The only other thing I can think of is a new magic token .ASIS or something
Then you could have:

.PATH: .ASIS ${RELCURDIR} .DOTLAST

but I think the path is a prefix of name and stat check is reasonable.
The ugly bit is the goto - but the alternative is an extra stat(2) which
I'd like to avoid.  Ie. we'd have:

--- dir.c.~1~	Mon Sep 16 16:16:14 2002
+++ dir.c	Wed Nov 13 17:36:32 2002
@@ -797,8 +797,24 @@ DirLookupSubdir(Path *p, char *name)
     struct stat	  stb;		/* Buffer for stat, if necessary */
     Hash_Entry	 *entry;	/* Entry for mtimes table */
     char 	 *file;		/* the current filename to check */
+    size_t	len;
 
     if (p != dot) {
+	if (p->name[0] == '.' && p->name[1] == '.' && p->name[2] == '/' &&
+	    strncmp(p->name, name, (len = strlen(p->name))) == 0 &&
+	    name[len] == '/' &&
+	    stat (name, &stb) == 0) {
+	    /*
+	     * We have a ../../ path in .PATH and we've previously found
+	     * a file via it and stored it in .depend, so 'name' is
+	     * correct as is. 
+	     */
+	    file = estrdup(name);
+	    if (DEBUG(DIR)) {
+		printf("checking %s...", file);
+	    }
+	    goto gotit;			/* skip the stat(2) */
+	}
 	file = str_concat (p->name, name, STR_ADDSLASH);
     } else {
 	/*
@@ -812,6 +828,7 @@ DirLookupSubdir(Path *p, char *name)
     }
 
     if (stat (file, &stb) == 0) {
+    gotit:
 	if (DEBUG(DIR)) {
 	    printf("got it.\n");
 	}

Thanks
--sjg