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

nheko: build with gcc 11 #211776

Closed
wants to merge 2 commits into from
Closed

nheko: build with gcc 11 #211776

wants to merge 2 commits into from

Conversation

rnhmjoj
Copy link
Contributor

@rnhmjoj rnhmjoj commented Jan 20, 2023

Description of changes

This should fix the aarch64 build where stdenv still defaults to gcc 9.

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux (cross-compiling)
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 23.05 Release Notes (or backporting 22.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

@rnhmjoj
Copy link
Contributor Author

rnhmjoj commented Jan 20, 2023

@GrahamcOfBorg build nheko

@ofborg ofborg bot added 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 11.by: package-maintainer This PR was created by the maintainer of the package it changes labels Jan 20, 2023
@uninsane
Copy link
Contributor

aarch64 build is still broken in the same way: https://github.com/NixOS/nixpkgs/pull/211776/checks?check_run_id=10782064902 (i've replicated this failure locally, as well)

looks like nheko's still building with gcc 9 for some reason: https://logs.nix.ci/?key=nixos/nixpkgs.211776&attempt_id=316673cb-1cf3-4250-88c5-38f1924ae8a7

-- The CXX compiler identification is GNU 9.5.0
-- The C compiler identification is GNU 9.5.0
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /nix/store/1aaqc3ln9rb8nbbjrr8izwr3flb9aiql-gcc-wrapper-9.5.0/bin/g++ - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: /nix/store/1aaqc3ln9rb8nbbjrr8izwr3flb9aiql-gcc-wrapper-9.5.0/bin/gcc - skipped

x86-64 shows an error too, about the new compiler flag being unrecognized. i assume that's because it's also using the old gcc, but haven't confirmed.

nheko doesn't actually use its stdenv argument for anything interesting. i'll bet overriding the cmake argument would be the path to get a newer gcc in there.

@uninsane
Copy link
Contributor

uninsane commented Jan 20, 2023

i'm able to actually override the gcc version with this:

   nheko = libsForQt5.callPackage ../applications/networking/instant-messengers/nheko {
     stdenv = gcc11Stdenv;
+    mkDerivation = libsForQt5.mkDerivationWith gcc11Stdenv.mkDerivation;
   };

the stdenv = override is probably not strictly necessary then. no idea what are the implications of doing this though.

edit: but now there's a linker error:

[ 71%] Linking CXX executable nheko
/nix/store/i8rnhjwfzjwvdkbhzyldi81jvraiz232-binutils-2.39/bin/ld: nheko: hidden symbol `__aarch64_cas4_acq_rel' in /nix/store/pq4x2qmdfnr4y982zmxwjq61m40cnz5r-gcc-11.3.0/lib/gcc/aarch64-unknown-linux-gnu/11.3.0/libgcc.a(cas_4_4.o) is referenced by DSO
/nix/store/i8rnhjwfzjwvdkbhzyldi81jvraiz232-binutils-2.39/bin/ld: final link failed: bad value
collect2: error: ld returned 1 exit status

This should fix the aarch64 build where stdenv still defaults to gcc 9.
@rnhmjoj
Copy link
Contributor Author

rnhmjoj commented Jan 21, 2023

Thanks, I suspected Qt had something to do with it: another reason to not use mkDerivation.
The link issue seems to be of the same underlying issue: setting the same flag doesn't work, but I suspect because it's being ignored somehow.

@NickCao
Copy link
Member

NickCao commented Jan 21, 2023

The qt5.mkDerivation function has been deprecated in #108888

diff --git a/pkgs/applications/networking/instant-messengers/nheko/default.nix b/pkgs/applications/networking/instant-messengers/nheko/default.nix
index 680833ca2e6..09819510136 100644
--- a/pkgs/applications/networking/instant-messengers/nheko/default.nix
+++ b/pkgs/applications/networking/instant-messengers/nheko/default.nix
@@ -1,6 +1,5 @@
 { lib
 , stdenv
-, mkDerivation
 , fetchFromGitHub
 , cmake
 , asciidoc
@@ -24,6 +23,7 @@
 , qtmultimedia
 , qtquickcontrols2
 , qttools
+, wrapQtAppsHook
 , re2
 , spdlog
 , voipSupport ? true
@@ -31,7 +31,7 @@
 , libnice
 }:

-mkDerivation rec {
+stdenv.mkDerivation rec {
   pname = "nheko";
   version = "0.11.1";

@@ -47,6 +47,7 @@ mkDerivation rec {
     cmake
     lmdbxx
     pkg-config
+    wrapQtAppsHook
   ];

   buildInputs = [

Edit: the linking error persists, guess that we just cannot bypass that problem introduced in our bootstrap process.

@ghost
Copy link

ghost commented Jan 21, 2023

No dice:

       > [ 71%] Linking CXX executable nheko
       > /nix/store/i8rnhjwfzjwvdkbhzyldi81jvraiz232-binutils-2.39/bin/ld: nheko: hidden symbol `__aarch64_cas4_acq_rel' in /nix/store/pq4x2qmdfnr4y982zmxwjq61m40cnz5r-gcc-11.3.0/lib/gcc/aarch64-unknown-linux-gnu/11.3.0/libgcc.a(cas_4_4.o) is referenced by DSO
       > /nix/store/i8rnhjwfzjwvdkbhzyldi81jvraiz232-binutils-2.39/bin/ld: final link failed: bad value
       > collect2: error: ld returned 1 exit status
       > make[2]: *** [CMakeFiles/nheko.dir/build.make:4119: nheko] Error 1
       > make[1]: *** [CMakeFiles/Makefile2:150: CMakeFiles/nheko.dir/all] Error 2
       > make: *** [Makefile:136: all] Error 2
       For full logs, run 'nix log /nix/store/19hniicm81cv9pjh2i86pbswm42k001q-nheko-0.11.1.drv'.

I don't think this approach is going to work unless you add -mno-outline-atomics to all of nheko's dependencies too.

Or we could just fix the bootstrap. 🤷

@rnhmjoj
Copy link
Contributor Author

rnhmjoj commented Jan 21, 2023

The link issue seems to be of the same underlying issue

I confirmed with NIX_DEBUG that the flag is being set, so it must be something else.

@ghost
Copy link

ghost commented Jan 21, 2023

I confirmed with NIX_DEBUG that the flag is being set, so it must be something else.

You need to set the flag in all of nheko's dependencies too. If you're trying to fix it that way.

@rnhmjoj
Copy link
Contributor Author

rnhmjoj commented Jan 21, 2023

You need to set the flag in all of nheko's dependencies too. If you're trying to fix it that way.

Well, then I guess it stays broken because this is unfeasible.

@uninsane
Copy link
Contributor

uninsane commented Jan 21, 2023 via email

@rnhmjoj
Copy link
Contributor Author

rnhmjoj commented Jan 21, 2023

unless it was a security fix or something, i’d push to revert that branch to a version that ...

Nothing in particular, I normally backport updates for leaf packages like nheko. Sorry I didn't pay attention to the aarch64 failure, I guess I assume it was already broken. I'm ok with reverting.

@NickCao
Copy link
Member

NickCao commented Jan 22, 2023

Revert done in #212014 and #212013

@rnhmjoj
Copy link
Contributor Author

rnhmjoj commented Jan 22, 2023

Uhm, I would have reverted only for 22.11. What are we supposed to do in case an update break a tier-2 platform?
Should a single platform hold the update for the rest?

@wegank
Copy link
Member

wegank commented Jan 22, 2023

No dice:

       > [ 71%] Linking CXX executable nheko
       > /nix/store/i8rnhjwfzjwvdkbhzyldi81jvraiz232-binutils-2.39/bin/ld: nheko: hidden symbol `__aarch64_cas4_acq_rel' in /nix/store/pq4x2qmdfnr4y982zmxwjq61m40cnz5r-gcc-11.3.0/lib/gcc/aarch64-unknown-linux-gnu/11.3.0/libgcc.a(cas_4_4.o) is referenced by DSO
       > /nix/store/i8rnhjwfzjwvdkbhzyldi81jvraiz232-binutils-2.39/bin/ld: final link failed: bad value
       > collect2: error: ld returned 1 exit status
       > make[2]: *** [CMakeFiles/nheko.dir/build.make:4119: nheko] Error 1
       > make[1]: *** [CMakeFiles/Makefile2:150: CMakeFiles/nheko.dir/all] Error 2
       > make: *** [Makefile:136: all] Error 2
       For full logs, run 'nix log /nix/store/19hniicm81cv9pjh2i86pbswm42k001q-nheko-0.11.1.drv'.

I don't think this approach is going to work unless you add -mno-outline-atomics to all of nheko's dependencies too.

Or we could just fix the bootstrap. 🤷

I think the canonical fix is to stick to -lgcc (and it works, see #212051).

@rnhmjoj rnhmjoj closed this Jan 22, 2023
@rnhmjoj rnhmjoj deleted the pr-nheko branch July 10, 2023 13:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 11.by: package-maintainer This PR was created by the maintainer of the package it changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants