bugmake - Bugs: bug #30714, List of shell commands is...

 
 

bug #30714: List of shell commands is outdated/Fallback to shell

Submitter:  Krzysztof Malinowski <raspy>
Submitted:  Mon 09 Aug 2010 12:53:19 PM UTC
   
 
Severity:  3 - Normal Item Group:  Bug
Status:  Fixed Privacy:  Public
Assigned to:  None Open/Closed:  Closed
Component Version:  4.0 Operating System:  MS Windows
Fixed Release:  4.0 Triage Status:  Verified
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Wed 11 Aug 2010 05:39:19 PM UTC, comment #10: 

"It depends on what you consider "normal". It's normal for CreateProcess, of course, but this is not how make should work. I don't see any reason why Windows version should behave differently than Unix version, and I believe that Unix version does not look for a command in the directory where 'make' is
installed in the first place."

Again, it's normal on Windows.  Every Windows program that invokes subsidiary programs behaves like that.  Make is working like that for a long time, so any serious changes are likely to cause trouble somewhere.

"As for shell's command line limit: it doesn't really matter, since this is the shell's limitation and the Makefile maintainer should be aware of limits of the selected shell."

It doesn't help "to be aware" when the command length exceeds the limits.  There's no easy escape, sometimes no escape at all.

"It's wrong, because it doesn't work the way it is supposed to work. 'make' claims to support custom shell selection by setting $(SHELL), but its value is ignored."

That's a different bug report, let's not mix them.  That other bug will be fixed when I have time to work on it.  Here we are talking about SHELL whose value is one of the two shells that Make on Windows pretends to know intimately, like the Unix version does with /bin/sh.  For that use-case, I think Make on Windows behaves like the Unix version does: if the command fails, Make does not try to run it via the shell.

Anyway, I think it's time to wrap this up.  Let me say in conclusion that I think Make works reasonably well on Windows, even though it might not be 100% clean in some cases.  OTOH, it supports 2 standard shells (sh.exe an cmd.exe) while the Unix build supports only one, so some rough corners are inevitable.  It took some non-trivial effort to get where we are now.  Given the feeble resources I'm able to devote to maintaining the Windows build, I cannot even review patches that make revolutionary changes in job.c and be responsible for  their effect on Make, let alone do it myself.  So any serious surgery in how Make invokes programs today will only happen if someone else will take over the maintenance of the Windows port.  FWIW, I will happily relinquish that responsibility to a willing volunteer.

Eli Zaretskii <eliz>
Group Member
Wed 11 Aug 2010 01:54:16 PM UTC, comment #9: 

"But this is normal behavior on Windows."

It depends on what you consider "normal". It's normal for CreateProcess, of course, but this is not how make should work. I don't see any reason why Windows version should behave differently than Unix version, and I believe that Unix version does not look for a command in the directory where 'make' is installed in the first place. This behavior is also not documented in make manual. Manual states that recipe lines "are executed by invoking a new subshell for each line", so one would expect selected shell's behavior: look at current working directory then search through PATH. The shortcut taken here by not running the shell is wrong, because the shell would interpret the command differently.

As for shell's command line limit: it doesn't really matter, since this is the shell's limitation and the Makefile maintainer should be aware of limits of the selected shell.

"Why is this wrong?"

It's wrong, because it doesn't work the way it is supposed to work. 'make' claims to support custom shell selection by setting $(SHELL), but its value is ignored. It is not used for running commands, not even $(SHELL) $(.SHELLFLAGS) <recipe line> as 'make' claims to do.

I propose the following approach:

1. Try to find a specific program to run the same way the shell would do, i.e. current working directory and then through PATH.
2. If the program cannot be found, run $(SHELL) $(.SHELLFLAGS) <recipe line>, as I believe 'make' works on Unix.

This would make 'make' consistent on Unix and Windows.

Krzysztof Malinowski <raspy>
Wed 11 Aug 2010 02:56:44 AM UTC, comment #8: 

"Now imagine that somebody writes a recipe, thinking that it would be parsed by shell (as documentation suggests). He creates for example a script named foo.bat which is being called by the recipe. What he does not know is that someone else created foo.bat in make's directory and this will be the script which will be called by CreateProcess. It's a very hard bug to track down for a Makefile developer, undocumented and non-intuitive. And this is not how the shell would behave."

But this is normal behavior on Windows.

"The second thing I've seen today is that even when a command is recognized as a built-in, it still will not honor $(SHELL) variable. This is because a batch file is created with the recipe line and this batch file is called directly, without using $(SHELL), so this will be loaded and evaluated by the system, with the default shell. This is wrong again."

Why is this wrong?



Eli Zaretskii <eliz>
Group Member
Tue 10 Aug 2010 08:18:42 PM UTC, comment #7: 

I agree that Savannah is not too well suited for discussion, but I would like to keep it here in order to track how the discussion develops. And maybe other people will join us :-)

I know that explicit call to "cmd /c move" will do the trick, but I think that's ridiculous to set $(SHELL) and then still calling cmd explicitly. Additionally it's against documentation (and maybe make requirements - I don't know) since it states that a subshell is called for a recipe line and this doesn't happen. This way make breaks at perfectly valid recipe shell script.

As for the places where the shell looks for programs: that's true that CreateProcess looks in additional places, but the question is whether this is the behavior that make should follow. After all, when creating a recipe one would assume that (s)he's writing a shell script (as all the documentation suggests). And the shell lookup should be considered correct: first current working directory, then through PATH. CreateProcess looks to make's directory (which is not intuitive for somebody writing a recipe) and then it looks to system and windows directory before checking PATH - so it's quite different than shell does.

Now imagine that somebody writes a recipe, thinking that it would be parsed by shell (as documentation suggests). He creates for example a script named foo.bat which is being called by the recipe. What he does not know is that someone else created foo.bat in make's directory and this will be the script which will be called by CreateProcess. It's a very hard bug to track down for a Makefile developer, undocumented and non-intuitive. And this is not how the shell would behave.

The second thing I've seen today is that even when a command is recognized as a built-in, it still will not honor $(SHELL) variable. This is because a batch file is created with the recipe line and this batch file is called directly, without using $(SHELL), so this will be loaded and evaluated by the system, with the default shell. This is wrong again.

Therefore I still believe that make should follow its own rules and invoke the shell as it claims to do (and obey $(SHELL) when doing that).

(And as for shell command line length: it appears that cmd.exe prior to Windows 2000 has 2K limit, while newer have 8K limit. See http://support.microsoft.com/kb/830473 for reference.)

Krzysztof Malinowski <raspy>
Tue 10 Aug 2010 05:43:42 PM UTC, comment #6: 

Savannah is not suited well for having a discussion.  (Maybe we should start a thread on make-w32@gnu.org, if this is going to continue.)  I respond to some of the comments after quoting the relevant portion of it.

>As for MOVE being external program in Windows 9x: please note >that Windows 9x uses command.com, while NT line uses cmd.exe -


Not true: there's Windows ME that uses cmd.exe, but is still W9X vintage under the hood.  It's a mess.

>Please also note that you already have cmd.exe's specific >commands on the list (like setlocal/endlocal).


These are never external programs, so the problem does not arise.

>Again, falling back to running shell would keep such problems >away.


So would an explicit use of "cmd /c move".  I actually think that this is TRT in a Makefile that should run on Windows, because you can never know what shells are installed on the end-user's machine.  For example, if there's a sh.exe anywhere on PATH, Make will invoke it by default, and then "move" will fail anyway.

>How far do you expect you can get with logic with people who >think that throwing two internal function names
>("process_begin: CreateProcess") is anyway good enough as an >error message for you stupid windoze user.


People who coded that are no longer active in Make maintenance.  Feel free to submit patches to make the diagnostics more user-friendly, such patches will be welcome here.  Alternatively, feel free to file a separate bug report about that.

>My point here is that the shell should be called here, not the >system directly. After all the system won't do anything more >than the shell would do, but it can do less


Are you sure?  AFAIR, CreateProcess doesn't only search PATH, but also a few other places, see the MS docs.  Are you positive that the shell searches exactly those same places?  What about with the directory where Make's executable lives, for example? will the shell look there?

You see, these additional places where CreateProcess looks for a program are one reason why we call CreateProcess even though the search along PATH fails.

(Invoking the shell has another disadvantage: cmd.exe supports only 4K long command lines, while CreateProcess supports 32K.)

Eli Zaretskii <eliz>
Group Member
Tue 10 Aug 2010 03:50:50 PM UTC, comment #5: 

But this is exactly the case. Searching for 'move' in PATH fails, so shell should be started to parse the command. However, instead of starting the shell the command is just being passed to the system, and the system fails to run it.

Let me illustrate it. Suppose the following Makefile:

foo: bar
        move /y $< $@

bar: baz
        copy $< $@

baz:
        mem > $@

'mem' in baz target is C:\WINDOWS\SYSTEM32\mem.exe, 'copy' in bar target is a recognized built-in, while 'move' in foo target is an unrecognized built-in.

See what happens:

$> make -d
[...]
Updating goal targets....
Considering target file `foo'.
 File `foo' does not exist.
  Considering target file `bar'.
   File `bar' does not exist.
    Considering target file `baz'.
     File `baz' does not exist.
     Finished prerequisites of target file `baz'.
    Must remake target `baz'.
Creating temporary batch file D:\Profiles\akm022\LOCALS~1\Temp\make8964-1.bat
mem > baz
CreateProcess(D:\Profiles\akm022\LOCALS~1\Temp\make8964-1.bat,D:\Profiles\akm022\LOCALS~1\Temp\make8964-1.bat,...)
Putting child 0x009647b0 (baz) PID 9860272 on the chain.
Live child 0x009647b0 (baz) PID 9860272
Main thread handle = 0x000007c0
Reaping winning child 0x009647b0 PID 9860272
Cleaning up temp batch file D:\Profiles\akm022\LOCALS~1\Temp\make8964-1.bat
Removing child 0x009647b0 PID 9860272 from chain.
    Successfully remade target file `baz'.
   Finished prerequisites of target file `bar'.
  Must remake target `bar'.
Creating temporary batch file D:\Profiles\akm022\LOCALS~1\Temp\make8964-1.bat
copy baz bar
CreateProcess(D:\Profiles\akm022\LOCALS~1\Temp\make8964-1.bat,D:\Profiles\akm022\LOCALS~1\Temp\make8964-1.bat,...)
Putting child 0x009669a0 (bar) PID 9860272 on the chain.
Live child 0x009669a0 (bar) PID 9860272
        1 file(s) copied.
Reaping winning child 0x009669a0 PID 9860272
Cleaning up temp batch file D:\Profiles\akm022\LOCALS~1\Temp\make8964-1.bat
Removing child 0x009669a0 PID 9860272 from chain.
  Successfully remade target file `bar'.
 Finished prerequisites of target file `foo'.
Must remake target `foo'.
move /y bar foo
CreateProcess(NULL,move /y bar foo,...)
process_begin: CreateProcess(NULL, move /y bar foo, ...) failed.
Putting child 0x009687e8 (foo) PID 9860272 on the chain.
Live child 0x009687e8 (foo) PID 9860272
make (e=2): The system cannot find the file specified.
Reaping losing child 0x009687e8 PID 9860272
make: * [foo] Error 2

Generally for baz and bar targets shell is executed (batch file created and run - note the first argument to CreateProcess is non-NULL). This happens when either a) command is found on PATH or b) command is a known built-in. What happens in foo target is that make tries to run the command directly through the system, which is against what documentations states about it ("they are executed by invoking a new subshell for each line"). My point here is that the shell should be called here, not the system directly. After all the system won't do anything more than the shell would do, but it can do less because of shell built-ins which are valid only in shell (and the system won't process them).

Krzysztof Malinowski <raspy>
Tue 10 Aug 2010 02:30:40 PM UTC, comment #4: 

I don't have any Windows systems; all my systems run Linux or some UNIX variant.  So, I can't support Windows myself and I rely on a group of volunteers to maintain that platform (just like with VMS and other non-POSIX platforms).

One concern with "falling back" is that you don't want to do it if the command fails for some reason other than the command is not found.  We definitely do not want to try to fall back to a shell method in any other error case.  I don't know enough Windows to know if we can determine that accurately.

Paul D. Smith <psmith>
Group administrator
Tue 10 Aug 2010 01:24:05 PM UTC, comment #3: 

How far do you expect you can get with logic with people who think that throwing two internal function names ("process_begin: CreateProcess") is anyway good enough as an error message for you stupid windoze user.

Anonymous
Tue 10 Aug 2010 08:21:13 AM UTC, comment #2: 

Well, the documentation states:

"When it is time to execute recipes to update a target, they are executed by invoking a new subshell for each line of the recipe, unless the .ONESHELL special target is in effect (see Using One Shell) (In practice, make may take shortcuts that do not affect the results.)"

So basically it means, that every recipe line should go through a shell. And in practice make takes shortcut which DOES affect the result - doesn't run the command through shell, although the shell would have no problems with executing the command.

As for MOVE being external program in Windows 9x: please note that Windows 9x uses command.com, while NT line uses cmd.exe - the list of known commands should be set depending on which shell is used. Please also note that you already have cmd.exe's specific commands on the list (like setlocal/endlocal). Furthermore, I was talking about falling back to shell when the command is not found - in case that MOVE is actually expanded to move.exe it should be run directly anyway, but even if not, the shell would still expand MOVE command on its own.

One more issue that I have found is that current implementation ignores the fact, that Windows shells are case-insensitive. Therefore if the Makefile actually looks like this:

foo: bar
        move /y $< $@

bar:
        COPY NUL $@

then make fails even sooner, on creating bar, since it doesn't recognize COPY as a built-in command:

$> make
COPY NUL bar
process_begin: CreateProcess(NULL, COPY NUL bar, ...) failed.
make (e=2): The system cannot find the file specified.
make: * [bar] Error 2

Again, falling back to running shell would keep such problems away.

Krzysztof Malinowski <raspy>
Mon 09 Aug 2010 05:04:22 PM UTC, comment #1: 

Unfortunately, the issue is a little bit more complicated: the MOVE command is a built-in on some versions of Windows, but an external program move.exe on others (Windows 9X).  That (and not outdated information regarding the current Windows shells) is the reason why MOVE is not in the list, while COPY is: we don't want to fall back to the shell unless we know a command is a built-in.

So the short-term solution you propose is a non-starter.

Regarding the fallback to the shell: I'm not sure this is a good idea, because that's not how GNU Make works on Unix (AFAIK).  The Windows code generally follows the Unix code, except where deviations are absolutely necessary, but even then it tries very hard not to change the general behavior.

Regarding a non-standard shell: I believe Make 3.82 should already support the feature whereby any unknown shell called out by the SHELL variable causes all the commands to be passed to that shell.  If that does not work on Windows (I didn't yet have time to try it), please feel free to file a separate bug report.  I believe the Unix version does support that, so the Windows build should do the same.

Unless you have convincing counter-arguments, I tend to close this bug as "Wont fix".

Eli Zaretskii <eliz>
Group Member
Mon 09 Aug 2010 12:53:19 PM UTC, original submission:  

List of known shell commands is hardcoded in GNU make's source code. This leads to problems when trying to run a command which is not on the list. Consider the following Makefile:

foo: bar
move /y $< $@

bar:
copy nul $@

When running such Makefile it fails:

$> make

copy nul bar
        1 file(s) copied.
move /y bar foo
process_begin: CreateProcess(NULL, move /y bar foo, ...) failed.
make (e=2): The system cannot find the file specified.
make: * [foo] Error 2

'copy' command is recognized and is being run through shell, while 'move' command is not on the list and is being passed to Windows with hope that system will somehow know how to run it. Unfortunately this is a built-in command and it cannot be run outside of shell.

There is a short-term and a long-term resolution for that. In the short-term the list of known commands should be updated (http://ss64.com/nt/ has a quite good list of what's built-in). But in order to work really reliably, the algorithm should be changed: when an unknown command is met, run the shell and pass the command through shell instead of operating system.

Current implementation is limited when it comes to shell support: suppose that somebody changes shell by setting SHELL to something different, i.e. PowerShell. There can be completely different commands, so hardcoded list of commands will not be suitable; all commands are routed to Windows, totally bypassing $(SHELL) which in turn appears to be useless.

The issue is also reproducible on 3.82.

Krzysztof Malinowski <raspy>

 

(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 psmith (Posted a comment)
  • -email is unavailable- added by eliz (Posted a comment)
  • -email is unavailable- added by raspy (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 5 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2013-05-18 eliz StatusNone Fixed
        Open/ClosedOpen Closed
        Component Version3.81 4.0
        Fixed ReleaseNone 4.0
        Triage StatusNone Verified

    Back to the top

    Powered by Savane 3.13-758e.
    Corresponding source code