Dear Maintainer,

Building clisp package in an OBS instance FTBFS because of the following
test failure:


https://salsa.debian.org/common-lisp-team/clisp/-/blob/0a8137b7caa777e1c08e06fa7161784f947fa5af/tests/streams.tst#L1290

  (let ((*reopen-open-file* nil)) ; stdout can be a file, it will be
detected!
    (with-open-file (copy s :direction :output) (streamp copy))) T

Build error log shows the following output for that test:

  $ cat usr/src/packages/BUILD/debian/build/tests/*.erg
  Form: (LET ((*REOPEN-OPEN-FILE* NIL)) (WITH-OPEN-FILE (COPY S
:DIRECTION :OUTPUT) (STREAMP COPY)))
  CORRECT: T
  CLISP  : ERROR
  OS-FILE-ERROR(13): Permission denied
  OUT:
  "[OS-FILE-ERROR]: OS-FILE-ERROR(13): Permission denied
  "

That Lisp form tries to open the stdout file descriptor, but in this
case it's failing with EACCES (Permission denied).

AFAICS, running `strace` on the test shows that on the OBS instance,
`stdout` is piped, causing the access issue:

  [  148s] lstat("/proc/2867342/fd/2", {st_mode=S_IFLNK|0300,
st_size=64, ...}) = 0
  [  148s] readlink("/proc/2867342/fd/2", "pipe:[4065697]", 64) = 14
  [  148s] stat("/proc/2867342/fd/2", {st_mode=S_IFIFO|0600, st_size=0,
...}) = 0
  [  148s] openat(AT_FDCWD, "/proc/2867342/fd/2",
O_WRONLY|O_CREAT|O_TRUNC, 0644) = -1 EACCES (Permission denied)

However, on a "local" build, `stdout` going directly to the pseudo
terminal doesn't raise any issue:

  lstat("/proc/2901662/fd/2", {st_mode=S_IFLNK|0700, st_size=64, ...}) = 0
  readlink("/proc/2901662/fd/2", "/dev/pts/1", 64) = 10
  readlink("/dev", 0x7fff7e9fbd30, 4095)  = -1 EINVAL (Invalid argument)
  readlink("/dev/pts", 0x7fff7e9fbd30, 4095) = -1 EINVAL (Invalid argument)
  lstat("/dev/pts/1", {st_mode=S_IFCHR|0620, st_rdev=makedev(0x88, 0x1),
...}) = 0
  stat("/dev/pts/1", {st_mode=S_IFCHR|0620, st_rdev=makedev(0x88, 0x1),
...}) = 0
  openat(AT_FDCWD, "/dev/pts/1", O_WRONLY|O_CREAT|O_TRUNC, 0644) = 10

This seems to be related to how output fds are handled. AFAICS there's a
difference between these two cases, see:


https://salsa.debian.org/common-lisp-team/clisp/-/blob/0a8137b7caa777e1c08e06fa7161784f947fa5af/src/pathname.d#L5426

Is this an issue and should be checked? What's the expected behaviour on
these cases?

Thanks in advance.

Regards,
Ariel D'Alessandro

It could be that OBS opens the pipe as a privileged process, forks,
then drops privileges afterwards such that the child process is
precluded from reopening the inode that backs the file descriptor
clisp operates on.

You'd somehow have to find a way to invoke "stat -L
/proc/<pid>/fd/<fdno>" on the failing file quickly enough to find out
if this is the case.  Inserting sleeps into the test could buy you
more time.

Alternatively (and assuming you use ocs) tools like socat/ptywrap
could maybe be used via $OSC_SU_WRAPPER to hook the child process onto
a pty (which should also be created after dropping the privileges).
Or you could try to run the build as root (osc build
--userootforbuild) and see if the error persists.

Regards,
Dennis

Hi Dennis,

Thanks for your helpful feedback.

Makes sense, agreed, this is probably the sequence.

lstat("/proc/1880876/fd/2", {st_mode=S_IFLNK|0300, st_size=64, ...}) = 0
readlink("/proc/1880876/fd/2", "pipe:[53088390]", 64) = 15
stat("/proc/1880876/fd/2", {st_mode=S_IFIFO|0600, st_size=0, ...}) = 0
openat(AT_FDCWD, "/proc/1880876/fd/2", O_WRONLY|O_CREAT|O_TRUNC, 0644) =
-1 EACCES (Permission denied)

This matches the case described above, opening the pipe as root, then
dropping and running clisp (during tests):

$ stat -L proc/1880876/fd/2
  File: proc/1880876/fd/2
  Size: 0         	Blocks: 0          IO Block: 4096   fifo
Device: ch/12d	Inode: 53088390    Links: 1
Access: (0600/prw-------)  Uid: (    0/    root)   Gid: (    0/    root)
Access: 2021-12-09 21:55:16.645805190 -0300
Modify: 2021-12-09 21:55:16.645805190 -0300
Change: 2021-12-09 21:55:16.645805190 -0300
 Birth: -

$ head proc/1880876/status
Name:	lisp.run
Umask:	0022
State:	S (sleeping)
Tgid:	1880876
Ngid:	0
Pid:	1880876
PPid:	1880873
TracerPid:	1880873
Uid:	399	399	399	399
Gid:	399	399	399	399
is no longer happening. Of course, now the pty is used instead of the
(root owned) pipe, so permissions are fine to re-open the file.

So, moving forward. This is the failing test simplified:

(streamp (setq s (make-stream :error))) T
(let ((*reopen-open-file* nil)) ; stdout can be a file, it will be detected!
  (with-open-file (copy s :direction :output) (streamp copy))) T

My question here would be: is this test expected to always succeed?

AFAIU, we can't assume to have the permissions to re-open the file
related to the file descriptor, which is what the code is explicitly
trying to do.

Please correct me if I'm wrong as I'm not used to clisp syntax, just
going through the source code :-)

From what I see, it responsibility of the clisp test code to check if it
has the permissions to re-open the file. Otherwise, it's not guaranteed
to succeed. For example, having this fd pointing to a (root owned) pipe
is a valid situation and should be properly handled by this test.

Thanks,
Ariel

control: forwarded -1 https://sourceforge.net/p/clisp/bugs/747/
X-Debbugs-CC: Ariel D'Alessandro <ariel.dalessandro@collabora.com>

You'd have to ask the original author of the test that.  In a perfect
world every test would be accompanied by a detailed description of
what exactly it tests for, what conditions it expects and what
constitutes a pass or failure.

In a test suite you usually can since it is probably not supposed to
be run by a privileged user in such a way that it fails.  Build
servers and CI pipelines, which are almost always involved in exposing
such bugs, really ought to put in more effort to make their
environment more similar to the one the developer develops and tests
the code in instead of silently expecting everyone else to put in the
additional effort to placate them.  Fooling a build/test suite into
thinking it is hooked to a terminal with a tool like ptywrap is really
not that big of an ask and it should be a default feature in all such
products.  Alternatively the privileged invoker could just as easily
call chown(2) on the pipe after opening it and solve the issue this
way.

That's debatable.  Yes, it could be argued that the test should do a
better job at disarming itself by testing if it can reopen the file
with e.g. access(2).  OTOH it could also be argued that it is the
invoker's burden to ensure conditions that allow the test to succeed.
Writing test suites is annoying enough as is.  No need to make it more
annoying by also expecting accommodation for build servers and CI
pipelines if they themselves could do the accommodation much more
easily.

Regards.

FYI, adding +Cc Sam Steingold.