Skip to content

atk: clean up cross-compilation logic#124417

Merged
jtojnar merged 1 commit intoNixOS:stagingfrom
Mindavi:atk/clean-up-cross
Jul 7, 2021
Merged

atk: clean up cross-compilation logic#124417
jtojnar merged 1 commit intoNixOS:stagingfrom
Mindavi:atk/clean-up-cross

Conversation

@Mindavi
Copy link
Contributor

@Mindavi Mindavi commented May 25, 2021

Motivation for this change

Clean up based on review comments on other packages. See e.g. #123712.

Remove conditionally adding gobject-introspection as nativeBuildInput (it doesn't matter if it's available or not), remove input parameter that can just be inlined.

I think it's nicer to have the cross support be as minimal as possible to have a clean package definition.

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/)
  • Added a release notes entry if the change is major or breaking
  • Fits CONTRIBUTING.md.

cc @7c6f434c

@7c6f434c
Copy link
Member

I of course find first claim unreliable and the second change net negative. You might be able to convince @jtojnar to merge it, I guess.

@Mindavi
Copy link
Contributor Author

Mindavi commented May 25, 2021

Keeping it like it was is fine by me, I don't have a strong opinion on either way. I'll leave this open for a bit so we can decide on how we'll do this.

@ofborg ofborg bot requested a review from 7c6f434c May 25, 2021 21:25
@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: 1001-2500 This PR causes many rebuilds on Darwin and should most likely 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 May 25, 2021
@Mindavi Mindavi force-pushed the atk/clean-up-cross branch from 8da5f61 to 0e81850 Compare May 27, 2021 21:13
@Mindavi
Copy link
Contributor Author

Mindavi commented May 27, 2021

Having some issues where atk doesn't want to build anymore on my pc. It's hard to debug where the issue lies. Also happens on master (by forcing a rebuild using --check or --rebuild, so it's not clear to me why it's not working.

I don't understand what's happening.

error: builder for '/nix/store/y3d8v5qhzrwmgb3pwb362d9dyvrxcazr-atk-2.36.0.drv' failed with exit code 1;
       last 10 log lines:
       > Dependency gobject-introspection-1.0 found: YES 1.68.0 (cached)
       > Program g-ir-compiler found: YES (/nix/store/jxrp8rx8kwxligp0zi5fdl44bgmf1d18-gobject-introspection-1.68.0-dev/bin/g-ir-compiler)
       > Build targets in project: 15
       >
       > Option buildtype is: plain [default: debugoptimized]
       > Found ninja-1.10.2 at /nix/store/6gzaz26a84m4bn33f1zrnj4zwbnwc8cd-ninja-1.10.2/bin/ninja
       > meson: enabled parallel building
       > building
       > build flags: -j8 -l8
       > [0/52] Generating atkmarshal_c with a custom commandninja: fatal: posix_spawn: No such file or directory
       For full logs, run 'nix log /nix/store/y3d8v5qhzrwmgb3pwb362d9dyvrxcazr-atk-2.36.0.drv'.

No idea

@7c6f434c
Copy link
Member

Ah, that's probably missing /bin/sh in the sandbox

@Mindavi
Copy link
Contributor Author

Mindavi commented May 27, 2021

Yes indeed, thanks! Doing something like this works: CaptainSpof/dotfiles@ad425fd

@SuperSandro2000
Copy link
Member

SuperSandro2000 commented May 31, 2021

Yes indeed, thanks! Doing something like this works: CaptainSpof/dotfiles@ad425fd

This should be solved by a fixShebang on the file.

@Mindavi
Copy link
Contributor Author

Mindavi commented May 31, 2021

@SuperSandro2000 it was also failing on master, probably due to this: #124215

So that fail is not related to this patchset.

@bjornfor bjornfor added the 6.topic: cross-compilation Building packages on a different platform than they will be used on label Jun 5, 2021
@Mindavi
Copy link
Contributor Author

Mindavi commented Jul 7, 2021

I'm not sure if we should move forward with this or just close it. I wanted to make this package more consistent with the other PRs that were applied, but the result of applying this or not will be the same, except for the looks.

If this doesn't move, I think I'll just close it in a week or so.

@jtojnar jtojnar merged commit 23261c3 into NixOS:staging Jul 7, 2021
@Mindavi Mindavi deleted the atk/clean-up-cross branch July 7, 2021 21:09
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: 1001-2500 This PR causes many rebuilds on Darwin and should most likely 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.

5 participants