Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
p6rt opened this issue Aug 3, 2016 · 8 comments
Closed

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

p6rt opened this issue Aug 3, 2016 · 8 comments
Labels

Comments

@p6rt
Copy link

p6rt commented Aug 3, 2016

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

Searchable as RT128831$

@p6rt
Copy link
Author

p6rt commented Aug 3, 2016

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 <unit> 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
Copy link
Author

p6rt commented Aug 3, 2016

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
Copy link
Author

p6rt commented Aug 3, 2016

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

@p6rt
Copy link
Author

p6rt commented Aug 3, 2016

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
Copy link
Author

p6rt commented Sep 23, 2016

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/<pid>/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
Copy link
Author

p6rt commented Sep 23, 2016

From cuong.manhle.vn@gmail.com

Hi,

I create a PR to fix this issue​: Raku/roast#159

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

Thanks.

Cuong Manh Le
https://cuonglm.xyz

@p6rt
Copy link
Author

p6rt commented Sep 23, 2017

From @smls

The PR was merged long ago. Closing the ticket.

@p6rt
Copy link
Author

p6rt commented Sep 23, 2017

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

@p6rt p6rt closed this as completed Sep 23, 2017
@p6rt p6rt added the spec label Jan 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant