Skip to content

Conversation

@Mindavi
Copy link
Contributor

@Mindavi Mindavi commented Mar 28, 2021

Motivation for this change

Fixes #117181.

I first thought that the patch for gtk-doc didn't make sense, but I think it does. Still have to disable building docs for json-glib, since it also uses introspection, which doesn't support cross-compilation (well).

Important for cross-compilation, since glib is used in a lot of (GUI) applications.

Please let me know if this is the right approach. I think we should disable introspection on cross-compiled packages until we have proper support for it, so we can at least get cross-compilation working for most of the system. However, I'm not 100% sure if that's the right approach. If there are objections, I'd like an explanation of why we need introspection / what it's useful for.

I'm planning on doing more fixes as I get further with cross-compiling. I've quite a lot done, but it needs tidying up and a good look before I want to send them here. Also, I'd like some feedback on this approach before pushing all of that into nixpkgs.

Cc @Ericson2314 @samueldr

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • 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 nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@Mindavi Mindavi added 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 labels Mar 28, 2021
@SuperSandro2000 SuperSandro2000 requested a review from jtojnar March 28, 2021 22:18
@ofborg ofborg bot added 10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-darwin: 5001+ This PR causes many rebuilds on Darwin and must target the staging branches. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 5001+ This PR causes many rebuilds on Linux and must target the staging branches. labels Mar 28, 2021
@ThibautMarty ThibautMarty mentioned this pull request Apr 5, 2021
10 tasks
@Mindavi
Copy link
Contributor Author

Mindavi commented Apr 5, 2021

Does it help if I split this up? Only fixing gtk-doc and submitting the json-glib change later?

@Mindavi
Copy link
Contributor Author

Mindavi commented Apr 5, 2021

I tried building some packages with these changes (native x86-64):

  • xwayland
  • gtk3
  • flatpak

I guess that gives pretty good confidence this at least builds.

Also built:

  • pkgsCross.aarch64-multiplatform.gtk-doc
  • pkgsCross.aarch64-multiplatform.json-glib

Note that disabling gobject-introspection has some disadvantages, e.g. python apps using pygobject won't work / compile. However, I think it's a reasonable tradeoff considering some application are able to be cross-compiled when you disable introspection, and something is more than nothing.

In the future we could consider re-enabling introspection whenever we figure out how to properly cross-compile apps using introspection (see also #88222).

Copy link
Member

@jtojnar jtojnar left a comment

Choose a reason for hiding this comment

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

I agree that disabling introspection for cross is good idea for now since it makes greater range of packages build.

I think including the gtk-doc changes here is fine, as it was done in order to make json-glib cross-compile. It would be nice to add assert !isCross to gobject-introspection / python3.pkgs.pygobject3 so that the apps relying on introspection could fail fast instead of at runtime but that is probably too out of scope.

My understanding of cross-compilation is still somewhat incomplete so I am not really qualified to review that.

@Mindavi
Copy link
Contributor Author

Mindavi commented Apr 6, 2021

Thanks for the comments. I'll have a good look at this and will try some things to make cross-compilation happy. Indeed soms of the depsBuildBuild feel wrong, it's good to have that confirmed.

@Mindavi Mindavi force-pushed the cross/gtk-doc-json-glib branch from cd76e2c to c4bfe17 Compare April 7, 2021 18:47
@github-actions github-actions bot removed the 6.topic: GNOME GNOME desktop environment and its underlying platform label Apr 7, 2021
@ofborg ofborg bot requested a review from jtojnar April 7, 2021 18:59
@Mindavi
Copy link
Contributor Author

Mindavi commented Apr 7, 2021

I've updated the PR so depsBuildBuild are not used anymore in json-glib. It appears that that was not needed / I couldn't make sense of it. Maybe it's something to look at in the future.

I did have to disable strictDeps again though. It would work when I put pkg-config in depsBuildBuild, but I didn't really understand why that was needed and I couldn't really figure it out together with @Ericson2314. It doesn't seem logical to put pkg-config there, and I can't find that pattern anywhere else in nixpkgs. Somehow when I enabled strictDeps, I got an error that gobject-introspection couldn't be found anymore (even though pkg-config and gobject-introspection are both in nativeBuildInputs).

@Ericson2314
Copy link
Member

It is logical to put pkg-config there too when it is needed for build time packages' pc files, and I do recommend that.

@Mindavi Mindavi force-pushed the cross/gtk-doc-json-glib branch from c4bfe17 to 8e14bce Compare April 8, 2021 18:02
@Mindavi
Copy link
Contributor Author

Mindavi commented Apr 15, 2021

This is ready IMO. Let me know if there's anything still to be addressed.

@Ericson2314 Ericson2314 merged commit 38c8ae9 into NixOS:staging Apr 15, 2021
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 10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-darwin: 5001+ This PR causes many rebuilds on Darwin and must target the staging branches. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 5001+ This PR causes many rebuilds on Linux and must target the staging branches.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants