godot3: refactor, rename from godot, and add mono builds #177578
Conversation
|
|
Rotaerk
force-pushed
the
godot-mono
branch
2 times, most recently
from
67703b4
to
cc6f876
Compare
There was a problem hiding this comment.
Looks good so far - however, I'm personally not that involved in godot (in nixpkgs and in general). I only "duplicated" the server derivation to headless to be able to build oh-my-git (see #119642).
But oh-my-git can be built successfully with the changes. The godot editor also builds and launches a sample project.
Result of nixpkgs-review pr 177578 run on x86_64-linux 1
9 packages built:
- godot
- godot-export-templates
- godot-headless
- godot-mono
- godot-mono-export-templates
- godot-mono-headless
- godot-mono-server
- godot-server
- oh-my-git
|
Result of 9 packages built:
|
|
That seems like a good idea; how do you do that? |
|
look at |
|
Thanks, never noticed aliases.nix. Added aliases for the old names. |
|
Pushed a new version that just adjusts the sh |
|
This push eliminates the detect.py patches and switches to autoPatchelfHook-based corrections to add udev, pulseaudio, and alsa as runtime dependencies. Also renamed the man page file to |
|
Result of 19 packages failed to build:
4 packages built:
|
|
huh. they built locally ... I'll check tomorrow. |
the build server restarted while it was building, gonna build it again later ^^ don't worry D: |
|
Result of 23 packages built:
|
There was a problem hiding this comment.
Okay, I do have one more set of review items, but I'll leave them to your discretion tbh. If you do not want to fix the aarch64 build, just mark it as meta.broken = stdenv.isAarch64; or something. Otherwise I did just test what I suggested as a change and it works for aarch64-linux on the community builder
Thank you again for this work!
The godot base derivation was implemented by passing a recursive attrset to mkDerivation. This is problematic because recursive references to attributes that are later overridden don't notice the override. Switched to the approach of giving mkDerivation a lambda that receives a self argument, which allows recursive references to the final value after overrides are applied. The related derivations (for export templates, headless, and server builds of godot) duplicated content from the base derivation. This refactor eliminated that by using overridable attributes to parameterize the scripts. The main motivation for this refactor was to help me add derivations for mono builds of godot. Renamed to godot3 to distinguish from version 4, which is a complete rewrite and effectively a different tool altogether.
|
Okay, the changes have been made. Here is what I'd suggest for testing this on aarch64:
|
There was a problem hiding this comment.
Okay I've given this one last look over and it's an impressive amount of work!
I didn't do much aarch64 testing (since I haven't had the time to set up NixOS on one of my Raspberry Pis or something, which I'm not even sure Godot will support anyway), but most of the applications don't even include aarch64-linux in meta.platforms, so I doubt it gets much use on that platform and given it actually builds now, this PR is likely an improvement regardless there
I have no further comments from a perspective of actual problems (any comments I would have would mostly be preferential or stylistic) and I don't see any unresolved review comments, so I'll say I think this is ready!
I'll give it just another couple days for anyone else to bring up potential issues or correct me if not all review comments are addressed, but if there's no dissent then I'll go ahead and merge this
Thank you for sticking with this @Rotaerk and doing all of this work, which I'm sure people appreciate ❤️
|
Awesome, thanks @lilyinstarlight and @Janik-Haag for helping me with finalizing this! And thanks everyone else who has provided feedback throughout the life of this PR. It's much better than it originally was. |
Description of changes
Refactored to reduce repetition of common installation steps across the different derivation variations, improving the granularity of overridability. This helped with the next step: Adding new derivations for the Mono version of Godot.
Things done
sandbox = trueset innix.conf? (See Nix manual)nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)nixos/doc/manual/md-to-db.shto update generated release notes