On Sun, 2013-04-28 at 21:14 +0300, Eli Zaretskii wrote: > > From: Paul Smith <p...@mad-scientist.net> > > Cc: make-...@gnu.org, bug-make@gnu.org > > Date: Sat, 27 Apr 2013 12:54:10 -0400 > > > > On Sat, 2013-04-27 at 19:17 +0300, Eli Zaretskii wrote: > > > The .ONESHELL feature is now supported on MS-Windows, for the default > > > Windows shell (cmd.exe) or compatible replacements, in the development > > > version (commit e56aad4). > > > > Nice! > > I see you followed this up with a commit (30843de) which moved code > around that deals with the one_shell use case. What was the reason > for that change? I think it breaks the use case where a Unixy shell > is used on MS-Windows under .ONESHELL; this used to work before.
I moved it because the way you had it caused one of the test cases to fail: you had changed that block of code to be inside the if-statement, when before it was outside/after the if-statement. > The code block that you moved is needed on MS-Windows as well, when > the shell passes the is_bourne_compatible_shell test. I think you're right, sorry. I thought that code was inside a unix-only section but I see now it wasn't. The code used to look like this: if (is_bourne_compatible_shell()) { ... modify the script to remove @-+ ... *t = '\0'; } /* create an argv list ... */ { ... new_argv[n++] = NULL; } return new_argv; So, the setup of the new_argv[] was AFTER and outside the if-statement. In the change you made, it was moved to be INSIDE the if-statement. That caused a test case to fail because in the situation where we do have one-shell and we do not have a POSIX shell, new_argv[] was empty and no command was invoked. I moved it back, but accidentally made it not work for Windows. The goal of this code in the if-statement is to implement a special case allowing ONESHELL to be easier to add in the case where you DO have a standard shell. In that case, and ONLY in that case, we remove the internal @-+ characters. This allows you to have something like: foo: @echo hi @echo there @echo how are you And have it continue to work if you add ONESHELL (for performance reasons) without rewriting all the recipes. However, if you do NOT have a POSIX shell, then we do NOT remove these internal characters: we simply provide the script as-is and only the first line is checked for special characters. This lets you use something like Perl, where @ is a special character, for example: SHELL = /usr/bin/perl foo: @print "hi"; @array = qw(there how are you); print "@array\n"; I think the implementation you have is not quite right. I think the parsing of the @-+ stuff is common across all platforms if we have a shell, so you don't need the "else /* non-posix shell */". I think it pseudo-code it would look something like this: if (posix-shell) { ...strip out @-+ from LINE... } #ifdef WINDOWS32 if (need a batch file) { ...write LINE to the batch file & setup new_argv for batch... } else #endif { ...chop LINE up into new_argv... } return new_argv; Or something. Also, I'm not sure about adding things like @echo off to the batch file. That assumes that we'll always be using command.com to run the batch file, but what if the user specified C:/perl/bin/perl.exe or something as their SHELL? I'm probably missing something about the implementation though. _______________________________________________ Bug-make mailing list Bug-make@gnu.org https://lists.gnu.org/mailman/listinfo/bug-make