Raku / old-issue-tracker

Tickets from RT
https://github.com/Raku/old-issue-tracker/issues
2 stars 1 forks source link

t/spec/S16-io/eof.t is unportably assuming that /proc/1/comm exists #5527

Closed p6rt closed 7 years ago

p6rt commented 8 years ago

Migrated from rt.perl.org#128831 (status was 'resolved')

Searchable as RT128831$

p6rt commented 8 years ago

From @nwc10

$ ./perl6-m -Ilib t/spec/S16-io/eof.t
1..2 ok 1 - Regular file EOF was reached Use of Nil in string context in block at t/spec/S16-io/eof.t line 26 Failed to open "/proc/1/comm"​:   in block at t/spec/S16-io/eof.t line 26   in block \ at t/spec/S16-io/eof.t line 24

# Looks like you planned 2 tests, but ran 1

It's assuming both that process 1 exists, and that /proc/1 for it contains a file comm.

(I *think* that the assumption about process 1 is bad. Certainly on Solaris with Zones, the init process may not have pid 1. Perl 5 had a failing regression test until we removed the assumption that process 1 exists. I don't know if every Linux virtualisation system ensures that PID 1 exists. It would be safer to test the /proc entry for the current process ID)

On this CentOS system there is a process 1, and its directory in /proc but it doesn't have a comm​:

$ ls /proc/1/
ls​: cannot read symbolic link /proc/1/cwd​: Permission denied ls​: cannot read symbolic link /proc/1/root​: Permission denied ls​: cannot read symbolic link /proc/1/exe​: Permission denied attr cpuset limits net root statm autogroup cwd loginuid numa_maps sched status auxv environ maps oom_adj schedstat syscall cgroup exe mem oom_score sessionid task clear_refs fd mountinfo oom_score_adj smaps wchan cmdline fdinfo mounts pagemap stack coredump_filter io mountstats personality stat

I don't know what file would be safer. Maybe mem?

Nicholas Clark

p6rt commented 8 years ago

From @geekosaur

On Wed, Aug 3, 2016 at 3​:12 PM, Nicholas Clark \<perl6-bugs-followup@​perl.org

wrote​:

I don't know what file would be safer. Maybe mem?

None of them. There's no guarantee that /proc exists (non-SVR4 commercial Unixes), or that it is in any way compatible with Linux's notion of /proc (FreeBSD, commercial SVR4 that didn't add Linux compatibility names like Solaris did).

-- brandon s allbery kf8nh sine nomine associates allbery.b@​gmail.com ballbery@​sinenomine.net unix, openafs, kerberos, infrastructure, xmonad http://sinenomine.net

p6rt commented 8 years ago

The RT System itself - Status changed from 'new' to 'open'

p6rt commented 8 years ago

From @nwc10

On Wed, Aug 03, 2016 at 03​:15​:49PM -0400, Brandon Allbery wrote​:

On Wed, Aug 3, 2016 at 3​:12 PM, Nicholas Clark \<perl6-bugs-followup@​perl.org

wrote​:

I don't know what file would be safer. Maybe mem?

None of them. There's no guarantee that /proc exists (non-SVR4 commercial Unixes), or that it is in any way compatible with Linux's notion of /proc (FreeBSD, commercial SVR4 that didn't add Linux compatibility names like Solaris did).

Sorry, wasn't clear (and *I* definitely confused things by mentioning Solaris Zones). *This* code is only run on Linux​:

my $procfile = '/proc/1/comm'; {   if $*KERNEL.name eq 'linux' {   my $fh = open $procfile or die qq/Failed to open "$procfile"​: $!/; ;   $fh.slurp-rest;

...

but it seems that it's not portable between different Linux variants.

I was mentioning Zones in the context of "we discovered that it's not actually safe to assume that process 1 exists", and I'm wary of assuming that it's only Solaris that can be strange. I wouldn't put it past Linux, somewhere, somehow.

Your comments about /proc generally are useful for us all to keep in mind.

Nicholas Clark

p6rt commented 8 years ago

From cuong.manhle.vn@gmail.com

Hi,

I'm the one who add this test.

First of all, this test only run on Linux, so we're safe to assume that /proc exists.

The second, `/proc/\/comm` is only available since Linux 2.6.33. I think we should pick the first file under `/proc/1/*` to read.

Thanks.

Cuong Manh Le https://cuonglm.xyz

p6rt commented 8 years ago

From cuong.manhle.vn@gmail.com

Hi,

I create a PR to fix this issue​: https://github.com/perl6/roast/pull/159

The test now picks the first readable file under `/proc/1`.

Thanks.

Cuong Manh Le https://cuonglm.xyz

p6rt commented 7 years ago

From @smls

The PR was merged long ago. Closing the ticket.

p6rt commented 7 years ago

@smls - Status changed from 'open' to 'resolved'