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

cinnamon.xapps: move gobject-introspection to buildInputs #79780

Merged
merged 1 commit into from Feb 13, 2020

Conversation

@mkg20001
Copy link
Member

@mkg20001 mkg20001 commented Feb 10, 2020

Motivation for this change
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.
@mkg20001 mkg20001 requested a review from worldofpeace Feb 10, 2020
Copy link
Contributor

@jtojnar jtojnar left a comment

Since the bindings are generated using g-ir-* tools, this needs to be in nativeBuildInputs. If this project also links against libgirepository-1.0.so or depends on one of the typelibs shipped along, then it should go to buildInputs as well.

Could you mention the reason in the commit message?

Also the message is confusing, as gobject (GLib’s object system) ≠ gobject-introspection (set of tooling for introspecting libraries based on GObject and generating bindable typelib libraries from the introspected data). To add to the confusion, gobject-introspection package also does provide typelibs for glib (including GObject-2.0.typelib).

@mkg20001
Copy link
Member Author

@mkg20001 mkg20001 commented Feb 11, 2020

I have no idea. It seems to reference glib-object.h in some places, but otherwise I just followed the pattern of @worldofpeace telling me that I should add gobject-introspection as a build input, assuming it would always be one.

@worldofpeace
Copy link
Member

@worldofpeace worldofpeace commented Feb 11, 2020

@mkg20001
Copy link
Member Author

@mkg20001 mkg20001 commented Feb 11, 2020

So build input or not?
I'm still not sure.

@worldofpeace
Copy link
Member

@worldofpeace worldofpeace commented Feb 11, 2020

I believe it falls under the "generating bindable typelib libraries from the introspected data" so it should be native. But we can confirm this with @jtojnar.

@jtojnar
Copy link
Contributor

@jtojnar jtojnar commented Feb 11, 2020

Well, that would be for the generator but typelibs itself are runtime dependencies so they should go to regular buildInputs. Perhaps even propagated ones since the GIR files include GObject (not sure).

@worldofpeace
Copy link
Member

@worldofpeace worldofpeace commented Feb 11, 2020

😄 Ahh, we should probably document this to help people out.

Copy link
Member

@worldofpeace worldofpeace left a comment

Can you fix your commit message?

@mkg20001 mkg20001 force-pushed the mkg20001:ge branch from d089dbf to 8f69b1d Feb 11, 2020
@mkg20001 mkg20001 requested a review from worldofpeace Feb 12, 2020
@worldofpeace worldofpeace changed the title cinnamon.xapps: make gobject build input cinnamon.xapps: move gobject-introspection to buildInputs Feb 13, 2020
@worldofpeace worldofpeace merged commit 129224e into NixOS:master Feb 13, 2020
15 checks passed
15 checks passed
Evaluation Performance Report Evaluator Performance Report
Details
cinnamon.xapps on aarch64-linux Success
Details
cinnamon.xapps on x86_64-linux Success
Details
grahamcofborg-eval ^.^!
Details
grahamcofborg-eval-check-maintainers matching changed paths to changed attrs...
Details
grahamcofborg-eval-check-meta config.nix: checkMeta = true
Details
grahamcofborg-eval-darwin nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A darwin-tested
Details
grahamcofborg-eval-nixos nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release-combined.nix -A tested
Details
grahamcofborg-eval-nixos-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release.nix -A manual
Details
grahamcofborg-eval-nixos-options nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release.nix -A options
Details
grahamcofborg-eval-nixpkgs-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A manual
Details
grahamcofborg-eval-nixpkgs-tarball nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A tarball
Details
grahamcofborg-eval-nixpkgs-unstable-jobset nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A unstable
Details
grahamcofborg-eval-package-list nix-env -qa --json --file .
Details
grahamcofborg-eval-package-list-no-aliases nix-env -qa --json --file . --arg config { allowAliases = false; }
Details
@worldofpeace
Copy link
Member

@worldofpeace worldofpeace commented Feb 13, 2020

Thank you @mkg20001.

@jtojnar
Copy link
Contributor

@jtojnar jtojnar commented Feb 13, 2020

Perhaps I should have been clearer, it should still be in nativeBuildInputs as well.

@mkg20001
Copy link
Member Author

@mkg20001 mkg20001 commented Feb 13, 2020

This PR:

confusion 100

Perhaps I should have been clearer, it should still be in nativeBuildInputs as well.

So it should be in there twice? Why?

@jtojnar
Copy link
Contributor

@jtojnar jtojnar commented Feb 13, 2020

The distinction between buildInputs and nativeBuildInputs stems out from the fact that the packages are built and run on different computers. The computers are usually running on the same platform but that does not have to be the case (see cross-compilation).

The build-time platform uses dependencies nativeBuildInputs, while run-time platform those from buildInputs. For historical reasons, by default, Nix will make buildInputs available at build time as well when one is not cross-compiling but proper expressions should distinguish between build-time and run-time environment.

@mkg20001
Copy link
Member Author

@mkg20001 mkg20001 commented Feb 13, 2020

This makes sense

Is this in the docs somewhere? Could be added if not under the gnome packaging section.

@jtojnar
Copy link
Contributor

@jtojnar jtojnar commented Feb 13, 2020

It is described in https://nixos.org/nixpkgs/manual/#chap-cross and rather hard to digest https://nixos.org/nixpkgs/manual/#ssec-stdenv-dependencies (see #57595 for rendered diagrams).

@worldofpeace
Copy link
Member

@worldofpeace worldofpeace commented Feb 13, 2020

Ohhh, strictDeps would have made this obvious.
@mkg20001 nixpkgs exposes the details of platforms and dependencies in a rather academic way sometimes 🤣

@jtojnar
Copy link
Contributor

@jtojnar jtojnar commented Feb 13, 2020

Not entirely academic – lot of people are cross-compiling stuff. Though to be fair gobject-introspection is currently borked for cross and strictDeps break setup hooks (#56943).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.