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

Don't let Qt require a minimum kernel of 3.17+ (even if we're building against headers satisfying that requirement) #40577

Closed
wants to merge 1 commit into from

Conversation

dtzWill
Copy link
Member

@dtzWill dtzWill commented May 16, 2018

Fixes #38832.

Fixes use of qt 5.10 on many distributions not shipping 3.17 or newer.

I believe even kernels with these features --if such exist-- won't work since
the check is specifically for version and not for the features in question.

This is especially "unfortunate" since libraries requiring versions newer
than what is available are silently overlooked during searches,
resulting in messages indicating that "libFoo.so" cannot be found
(instead of "libFoo.so found but requires kernel X.Y, you're running A.B" or whatever).

As indicated in the commit, someone checking this situation would be appreciated :).
And to be completely clear, this compatibility is achieved by disabling use of
the renameat2 and getentropy syscalls (even if they are supported);
this does not seem problematic to me but is something to be considered.

  • Tested using sandboxing (nix.useSandbox on NixOS, or option build-use-sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

Before this change:
$ readelf --notes /nix/store/zf5yja02g8n8dzgs25pqfd8w3myfzgzc-qtbase-5.10.1/lib/libQt5Core.so

Displaying notes found at file offset 0x004a7778 with length 0x00000020:
  Owner                 Data size       Description
  GNU                  0x00000010       NT_GNU_ABI_TAG (ABI version tag)
    OS: Linux, ABI: 3.17.0

After:
$ readelf --notes /nix/store/sg1s9hdw0b7p6h0dwg09g4lxy1acq7y6-qtbase-5.10.1/lib/libQt5Core.so

Displaying notes found at file offset 0x004a7dcc with length 0x00000020:
  Owner                 Data size       Description
  GNU                  0x00000010       NT_GNU_ABI_TAG (ABI version tag)
    OS: Linux, ABI: 2.6.28

-----------

The above paths were before rebasing the commit onto staging,
and it'd probably be good to have someone confirm the same happens
when built on a hydra builder or other non-dtzWill machine :).
@dezgeg
Copy link
Contributor

dezgeg commented May 16, 2018

I'm confused. The code at http://code.qt.io/cgit/qt/qtbase.git/tree/src/corelib/io/qfilesystemengine_unix.cpp?h=dev first tries renameat2 and then if that fails due to not being supported, falls back to the same code that would be executed if renameat2 was disabled at compile time. What does strace say for that renameat2 call?

@dezgeg
Copy link
Contributor

dezgeg commented May 16, 2018

Ah now I get what you mean:

$ file result/lib/libQt5Core.so.5.10.1
result/lib/libQt5Core.so.5.10.1: ELF 64-bit LSB shared object, x86-64, version 1 (GNU/Linux), dynamically linked, interpreter /nix/store/27x7pinqdsl9f3rpbm8bsszd9fhwq266-glibc-2.27/lib/ld-linux-x86-64.so.2, for GNU/Linux 3.17.0, stripped
                                                                                                                                                                                                               ^^^^^^^^^^^^^^^^^^^^

Well that's crazy. I wonder what's causing the kernel version tag to be emitted.

@dtzWill
Copy link
Member Author

dtzWill commented May 16, 2018

If no libc wrapper is detected (not avail in our glibc, unsure what version adds it), but SYS_renameat2 is defined (new kernel headers), qt emits its own syscall wrapper and sets kernel version requirement (non-standard, glibc-only). Handling ENOSYS instead might be better, but that's not really the point. The real problem is that our Qt is imposing a kernel requirement unexpectedly and significantly newer than we currently require.

I also found the logs a bit confusing at first, since they report that checks for these functions (renameat2, etc.) fail but don't mention they'll be invoking them as syscalls and whatnot.

I didn't track down where the version is set, would be curious how they manage to tie that into their build system. (understanding this isn't important, just curious)

@dtzWill
Copy link
Member Author

dtzWill commented May 16, 2018

IMHO this should be backported to 18.03 as well. Perhaps sent to staging-18.03 first, if we're doing that? :)

@dezgeg
Copy link
Contributor

dezgeg commented May 16, 2018

Qt emits its own syscall wrapper and sets kernel version requirement (non-standard, glibc-only)

Right, that should be reported as a Qt bug. There is no reason to do anything like that, checking for ENOSYS is the way to handle this.

But yes, in the meantime, let's merge this.

@dtzWill
Copy link
Member Author

dtzWill commented May 17, 2018

(Should we wait to hear from @ttuegel?)

@dezgeg
Copy link
Contributor

dezgeg commented May 18, 2018

Applied in 39696b6. I added some comments.

@dezgeg dezgeg closed this May 18, 2018
@ttuegel
Copy link
Member

ttuegel commented May 18, 2018

Should we wait to hear from @ttuegel?

No 😉 applying this was the right thing to do. Is this backported to 18.03 yet? (It doesn't look like it, to me.) If not, I'm happy to take care of it.

@dtzWill
Copy link
Member Author

dtzWill commented May 21, 2018

@ttuegel if it's no trouble, please do so if you haven't already!

Also can we pick this to master too? :3. Or do we think staging will be merged soon-ish?

@knedlsepp
Copy link
Member

knedlsepp commented Jun 13, 2018

@ttuegel Could this be backported to 18.03? I guess this would solve: #38832

@dezgeg
Copy link
Contributor

dezgeg commented Jun 13, 2018

Applied in a7b6a91.

drbenmorgan added a commit to SuperNEMO-DBD/homebrew-cadfael that referenced this pull request Jan 21, 2019
Running an ubuntu 18 container on a CentOS 7 system warned about
libQt5Core.so not being found. Binaries were checked for correct
RUNPATHs and found to be o.k. Other Qt libraries were found and loaded
without issue. Probable cause found to be Qt encoding kernel ABI into
library (and thus container shared kernel not supporting this) per
report here:

NixOS/nixpkgs#40577

Use same set of arguments in that patch to build binaries with
compatibility for older kernels.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants