[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: .ONESEHLL not working as expected in 3.82
From: |
Paul Smith |
Subject: |
Re: .ONESEHLL not working as expected in 3.82 |
Date: |
Sun, 28 Apr 2013 15:15:09 -0400 |
On Sun, 2013-04-28 at 21:14 +0300, Eli Zaretskii wrote:
> > From: Paul Smith <address@hidden>
> > Cc: address@hidden, address@hidden
> > 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.
- Re: .ONESEHLL not working as expected in 3.82, (continued)
- Re: .ONESEHLL not working as expected in 3.82, Paul Smith, 2013/04/27
- Re: .ONESEHLL not working as expected in 3.82, Eli Zaretskii, 2013/04/27
- Re: .ONESEHLL not working as expected in 3.82, Paul Smith, 2013/04/27
- Re: .ONESEHLL not working as expected in 3.82, Eli Zaretskii, 2013/04/27
- Re: .ONESEHLL not working as expected in 3.82, Paul Smith, 2013/04/27
- Re: .ONESEHLL not working as expected in 3.82, Eli Zaretskii, 2013/04/28
- Re: .ONESEHLL not working as expected in 3.82, Paul Smith, 2013/04/28
- Re: .ONESEHLL not working as expected in 3.82, Eli Zaretskii, 2013/04/28
- Re: .ONESEHLL not working as expected in 3.82, Eli Zaretskii, 2013/04/29
Message not available
Re: .ONESEHLL not working as expected in 3.82, Eli Zaretskii, 2013/04/29