Skip to content

Conversation

@Artturin
Copy link
Member

@Artturin Artturin commented Jun 5, 2022

with this json-glib cross-compiles and the gir files exist, have not tested running any binaries on aarch64

nix build ".#pkgsCross.aarch64-multiplatform.json-glib"

#88222
#72868

https://github.com/void-linux/void-packages/blob/master/srcpkgs/gobject-introspection/template and the buildroot mk file were inspirations

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.

@Artturin Artturin added the 6.topic: cross-compilation Building packages on a different platform than they will be used on label Jun 5, 2022
@Artturin Artturin requested a review from Mindavi June 5, 2022 20:04
@Artturin Artturin marked this pull request as draft June 5, 2022 20:04
@Artturin Artturin requested a review from Ericson2314 June 5, 2022 20:04
@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: 2501-5000 This PR causes many rebuilds on Darwin and should 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 Jun 5, 2022
@Mindavi
Copy link
Contributor

Mindavi commented Jun 5, 2022

Hmm, will we need to build such wrapper for each package using gobject-introspection?

Copy link
Member

@SuperSandro2000 SuperSandro2000 left a comment

Choose a reason for hiding this comment

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

Only some formatting improvements. Thanks for trying to bring this forward. I really really appreciate it!

@ofborg ofborg bot requested review from 7c6f434c and edolstra June 6, 2022 01:52
@Mindavi
Copy link
Contributor

Mindavi commented Jun 7, 2022

I do like this better than having it in json-glib at least :). Still need to try it out

@ofborg ofborg bot added 8.has: package (new) This PR adds a new package 8.has: clean-up This PR removes packages or removes other cruft labels Jun 7, 2022
@Artturin Artturin changed the title WIP: gobject-introspection fix cross gobject-introspection: support cross-compilation Jun 7, 2022
Copy link
Member

@SuperSandro2000 SuperSandro2000 left a comment

Choose a reason for hiding this comment

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

I think we are in a good state here now!

@Mindavi what do you think? Can we merge this soon?

@github-actions github-actions bot added the 8.has: documentation This PR adds or changes documentation label Jun 8, 2022
@Artturin
Copy link
Member Author

Artturin commented Jun 9, 2022

I think we are in a good state here now!
@Mindavi what do you think? Can we merge this soon?

I think we can merge this soon-ish. I want to do some testing, but I tried out an older version (from yesterday) and that was already looking promising. Let's also address the review comments 😄.

Just to get some idea of the status:

1. For any package _not_ using meson, we don't need to add the hook, right? It should work out of the box?

2. For any package using meson we need the hook because meson does it's own handing of gobject-introspection, correct?

I'm not really sure if we really want to make a separate hook or if we always want to do this when meson and gobject-introspection are both used. I think it would be more convenient and cause less friction if we could automatically add the cross-file when needed instead of having to explicitly define it.

In general, most of this is looking good!

The hook is not always needed even with meson, small example
json-glib doesn't need it while gtk3 does

@Mindavi
Copy link
Contributor

Mindavi commented Jun 9, 2022

Cool, yeah, if it's not always needed I think it's preferred to not have to have an emulator unless it's really needed.

@Artturin Artturin force-pushed the gobjecfix branch 2 times, most recently from 52d21bf to 047597d Compare June 10, 2022 16:35
@Artturin Artturin force-pushed the gobjecfix branch 2 times, most recently from ebf1f89 to 8e18a52 Compare June 10, 2022 17:37
@Artturin Artturin requested review from Ericson2314, Mindavi and SuperSandro2000 and removed request for Mindavi and SuperSandro2000 June 10, 2022 20:10
@bobby285271 bobby285271 added the 12.approvals: 1 This PR was reviewed and approved by one person. label Jun 12, 2022
Copy link
Contributor

@Mindavi Mindavi left a comment

Choose a reason for hiding this comment

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

Diff now LGTM

@Artturin Artturin merged commit ce62ff7 into NixOS:staging Jun 12, 2022
@Artturin Artturin deleted the gobjecfix branch June 12, 2022 14:32
@Artturin
Copy link
Member Author

Thanks everyone!

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 8.has: clean-up This PR removes packages or removes other cruft 8.has: documentation This PR adds or changes documentation 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-darwin: 2501-5000 This PR causes many rebuilds on Darwin and should 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. 11.by: package-maintainer This PR was created by a maintainer of all the package it changes. 12.approvals: 1 This PR was reviewed and approved by one person.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants