[PATCH] Pass all of spawnw.t on Win32/MSVC++

[Jonathan Worthington]

Hi,

This patch implements a missing function in config\gen\win32\exec.c to make
all of the tests in t\op\spawnw.t work.

Any input on why we have the tests right shift by 8 bits what spwanw gives
them back would be helpful. If its that the exit code on a UNIXy platform is
in the upper byte, then surely it's better to right shift it in the platform
specific stuff (in platform.c), so that an exit code of 1 comes back from
spawnw as 1, not 1 << 8? I'll happily send it a patch to make this so if
there's agreement it's the Right Thing.

Thanks,

Jonathan

[Jerry Gay]

On 7/15/05, Jonathan Worthington <jona...@jwcs.net> wrote:
> Hi,
>
> This patch implements a missing function in config\gen\win32\exec.c to make
> all of the tests in t\op\spawnw.t work.
>

excellent!

> Any input on why we have the tests right shift by 8 bits what spwanw gives
> them back would be helpful. If its that the exit code on a UNIXy platform is
> in the upper byte, then surely it's better to right shift it in the platform
> specific stuff (in platform.c), so that an exit code of 1 comes back from
> spawnw as 1, not 1 << 8? I'll happily send it a patch to make this so if
> there's agreement it's the Right Thing.
>

what spawned processes return was never fully specified, so this test
has remained broken on windows for a long time. your guess is correct;
the exit code is the upper byte in unix-land (how silly.)

if i recall correctly, the options here are: 1) unix-like return value
from spawnw everywhere, 2) return platform-specific value, or 3)
return some object-like thingy.

i don't like option 1, as i believe spawnw should return something
platform-specific. an object-like thingy (option 3) might be nice, and
is definitely shiny, but it doesn't exist, and requires more
specification.

in the meantime, you've (happily!) volunteered to implement option 2,
which i believe is the Right Thing. if nobody else disagrees, i'll not
apply this patch, and instead wait for your new patch which gets
spawnw working in a platform-specific manner.

i'm looking forward to a working spawnw on all platforms.
~jerry

[Jonathan Worthington]

"jerry gay" <jerr...@gmail.com> wrote:
>> Any input on why we have the tests right shift by 8 bits what spwanw
>> gives
>> them back would be helpful. If its that the exit code on a UNIXy platform
>> is
>> in the upper byte, then surely it's better to right shift it in the
>> platform
>> specific stuff (in platform.c), so that an exit code of 1 comes back from
>> spawnw as 1, not 1 << 8? I'll happily send it a patch to make this so if
>> there's agreement it's the Right Thing.
>>
> what spawned processes return was never fully specified, so this test
> has remained broken on windows for a long time. your guess is correct;
> the exit code is the upper byte in unix-land (how silly.)
>

I'm sure there's a reason for it, though I can't think what.

> if i recall correctly, the options here are: 1) unix-like return value
> from spawnw everywhere, 2) return platform-specific value, or 3)
> return some object-like thingy.
>
> i don't like option 1, as i believe spawnw should return something
> platform-specific. an object-like thingy (option 3) might be nice, and
> is definitely shiny, but it doesn't exist, and requires more
> specification.
>

Sorry, I wasn't clear in what I wrote. I was suggesting a 4th option -
always return the exit code so the user can use it right away, as in we >> 8
it on UNIX platforms, leave it as is on Win32, etc. So if the exit code is
1, then Ix contains 1. No matter which platform you're on. User code has
to do no shifts.

Part of the point of writing a VM is to hide all that it's appropriate to
hide about the underlying OS and platform to stuff at a bytecode level.
This is one thing we should be hiding. I don't want my bytecode that uses
spawnw to work on UNIX just fine, but break on Win32. Which is the current
situation if we leave out the thing we've got now to do option 1, or if we
return whatever the OS gives us back without touching it.

> in the meantime, you've (happily!) volunteered to implement option 2,
> which i believe is the Right Thing. if nobody else disagrees, i'll not
> apply this patch, and instead wait for your new patch which gets
> spawnw working in a platform-specific manner.
>

Seems we slight disagree on what is the Right Thing. I'm going to be away
for a few days too (visiting family, seeing Rammstein play) so I suggest
apply this now and if there's agreement on what to do when I return, I'll
implement the other changes then.

Thanks,

Jonathan

[Jonathan Worthington]

"Jonathan Worthington" <jona...@jwcs.net> wrote:

> "jerry gay" <jerr...@gmail.com> wrote:
>> if i recall correctly, the options here are: 1) unix-like return value
>> from spawnw everywhere, 2) return platform-specific value, or 3)
>> return some object-like thingy.
>>
>> i don't like option 1, as i believe spawnw should return something
>> platform-specific. an object-like thingy (option 3) might be nice, and
>> is definitely shiny, but it doesn't exist, and requires more
>> specification.
>>
> Sorry, I wasn't clear in what I wrote. I was suggesting a 4th option -
> always return the exit code so the user can use it right away, as in we >>
> 8 it on UNIX platforms, leave it as is on Win32, etc. So if the exit code
> is 1, then Ix contains 1. No matter which platform you're on. User code
> has to do no shifts.
>

Just went and looked at what $? holds in Perl 5 and, well, it holds the
POSIX value. And the lower 8 bits actually can contain something useful
(the signal that terminated the program and whether it core dumped). So my
idea would have thrown away potentially useful info. That'd suck.

I still think consistent is important, not something platform specific. So
here's my patch again, with the comments cleared up to say why we left shift
by 8. I think it's the right thing to do.

All spawnw.t passes on Win32 with this patch.

Thanks,

Jonathan

[Jerry Gay]

after further discussion (on #parrot) with chip and others, chip
decided that the proper option for spawnw is to return an object-like
thingy. and after this discussion, i fully agree, and take back my
previous statements about platform-specific values being the way to
go. that said...

> All spawnw.t passes on Win32 with this patch.
>

i'll apply this patch today. additionally, i'll add some text to
ops/sys.ops specifying that spawnw should return something
object-like. i've created a TODO ticket in RT for the spawnw
object-like return.

~jerry

[Jerry Gay]

On 7/21/05, jerry gay <jerr...@gmail.com> wrote:
> i'll apply this patch today. additionally, i'll add some text to
> ops/sys.ops specifying that spawnw should return something
> object-like. i've created a TODO ticket in RT for the spawnw
> object-like return.
>

applied, r8660. also opened [perl #36619].