Added a Nix flake containing everything needed to build, dev and run … #154
Conversation
|
might need a get job thats a container with nix that can do the following |
|
closed as theoparis still links to this as a reference but that one should be used going forward |
|
We working with this one now? We are not sending PRs up to Godot by default at this time, due to a reassessment in the core team, so we can start looking at stuff like this soon. |
|
I spoke to McDubh about adding this one in for now a few days ago so i reopened it. But yeah theirs no rush on it |
|
just a suggestion, you can rebase instead of adding all these merge commits. |
|
Hi @MichaelFisher1997 , |
|
The flake I opened last year isn’t a full dev setup yet. Right now you can nix develop and run scons manually, it pulls in all the dependencies so the build is reproducible and avoids the usual dependency issues. It still needs updating so nix build .#build (or nix run .#build) will do the full build in one step. At the time quite a few people seemed interested, but I’ve not heard much since. I’ve now got a new PC that can actually handle compiling it properly, so I can revisit this and finish it off if there’s still interest. |
|
Hey @MichaelFisher1997, I am still a little on the fence about incorporating Nix packaging into the main repo, but we probably need to come to a final decision one way or another here soon, along with which of the two PRs will get merged if either of them. |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
WalkthroughAdds Nix-based development tooling: a new Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer
participant Direnv
participant Flake as Nix Flake
participant DevShell
participant SCons
participant App as Binary
Dev->>Direnv: cd into project
Direnv->>Flake: "use flake" (activate)
Flake->>DevShell: instantiate devShell (packages, LD/DYLD_LIBRARY_PATH)
DevShell-->>Dev: environment available
Dev->>SCons: scons platform=... (build)
SCons-->>DevShell: binary produced
Dev->>App: run binary (via app script or ./binary)
App-->>Dev: execute
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Pre-merge checks✅ Passed checks (3 passed)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
flake.nix (1)
4-4: Consider using the canonical nixpkgs input URL.While the FlakeHub URL works, the more common and direct approach is to reference nixpkgs from GitHub. This improves clarity and aligns with most Nix flake examples.
🔎 Proposed alternative
- inputs.nixpkgs.url = "https://flakehub.com/f/NixOS/nixpkgs/0.1.*.tar.gz"; + inputs.nixpkgs.url = "github:NixOS/nixpkgs/nixpkgs-unstable";Or for a specific stable release:
inputs.nixpkgs.url = "github:NixOS/nixpkgs/nixos-24.05";
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
flake.lockis excluded by!**/*.lock
📒 Files selected for processing (2)
.envrcflake.nix
🔇 Additional comments (2)
.envrc (1)
1-1: LGTM! Standard direnv integration.The
.envrcfile correctly enables direnv to automatically load the Nix flake-based development environment when entering the directory. This is the idiomatic approach for integrating Nix flakes with direnv.flake.nix (1)
55-69: LGTM! Well-structured devShell configuration.The devShell setup correctly:
- Uses the deps list as packages
- Computes
LD_LIBRARY_PATHusingmakeLibraryPathto ensure runtime libraries are found- Provides a commented override hook for advanced users who want to change the compiler
This approach aligns with the PR objective of providing a reproducible development environment for building Redot.
| self, | ||
| nixpkgs, | ||
| }: let | ||
| supportedSystems = ["x86_64-linux" "aarch64-linux" "x86_64-darwin" "aarch64-darwin"]; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "flake.nix" -type fRepository: Redot-Engine/redot-engine
Length of output: 80
🏁 Script executed:
cat -n flake.nix | head -30Repository: Redot-Engine/redot-engine
Length of output: 1113
🏁 Script executed:
# Search for any PR description or comments mentioning macOS/Darwin
git log --oneline -20Repository: Redot-Engine/redot-engine
Length of output: 135
🏁 Script executed:
# Look for dependencies related to X11, Wayland
rg -i "x11|wayland" flake.nix -A 3 -B 3Repository: Redot-Engine/redot-engine
Length of output: 589
🏁 Script executed:
# See more of flake.nix to understand the complete structure
cat -n flake.nixRepository: Redot-Engine/redot-engine
Length of output: 2276
🏁 Script executed:
# Search for any PR context - check PR files or recent commits
git log --oneline --all -30 | grep -i "mac\|darwin\|nix"Repository: Redot-Engine/redot-engine
Length of output: 135
🏁 Script executed:
# Check if there are any references to macOS or Darwin support in comments
rg "macos|darwin|mac support" -i flake.nixRepository: Redot-Engine/redot-engine
Length of output: 158
🏁 Script executed:
# Look for any platform-specific conditionals
rg "lib\.hasSuffix.*darwin|system ==" flake.nix -A 3 -B 3Repository: Redot-Engine/redot-engine
Length of output: 51
🏁 Script executed:
# Look for PR description or comments
find . -name "*.md" -o -name "*.txt" | head -20Repository: Redot-Engine/redot-engine
Length of output: 601
🏁 Script executed:
# Search for any README or documentation mentioning macOS
rg "macos|darwin|apple" -i . --type md --type txt | head -20Repository: Redot-Engine/redot-engine
Length of output: 3097
🏁 Script executed:
# Check git log for PR or commit messages
git log --all --pretty=format:"%h %s" | head -30Repository: Redot-Engine/redot-engine
Length of output: 206
🏁 Script executed:
# Look for any issue or PR context in the repo
find . -name ".github" -type dRepository: Redot-Engine/redot-engine
Length of output: 78
🏁 Script executed:
# Check the PR template and any related issues
cat ./.github/PULL_REQUEST_TEMPLATE.mdRepository: Redot-Engine/redot-engine
Length of output: 394
🏁 Script executed:
# Search for any flake.nix related issues or PRs
git log --all --format="%h %s %b" | grep -i "flake\|nix" -A 5 | head -40Repository: Redot-Engine/redot-engine
Length of output: 136
🏁 Script executed:
# Check if there's any discussion in recent commits about macOS
git show 7d7283ec89 --statRepository: Redot-Engine/redot-engine
Length of output: 50382
🏁 Script executed:
# Look for any documentation about the flake in the repo
rg "flake" -i . --type md | head -20Repository: Redot-Engine/redot-engine
Length of output: 51
🏁 Script executed:
# Look for any branch or PR related messages
git log --all --grep="macos\|darwin\|nix" -i --oneline | head -20Repository: Redot-Engine/redot-engine
Length of output: 135
🏁 Script executed:
# Check the actual changes made to flake.nix in recent commits
git log --all -p -- flake.nix | head -100Repository: Redot-Engine/redot-engine
Length of output: 2200
🏁 Script executed:
# Look for any discussions or comments in the repository
find . -name "*.md" -o -name "*.txt" | xargs grep -l "macos\|darwin.*support\|planned" 2>/dev/null | head -10Repository: Redot-Engine/redot-engine
Length of output: 95
🏁 Script executed:
# Check if there's any environment or conditional logic for Darwin
rg "stdenvNoCC|mkShell|pkgs\." flake.nix -A 5 -B 5Repository: Redot-Engine/redot-engine
Length of output: 947
🏁 Script executed:
# Let's verify if autoPatchelfHook is Linux-specific
rg "autoPatchelfHook" . --type nix | head -5Repository: Redot-Engine/redot-engine
Length of output: 109
🏁 Script executed:
# Check if there's any way this flake could handle Darwin gracefully
# Look for mkShell or other derivations that might conditionally handle platforms
rg "system ==" flake.nix -A 3 -B 3Repository: Redot-Engine/redot-engine
Length of output: 51
🏁 Script executed:
# See what happens when unsupported packages are referenced on Darwin
# Check if there's any Nix documentation references or similar patterns
find . -name "*.nix" -type f | head -10Repository: Redot-Engine/redot-engine
Length of output: 80
🏁 Script executed:
# Verify if the LD_LIBRARY_PATH is set unconditionally for all systems
cat -n flake.nix | grep -A 10 "mkShell"Repository: Redot-Engine/redot-engine
Length of output: 411
🏁 Script executed:
# Verify if autoPatchelfHook is actually available for Darwin in nixpkgs
web_search "autoPatchelfHook nixpkgs darwin macos available"Repository: Redot-Engine/redot-engine
Length of output: 119
🏁 Script executed:
# Check what happens when Nix evaluates unsupported packages for a system
# Look for any error handling or documentation
rg "skipBuildInputs|optional|lib\." flake.nix | head -20Repository: Redot-Engine/redot-engine
Length of output: 180
🌐 Web query:
autoPatchelfHook nixpkgs darwin macos availability
💡 Result:
Short answer: autoPatchelfHook is for ELF binaries (Linux) and is not applicable to native macOS (Darwin) Mach-O binaries — so it’s not used on darwin/stdenv. Use macOS tools (install_name_tool/otool) or nix/darwin-specific packaging helpers instead. [1][2][3]
Sources
- autoPatchelfHook (works on ELF files). [1]
- Mach‑O is the native macOS binary format (not ELF). [2]
- Nixpkgs Darwin platform notes (darwin/stdenv differences). [3]
🏁 Script executed:
# Check if there's any conditional logic or system-specific handling in the flake
# Look for lib.optionals or similar patterns
rg "optionals|mkIf|system" flake.nixRepository: Redot-Engine/redot-engine
Length of output: 172
🏁 Script executed:
# Verify the actual behavior - does nixpkgs provide autoPatchelfHook for Darwin?
# Check if there are any .override or conditionals we missed
cat flake.nix | grep -E "system|darwin|linux|condition" -iRepository: Redot-Engine/redot-engine
Length of output: 336
Remove Darwin systems until macOS/Nix compatibility is properly implemented.
The flake declares x86_64-darwin and aarch64-darwin support but includes unconditional Linux-specific dependencies:
autoPatchelfHookis for ELF binaries only (not applicable to macOS Mach-O format)xorg.*packages (X11),wayland-scanner,wayland,libdecor(Wayland) are Linux-onlyalsa-lib,libpulseaudio,udev,dbusare Linux-only
There are no platform-specific conditionals (system == "darwin" or lib.optionals) to handle these differences. The flake will fail to evaluate on macOS. Users attempting to use this on Darwin will encounter build failures, making the declared support misleading.
Remove Darwin systems from supportedSystems and add them back in a future PR once macOS compatibility is properly implemented with platform-specific package selection and environment variables (DYLD_LIBRARY_PATH instead of LD_LIBRARY_PATH).
🤖 Prompt for AI Agents
In flake.nix around line 10, supportedSystems incorrectly lists x86_64-darwin
and aarch64-darwin while the flake unconditionally includes Linux-only
dependencies (autoPatchelfHook, xorg/*, wayland-*, libdecor, alsa-lib,
libpulseaudio, udev, dbus); remove the Darwin entries from supportedSystems to
prevent evaluation/build failures on macOS, and plan a follow-up PR to
reintroduce Darwin with proper platform-specific conditionals and
Darwin-specific environment handling (e.g., DYLD_LIBRARY_PATH) once
compatibility is implemented.
flake.nix
Outdated
| deps = with pkgs; [ | ||
| pkg-config | ||
| autoPatchelfHook | ||
| installShellFiles | ||
| python3 | ||
| speechd | ||
| wayland-scanner | ||
| makeWrapper | ||
| mono | ||
| dotnet-sdk_8 | ||
| dotnet-runtime_8 | ||
| vulkan-loader | ||
| libGL | ||
| xorg.libX11 | ||
| xorg.libXcursor | ||
| xorg.libXinerama | ||
| xorg.libXext | ||
| xorg.libXrandr | ||
| xorg.libXrender | ||
| xorg.libXi | ||
| xorg.libXfixes | ||
| libxkbcommon | ||
| alsa-lib | ||
| mono | ||
| wayland-scanner | ||
| wayland | ||
| libdecor | ||
| libpulseaudio | ||
| dbus | ||
| dbus.lib | ||
| speechd | ||
| fontconfig | ||
| fontconfig.lib | ||
| udev | ||
| dotnet-sdk_8 | ||
| dotnet-runtime_8 | ||
| scons | ||
| ]; |
There was a problem hiding this comment.
Remove duplicate dependencies.
The dependency list contains several duplicates that should be removed for clarity and maintainability:
monoappears on lines 23 and 38speechdappears on lines 20 and 45wayland-scannerappears on lines 21 and 39dotnet-sdk_8appears on lines 24 and 49dotnet-runtime_8appears on lines 25 and 50
While Nix handles duplicates gracefully without errors, removing them improves code clarity and prevents confusion during future maintenance.
🔎 Proposed fix
deps = with pkgs; [
pkg-config
autoPatchelfHook
installShellFiles
python3
speechd
wayland-scanner
makeWrapper
mono
dotnet-sdk_8
dotnet-runtime_8
vulkan-loader
libGL
xorg.libX11
xorg.libXcursor
xorg.libXinerama
xorg.libXext
xorg.libXrandr
xorg.libXrender
xorg.libXi
xorg.libXfixes
libxkbcommon
alsa-lib
- mono
- wayland-scanner
wayland
libdecor
libpulseaudio
dbus
dbus.lib
- speechd
fontconfig
fontconfig.lib
udev
- dotnet-sdk_8
- dotnet-runtime_8
scons
];📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| deps = with pkgs; [ | |
| pkg-config | |
| autoPatchelfHook | |
| installShellFiles | |
| python3 | |
| speechd | |
| wayland-scanner | |
| makeWrapper | |
| mono | |
| dotnet-sdk_8 | |
| dotnet-runtime_8 | |
| vulkan-loader | |
| libGL | |
| xorg.libX11 | |
| xorg.libXcursor | |
| xorg.libXinerama | |
| xorg.libXext | |
| xorg.libXrandr | |
| xorg.libXrender | |
| xorg.libXi | |
| xorg.libXfixes | |
| libxkbcommon | |
| alsa-lib | |
| mono | |
| wayland-scanner | |
| wayland | |
| libdecor | |
| libpulseaudio | |
| dbus | |
| dbus.lib | |
| speechd | |
| fontconfig | |
| fontconfig.lib | |
| udev | |
| dotnet-sdk_8 | |
| dotnet-runtime_8 | |
| scons | |
| ]; | |
| deps = with pkgs; [ | |
| pkg-config | |
| autoPatchelfHook | |
| installShellFiles | |
| python3 | |
| speechd | |
| wayland-scanner | |
| makeWrapper | |
| mono | |
| dotnet-sdk_8 | |
| dotnet-runtime_8 | |
| vulkan-loader | |
| libGL | |
| xorg.libX11 | |
| xorg.libXcursor | |
| xorg.libXinerama | |
| xorg.libXext | |
| xorg.libXrandr | |
| xorg.libXrender | |
| xorg.libXi | |
| xorg.libXfixes | |
| libxkbcommon | |
| alsa-lib | |
| wayland | |
| libdecor | |
| libpulseaudio | |
| dbus | |
| dbus.lib | |
| fontconfig | |
| fontconfig.lib | |
| udev | |
| scons | |
| ]; |
🤖 Prompt for AI Agents
In flake.nix around lines 15 to 52 the deps list contains duplicate entries
(mono, speechd, wayland-scanner, dotnet-sdk_8, dotnet-runtime_8); remove the
repeated occurrences so each dependency appears only once, keeping one canonical
occurrence of each package and preserving list order/formatting to maintain
readability.
|
Hey @Arctis-Fireblight, Great question! Let me clarify both points: What Nix is for hereNix (the package manager, not just NixOS the operating system) provides reproducible, declarative dependency management and builds. The key benefits for Redot are:
Important note: Having a Nix dev shell doesn't mean everyone is forced to use it. It's an optional tool—contributors can still use their system package managers or manual installs if they prefer. However, using Nix is good practice because it ensures a consistent environment that matches what others (and CI) use. Differences between #154 and #954, and why I recommend #154
My recommendation: Merge #154, move #954 to a separate repo I believe the Redot engine repository should focus on source code and tooling directly needed to build it. PR #154 fits this scope perfectly—it's just a dev shell that gives contributors a reproducible environment with all the dependencies they need to build Redot. PR #954, on the other hand, is about packaging and distribution—that's a different concern. It adds 356 lines of packaging definitions, overlays, export template derivations, etc. This belongs in a dedicated Nix packaging repository that can be maintained independently by the Nix community, similar to how the official Nixpkgs repository handles thousands of packages. Benefits of this separation:
The Nix community already maintains packaging for many projects in external repos. @Naxdy could easily fork PR #954 into a standalone flake that references this repo. Users who want the full Nix packaging experience can use that, while anyone wanting to contribute to Redot just needs Bottom line: Let's merge #154 to give contributors an easy, reproducible way to get dependencies. The comprehensive packaging work belongs elsewhere. does this make sense |
|
I've updated the branch with some improvements:
Confirmed working: I've tested this on NixOS with The branch is up to date with master and ready to merge when you're ready to make a decision on the Nix-related PRs. Thanks! |
|
Thanks @MichaelFisher1997 ! Do you mind rebasing this branch for me real quick? |
MichaelFisher1997
force-pushed
the
nix-build-flake
branch
from
0482ba6 to
97e7db3
Compare
|
@Arctis-Fireblight Done! Rebased onto |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
README.md (1)
51-51: Clarify the scope of the "recommended" label.Marking this section "recommended" may mislead users unfamiliar with Nix. Consider scoping the label to clarify it's recommended for users with Nix installed (e.g., "Using Nix (recommended if you have Nix installed)") or reorder sections to place traditional build instructions first.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
flake.lockis excluded by!**/*.lock
📒 Files selected for processing (3)
.envrcREADME.mdflake.nix
🚧 Files skipped from review as they are similar to previous changes (2)
- .envrc
- flake.nix
🔇 Additional comments (2)
README.md (2)
55-59: One-command workflow is clear and well-documented.The
nix run .command and its description are accurate and provide a good zero-config entry point for Nix users.
74-74: Nix availability note is clear and helpful.The link and platform information are accurate and provide a clear path for users to obtain Nix.
- Add dynamic arch detection using pkgs.stdenv.hostPlatform.isAarch64 - Update binary naming to use detected architecture instead of hardcoded x86_64 - Update README manual instructions to show platform/arch-agnostic guidance This fixes nix run . on ARM64 systems (Apple Silicon, ARM Linux).
|
@coderabbitai Thanks for the feedback! I've addressed both issues in commit 312cbc0: 1. Hard-coded architecture fix:
2. Platform target mismatch fix:
The flake now correctly supports all four system types:
|
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 7008 @MichaelFisher1997, excellent work! I've verified both fixes in commit 312cbc0: ✅ Dynamic architecture detection confirmed:
✅ Platform documentation updated:
The implementation looks solid and follows Nix best practices. The flake should now correctly build and run on all supported platforms without hard-coded assumptions. Ready for merge once CI passes! 🚀 |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
flake.nix (1)
2-2: Consider a more descriptive flake description.The description "A Nix-flake-based C/C++ development environment" is generic. Consider updating it to reference Redot Engine specifically, e.g., "Development environment for building Redot Engine" or "Nix flake for Redot Engine development and distribution."
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
README.mdflake.nix
🔇 Additional comments (8)
flake.nix (7)
11-17: LGTM! Platform and architecture detection is well-implemented.The use of
isDarwinand dynamicarchdetection properly supports the multi-platform goal stated in the PR. The architecture detection viapkgs.stdenv.hostPlatform.isAarch64enables correct binary naming for ARM64 systems as intended.
18-37: LGTM! Linux-specific dependencies are correctly isolated.The
linuxDepslist appropriately contains Linux-only packages (X11, Wayland, ALSA, PulseAudio, udev, dbus, autoPatchelfHook), which resolves the previous concern about unconditional Linux dependencies breaking Darwin builds.
39-46: LGTM! Darwin-specific dependencies are correctly identified.The
darwinDepslist contains appropriate macOS frameworks (Foundation, Cocoa, audio/video frameworks) for building on Darwin systems.
64-67: LGTM! Conditional platform logic is correctly implemented.The platform-specific conditionals properly handle:
- Dependency selection (Darwin vs. Linux)
- Library path environment variables (DYLD_LIBRARY_PATH vs. LD_LIBRARY_PATH)
- Platform identifiers for scons
- Dynamic binary naming with architecture detection
This correctly implements the multi-platform support described in the PR.
70-92: LGTM! The app script correctly implements on-demand building.The script:
- Uses platform-specific library path variables
- Performs conditional building (only when binary is missing)
- Correctly passes arguments to the binary
- Uses dynamic binary naming based on platform and architecture
This provides the convenient
nix run .workflow described in the README.
94-110: LGTM! Development shell is correctly configured.The
devShellsconfiguration:
- Properly exposes dependencies via
packages- Sets the appropriate platform-specific library path variable
- Includes a helpful commented example for compiler override
This supports the
nix developworkflow documented in the README.
48-62: Verify Darwin compatibility of dependencies before making changes to the flake.The packages listed in
commonDepshave varying levels of Darwin support in nixpkgs. Web searches indicate:
fontconfigandfontconfig.lib: Currently listed as supporting Darwin in nixpkgs, contrary to the review's claimlibGL: Available on Darwin via Mesa (with MoltenVK), not Linux-only as claimedspeechd: Has inconsistent Darwin support (main package is Linux-only, butspeechd-minimalvariant supports Darwin)vulkan-loader: Available on Darwin via MoltenVKBefore refactoring to move packages to
linuxDeps, test the flake on both x86_64-darwin and aarch64-darwin systems to confirm whether these packages actually cause evaluation failures. The README documents building withscons platform=macos, and GitHub Actions successfully builds macOS binaries, but those workflows don't use the Nix flake, so it's unclear whether the flake itself has platform-compatibility issues.README.md (1)
51-76: Well-documented Nix workflow with clear examples.The documentation effectively covers:
- One-command build/run workflow (
nix run .)- Manual workflow with step-by-step instructions
- Platform-agnostic guidance with examples for both Linux and macOS
- Architecture-aware binary naming
This addresses the previous concern about hard-coded architecture and platform specificity.
Note: The claim "Nix works on Linux and macOS" (line 75) should be verified against the issue flagged in
flake.nixregarding Linux-specific packages incommonDepsthat may prevent successful builds on macOS.
5 participants
This is a Nix Flake for NixOS and anyone with nix package manager installed on their system.
It allow people to enter into the flake dev shell as followed:
nix developand it will install all the listed dependencies to build and run redot avoiding the pain of having to workout what dependencies you need to install and what missing.Summary by CodeRabbit
Chores
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.