fix(make): detect nix in Docker where profile path differs - #1217
Conversation
NIX_ENV sourced only ~/.nix-profile/etc/profile.d/nix.sh, which doesn't exist in the Docker CI image where nix is pre-installed at /usr/bin. Fall back to the Determinate Systems daemon profile path and `command -v nix` to avoid re-running the installer when nix is already present. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Entire-Checkpoint: f6b1317c5ea8
|
You do not have enough credits to review this pull request. Please purchase more credits to continue. |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses an issue where the Makefile failed to correctly detect Nix installations in certain Docker CI environments, leading to unnecessary re-attempts to install Nix and subsequent errors. By enhancing the logic for identifying an existing Nix setup, the changes ensure smoother and more reliable execution of build processes in diverse environments. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
Disabled knowledge base sources:
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~5 minutes Suggested labels
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
The pull request enhances the detection of the Nix environment within the Makefile by adding additional fallback mechanisms. This is a positive change, making the build process more robust across different environments, especially Docker CI where Nix might be installed in non-standard locations. The added checks for /nix/var/nix/profiles/default/etc/profile.d/nix-daemon.sh and command -v nix are valuable for broader compatibility.
| echo "$(shell whoami)"; \ | ||
| fi) | ||
| NIX_ENV := $(shell . ~/.nix-profile/etc/profile.d/nix.sh 2>/dev/null || echo "not_found") | ||
| NIX_ENV := $(shell . ~/.nix-profile/etc/profile.d/nix.sh 2>/dev/null || . /nix/var/nix/profiles/default/etc/profile.d/nix-daemon.sh 2>/dev/null || command -v nix >/dev/null 2>&1 || echo "not_found") |
There was a problem hiding this comment.
The command -v nix >/dev/null 2>&1 check, while useful for detecting the nix executable, only verifies its presence in the PATH. Unlike sourcing the profile scripts (. ~/.nix-profile/... or . /nix/var/nix/profiles/...), it does not ensure that the necessary Nix environment variables (like NIX_PATH) are correctly set up. If only this check succeeds, the NIX_ENV variable will be empty, which the nix-check target interprets as a functional Nix environment. This could lead to subsequent Nix commands failing due to an incomplete environment setup, even though nix-check passes.
To make this fallback more robust, consider replacing command -v nix >/dev/null 2>&1 with a check that also verifies basic Nix functionality, such as nix --version >/dev/null 2>&1. This would ensure that if nix is found, it is also capable of executing basic commands, implying a more complete environment.
NIX_ENV := $(shell . ~/.nix-profile/etc/profile.d/nix.sh 2>/dev/null || . /nix/var/nix/profiles/default/etc/profile.d/nix-daemon.sh 2>/dev/null || nix --version >/dev/null 2>&1 || echo "not_found")
There was a problem hiding this comment.
Pull request overview
Adjusts Nix detection in the Makefile to better handle Docker/CI environments where the standard ~/.nix-profile/.../nix.sh path is absent, avoiding unnecessary (and failing) re-install attempts.
Changes:
- Extend
NIX_ENVdetection to try Determinate Systems’ daemon profile script path. - Add a final fallback that treats
nixbeing present onPATHas “installed”.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| echo "$(shell whoami)"; \ | ||
| fi) | ||
| NIX_ENV := $(shell . ~/.nix-profile/etc/profile.d/nix.sh 2>/dev/null || echo "not_found") | ||
| NIX_ENV := $(shell . ~/.nix-profile/etc/profile.d/nix.sh 2>/dev/null || . /nix/var/nix/profiles/default/etc/profile.d/nix-daemon.sh 2>/dev/null || command -v nix >/dev/null 2>&1 || echo "not_found") |
There was a problem hiding this comment.
NIX_ENV is computed with :=, so it’s evaluated once when Make parses the file. If make nix-setup runs on a machine without Nix, nix-install can install Nix but nix-check will still see the original NIX_ENV=not_found and fail in the same invocation. Also, sourcing nix.sh/nix-daemon.sh inside $(shell ...) can’t actually set environment for later Make recipes (it runs in a subshell), so this variable is effectively just a presence check.
Consider moving the detection into the nix-check/nix-install recipes (e.g., command -v nix), or change NIX_ENV to a recursively-expanded variable (=) / function so it’s evaluated at use time and reflects installs done earlier in the same make run.
| NIX_ENV := $(shell . ~/.nix-profile/etc/profile.d/nix.sh 2>/dev/null || . /nix/var/nix/profiles/default/etc/profile.d/nix-daemon.sh 2>/dev/null || command -v nix >/dev/null 2>&1 || echo "not_found") | |
| NIX_ENV = $(shell command -v nix >/dev/null 2>&1 || echo "not_found") |
Mesa DescriptionTL;DRFixed Nix environment detection in Docker CI by adding fallback profile paths, preventing installer re-runs and failures. What changed?
Description generated by Mesa. Update settings |
Summary
NIX_ENVonly sourced~/.nix-profile/etc/profile.d/nix.sh, which doesn't exist in the Docker CI image where nix is pre-installed at/usr/bin/nix/var/nix/profiles/default/etc/profile.d/nix-daemon.sh) andcommand -v nixTest plan
make: *** [Makefile:301: nix-install] Error 1🤖 Generated with Claude Code
Summary by cubic
Fix Nix detection in Docker/CI by adding daemon profile and command-based fallbacks, so the installer skips when
nixis already available. Preventsmake nix-installfailures in images wherenixlives at/usr/bin.NIX_ENVto source~/.nix-profile/..., or/nix/var/nix/profiles/default/etc/profile.d/nix-daemon.sh, or usecommand -v nixas a final check.Written for commit cb8bba4. Summary will update on new commits.