Perl / docker-perl

Dockerfiles for index.docker.io (official Perl Docker image)
https://registry.hub.docker.com/_/perl/
Artistic License 2.0
117 stars 51 forks source link

$! not set properly for unlink on < 5.18 #78

Closed toddr closed 1 year ago

toddr commented 4 years ago

I found a bug today switching Test::MockFile over to github actions.

https://github.com/cpanel/Test-MockFile/runs/418056154?check_suite_focus=true

You can see the test failure here:

2020-01-30T21:04:02.1945833Z t/touch.t .. 
2020-01-30T21:04:02.1947864Z # Seeded srand with seed '20200130' from local date.
2020-01-30T21:04:02.1955248Z # -------------- REAL MODE --------------
2020-01-30T21:04:02.1956268Z ok 1 - /tmp/OkmKzp4LAr is there
2020-01-30T21:04:02.1957157Z ok 2 - unlink on a dir fails
2020-01-30T21:04:02.1958021Z not ok 3 - unlink /dir is non-zero (0)
2020-01-30T21:04:02.1958467Z 
2020-01-30T21:04:02.1959272Z # Failed test 'unlink /dir is non-zero (0)'
2020-01-30T21:04:02.1960789Z # at t/touch.t line 20.
2020-01-30T21:04:02.1961664Z # -------------- MOCK MODE --------------
2020-01-30T21:04:02.1962537Z ok 4 - unlink /link works.
2020-01-30T21:04:02.1963361Z ok 5 - /link is now gone
2020-01-30T21:04:02.1964188Z not ok 6 -    ... and throws a $!
2020-01-30T21:04:02.1964668Z 
2020-01-30T21:04:02.1965141Z # Failed test '   ... and throws a $!'
2020-01-30T21:04:02.1965452Z # at t/touch.t line 35.
2020-01-30T21:04:02.1965915Z # +-----+----+-------+
2020-01-30T21:04:02.1966217Z # | GOT | OP | CHECK |
2020-01-30T21:04:02.1966670Z # +-----+----+-------+
2020-01-30T21:04:02.1966992Z # | 21  | eq | 0     |
2020-01-30T21:04:02.1967450Z # +-----+----+-------+
2020-01-30T21:04:02.1967938Z ok 7 - touch /dir doesn't work.
2020-01-30T21:04:02.1968441Z ok 8 - touch /link doesn't work.
2020-01-30T21:04:02.1968925Z ok 9 - Set mtime to 1970
2020-01-30T21:04:02.1969398Z ok 10 - Set ctime to 1970
2020-01-30T21:04:02.1969887Z ok 11 - Set atime to 1970
2020-01-30T21:04:02.1970385Z ok 12 - Touch a missing file.
2020-01-30T21:04:02.1970873Z ok 13 - mtime is set.
2020-01-30T21:04:02.1971343Z ok 14 - ctime is set.
2020-01-30T21:04:02.1971822Z ok 15 - atime is set.
2020-01-30T21:04:02.1972644Z ok 16 - /file exists with -e
2020-01-30T21:04:02.1973184Z ok 17 - /file is removed via unlink method
2020-01-30T21:04:02.1973697Z ok 18 - /file is missing via contents check
2020-01-30T21:04:02.1976357Z ok 19 - /file is missing via size method
2020-01-30T21:04:02.1976755Z ok 20 - /file is removed via -e check
2020-01-30T21:04:02.1977039Z ok 21 - Set file to have stuff in it.
2020-01-30T21:04:02.1977313Z ok 22 - Touch an existing file.
2020-01-30T21:04:02.1977577Z ok 23 - mtime is set to 1234.
2020-01-30T21:04:02.1977849Z ok 24 - ctime is set to 1234.
2020-01-30T21:04:02.1978135Z ok 25 - atime is set to 1234.
2020-01-30T21:04:02.1978243Z 1..25
2020-01-30T21:04:02.1978332Z Dubious, test returned 2 (wstat 512, 0x200)
2020-01-30T21:04:02.1978438Z Failed 2/25 subtests 
2020-01-30T21:04:02.1978488Z 
2020-01-30T21:04:02.1978590Z Test Summary Report
2020-01-30T21:04:02.1978846Z -------------------
2020-01-30T21:04:02.1978953Z t/touch.t (Wstat: 512 Tests: 25 Failed: 2)
2020-01-30T21:04:02.1979047Z   Failed tests:  3, 6

The issue appears to be that $! is not set when unlink is tried on a directory.

I simplified this problem to do: mkdir -p /tmp/foo; ls -ld /tmp/foo; perl -E'CORE::unlink "/tmp/foo"; print $! + 0; print "\n"'; ls -ld /tmp/foo

On everything above 5.18, I get:

drwxr-xr-x 2 root root 4096 Jan 30 21:45 /tmp/foo
21
drwxr-xr-x 2 root root 4096 Jan 30 21:45 /tmp/foo

On everything at and below 5.18, I get:

drwxr-xr-x 2 root root 4096 Jan 30 21:45 /tmp/foo
0
drwxr-xr-x 2 root root 4096 Jan 30 21:45 /tmp/foo

For whatever reason, perl is broken subtly on the older perls OR jesse is a problem?

atoomic commented 4 years ago

I wonder if we would see the same issue if we decide to build Perl on more recent version of Debian container.

Note that jessie LTS will end in June 2020. Maybe we could consider updating these containers? or provide a different flavors for them?

zakame commented 4 years ago

@toddr thanks for the report, let me check further shortly :muscle: Note that those using docker-perl images before the currently supported perlpolicy versions are advised to rebuild and customize their own images using the Dockerfiles here in this repo as needed (note to self: probably should note this down in the README.md.) As @atoomic notes, it might be best to try rebuilding 5.18 against a stretch base, so essentially a one-line change in https://github.com/Perl/docker-perl/blob/28f5306b9e608844e727c455a80b428697e8c6b1/5.018.004-main-jessie/Dockerfile#L1 to use the stretch tag.

@atoomic for supported Perls we default on buster-based debian and buildpack-deps images actually, retaining some jessie-based ones on the older Perl versions up to 5.18 perhaps for a little while beyond the LTS. And in between, stretch of course is still supported from Perl 5.20 and up (see https://github.com/Perl/docker-perl/blob/master/config.yml.)

FGasper commented 4 years ago

This may relate to the fact that Perl doesn’t actually unlink() a directory because some OSes actually allow this, even though it corrupts the filesystem.

zakame commented 4 years ago

Running under docker run --rm -it perl:5.18 (pulling the image afresh from Docker Hub,) can certainly observe it:

# after installdeps for Test::Mockfile@0.020...
root@545808d206c3:~# SHELL=/bin/bash cpanm --look Test::MockFile@0.020
--> Working on Test::MockFile
Fetching http://backpan.perl.org/authors/id/T/TO/TODDR/Test-MockFile-0.020.tar.gz ... OK
Entering /root/.cpanm/work/1580471299.2635/Test-MockFile-0.020 with /bin/bash
root@545808d206c3:~/.cpanm/work/1580471299.2635/Test-MockFile-0.020# prove -lv t/touch.t
t/touch.t .. 
# Seeded srand with seed '20200131' from local date.
# -------------- REAL MODE --------------
ok 1 - unlink on a dir fails
# -------------- MOCK MODE --------------
ok 2 - unlink /link works.
ok 3 - /link is now gone
ok 4 - unlink /dir doesn't work.
not ok 5 -    ... and throws a $!

# Failed test '   ... and throws a $!'
# at t/touch.t line 33.
# +-----+----+-------+
# | GOT | OP | CHECK |
# +-----+----+-------+
# | 21  | eq | 0     |
# +-----+----+-------+
ok 6 - touch /dir doesn't work.
ok 7 - touch /link doesn't work.
ok 8 - Set mtime to 1970
ok 9 - Set ctime to 1970
ok 10 - Set atime to 1970
ok 11 - Touch a missing file.
ok 12 - mtime is set.
ok 13 - ctime is set.
ok 14 - atime is set.
ok 15 - /file exists with -e
ok 16 - /file is removed via unlink method
ok 17 - /file is missing via contents check
ok 18 - /file is missing via size method
ok 19 - /file is removed via -e check
ok 20 - Set file to have stuff in it.
ok 21 - Touch an existing file.
ok 22 - mtime is set to 1234.
ok 23 - ctime is set to 1234.
ok 24 - atime is set to 1234.
1..24
Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/24 subtests 

Test Summary Report
-------------------
t/touch.t (Wstat: 256 Tests: 24 Failed: 1)
  Failed test:  5
  Non-zero exit status: 1
Files=1, Tests=24,  0 wallclock secs ( 0.03 usr  0.01 sys +  0.43 cusr  0.04 csys =  0.51 CPU)
Result: FAIL

Let me check with a rebuild of 5.18 against buildpack-deps:stretch...

zakame commented 4 years ago

@FGasper is probably right, it is noted in perldoc after all:

Note: unlink will not attempt to delete directories unless you are superuser and the -U flag is supplied to Perl. Even if these conditions are met, be warned that unlinking a directory can inflict damage on your filesystem. Finally, using unlink on directories is not supported on many operating systems. Use rmdir instead.

That last bit probably warrants a perlport note as well.

zakame commented 4 years ago

Heh, and current the 5.18 build will still fail make test anyway due to #76

Test Summary Report
-------------------
../cpan/Time-Local/t/Local.t                                    (Wstat: 512 Tests: 187 Failed: 2)
  Failed tests:  6, 12
  Non-zero exit status: 2
Files=2395, Tests=675658, 894 wallclock secs (82.87 usr 12.20 sys + 550.37 cusr 59.48 csys = 704.92 CPU)
Result: FAIL
makefile:958: recipe for target 'test_harness' failedmake: *** [test_harness] Error 2
The command '/bin/sh -c true     && curl -SL https://www.cpan.org/src/5.0/perl-5.18.4.tar.bz2 -o perl-5.18.4.tar.bz2     && echo '1fb4d27b75cd244e849f253320260efe1750641aaff4a18ce0d67556ff1b96a5 *perl-5.18.4.tar.bz2' | sha256sum -c -     && tar --strip-components=1 -xaf perl-5.18.4.tar.bz2 -C /usr/src/perl     && rm perl...
atoomic commented 4 years ago

@zakame Could we consider updating the debian release s{jessie}{stretch} for all Perl versions listed in https://github.com/Perl/docker-perl/blob/master/config.yml then regenerate the Dockerfile & publish the new images?

Do you see any issues updating the containers this way?

zakame commented 4 years ago

@atoomic for the unsupported perls from 5.24 down, we no longer automatically publish updates; only those listed in https://github.com/docker-library/official-images/blob/master/library/perl get rebuilt by docker-library's Jenkins pipeline (see also https://github.com/docker-library/oi-janky-groovy.)

The main issue is the sheer size; for these older perls, we'd be asking docker-library to rebuild around 24 images, yet not all of them are guaranteed to pass building (we still have the Time-Local test issue to fix/disable, as well as perhaps other things to check.)

Again, I'd encourage users/orgs to fork and customize the Dockerfiles here especially for running older Perls on newer OS releases (note that one doesn't have to be limited to Debian, they can use ubuntu, centos, or whatever other OS that might suit their needs better.)

zakame commented 4 years ago

Back to @toddr's original issue, it seems that even when rebuilding to stretch the problem in t/touch.t persists:

Summary of my perl5 (revision 5 version 18 subversion 4) configuration:

  Platform:
    osname=linux, osvers=5.4.1, archname=x86_64-linux-gnu
    uname='linux 13b3ebe5a7e4 5.4.1 #1 smp sat nov 30 14:09:29 cst 2019 x86_64 gnulinux '
    config_args='-Darchname=x86_64-linux-gnu -Duse64bitall -Duseshrplib -Dvendorprefix=/usr/local -A ccflags=-fwrapv -des'
    hint=recommended, useposix=true, d_sigaction=define
    useithreads=undef, usemultiplicity=undef
    useperlio=define, d_sfio=undef, uselargefiles=define, usesocks=undef
    use64bitint=define, use64bitall=define, uselongdouble=undef
    usemymalloc=n, bincompat5005=undef
  Compiler:
    cc='cc', ccflags ='-fwrapv -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64',
    optimize='-O2',
    cppflags='-fwrapv -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include'
    ccversion='', gccversion='6.3.0 20170516', gccosandvers=''
    intsize=4, longsize=8, ptrsize=8, doublesize=8, byteorder=12345678
    d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=16
    ivtype='long', ivsize=8, nvtype='double', nvsize=8, Off_t='off_t', lseeksize=8
    alignbytes=8, prototype=define
  Linker and Libraries:
    ld='cc', ldflags =' -fstack-protector -L/usr/local/lib'
    libpth=/usr/local/lib /lib/x86_64-linux-gnu /lib/../lib /usr/lib/x86_64-linux-gnu /usr/lib/../lib /lib /usr/lib
    libs=-lnsl -lgdbm -ldb -ldl -lm -lcrypt -lutil -lc -lgdbm_compat
    perllibs=-lnsl -ldl -lm -lcrypt -lutil -lc
    libc=libc-2.24.so, so=so, useshrplib=true, libperl=libperl.so
    gnulibc_version='2.24'
  Dynamic Linking:
    dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,-E -Wl,-rpath,/usr/local/lib/perl5/5.18.4/x86_64-linux-gnu/CORE'
    cccdlflags='-fPIC', lddlflags='-shared -O2 -L/usr/local/lib -fstack-protector'

Characteristics of this binary (from libperl): 
  Compile-time options: HAS_TIMES PERLIO_LAYERS PERL_DONT_CREATE_GVSV
                        PERL_HASH_FUNC_ONE_AT_A_TIME_HARD PERL_MALLOC_WRAP
                        PERL_PRESERVE_IVUV PERL_SAWAMPERSAND USE_64_BIT_ALL
                        USE_64_BIT_INT USE_LARGE_FILES USE_LOCALE
                        USE_LOCALE_COLLATE USE_LOCALE_CTYPE
                        USE_LOCALE_NUMERIC USE_PERLIO USE_PERL_ATOF
  Built under linux
  Compiled at Feb  1 2020 02:44:57
  @INC:
    /usr/local/lib/perl5/site_perl/5.18.4/x86_64-linux-gnu
    /usr/local/lib/perl5/site_perl/5.18.4
    /usr/local/lib/perl5/vendor_perl/5.18.4/x86_64-linux-gnu
    /usr/local/lib/perl5/vendor_perl/5.18.4
    /usr/local/lib/perl5/5.18.4/x86_64-linux-gnu
    /usr/local/lib/perl5/5.18.4
    .

