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

lots of cross fixes #189397

Merged
merged 7 commits into from
Sep 8, 2022
Merged

lots of cross fixes #189397

merged 7 commits into from
Sep 8, 2022

Conversation

Artturin
Copy link
Member

@Artturin Artturin commented Sep 2, 2022

Description of changes
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/)
  • 22.11 Release Notes (or backporting 22.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
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

@github-actions github-actions bot added 6.topic: GNOME GNOME desktop environment and its underlying platform 6.topic: python labels Sep 2, 2022
@Artturin Artturin changed the title fix lot of cross builds lots of cross fixes Sep 2, 2022
@Artturin Artturin added the 6.topic: cross-compilation Building packages on a different platform than they will be used on label Sep 2, 2022
@@ -41,6 +41,8 @@ stdenv.mkDerivation rec {
] ++ lib.optional gstreamerSupport "--enable-cogl-gst"
++ lib.optionals (!stdenv.isDarwin) [ "--enable-gles1" "--enable-gles2" ];

# TODO: this shouldn't propagate so many things
# especially not gobject-introspection
Copy link
Contributor

Choose a reason for hiding this comment

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

"-Dinstalled_test_datadir=${placeholder "installedTests"}/share"
"-Dinstalled_test_bindir=${placeholder "installedTests"}/libexec"
];

doCheck = true;

postPatch = ''
substituteInPlace tests/meson.build \
Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds very hacky :)

Copy link
Member Author

@Artturin Artturin Sep 2, 2022

Choose a reason for hiding this comment

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

wonder why i did that instead of just making the installedTests conditional on host == build
changed it to that

@Mindavi
Copy link
Contributor

Mindavi commented Sep 2, 2022

Thanks! Currently diff LGTM, but indeed some things should go upstream sooner rather than later. Do you want to track that somewhere or would the source of nixpkgs be enough to remember?

Sprinkled some small comments, nothing major.

@Artturin Artturin force-pushed the gobjectfunfixessplit branch 2 times, most recently from ced75a7 to a57e5de Compare September 2, 2022 13:13
@Artturin Artturin force-pushed the gobjectfunfixessplit branch 2 times, most recently from 41e0af9 to f813b1f Compare September 2, 2022 15:30
doesn't work yet

```
gnome-tour-aarch64-unknown-linux-gnu> error: linking with `/nix/store/3d9zjv5vaqjxb9kwbdqsd194alwm97x1-gcc-wrapper-11.3.0/bin/cc` failed: exit status: 1
gnome-tour-aarch64-unknown-linux-gnu>   |
gnome-tour-aarch64-unknown-linux-gnu>   = note: "/nix/store/3d9zjv5vaqjxb9kwbdqsd194alwm97x1-gcc-wrapper-11.3.0/bin/cc" "-m64" "/build/rustcd8pO0A/symbols.o" "/build/gnome-tour-42.0/build/src/release/deps/gnome_to
...
gnome-tour-aarch64-unknown-linux-gnu>           /nix/store/vhf2cr6immz4qdxd83y025fnxim8mmfg-binutils-2.38/bin/ld: skipping incompatible /nix/store/c84gzzifyh10w9m612wxcas04r7fq4h0-glib-aarch64-unknown-linux-gnu-2.
72.3/lib/libglib-2.0.so when searching for -lglib-2.0
gnome-tour-aarch64-unknown-linux-gnu>           /nix/store/vhf2cr6immz4qdxd83y025fnxim8mmfg-binutils-2.38/bin/ld: cannot find -lglib-2.0: No such file or directory
...
gnome-tour-aarch64-unknown-linux-gnu>           collect2: error: ld returned 1 exit status
...
gnome-tour-aarch64-unknown-linux-gnu> FAILED: src/gnome-tour
gnome-tour-aarch64-unknown-linux-gnu> /nix/store/m5n32vy7rbfrqcxigw1p6wyx3cj7smg9-coreutils-9.1/bin/env CARGO_HOME=/build/gnome-tour-42.0/build/cargo-home /nix/store/76l9r44x1sv323c90j6vi401n4fvims2-cargo-1.62.1/b
in/cargo build --manifest-path /build/gnome-tour-42.0/Cargo.toml --target-dir /build/gnome-tour-42.0/build/src --release && cp src/release/gnome-tour src/gnome-tour
gnome-tour-aarch64-unknown-linux-gnu> ninja: build stopped: subcommand failed.
```
@Artturin Artturin marked this pull request as ready for review September 2, 2022 16:00
];

