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

lomiri.geonames: init at 0.3.0 #241259

Merged
merged 3 commits into from
Jul 22, 2023

Conversation

OPNA2608
Copy link
Contributor

@OPNA2608 OPNA2608 commented Jul 3, 2023

Description of changes

Working towards #99090.

Geonames, a library for parsing and querying a local copy of the geonames.org database. Required by Lomiri & some of its system applications.

Cross-compilation is known-broken without some hostPlatform emulation: https://gitlab.com/ubports/development/core/geonames/-/issues/1. I've used stdenv.hostPlatform.emulator buildPackages to make at least aarch64-linux and armv7l-linux cross work. Upstream issue also mentions glibc -> musl cross for same buildPlatform being broken, but that fails much earlier in some dependencies.

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • 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.11 Release Notes (or backporting 23.05 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
  • Fits CONTRIBUTING.md.

"-DWANT_DEMO=${lib.boolToString withExamples}"
"-DWANT_TESTS=${lib.boolToString finalAttrs.doCheck}"
# Keeps finding & using glib-compile-resources from buildInputs otherwise
"-DCMAKE_PROGRAM_PATH=${lib.makeBinPath [ buildPackages.glib.dev ]}"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have added glib to nativeBuildInputs, Setting CMAKE_PROGRAM_PATH here may no longer be necessary

Copy link
Contributor Author

@OPNA2608 OPNA2608 Jul 6, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without this, it still picks up the one from buildInputs instead of nativeBuildInputs.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

--argstr crossSystem riscv64-linux with this line removed:

[ 41%] Generating geonames-resources.c
/nix/store/51sszqz1d9kpx480scb1vllc00kxlx79-bash-5.2-p15/bin/bash: line 1: /nix/store/76cd2vr6rsp3fzwlrpaq6n8phdklnvq0-glib-riscv64-unknown-linux-gnu-2.76.3-dev/bin/glib-compile-resources: cannot execute binary file: Exec format error
make[2]: *** [src/CMakeFiles/geonames.dir/build.make:74: src/geonames-resources.c] Error 126

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test nix-build -A pkgsCross.riscv64.lomiri.geonames , It should be built successfully without CMAKE_PROGRAM_PATH

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does not, same error.

[ 41%] Generating geonames-resources.c
/nix/store/51sszqz1d9kpx480scb1vllc00kxlx79-bash-5.2-p15/bin/bash: line 1: /nix/store/76cd2vr6rsp3fzwlrpaq6n8phdklnvq0-glib-riscv64-unknown-linux-gnu-2.76.3-dev/bin/glib-compile-resources: cannot execute binary file: Exec format error
make[2]: *** [src/CMakeFiles/geonames.dir/build.make:74: src/geonames-resources.c] Error 126
make[1]: *** [CMakeFiles/Makefile2:199: src/CMakeFiles/geonames.dir/all] Error 2
make: *** [Makefile:136: all] Error 2
error: builder for '/nix/store/vccsfh93aaznxm0q30y4q6vjld7qa4nh-geonames-riscv64-unknown-linux-gnu-0.3.0.drv' failed with exit code 2;

Copy link
Member

@wineee wineee Jul 11, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nix-shell:~/.cache/nixpkgs-review/pr-241259/nixpkgs]$ git diff

pkgs/desktops/lomiri/development/geonames/default.nix
─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────

───────────────────────────────────────┐
85: stdenv.mkDerivation (finalAttrs: { │
───────────────────────────────────────┘
    "-DWANT_DEMO=${lib.boolToString withExamples}"
    "-DWANT_TESTS=${lib.boolToString finalAttrs.doCheck}"
    # Keeps finding & using glib-compile-resources from buildInputs otherwise
    "-DCMAKE_PROGRAM_PATH=${lib.makeBinPath [ buildPackages.glib.dev ]}"
    #"-DCMAKE_PROGRAM_PATH=${lib.makeBinPath [ buildPackages.glib.dev ]}"
  ];

  preInstall = lib.optionalString withDocumentation ''

[nix-shell:~/.cache/nixpkgs-review/pr-241259/nixpkgs]$ nix-build -A lomiri.geonames --argstr crossSystem riscv64-linux
/nix/store/3m45psnzk57q55zy8xgwd2q449a4jahq-geonames-riscv64-unknown-linux-gnu-0.3.0

Unable to reproduce build failure

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Weird.

[ 25%] Built target generated-files
[ 33%] Generating locales-gen
[ 33%] Built target localesgen
[ 41%] Generating geonames-resources.c
/nix/store/51sszqz1d9kpx480scb1vllc00kxlx79-bash-5.2-p15/bin/bash: line 1: /nix/store/76cd2vr6rsp3fzwlrpaq6n8phdklnvq0-glib-riscv64-unknown-linux-gnu-2.76.3-dev/bin/glib-compile-resources: cannot execute binary file: Exec format error
make[2]: *** [src/CMakeFiles/geonames.dir/build.make:74: src/geonames-resources.c] Error 126
make[1]: *** [CMakeFiles/Makefile2:199: src/CMakeFiles/geonames.dir/all] Error 2
make: *** [Makefile:136: all] Error 2
error: builder for '/nix/store/vccsfh93aaznxm0q30y4q6vjld7qa4nh-geonames-riscv64-unknown-linux-gnu-0.3.0.drv' failed with exit code 2;
       last 10 log lines:
       > /build/source/data/alternatenames/GS.txt
       > /build/source/data/alternatenames/GT.txt
       > [ 25%] Built target generated-files
       > [ 33%] Generating locales-gen
       > [ 33%] Built target localesgen
       > [ 41%] Generating geonames-resources.c
       > /nix/store/51sszqz1d9kpx480scb1vllc00kxlx79-bash-5.2-p15/bin/bash: line 1: /nix/store/76cd2vr6rsp3fzwlrpaq6n8phdklnvq0-glib-riscv64-unknown-linux-gnu-2.76.3-dev/bin/glib-compile-resources: cannot execute binary file: Exec format error
       > make[2]: *** [src/CMakeFiles/geonames.dir/build.make:74: src/geonames-resources.c] Error 126
       > make[1]: *** [CMakeFiles/Makefile2:199: src/CMakeFiles/geonames.dir/all] Error 2
       > make: *** [Makefile:136: all] Error 2
       For full logs, run 'nix log /nix/store/vccsfh93aaznxm0q30y4q6vjld7qa4nh-geonames-riscv64-unknown-linux-gnu-0.3.0.drv'.

[nix-shell:~/.cache/nixpkgs-review/pr-241259/nixpkgs]$ git diff
diff --git a/pkgs/desktops/lomiri/development/geonames/default.nix b/pkgs/desktops/lomiri/development/geonames/default.nix
index c4e327caf93..3532916a50c 100644
--- a/pkgs/desktops/lomiri/development/geonames/default.nix
+++ b/pkgs/desktops/lomiri/development/geonames/default.nix
@@ -86,7 +86,7 @@ stdenv.mkDerivation (finalAttrs: {
     "-DWANT_DEMO=${lib.boolToString withExamples}"
     "-DWANT_TESTS=${lib.boolToString finalAttrs.doCheck}"
     # Keeps finding & using glib-compile-resources from buildInputs otherwise
-    "-DCMAKE_PROGRAM_PATH=${lib.makeBinPath [ buildPackages.glib.dev ]}"
+    #"-DCMAKE_PROGRAM_PATH=${lib.makeBinPath [ buildPackages.glib.dev ]}"
   ];
 
   preInstall = lib.optionalString withDocumentation ''

Copy link
Contributor Author

@OPNA2608 OPNA2608 Jul 11, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you maybe have riscv64-linux in your boot.binfmt.emulatedSystems so you can execute risc-v binaries on your system? I think that would cause it to "work".

Warning: the builder can execute all emulated systems within the same build, which introduces impurities in the case of cross compilation.

I have aarch64-linux in mine, and cross-compiling for that likely "works" without CMAKE_PROGRAM_PATH because of it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, that makes sense

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-ready-for-review/3032/2415

Comment on lines 44 to 51
substituteInPlace doc/reference/CMakeLists.txt \
--replace "\''${CMAKE_INSTALL_DATADIR}/gtk-doc/html/\''${PROJECT_NAME}" "\''${CMAKE_INSTALL_DOCDIR}"
substituteInPlace demo/CMakeLists.txt \
--replace 'RUNTIME DESTINATION bin' 'RUNTIME DESTINATION ''${CMAKE_INSTALL_BINDIR}'
'' + lib.optionalString (stdenv.buildPlatform != stdenv.hostPlatform) ''
# Built for hostPlatform, executed during build
substituteInPlace src/CMakeLists.txt \
--replace 'COMMAND mkdb' 'COMMAND ${stdenv.hostPlatform.emulator buildPackages} mkdb'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this upstreamable?

Copy link
Contributor Author

@OPNA2608 OPNA2608 Jul 12, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see how this could be upstreamed. Ideally upstream shouldn't rely on the user having an emulator to run the build-time executable - building for the native system (buildPlatform) in a cross situation with CMake is abit tricky but IMO the real solution here. This is more of a crutch to deal with CMake not having first-class support for something like that, and relies on us knowing the correct emulator command for the cross platform.

We could use CMAKE_CROSSCOMPILING_EMULATOR=${stdenv.hostPlatform.emulator buildPackages} instead of patching in our emulator call, I guess that would be slightly better, but still not a real fix.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Switched to setting CMAKE_CROSSCOMPILING_EMULATOR, confirmed that it works by cross-compiling for armv7l-linux.

Ideally upstream will address this with a proper fix Soon:tm:, either for or after the 22.04 release of ubtouch.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

things like CMAKE_INSTALL_DOCDIR/CMAKE_INSTALL_BINDIR should be upstreamable

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@OPNA2608 OPNA2608 requested a review from wineee July 14, 2023 16:07
@wineee
Copy link
Member

wineee commented Jul 15, 2023

Result of nixpkgs-review pr 241259 run on x86_64-linux 1

4 packages built:
  • lomiri.geonames
  • lomiri.geonames.bin
  • lomiri.geonames.dev
  • lomiri.geonames.doc

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-already-reviewed/2617/1027

@SuperSandro2000 SuperSandro2000 merged commit 61677c9 into NixOS:master Jul 22, 2023
10 checks passed
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.

4 participants