root@443ab9aaa8a2:/# SHELL=/bin/bash cpanm --look Test::MockFile@0.020
--> Working on Test::MockFile
Fetching http://backpan.perl.org/authors/id/T/TO/TODDR/Test-MockFile-0.020.tar.gz ... OK
Entering /root/.cpanm/work/1580525449.2518/Test-MockFile-0.020 with /bin/bash
root@443ab9aaa8a2:~/.cpanm/work/1580525449.2518/Test-MockFile-0.020# prove -lv t/touch.t 
t/touch.t .. 
# Seeded srand with seed '20200201' from local date.
# -------------- REAL MODE --------------
ok 1 - unlink on a dir fails
# -------------- MOCK MODE --------------
ok 2 - unlink /link works.
ok 3 - /link is now gone
ok 4 - unlink /dir doesn't work.
not ok 5 -    ... and throws a $!

# Failed test '   ... and throws a $!'
# at t/touch.t line 33.
# +-----+----+-------+
# | GOT | OP | CHECK |
# +-----+----+-------+
# | 21  | eq | 0     |
# +-----+----+-------+
ok 6 - touch /dir doesn't work.
ok 7 - touch /link doesn't work.
ok 8 - Set mtime to 1970
ok 9 - Set ctime to 1970
ok 10 - Set atime to 1970
ok 11 - Touch a missing file.
ok 12 - mtime is set.
ok 13 - ctime is set.
ok 14 - atime is set.
ok 15 - /file exists with -e
ok 16 - /file is removed via unlink method
ok 17 - /file is missing via contents check
ok 18 - /file is missing via size method
ok 19 - /file is removed via -e check
ok 20 - Set file to have stuff in it.
ok 21 - Touch an existing file.
ok 22 - mtime is set to 1234.
ok 23 - ctime is set to 1234.
ok 24 - atime is set to 1234.
1..24
Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/24 subtests 

Test Summary Report
-------------------
t/touch.t (Wstat: 256 Tests: 24 Failed: 1)
  Failed test:  5
  Non-zero exit status: 1
Files=1, Tests=24,  0 wallclock secs ( 0.02 usr  0.01 sys +  0.13 cusr  0.00 csys =  0.16 CPU)
Result: FAIL

root@443ab9aaa8a2:~/.cpanm/work/1580525449.2518/Test-MockFile-0.020# cat /etc/os-release 
PRETTY_NAME="Debian GNU/Linux 9 (stretch)"
NAME="Debian GNU/Linux"
VERSION_ID="9"
VERSION="9 (stretch)"
VERSION_CODENAME=stretch
ID=debian
HOME_URL="https://www.debian.org/"
SUPPORT_URL="https://www.debian.org/support"
BUG_REPORT_URL="https://bugs.debian.org/"
root@443ab9aaa8a2:~/.cpanm/work/1580525449.2518/Test-MockFile-0.020# 

I suspect @FGasper is right on this one and we should be testing for file type prior to unlink/rmdir.

toddr commented 4 years ago

Back to @toddr's original issue, it seems that even when rebuilding to stretch the problem in t/touch.t persists:

I suspect @FGasper is right on this one and we should be testing for file type prior to unlink/rmdir.

The point of the test is to confirm a consistent behavior of the test vs the operating system it is on. I would be shocked if you could show me that this particular failure to set $! happens on any debian outside of Docker.

In my case I did test for the existence of the directory and it was there so $! should be set if I try to unlink it in perl.

zakame commented 4 years ago

The point of the test is to confirm a consistent behavior of the test vs the operating system it is on. I would be shocked if you could show me that this particular failure to set $! happens on any debian outside of Docker.

I wouldn't be shocked if this would happen on any container/OS combination. There are also subtle differences in syscall or other behavior depending when its on a container or not, see for example #44.

FWIW this issue seems confirmed to be present on jessie and stretch containers but not on buster:

root@56750b1d4055:~/.cpanm/work/1580752130.1090/Test-MockFile-0.020# perl -v

This is perl 5, version 30, subversion 1 (v5.30.1) built for x86_64-linux-gnu

Copyright 1987-2019, Larry Wall

Perl may be copied only under the terms of either the Artistic License or the
GNU General Public License, which may be found in the Perl 5 source kit.

Complete documentation for Perl, including FAQ lists, should be found on
this system using "man perl" or "perldoc perl".  If you have access to the
Internet, point your browser at http://www.perl.org/, the Perl Home Page.

root@56750b1d4055:~/.cpanm/work/1580752130.1090/Test-MockFile-0.020# cat /etc/os-release
PRETTY_NAME="Debian GNU/Linux 10 (buster)"
NAME="Debian GNU/Linux"
VERSION_ID="10"
VERSION="10 (buster)"
VERSION_CODENAME=buster
ID=debian
HOME_URL="https://www.debian.org/"
SUPPORT_URL="https://www.debian.org/support"
BUG_REPORT_URL="https://bugs.debian.org/"
root@56750b1d4055:~/.cpanm/work/1580752130.1090/Test-MockFile-0.020# prove -lv t/touch.t 
t/touch.t .. 
# Seeded srand with seed '20200203' from local date.
# -------------- REAL MODE --------------
ok 1 - unlink on a dir fails
# -------------- MOCK MODE --------------
ok 2 - unlink /link works.
ok 3 - /link is now gone
ok 4 - unlink /dir doesn't work.
ok 5 -    ... and throws a $!
ok 6 - touch /dir doesn't work.
ok 7 - touch /link doesn't work.
ok 8 - Set mtime to 1970
ok 9 - Set ctime to 1970
ok 10 - Set atime to 1970
ok 11 - Touch a missing file.
ok 12 - mtime is set.
ok 13 - ctime is set.
ok 14 - atime is set.
ok 15 - /file exists with -e
ok 16 - /file is removed via unlink method
ok 17 - /file is missing via contents check
ok 18 - /file is missing via size method
ok 19 - /file is removed via -e check
ok 20 - Set file to have stuff in it.
ok 21 - Touch an existing file.
ok 22 - mtime is set to 1234.
ok 23 - ctime is set to 1234.
ok 24 - atime is set to 1234.
1..24
ok
All tests successful.
Files=1, Tests=24,  0 wallclock secs ( 0.03 usr  0.01 sys +  0.17 cusr  0.01 csys =  0.22 CPU)
Result: PASS

so I suppose that's as good a lead as any to trace this issue. In particular, I note https://metacpan.org/source/TODDR/Test-MockFile-0.020/t/touch.t#L17 being done without any testing for superuser or capabilities at all; given a container may have superuser-like capabilities on a non-root to run processes with (or go the opposite, have root but drop some capabilities,) this might be an issue here.

clayne commented 4 years ago

FWIW, our RHEL/OEL6 systems use 5.10.1 our RHEL/OEL7 systems use 5.16.3, another system I have is 5.30.3 and every single one of them sees '21' rather than 0. Are we sure the unit test is not implicitly assuming something else about it's runtime environment?

$ perl -V | head
Summary of my perl5 (revision 5 version 30 subversion 3) configuration:

  Platform:
    osname=linux
    osvers=4.11.6
    archname=x86_64-linux-thread-multi
    uname='linux localhost 4.11.6 #2 smp thu jun 22 19:15:26 pdt 2017 x86_64 amd fx(tm)-8350 eight-core processor authenticamd gnulinux '
    config_args='-des -Dinstallprefix=/usr -Dinstallusrbinperl=n -Ui_xlocale -Di_ndbm -Di_gdbm -Di_db -Dusethreads -DDEBUGGING=-g -Dinc_version_list=5.30.1/x86_64-linux-thread-multi 5.30.1 5.28.2 5.24.1 5.12.4 5.12.3 5.12.2  -Dlibpth=/usr/local/lib64 /lib64 /usr/lib64 -Dnoextensions=ODBM_File -Duseshrplib -Darchname=x86_64-linux-thread -Dcc=x86_64-pc-linux-gnu-gcc -Doptimize=-O2 -march=native -mtune=native -pipe -g -ggdb -Dldflags=-Wl,-O1 -Wl,--as-needed -Dprefix=/usr -Dsiteprefix=/usr/local -Dvendorprefix=/usr -Dscriptdir=/usr/bin -Dprivlib=/usr/lib64/perl5/5.30.3 -Darchlib=/usr/lib64/perl5/5.30.3/x86_64-linux-thread-multi -Dsitelib=/usr/local/lib64/perl5/5.30.3 -Dsitearch=/usr/local/lib64/perl5/5.30.3/x86_64-linux-thread-multi -Dvendorlib=/usr/lib64/perl5/vendor_perl/5.30.3 -Dvendorarch=/usr/lib64/perl5/vendor_perl/5.30.3/x86_64-linux-thread-multi -Dman1dir=/usr/share/man/man1 -Dman3dir=/usr/share/man/man3 -Dsiteman1dir=/usr/local/man/man1 -Dsiteman3dir=/usr/local/man/man3 -Dvendorman1dir=/usr/share/man/man1 -Dvendorman3dir=/usr/share/man/man3 -Dman1ext=1 -Dman3ext=3pm -Dlibperl=libperl.so.5.30.3 -Dlocincpth=/usr/include  -Dglibpth=/lib64 /usr/lib64  -Duselargefiles -Dd_semctl_semun -Dcf_by=Gentoo -Dmyhostname=localhost -Dperladmin=root@localhost -Ud_csh -Dsh=/bin/sh -Dtargetsh=/bin/sh -Uusenm -Ui_xlocale -Di_ndbm -Di_gdbm -Di_db -Dusethreads -DDEBUGGING=-g -Dinc_version_list=5.30.1/x86_64-linux-thread-multi 5.30.1 5.28.2 5.24.1 5.12.4 5.12.3 5.12.2  -Dlibpth=/usr/local/lib64 /lib64 /usr/lib64 -Dnoextensions=ODBM_File'
    hint=recommended
    useposix=true

$ rm -rf /tmp/foo && mkdir -p /tmp/foo; ls -ld /tmp/foo; perl -E'CORE::unlink "/tmp/foo"; print $! + 0; print "\n"'; ls -ld /tmp/foo
drwxr-xr-x 2 clayne clayne 6 Jun 16 12:29 /tmp/foo
21
drwxr-xr-x 2 clayne clayne 6 Jun 16 12:29 /tmp/foo
$ perl -V | head
Summary of my perl5 (revision 5 version 16 subversion 3) configuration:

  Platform:
    osname=linux, osvers=4.14.35-1902.4.8.el7uek.x86_64, archname=x86_64-linux-thread-multi
    uname='linux jenkins-10-147-72-125-a976e1f6-bf96-46ca-a9ea-5e52c752f19c 4.14.35-1902.4.8.el7uek.x86_64 #2 smp sun aug 4 22:25:18 gmt 2019 x86_64 x86_64 x86_64 gnulinux '
    config_args='-des -Doptimize=-O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches   -m64 -mtune=generic -Dccdlflags=-Wl,--enable-new-dtags -Dlddlflags=-shared -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches   -m64 -mtune=generic -Wl,-z,relro  -DDEBUGGING=-g -Dversion=5.16.3 -Dmyhostname=localhost -Dperladmin=root@localhost -Dcc=gcc -Dcf_by=Red Hat, Inc. -Dprefix=/usr -Dvendorprefix=/usr -Dsiteprefix=/usr/local -Dsitelib=/usr/local/share/perl5 -Dsitearch=/usr/local/lib64/perl5 -Dprivlib=/usr/share/perl5 -Dvendorlib=/usr/share/perl5/vendor_perl -Darchlib=/usr/lib64/perl5 -Dvendorarch=/usr/lib64/perl5/vendor_perl -Darchname=x86_64-linux-thread-multi -Dlibpth=/usr/local/lib64 /lib64 /usr/lib64 -Duseshrplib -Dusethreads -Duseithreads -Dusedtrace=/usr/bin/dtrace -Duselargefiles -Dd_semctl_semun -Di_db -Ui_ndbm -Di_gdbm -Di_shadow -Di_syslog -Dman3ext=3pm -Duseperlio -Dinstallusrbinperl=n -Ubincompat5005 -Uversiononly -Dpager=/usr/bin/less -isr -Dd_gethostent_r_proto -Ud_endhostent_r_proto -Ud_sethostent_r_proto -Ud_endprotoent_r_proto -Ud_setprotoent_r_proto -Ud_endservent_r_proto -Ud_setservent_r_proto -Dscriptdir=/usr/bin -Dusesitecustomize'
    hint=recommended, useposix=true, d_sigaction=define
    useithreads=define, usemultiplicity=define
    useperlio=define, d_sfio=undef, uselargefiles=define, usesocks=undef
    use64bitint=define, use64bitall=define, uselongdouble=undef

$ rm -rf /tmp/foo && mkdir -p /tmp/foo; ls -ld /tmp/foo; perl -E'CORE::unlink "/tmp/foo"; print $! + 0; print "\n"'; ls -ld /tmp/foo
drwxrwxr-x 2 clayne clayne 4096 Jun 16 19:26 /tmp/foo
21
drwxrwxr-x 2 clayne clayne 4096 Jun 16 19:26 /tmp/foo
$ perl -V | head
Summary of my perl5 (revision 5 version 10 subversion 1) configuration:

  Platform:
    osname=linux, osvers=3.8.13-55.1.5.el6uek.x86_64, archname=x86_64-linux-thread-multi
    uname='linux x86-ol6-builder-04 3.8.13-55.1.5.el6uek.x86_64 #2 smp wed jan 28 17:03:28 pst 2015 x86_64 x86_64 x86_64 gnulinux '
    config_args='-des -Doptimize=-O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic -DDEBUGGING=-g -Dversion=5.10.1 -Dmyhostname=localhost -Dperladmin=root@localhost -Dcc=gcc -Dcf_by=Red Hat, Inc. -Dprefix=/usr -Dvendorprefix=/usr -Dsiteprefix=/usr/local -Dsitelib=/usr/local/share/perl5 -Dsitearch=/usr/local/lib64/perl5 -Dprivlib=/usr/share/perl5 -Darchlib=/usr/lib64/perl5 -Dvendorlib=/usr/share/perl5/vendor_perl -Dvendorarch=/usr/lib64/perl5/vendor_perl -Dinc_version_list=5.10.0 -Darchname=x86_64-linux-thread-multi -Dlibpth=/usr/local/lib64 /lib64 /usr/lib64 -Duseshrplib -Dusethreads -Duseithreads -Duselargefiles -Dd_dosuid -Dd_semctl_semun -Di_db -Ui_ndbm -Di_gdbm -Di_shadow -Di_syslog -Dman3ext=3pm -Duseperlio -Dinstallusrbinperl=n -Ubincompat5005 -Uversiononly -Dpager=/usr/bin/less -isr -Dd_gethostent_r_proto -Ud_endhostent_r_proto -Ud_sethostent_r_proto -Ud_endprotoent_r_proto -Ud_setprotoent_r_proto -Ud_endservent_r_proto -Ud_setservent_r_proto -Dscriptdir=/usr/bin -Dusesitecustomize'
    hint=recommended, useposix=true, d_sigaction=define
    useithreads=define, usemultiplicity=define
    useperlio=define, d_sfio=undef, uselargefiles=define, usesocks=undef
    use64bitint=define, use64bitall=define, uselongdouble=undef

