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

feedbackd: support cross-compilation #175998

Merged
merged 3 commits into from
Jun 8, 2022
Merged

Conversation

Mindavi
Copy link
Contributor

@Mindavi Mindavi commented Jun 2, 2022

Disables introspection for now.

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux (cross-compiled)
    • 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/)
  • Fits CONTRIBUTING.md.

Disables introspection for now.
@Mindavi Mindavi added the 6.topic: cross-compilation Building packages on a different platform than they will be used on label Jun 2, 2022
@Mindavi Mindavi marked this pull request as draft June 2, 2022 20:17
@Mindavi
Copy link
Contributor Author

Mindavi commented Jun 2, 2022

So yeah, this all seems correct, but the wrapGAppsHook produces a binary for x86-64... 😞. See #175045. Guess we can still merge this anyway, but another fix is needed to actually make it fully work.

@Mindavi Mindavi marked this pull request as ready for review June 2, 2022 20:23
@ofborg ofborg bot requested a review from tomfitzhenry June 3, 2022 17:04
@tomfitzhenry
Copy link
Contributor

tomfitzhenry commented Jun 4, 2022

So it looks like cross-compilation and gobject introspection are mutually exclusive, until #72868 is fixed.

The pro case for the status quo of keeping introspection:

Cross-compilation is important to have, but is it worth losing the pros of keeping introspection?

FWIW, I think it's common for mobile NixOS users to compile natively (either on an existing aarch64 device, or via binfmt emulation).

@Mindavi
Copy link
Contributor Author

Mindavi commented Jun 6, 2022

To me it wouldn't make sense that the option of disabling introspection would be available if it broke everything, but that may be naive.

I agree that keeping simple would be nice. I don't agree that it aligns with upstream if they provide the option.

Some progress is being made on improving the gir situation! #176464

So we may be able to remove the disables in the near future.

Personally I like to be able to cross-compile, even if it reduces functionality a bit. I do try it out from time to time and am aiming to get a full-blown image (with DM) working at some point, just because I like to prove it's possible and to be able to use my own hardware to build the image. But of course it would be nice to have more functionality.

Thanks for keeping the discussion alive btw, it's good that we think about these choices.

@tomfitzhenry
Copy link
Contributor

SGTM. I see low risk of breaking usecases, and since feedbackd is typically used on aarch64, I can see how cross-compilation would be convenient.

Have you tried compilation via binfmt? That should work, even without cross-compilation.

Co-authored-by: Sandro <sandro.jaeckel@gmail.com>
@Mindavi Mindavi merged commit dd16129 into NixOS:master Jun 8, 2022
@ofborg ofborg bot requested a review from tomfitzhenry June 8, 2022 19:07
@github-actions
Copy link
Contributor

github-actions bot commented Jun 8, 2022

Successfully created backport PR #176942 for release-22.05.

@Mindavi Mindavi deleted the feedbackd/cross branch June 15, 2022 18:36
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: 0 10.rebuild-linux: 1-10
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants