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.

Sign up for GitHub

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

Jump to bottom

remove godot_headers in favour of a dev output on godot #37992

Merged
merged 2 commits into from May 2, 2018
Merged

remove godot_headers in favour of a dev output on godot #37992

merged 2 commits into from May 2, 2018

Conversation

Twey
Copy link
Contributor

@Twey Twey commented Mar 28, 2018

Motivation for this change

It turns out that godot_headers is not guaranteed to be in sync with godot. Rather than suffer two packages that must be in sync, let's copy the actual generated headers from the godot build out into a dev output, as is conventional.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option build-use-sandbox in nix.conf on non-NixOS)
  • 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 nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

@Twey
Copy link
Contributor Author

Twey commented Mar 28, 2018

(thanks to @tilpner for pointing this out)

@xeji
Copy link
Contributor

xeji commented Mar 28, 2018

This should probably go into 18.03 as well since we just applied the change introducing godot_headers there.

@Twey
Copy link
Contributor Author

Twey commented Apr 10, 2018

Did this make it into 18.03 in the end?

@tilpner
Copy link
Member

tilpner commented Apr 10, 2018

No, this PR hasn't been merged yet. godot_headers still exist on master and nixos-18.03

matthewbauer
matthewbauer requested changes Apr 25, 2018
View reviewed changes
Copy link
Member

@matthewbauer matthewbauer left a comment

Choose a reason for hiding this comment

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

Looks good to me! Can you resolve the merge conflicts?

@Twey
Copy link
Contributor Author

Twey commented May 2, 2018

Rebased!

@xeji
Copy link
Contributor

xeji commented May 2, 2018

@GrahamcOfBorg build godot

@GrahamcOfBorg
Copy link

No attempt on aarch64-linux (full log)

The following builds were skipped because they don't evaluate on aarch64-linux: godot

Partial log (click to expand)


a) For `nixos-rebuild` you can set
  { nixpkgs.config.allowUnsupportedSystem = true; }
in configuration.nix to override this.

b) For `nix-env`, `nix-build`, `nix-shell` or any other Nix command you can add
  { allowUnsupportedSystem = true; }
to ~/.config/nixpkgs/config.nix.

@GrahamcOfBorg
Copy link

Success on x86_64-linux (full log)

Attempted: godot

Partial log (click to expand)

shrinking RPATHs of ELF executables and libraries in /nix/store/0ij7mwrwf6msi1g7q9vgqkbqcis6jx64-godot-3.0.2-dev
strip is /nix/store/j75dgadrff2d1fyc4fczmcgqkid2imdx-binutils-2.30/bin/strip
patching script interpreter paths in /nix/store/0ij7mwrwf6msi1g7q9vgqkbqcis6jx64-godot-3.0.2-dev
checking for references to /build in /nix/store/0ij7mwrwf6msi1g7q9vgqkbqcis6jx64-godot-3.0.2-dev...
shrinking RPATHs of ELF executables and libraries in /nix/store/qx76c37gnqzchd3p9sbmccx4xmh6ncbx-godot-3.0.2-man
gzipping man pages under /nix/store/qx76c37gnqzchd3p9sbmccx4xmh6ncbx-godot-3.0.2-man/share/man/
strip is /nix/store/j75dgadrff2d1fyc4fczmcgqkid2imdx-binutils-2.30/bin/strip
patching script interpreter paths in /nix/store/qx76c37gnqzchd3p9sbmccx4xmh6ncbx-godot-3.0.2-man
checking for references to /build in /nix/store/qx76c37gnqzchd3p9sbmccx4xmh6ncbx-godot-3.0.2-man...
/nix/store/qgfv7yk6f4mpyd19ad31ihspwlf6lfxk-godot-3.0.2

@xeji xeji merged commit c276939 into NixOS:master May 2, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@matthewbauer matthewbauer matthewbauer requested changes

@xeji xeji xeji approved these changes

Assignees
No one assigned
Labels
8.has: clean-up 10.rebuild-darwin: 0 10.rebuild-linux: 1-10
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@Twey @xeji @tilpner @GrahamcOfBorg @matthewbauer