[bug #56449] job.c (construct_command_argv_internal): Must use shell if '%' character is present in recipe line

classic Classic list List threaded Threaded
16 messages Options
Reply | Threaded
Open this post in threaded view
|

[bug #56449] job.c (construct_command_argv_internal): Must use shell if '%' character is present in recipe line

anonymous
URL:
  <https://savannah.gnu.org/bugs/?56449>

                 Summary: job.c (construct_command_argv_internal): Must use
shell if '%' character is present in recipe line
                 Project: make
            Submitted by: None
            Submitted on: Thu 06 Jun 2019 11:13:33 AM UTC
                Severity: 3 - Normal
              Item Group: Bug
                  Status: None
                 Privacy: Public
             Assigned to: None
             Open/Closed: Open
         Discussion Lock: Any
       Component Version: SCM
        Operating System: MS Windows
           Fixed Release: None
           Triage Status: None

    _______________________________________________________

Details:

The slow path must be taken if a shell escape character ('%') is present in
the command line. Otherwise escaped characters on the command line will not be
properly unescaped by the shell.


Demo:
======

argv1.c:
--------%<----------------------
#include <stdio.h>

int main(int argc, char *argv[])
{
        puts(argv[1]);
        return 0;
}
-------->%----------------------

> gcc -Wall -o argv1.exe argv1.c

Makefile:
--------%<----------------------
$ cat Makefile
.PHONY: all

SHELL = cmd.exe

all:
        @argv1 %%
        @argv1 "%%"
        @argv1 %% > con

-------->%----------------------

*** Please run mingw32-make from cmd.exe, not from Msys shell ***

> mingw32-make.exe
%%
%%
%


Expected result:
%
%
%

Result after applying attached patch:
%
%
%





    _______________________________________________________

File Attachments:


-------------------------------------------------------
Date: Thu 06 Jun 2019 11:13:33 AM UTC  Name:
0001-src-job.c-construct_command_argv_internal-Use-shell-.patch  Size: 2KiB  
By: None

<http://savannah.gnu.org/bugs/download.php?file_id=47045>

    _______________________________________________________

Reply to this item at:

  <https://savannah.gnu.org/bugs/?56449>

_______________________________________________
  Message sent via Savannah
  https://savannah.gnu.org/


_______________________________________________
Bug-make mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/bug-make
Reply | Threaded
Open this post in threaded view
|

[bug #56449] job.c (construct_command_argv_internal): Must use shell if '%' character is present in recipe line

anonymous
Update of bug #56449 (project make):

                  Status:                    None => Fixed                  
             Open/Closed:                    Open => Closed                

    _______________________________________________________

Follow-up Comment #1:

Thanks for the bug report!

    _______________________________________________________

Reply to this item at:

  <https://savannah.gnu.org/bugs/?56449>

_______________________________________________
  Message sent via Savannah
  https://savannah.gnu.org/


_______________________________________________
Bug-make mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/bug-make
Reply | Threaded
Open this post in threaded view
|

[bug #56449] job.c (construct_command_argv_internal): Must use shell if '%' character is present in recipe line

anonymous
Update of bug #56449 (project make):

             Open/Closed:                  Closed => Open                  

    _______________________________________________________

Follow-up Comment #2:

I see that this bug was fixed and closed, but looking at the result, I'm not
sure the fix is correct.  If I invoke a Windows program from the cmd.exe
prompt with %% as argument, the program gets "%%" as its argv[1], not just a
single %.

So I guess I don't understand why the OP expected to get a single % character.
 Am I missing something?


    _______________________________________________________

Reply to this item at:

  <https://savannah.gnu.org/bugs/?56449>

_______________________________________________
  Message sent via Savannah
  https://savannah.gnu.org/


_______________________________________________
Bug-make mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/bug-make
Reply | Threaded
Open this post in threaded view
|

[bug #56449] job.c (construct_command_argv_internal): Must use shell if '%' character is present in recipe line

anonymous
Follow-up Comment #3, bug #56449 (project make):

I wasn't thinking about % as an escape character so much as a variable
introduction character.

But I keep forgetting that variable expansion in Windows works very
differently than in POSIX.

I was assuming that:


all: ; argv1 %Path%


would need to use command.com to ensure that %Path% was expanded.  But maybe
that's not correct on Windows.

    _______________________________________________________

Reply to this item at:

  <https://savannah.gnu.org/bugs/?56449>

_______________________________________________
  Message sent via Savannah
  https://savannah.gnu.org/


_______________________________________________
Bug-make mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/bug-make
Reply | Threaded
Open this post in threaded view
|

[bug #56449] job.c (construct_command_argv_internal): Must use shell if '%' character is present in recipe line

anonymous
Follow-up Comment #4, bug #56449 (project make):

It is true that %Path% on a command line should expand to the value of Path in
the environment.  It is also true that to have this expansion, we need to
invoke the command through the shell.  However, this is only true for any
arbitrary %FOO% where an environment variable FOO is defined.  If FOO is not
defined, then %FOO% is expanded to just %FOO%, i.e. is not changed at all.  In
particular, %% is left alone by the shell.

So to support %FOO% expansion correctly, we need to figure out whether FOO is
defined or not.  IOW, we need to call 'getenv', at least.  And even that might
not be 100% accurate, since the command could modify the environment as part
of itself (but maybe we should ignore this possibility, even if it exists).


    _______________________________________________________

Reply to this item at:

  <https://savannah.gnu.org/bugs/?56449>

_______________________________________________
  Message sent via Savannah
  https://savannah.gnu.org/


_______________________________________________
Bug-make mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/bug-make
Reply | Threaded
Open this post in threaded view
|

[bug #56449] job.c (construct_command_argv_internal): Must use shell if '%' character is present in recipe line

anonymous
Follow-up Comment #5, bug #56449 (project make):

Sorry, I think there's some confusion.  All this patch does is detect if we
see a % in the command line and if so we do not take the fast path: instead we
take the slow path and invoke command.com.

So we're not trying to handle environment variables etc., we're just saying
"hey make, if the recipe contains % then give up and pass it to command.com to
take care of".

    _______________________________________________________

Reply to this item at:

  <https://savannah.gnu.org/bugs/?56449>

_______________________________________________
  Message sent via Savannah
  https://savannah.gnu.org/


_______________________________________________
Bug-make mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/bug-make
Reply | Threaded
Open this post in threaded view
|

[bug #56449] job.c (construct_command_argv_internal): Must use shell if '%' character is present in recipe line

anonymous
Follow-up Comment #6, bug #56449 (project make):

But the result is wrong when %..% doesn't specify a known variable, most
probably because we invoke the command through a batch file.

Try this from cmd.exe prompt:

 > echo %foo%

You will see %foo%.

Now try with make:

  D:\usr\eli>\gnu\make-4.2.90-guile\GccRel\gnumake -f-
  SHELL=cmd.exe

  all:
          @echo %foo%
  ^Z
  ECHO is off.

In other words, 'echo' was invoked with an empty string, which is not what we
want.


    _______________________________________________________

Reply to this item at:

  <https://savannah.gnu.org/bugs/?56449>

_______________________________________________
  Message sent via Savannah
  https://savannah.gnu.org/


_______________________________________________
Bug-make mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/bug-make
Reply | Threaded
Open this post in threaded view
|

Re: [bug #56449] job.c (construct_command_argv_internal): Must use shell if '%' character is present in recipe line

Eli Zaretskii
> From: Eli Zaretskii <[hidden email]>
> Date: Wed, 28 Aug 2019 15:24:21 -0400 (EDT)
>
> Follow-up Comment #6, bug #56449 (project make):
>
> But the result is wrong when %..% doesn't specify a known variable, most
> probably because we invoke the command through a batch file.
>
> Try this from cmd.exe prompt:
>
>  > echo %foo%
>
> You will see %foo%.
>
> Now try with make:
>
>   D:\usr\eli>\gnu\make-4.2.90-guile\GccRel\gnumake -f-
>   SHELL=cmd.exe
>
>   all:
>           @echo %foo%
>   ^Z
>   ECHO is off.
>
> In other words, 'echo' was invoked with an empty string, which is not what we
> want.

Christian, would you please chime in and explain why you think your
change makes GNU Make's behavior on Windows more correct?  In my
testing, the results are against my expectations in all cases except
when the string between the percent signs refers to an existing
environment variable.

TIA

_______________________________________________
Bug-make mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/bug-make
Reply | Threaded
Open this post in threaded view
|

[bug #56449] job.c (construct_command_argv_internal): Must use shell if '%' character is present in recipe line

anonymous
In reply to this post by anonymous
Follow-up Comment #7, bug #56449 (project make):

Hi, I'm the original author of this bug. It's some time ago I wrote this, so
maybe I've forgotten some details...

[comment #2 comment #2:]
> I see that this bug was fixed and closed, but looking at the result, I'm not
sure the fix is correct.  If I invoke a Windows program from the cmd.exe
prompt with %% as argument, the program gets "%%" as its argv[1], not just a
single %.
>
> So I guess I don't understand why the OP expected to get a single %
character.  Am I missing something?

I guess that cmd.exe may distinguish between "interactive mode" and "script
mode".
So in interactive mode it will not change double % to a single %. But in
script mode it does. For this moment I have no windows machine, but on
wineconsole I get different output when I

- type "echo %%"                         --> I get double %
- insert "echo %%" into a test.cmd file  --> I get single %

I also remember that using *echo* is not a good idea for demonstration with
*make*. As make recognizes echo as a shell builtin it always invokes this via
a shell.
That is the reason why I sent the demo program.

    _______________________________________________________

Reply to this item at:

  <https://savannah.gnu.org/bugs/?56449>

_______________________________________________
  Message sent via Savannah
  https://savannah.gnu.org/


_______________________________________________
Bug-make mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/bug-make
Reply | Threaded
Open this post in threaded view
|

[bug #56449] job.c (construct_command_argv_internal): Must use shell if '%' character is present in recipe line

anonymous
Follow-up Comment #8, bug #56449 (project make):

[comment #5 comment #5:]
> Sorry, I think there's some confusion.  All this patch does is detect if we
see a % in the command line and if so we do not take the fast path: instead we
take the slow path and invoke command.com.
>
> So we're not trying to handle environment variables etc., we're just saying
"hey make, if the recipe contains % then give up and pass it to command.com to
take care of".

Yes, that is exactly what I intended to change.

    _______________________________________________________

Reply to this item at:

  <https://savannah.gnu.org/bugs/?56449>

_______________________________________________
  Message sent via Savannah
  https://savannah.gnu.org/


_______________________________________________
Bug-make mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/bug-make
Reply | Threaded
Open this post in threaded view
|

[bug #56449] job.c (construct_command_argv_internal): Must use shell if '%' character is present in recipe line

anonymous
Follow-up Comment #9, bug #56449 (project make):

[comment #6 comment #6:]
> But the result is wrong when %..% doesn't specify a known variable, most
probably because we invoke the command through a batch file.

The question may be, which of both results is the correct one. So if cmd.exe
has something like an "interactive" and "batch" mode, I personally would
consider the batch mode as the correct one (I would rather enter all my build
commands into a long batch file than typing everything manually).

So I would expect that recipe lines in make behave equally to the batch
mode...

    _______________________________________________________

Reply to this item at:

  <https://savannah.gnu.org/bugs/?56449>

_______________________________________________
  Message sent via Savannah
  https://savannah.gnu.org/


_______________________________________________
Bug-make mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/bug-make
Reply | Threaded
Open this post in threaded view
|

[bug #56449] job.c (construct_command_argv_internal): Must use shell if '%' character is present in recipe line

anonymous
Follow-up Comment #10, bug #56449 (project make):

GNU Make emulates the behavior of the shell as if the user typed the commands
at the shell's prompt.  It is true that you need to double the % characters in
batch files, but GNU Make doesn't behave like batch files do.

So I think this change is for the worse, and should be reverted.


    _______________________________________________________

Reply to this item at:

  <https://savannah.gnu.org/bugs/?56449>

_______________________________________________
  Message sent via Savannah
  https://savannah.gnu.org/


_______________________________________________
Bug-make mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/bug-make
Reply | Threaded
Open this post in threaded view
|

[bug #56449] job.c (construct_command_argv_internal): Must use shell if '%' character is present in recipe line

anonymous
Follow-up Comment #11, bug #56449 (project make):

> I also remember that using echo is not a good idea for demonstration with
make.

I also mentioned calling a Windows program with the argument of %%.  The
program in this case gets the literal two % characters in its argv[] array.

If you invoke your argv.exe program from cmd.exe prompt with "%%" as a
command-line argument, don't you see the same output as what mingw-make.exe
produced before this change?  That is what I see, and that tells me that the
change is simply wrong, as it makes GNU Make behave like a batch file
processor, which is not what's expected.


    _______________________________________________________

Reply to this item at:

  <https://savannah.gnu.org/bugs/?56449>

_______________________________________________
  Message sent via Savannah
  https://savannah.gnu.org/


_______________________________________________
Bug-make mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/bug-make
Reply | Threaded
Open this post in threaded view
|

[bug #56449] job.c (construct_command_argv_internal): Must use shell if '%' character is present in recipe line

anonymous
Follow-up Comment #12, bug #56449 (project make):

[comment #10 comment #10:]
> GNU Make emulates the behavior of the shell as if the user typed the
commands at the shell's prompt.  It is true that you need to double the %
characters in batch files, but GNU Make doesn't behave like batch files do.
>
> So I think this change is for the worse, and should be reverted.

Ack. If I remember correctly, the double % were caused by cmake (unfortunately
I cannot verify this in short term). Maybe cmake expects that for MingW
Makefiles it needs to convert a single % into a double one. If this is wrong,
this behavior should changed in cmake.

    _______________________________________________________

Reply to this item at:

  <https://savannah.gnu.org/bugs/?56449>

_______________________________________________
  Message sent via Savannah
  https://savannah.gnu.org/


_______________________________________________
Bug-make mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/bug-make
Reply | Threaded
Open this post in threaded view
|

[bug #56449] job.c (construct_command_argv_internal): Must use shell if '%' character is present in recipe line

anonymous
Update of bug #56449 (project make):

                Severity:              3 - Normal => 1 - Wish              
                  Status:                   Fixed => Wont Fix              
             Open/Closed:                    Open => Closed                
           Fixed Release:                    None => SCM                    

    _______________________________________________________

Follow-up Comment #13:

Per recent discussions, I've now reverted the fix for this bug, and I'm
closing it as "wont fix".


    _______________________________________________________

Reply to this item at:

  <https://savannah.gnu.org/bugs/?56449>

_______________________________________________
  Message sent via Savannah
  https://savannah.gnu.org/


_______________________________________________
Bug-make mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/bug-make
Reply | Threaded
Open this post in threaded view
|

[bug #56449] job.c (construct_command_argv_internal): Must use shell if '%' character is present in recipe line

anonymous
Update of bug #56449 (project make):

           Fixed Release:                     SCM => None                  


    _______________________________________________________

Reply to this item at:

  <https://savannah.gnu.org/bugs/?56449>

_______________________________________________
  Message sent via Savannah
  https://savannah.gnu.org/


_______________________________________________
Bug-make mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/bug-make