$ rm -rf /tmp/foo && mkdir -p /tmp/foo; ls -ld /tmp/foo; perl -E'CORE::unlink "/tmp/foo"; print $! + 0; print "\n"'; ls -ld /tmp/foo
drwxrwxr-x 2 clayne clayne 4096 Jun 16 19:28 /tmp/foo
21
drwxrwxr-x 2 clayne clayne 4096 Jun 16 19:28 /tmp/foo
toddr commented 4 years ago

I continue to believe there is something wrong with the docker images. I don't see how this should ever not be an error.

waterkip commented 4 years ago

so I suppose that's as good a lead as any to trace this issue. In particular, I note https://metacpan.org/source/TODDR/Test-MockFile-0.020/t/touch.t#L17 being done without any testing for superuser or capabilities at all; given a container may have superuser-like capabilities on a non-root to run processes with (or go the opposite, have root but drop some capabilities,) this might be an issue here.

You hit the dot with root:

FROM perl:5.18

RUN useradd -m foo

USER foo

WORKDIR /home/foo

ENV PERL_LOCAL_LIB_ROOT /home/foo/.local
ENV PERL_MB_OPT "--install_base '$PERL_LOCAL_LIB_ROOT'"
ENV PERL_MM_OPT "INSTALL_BASE=$PERL_LOCAL_LIB_ROOT"
ENV PERL5LIB "./lib:$PERL_LOCAL_LIB_ROOT/lib/perl5"

RUN cpanm --installdeps Test::MockFile@0.020
ENV SHELL /bin/bash
RUN cpanm --test-only Test::MockFile@0.020

Yields:

Step 10/11 : ENV SHELL /bin/bash
---> Running in 8c22d3f758e0
Removing intermediate container 8c22d3f758e0
---> 1116b3c330ae
Step 11/11 : RUN cpanm --test-only Test::MockFile@0.020
---> Running in 2b2a5726a421
--> Working on Test::MockFile
Fetching http://backpan.perl.org/authors/id/T/TO/TODDR/Test-MockFile-0.020.tar.gz ... OK
Configuring Test-MockFile-0.020 ... OK
Building and testing Test-MockFile-0.020 ... OK
Successfully tested Test-MockFile-0.020
Removing intermediate container 2b2a5726a421
---> 969f036ea2e7
Successfully built 969f036ea2e7
toddr commented 4 years ago

None of that is needed. Just try this simplified example provided by @clayne. https://github.com/Perl/docker-perl/issues/78#issuecomment-644969993

we should be getting 21 always but it appears the older perl docker containers do not behave as expected.

rm -rf /tmp/foo && mkdir -p /tmp/foo; ls -ld /tmp/foo; perl -E'CORE::unlink "/tmp/foo"; print $! + 0; print "\n"'; ls -ld /tmp/foo

waterkip commented 4 years ago
$ docker run --rm -ti c61fc57390b0 bash
foo@fe3af326f385:~$ rm -rf /tmp/foo && mkdir -p /tmp/foo; ls -ld /tmp/foo; perl -E'CORE::unlink "/tmp/foo"; print $! + 0; print "\n"'; ls -ld /tmp/foo
drwxr-xr-x 2 foo foo 4096 Jun 16 21:37 /tmp/foo
21
drwxr-xr-x 2 foo foo 4096 Jun 16 21:37 /tmp/foo
foo@fe3af326f385:~$
$ docker run --rm -ti -u root c61fc57390b0 bash
root@3e006f0cf82d:/home/foo# rm -rf /tmp/foo && mkdir -p /tmp/foo; ls -ld /tmp/foo; perl -E'CORE::unlink "/tmp/foo"; print $! + 0; print "\n"'; ls -ld /tmp/foo
drwxr-xr-x 2 root root 4096 Jun 16 21:37 /tmp/foo
0
drwxr-xr-x 2 root root 4096 Jun 16 21:37 /tmp/foo
root@3e006f0cf82d:/home/foo#
zakame commented 4 years ago

Hi @toddr, @waterkip, and @clayne, thanks for the updates, I can replicate this certainly with the simple test:

[zakame:~] % # host system perl
[zakame:~] % rm -rf /tmp/foo && mkdir -p /tmp/foo; ls -ld /tmp/foo; perl -E'CORE::unlink "/tmp/foo"; print $! + 0; print "\n"'; ls -ld /tmp/foo
drwxr-xr-x 2 zakame users 40 Jun 20 17:28 /tmp/foo
21
drwxr-xr-x 2 zakame users 40 Jun 20 17:28 /tmp/foo
[zakame:~] %
[zakame:~] % # perl 5.18 jessie container
[zakame:~] % docker run --rm -it perl:5.18 /bin/bash
root@2830413c5a99:/# rm -rf /tmp/foo && mkdir -p /tmp/foo; ls -ld /tmp/foo; perl -E'CORE::unlink "/tmp/foo"; print $! + 0; print "\n"'; ls -ld /tmp/foo
drwxr-xr-x 2 root root 4096 Jun 20 09:28 /tmp/foo
0
drwxr-xr-x 2 root root 4096 Jun 20 09:28 /tmp/foo
root@2830413c5a99:/# useradd zakame
root@2830413c5a99:/# rm -fr /tmp/foo
root@2830413c5a99:/# su - zakame
No directory, logging in with HOME=/
$
$ # newly created user in-container
$ rm -rf /tmp/foo && mkdir -p /tmp/foo; ls -ld /tmp/foo; perl -E'CORE::unlink "/tmp/foo"; print $! + 0; print "\n"'; ls -ld /tmp/foo
drwxr-xr-x 2 zakame zakame 4096 Jun 20 09:29 /tmp/foo
21
drwxr-xr-x 2 zakame zakame 4096 Jun 20 09:29 /tmp/foo
$ 

So I think that while you might be checking for the directory existence, you lack checking for whether you're root or not, precisely as what the perldoc in https://github.com/Perl/docker-perl/issues/78#issuecomment-580706205 warns about.

@clayne's insight about assumptions on runtime is apt; FWIW, here's the same pattern when running under Kubernetes, emphasizing the lack of /.dockerenv (as we really should be checking for namespaces in /proc/1/cgroup anyway:)

[zakame:~] 1 % kubectl run --restart=Never --image=perl:5.18 test-mockfile-pod -- /bin/sleep 3600
pod/test-mockfile-pod created
[zakame:~] % kubectl exec test-mockfile-pod -it -- /bin/bash       
root@test-mockfile-pod:/# ls -l /.dockerenv
ls: cannot access /.dockerenv: No such file or directory
root@test-mockfile-pod:/# rm -rf /tmp/foo && mkdir -p /tmp/foo; ls -ld /tmp/foo; perl -E'CORE::unlink "/tmp/foo"; print $! + 0; print "\n"'; ls -ld /tmp/foo
drwxr-xr-x 2 root root 4096 Jun 20 11:08 /tmp/foo
0
drwxr-xr-x 2 root root 4096 Jun 20 11:08 /tmp/foo
root@test-mockfile-pod:/# useradd zakame
root@test-mockfile-pod:/# rm -fr /tmp/foo
root@test-mockfile-pod:/# su - zakame
No directory, logging in with HOME=/
$ rm -rf /tmp/foo && mkdir -p /tmp/foo; ls -ld /tmp/foo; perl -E'CORE::unlink "/tmp/foo"; print $! + 0; print "\n"'; ls -ld /tmp/foo
drwxr-xr-x 2 zakame zakame 4096 Jun 20 11:08 /tmp/foo
21
drwxr-xr-x 2 zakame zakame 4096 Jun 20 11:08 /tmp/foo
$ 

From the PoV of a Test::MockFile user, I'd suggest letting https://metacpan.org/pod/Test::MockFile#unlink be consistent with what's documented in the perlfunc and not support unlinking directories; you're supposed to be mocking existing (if albeit unreliable) behavior after all, not deviating from it.

zakame commented 4 years ago

As a follow up, I tested again with 5.32 on jessie, and it is consistent with the original observation for $] >= 5.018_000:

docker run --rm -it perl:5.32-slim-jessie /bin/bash
root@ca2c0819d4a8:/# rm -rf /tmp/foo && mkdir -p /tmp/foo; ls -ld /tmp/foo; perl -E'CORE::unlink "/tmp/foo"; print $! + 0; print "\n"'; ls -ld /tmp/foo
drwxr-xr-x 2 root root 4096 Jun 21 05:28 /tmp/foo
21
drwxr-xr-x 2 root root 4096 Jun 21 05:28 /tmp/foo
root@ca2c0819d4a8:/# useradd zakame
root@ca2c0819d4a8:/# rm -fr /tmp/foo
root@ca2c0819d4a8:/# su - zakame
No directory, logging in with HOME=/
$ rm -rf /tmp/foo && mkdir -p /tmp/foo; ls -ld /tmp/foo; perl -E'CORE::unlink "/tmp/foo"; print $! + 0; print "\n"'; ls -ld /tmp/foo
drwxr-xr-x 2 zakame zakame 4096 Jun 21 05:28 /tmp/foo
21
drwxr-xr-x 2 zakame zakame 4096 Jun 21 05:28 /tmp/foo
$ 

Again, all of this simply points out the fact that CORE::unlink($directory) is unreliable and documented in perlfunc to be so.

tonycoz commented 2 years ago

This was a bug in perl that was fixed in 1dcae8b8dd1e2aa373ab045fee3d4f95d34f0b3c

zakame commented 1 year ago

@tonycoz thanks for that bit of info! 🙏

@toddr and everyone else, let me know if this is still an issue for you, or if we can close out this one. 🙏