bugmake - Bugs: bug #40503, Shebang detection is flawed

 
 

bug #40503: Shebang detection is flawed

Submitter:  Mike Hommey <glandium>
Submitted:  Thu 07 Nov 2013 01:01:58 AM UTC
   
 
Severity:  2 - Minor Item Group:  Enhancement
Status:  None Privacy:  Public
Assigned to:  eliz Open/Closed:  Open
Component Version:  4.1 Operating System:  MS Windows
Fixed Release:  None Triage Status:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Thu 07 Nov 2013 04:20:05 PM UTC, comment #3: 

It will slow down Make for no good reason, for starters.

It also makes no sense with "absolute" paths such as /bin/sh.

But the main reason is: this is an enhancement.  Enhancements should come with a good reason why they would make sense, and asking "why not?" is not such a reason, sorry.

GNU Make on Windows already has quite a lot of hidden features whose aim is to help in some corner use cases.  (E.g., did you know that it transparently supports Unix-style foo:bar:baz format of PATH, even if the directories specify drive letters, as in "d:/foo:c:/bar:z:/baz"?)  These features sometimes interact in unpredictable ways, making maintenance harder, and each additional one makes Make on Windows less maintainable.  I'd like to avoid adding more cruft of this sort, unless it is really useful.

Eli Zaretskii <eliz>
Group Member
Thu 07 Nov 2013 10:49:54 AM UTC, comment #2: 

Any reason not to give the absolute path a try after not finding the .exe via the current algorithm and before failing?

David Boyce <boyski>
Thu 07 Nov 2013 05:27:53 AM UTC, comment #1: 

Detection of Unix scripts with shebang is a compatibility feature, meant for running Makefile's that invoke Unix shell scripts.  On Unix, what follows the shebang is the absolute file name of the interpreter in Unix format, like "/bin/sh" or "/usr/local/bin/perl".  However, it doesn't make sense to use these absolute file names verbatim on Windows, since the chances of having /bin or /usr/local/bin directory on every drive are nil.  Therefore, Make uses the reasonable alternative of using the basename of the interpreter's path, assuming that it will be on PATH.

So far what Make does.  I'm not sure what you suggest to do instead, when you say "same as the logic for recipes".  Please elaborate.

In any case, using the whole absolute file name is a non-starter, as it will punish too many use cases that worked until now.  Likewise, this feature will never use backslashes, since those cannot possibly happen in Unix scripts.  I'm unsure how or why you even have such a script with a shebang.


Eli Zaretskii <eliz>
Group Member
Thu 07 Nov 2013 01:01:58 AM UTC, original submission:  

According to
http://git.savannah.gnu.org/cgit/make.git/tree/w32/subproc/sub_proc.c?id=9d58570c77240fed53d1f88217877f8e778f4bb2#n622

shebang detection just uses the file base name and assumes whatever in $PATH has that name will work.

Except that doesn't work when you deliberately use a path that is not in $PATH for the shebang. We're hitting this in the Firefox build system, where a script has a shebang pointing to a virtualenv python executable, and the script doesn't work with the system python.

I think the shebang executable detection should use the same logic (or kind of) as the logic for recipes.

Ironically, make fails to get the file base name when the path uses back slashes instead of forward slashes, which makes shebang with back slashes have a different behavior.

Mike Hommey <glandium>

 

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

Attach Files:
   
   
Comment:
   

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by boyski (Posted a comment)
  • -email is unavailable- added by eliz (Posted a comment)
  • -email is unavailable- added by glandium (Submitted the item)
  •  

    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 3 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2013-11-07 eliz Severity3 - Normal 2 - Minor
        Item GroupBug Enhancement
        Assigned toNone eliz

    Back to the top

    Powered by Savane 3.13-f8d8.
    Corresponding source code