buildInputs = [
glib
Copy link
Member

Choose a reason for hiding this comment

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

# for some reason upstream does the following instead of using meson builtins
# TODO: tell upstream to use meson builtins / remove the duplicate check

# dependency('pkg-config', version : '>= 0.24')
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

The extraneous mkenums check was removed in https://gitlab.gnome.org/GNOME/gnome-logs/-/merge_requests/37

Copy link
Member Author

Choose a reason for hiding this comment

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

pulled both in patches

@@ -38,6 +38,7 @@ stdenv.mkDerivation rec {
python3
desktop-file-utils
wrapGAppsHook4
gjs
Copy link
Member

@jtojnar jtojnar Sep 2, 2022

Choose a reason for hiding this comment

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

This should be fixed by adding native: true kwarg to find_program here:

https://gitlab.gnome.org/GNOME/gnome-characters/-/blob/b72bf4e1a7cbe8446ef2659d432b78ac8b50df83/src/meson.build#L2

Edit: that might not work looking at https://mesonbuild.com/Reference-manual_functions.html#find_program but it is definitely a native build input.

Copy link
Member Author

Choose a reason for hiding this comment

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

find_program already looks in PATH

ther reason its needed in buildInputs is https://gitlab.gnome.org/GNOME/gnome-characters/-/blob/b72bf4e1a7cbe8446ef2659d432b78ac8b50df83/meson.build#L47

Copy link
Member

Choose a reason for hiding this comment

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

buildInputs I am fine with since gjs is an interpreter used for running the app. But it should not be necessary at build time, as far as I can tell.

Copy link
Member Author

@Artturin Artturin Sep 2, 2022

Choose a reason for hiding this comment

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

Comment on lines +48 to +50
# error: Package `...' not found in specified Vala API directories or GObject-Introspection GIR directories
# TODO: the vala setuphook should look for vala filess in targetOffset instead of hostOffset
gsound
Copy link
Member

Choose a reason for hiding this comment

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

Then it should be fixed there.

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah im going to do it in a separate pr

Copy link
Member Author

Choose a reason for hiding this comment

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

@@ -25,6 +27,11 @@ stdenv.mkDerivation rec {
glib
];

postPatch = ''
Copy link
Member

@jtojnar jtojnar Sep 2, 2022

Choose a reason for hiding this comment

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

We are switching to meson in #182618, we can probably cherry-pick that to master.

@Artturin Artturin force-pushed the gobjectfunfixessplit branch 6 times, most recently from 2c9bff3 to 470e9ba Compare September 2, 2022 19:35
@teto
Copy link
Member

teto commented Sep 3, 2022

I won't/can't comment on the technical side but @Artturin I wonder what's your motivation for all these cross-building fixes ? It is great, just curious :)

@Artturin
Copy link
Member Author

Artturin commented Sep 7, 2022

I won't/can't comment on the technical side but @Artturin I wonder what's your motivation for all these cross-building fixes ? It is great, just curious :)

I want to see how much of nixpkgs we can get to cross-compile

they weren't being built before at all
no idea why this still happens here when this issue was fixed in
gobject-introspection hook and the fix works for other packages
@Artturin Artturin merged commit 5fcc288 into NixOS:staging Sep 8, 2022
@Artturin Artturin deleted the gobjectfunfixessplit branch September 8, 2022 20:54
uninsane added a commit to uninsane/nixpkgs that referenced this pull request Nov 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: cross-compilation Building packages on a different platform than they will be used on 6.topic: GNOME GNOME desktop environment and its underlying platform 6.topic: python 10.rebuild-darwin: 501+ 10.rebuild-darwin: 1001-2500 10.rebuild-linux: 501+ 10.rebuild-linux: 5001+
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants