libutil-tests: fix openFileEnsureBeneathNoSymlinks.works on cygwin

[corngood]

Motivation

This fixes a test failure I get on cygwin.

Context

[----------] 1 test from openFileEnsureBeneathNoSymlinks
[ RUN      ] openFileEnsureBeneathNoSymlinks.works
unknown file: Failure
C++ exception with description "error: relative path 'a/broken_symlink' points to a symlink, which is not allowed" thrown in the test body.

[  FAILED  ] openFileEnsureBeneathNoSymlinks.works (12 ms)
[----------] 1 test from openFileEnsureBeneathNoSymlinks (12 ms total)

EEXIST
pathname already exists and O_CREAT and O_EXCL were used.

ELOOP
Too many symbolic links were encountered in resolving pathname, or O_NOFOLLOW was specified but pathname was a symbolic link.

If I understand correctly, both of these conditions are true, so maybe cygwin isn't actually doing anything wrong? If so, we could check for both codes, but the fact that ELOOP gets turned into an exception makes that a little ugly.

---

Add 👍 to pull requests you find important.

The Nix maintainer team uses a GitHub project board to schedule and track reviews.

[xokdvium]

Hm, I do think that this is an issue with cygwin. From IEEE Std 1003.1-2008:

[EEXIST]
O_CREAT and O_EXCL are set, and the named file exists.

O_EXCL
If O_CREAT and O_EXCL are set, open() shall fail if the file exists. The check for the existence of the file and the creation of the file if it does not exist shall be atomic with respect to other threads executing open() naming the same filename in the same directory with O_EXCL and O_CREAT set. If O_EXCL and O_CREAT are set, and path names a symbolic link, open() shall fail and set errno to [EEXIST], regardless of the contents of the symbolic link.

I think it's pretty clear that Cygwin is non-compliant here (if I'm not misreading the spec, but it's pretty clear-cut IMO)

[xokdvium]

If O_EXCL and O_CREAT are set, and path names a symbolic link, open() shall fail and set errno to [EEXIST], regardless of the contents of the symbolic link.

Specifically this. Actually this behavior is one of the important factors for safe NAR unpacking. See RestoreSink. But I guess it's not the end of the world if it reports ELOOP, but should probably be reported upstream if it's not a known issue?

You can also just do GTEST_SKIP for this check since it's buggy.

[corngood]

I just don't understand why this wouldn't be allowed to take precedence:

O_NOFOLLOW
If path names a symbolic link, fail and set errno to [ELOOP].

It does say "regardless of the contents of the symbolic link", but the O_NOFOLLOW description doesn't mention contents.

You can also just do GTEST_SKIP for this check since it's buggy.

If we consider cygwin broken, I think I'd rather #ifndef __CYGWIN__ this portion of the test, since the rest of the test may be valuable, and what's here now isn't a good idea if we're going to fix it in cygwin.

[xokdvium]

I just don't understand why this wouldn't be allowed to take precedence:

Hm the spec is indeed underspecified in that regard. I guess the best argument that we have for this is that FreeBSD, Darwin and Linux all return EEXIST here.

I think I'd rather #ifndef CYGWIN this portion of the test

Sure.

[corngood]

Okay, I've made that change

FreeBSD, Darwin and Linux all return EEXIST here

This is also a decent argument for changing cygwin. I've already got at least one other posix fix for cygwin as a result of the nix tests, so I might attempt this as well.

[xokdvium]

FreeBSD

Might be best to double-check, but looking at some recent changes upstream I think that's the intenion https://reviews.freebsd.org/D50531:

if ((fmode & O_EXCL) != 0)
ndp->ni_cnd.cn_flags &= ~FOLLOW;