Re: [PATCH] t/spawn: Find invalid PID to try to join its process group

[Thiago Jung Bauermann <bauermann@kolabnow.com>]

Hello Eric,

Thank you for responding so quickly!

Eric Wong <e@80x24.org> writes:

> Thiago Jung Bauermann <bauermann@kolabnow.com> wrote:
>> In the container used to build packages of the GNU Guix distribution, PID 1
>> runs as the same user as the test so this spawn that should fail actually
>> succeeds.
>
> Ah, interesting...

It's amazing how much variation there is out there, right? :-)

>> First of all, thank you for public-inbox! Mailing list archives are so much
>> better now for projects that use this project!
>> 
>> I've also been using lei and it's extremely helpful, thank you for this new
>> tool as well.
>
> You're welcome, hoping I'll soon be able to get my brain working
> again to work on it..

Sorry to hear about your brain state. Hope that hacking on public-inbox
is fun and exciting and helps you get it working again!

>> I packaged v1.8.0 for the GNU Guix distribution, but its build environment
>> causes this test to fail. This patch fixes it.
>
> Thanks for packaging and this patch.

You're most welcome. Guix was stuck in version 1.6 for various reasons
and lei kept coming up in the Guix mailing list during discussions about
improved email workflows. We were eager to get at least 1.7 so that we
could all enjoy it.

>> I'm not a Perl developer, so please let me know if there's anything I should
>> improve in the code.
>
> No worries, neither are most people these days :)  Some comments below.

Thank you for the comments.

>> diff --git a/t/spawn.t b/t/spawn.t
>> index 6168c1f6171c..4af215809962 100644
>> --- a/t/spawn.t
>> +++ b/t/spawn.t
>> @@ -24,7 +24,17 @@ SKIP: {
>>  	is(waitpid($pid, 0), $pid, 'waitpid succeeds on spawned process');
>>  	is($?, 0, 'true exited successfully');
>>  	pipe(my ($r, $w)) or BAIL_OUT;
>> -	$pid = eval { spawn(['true'], undef, { pgid => 1, 2 => $w }) };
>> +
>> +        # Find invalid PID to try to join its process group.
>> +        my $wrong_pgid = 1;
>> +        for (my $i=65534; $i >= 2; $i--) {
>> +            if (kill(0, $i) == 0) {
>> +                $wrong_pgid = $i;
>> +                last;
>> +            }
>> +        }
>> +
>
> Perhaps 65534 is enough for common purposes, but my FreeBSD VM
> and newer Linux systems see PIDs greater than that (in the
> millions, even).
>
> Instead of 65534, starting the loop with some absurdly high
> number (e.g 0x7fffffff) seems to work just as quickly on both
> Linux and FreeBSD 12.  Not sure if that causes problems on Guix
> or other systems, but it's unlikely.

Makes sense. 0x7fffffff worked for me so I changed the loop to start
from there.

> Style note: we indent exclusively with hard tabs.  Our indentation
> is more consistent with Linux kernel/git (C) style
> than what is commonly found in CPAN and other Perl projects.

Sorry, I should have paid attention to indentation style and tab vs
spaces. I fixed my code to fix both issues. I'll send a v2 shortly.

I'll also propose a patch adding an .editorconfig file¹ that adjusts
these settings automatically for various editors.

> Thanks again

Thank you again for quickly reviewing this patch.

-- 
Thanks
Thiago

¹ https://EditorConfig.org