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

zeroad: Split data from binaries again #119643

Merged
merged 2 commits into from May 8, 2021
Merged

Conversation

chvp
Copy link
Member

@chvp chvp commented Apr 16, 2021

Motivation for this change

The rootdir patch was lost in the update to 0.0.24. Adding it allows hydra to build the game (which takes a few minutes). It can't when the binaries depend on the data since the data is too big for hydra.

ZHF: #122042
CC @NixOS/nixos-release-managers

Things done

I started a game using this build.

  • 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.

The rootdir patch was lost in the update to 0.0.24b. Adding it allows
hydra to build the game. It can't when the binaries depend on the data
since the data is too big for hydra.
@Mindavi
Copy link
Contributor

Mindavi commented Apr 16, 2021

Great. It's nice if hydra can build it for us. Also good to see that you're willing to maintain the package.

Haven't tried this yet, might try later. Overall the idea is good and I like it.

@chvp
Copy link
Member Author

chvp commented Apr 16, 2021

Great. It's nice if hydra can build it for us. Also good to see that you're willing to maintain the package.

I'm playing it weekly with a group of friends, it's the least I could do. 😄

Haven't tried this yet, might try later. Overall the idea is good and I like it.

Note that this is not a new idea (although I only realized this after I fixed it). This also how it was made sure that it was built on hydra before 0.0.24. (See https://github.com/NixOS/nixpkgs/blob/7f21b53fc075755d34a16ec7a352389578a3de50/pkgs/games/0ad/rootdir_env.patch and https://github.com/NixOS/nixpkgs/blob/7f21b53fc075755d34a16ec7a352389578a3de50/pkgs/games/0ad/game.nix.)

@Mindavi
Copy link
Contributor

Mindavi commented Apr 16, 2021

Yes indeed. I remember seeing that. Not sure why it was changed, I missed that. I also play this sometimes with friends, but not weekly :).

@chvp
Copy link
Member Author

chvp commented May 8, 2021

I've been using this patch for three weeks now, without issues.

Copy link
Contributor

@jonringer jonringer left a comment

Choose a reason for hiding this comment

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

LGTM

Result of nixpkgs-review pr 119643 run on x86_64-linux 1

1 package built:
  • zeroad

@jonringer jonringer merged commit b651550 into NixOS:master May 8, 2021
@chvp chvp deleted the zeroad-hydra-build branch May 8, 2021 17:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants