bugmake - Bugs: bug #63516, `include` of absolute path...

 
 

bug #63516: `include` of absolute path prepends path with `./`

Submitter:  None
Submitted:  Mon 12 Dec 2022 10:17:30 PM UTC
   
 
Severity:  3 - Normal Item Group:  Bug
Status:  Fixed Privacy:  Public
Assigned to:  psmith Open/Closed:  Closed
Component Version:  4.4 Operating System:  MS Windows
Fixed Release:  4.4.1 Triage Status:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Tue 20 Dec 2022 07:29:11 AM UTC, comment #10: 

Fix pushed.  Thanks for the report!

Paul D. Smith <psmith>
Group administrator
Fri 16 Dec 2022 03:14:46 PM UTC, comment #9: 


> The file name "d:foo" means on Windows "the file 'foo' in the directory that is current on drive 'd'


Yep, I don't know much about Windows but I do know that much! :)

> So in my opinion we should treat such file names as absolute, because prepending a "./" to them is wrong.


Right so you're not saying that all instances of "D:foo" should be considered absolute, you're saying in this specific situation where we are trying to decide whether to prepend with "./", we should not prepend with "./" (just as if it were absolute).  That was my question, thanks!

Paul D. Smith <psmith>
Group administrator
Fri 16 Dec 2022 02:56:53 PM UTC, comment #8: 

The file name "d:foo" means on Windows "the file 'foo' in the directory that is current on drive 'd'.  Yes, windows programs can have a separate current directory on each drive.  You can see that if you do the following dance:

  C:\> cd foobar
  C:\foobar> d:
  D:\> cd quux
  D:\quux> c:
  C:\foobar> dir d:

The last command will show the listing of d:\quux, not of d:\.

IOW, "d:foo" is the same as "d:./foo".

So in my opinion we should treat such file names as absolute, because prepending a "./" to them is wrong.

Eli Zaretskii <eliz>
Group Member
Fri 16 Dec 2022 02:03:48 PM UTC, comment #7: 

Thanks for the patch but there's no need to spend more time on this; as I said I have a fix.  During this time of year I've got a lot going on so it takes me a bit longer to finish things.

But Eli I was curious about your comment that we should treat a drive-relative path as absolute even if it isn't (doesn't start with a "/" or "\").  You are the Windows expert and I defer to you on these issues but I wouldn't have thought that so can you clarify?  Thanks!

Do you mean in this specific instance, where we are potentially going to add "./" to it (why are we doing that?  I need to go back and look at the code)?  Or do you mean in general?

Paul D. Smith <psmith>
Group administrator
Fri 16 Dec 2022 01:38:53 PM UTC, comment #6: 


comment #5:

> Thanks for the patch.
>
> I have three comments:
>
> (1) The test under "WINDOWS32" should allow a backslash after the colon as well as a forward slash.


Sounds right.

>
> (2) Do we want Make to consider "drive-relative" file names, as in "d:foo/bar", relative or absolute?  I think the latter, so I suggest not to require the 3rd character to be a slash, in which case (1) gets solved automatically.


`d:foo/bar` is wierd; it's relative if your current drive is `d:`, otherwise it's absolute.

>
> (3) Unis-style absolute file names, like "/foo/bar", should also be considered "absolute" on Windows, and likewise "\foo\bar".  Which brings the backslash issue back.


You're right.  I had just made a quick change to keep me going.  The proper changes will be somewhat more complicated.  If I get the chance to provide a proper fix before I see a patch in the repo, I'll post a better fix here.

On the `bootstrap.bat` change: I don't know if that is peculiar to my setup; I'm running from powershell.  I don't think that that would make a difference to the interpretation of `.bat` files, but windows quoting is weird, so who knows.

Thanks for the feedback.

Anonymous
Fri 16 Dec 2022 07:06:23 AM UTC, comment #5: 

Thanks for the patch.

I have three comments:

(1) The test under "WINDOWS32" should allow a backslash after the colon as well as a forward slash.

(2) Do we want Make to consider "drive-relative" file names, as in "d:foo/bar", relative or absolute?  I think the latter, so I suggest not to require the 3rd character to be a slash, in which case (1) gets solved automatically.

(3) Unis-style absolute file names, like "/foo/bar", should also be considered "absolute" on Windows, and likewise "\foo\bar".  Which brings the backslash issue back.


Eli Zaretskii <eliz>
Group Member
Thu 15 Dec 2022 09:16:35 PM UTC, comment #4: 

I've attached my fix in case it's helpful.  Also attached is a small fix to `bootstrap.bat` that I had to make to get it to build with `tcc`.

(file #54105, file #54106)

Anonymous
Mon 12 Dec 2022 11:03:05 PM UTC, comment #3: 

Yes, that's it.  I have a fix and will put in up a patch shortly.

Paul D. Smith <psmith>
Group administrator
Mon 12 Dec 2022 10:40:38 PM UTC, comment #2: 

Line 376 in `src/read.c` may be the culprit:


  if (ebuf.fp == NULL && deps->error == ENOENT && (flags & RM_INCLUDED)
      && *filename != '/' && include_directories)


Looks like it's trying to determine whether the path is absolute in a way that doesn't work under windows.

Anonymous
Mon 12 Dec 2022 10:22:57 PM UTC, comment #1: 

`make` was installed with `scoop`.

Anonymous
Mon 12 Dec 2022 10:17:30 PM UTC, original submission:  

When trying to `include` a file with an absolute path (including drive path), the include directive prepends a `./` to the path, causing the `include` directive to fail.

E.g.


OUTPUT_DIR := $(abspath .)
DEPS_FILE := $(OUTPUT_DIR)/makefile-deps.mk
include $(DEPS_FILE)
$(DEPS_FILE): $(CSRCS) $(OUTPUT_DIR)
        @echo MAKEDEPS
        @touch $@


outputs the following error:


makefile:6: ./D:/Users/t0031450/Workspace/test/make/makefile-deps.mk: Invalid argument
make: *** No rule to make target './D:/Users/t0031450/Workspace/test/make/makefile-deps.mk'.  Stop.


I've verified that the `DEPS_FILE` variable does not contain the `./` prefix, so it looks like it is prepended by the `include` directive itself.

Anonymous

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files
file #54105:  0001-Remove-extra-backslash-in-sed-invocation.patch added by None (878B - application/octet-stream)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by eliz (Posted a comment)
  • -email is unavailable- added by psmith (Posted a comment)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only logged-in users can vote.

     

    Follow 6 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2022-12-20 psmith StatusNone Fixed
        Assigned toNone psmith
        Open/ClosedOpen Closed
        Fixed ReleaseNone 4.4.1
    2022-12-15 None Attached File- Added 0001-Remove-extra-backslash-in-sed-invocation.patch, #54105
        Attached File- Added 0002-Windows-Fix-check-for-absolute-pathnames-of-include-.patch, #54106

    Back to the top

    Powered by Savane 3.13-02a9.
    Corresponding source code