Rewrite of Nix Package Manager Support#6861
Conversation
Summary of ChangesHello @ZZBaron, 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 introduces a complete overhaul of xmake's Nix package manager support, transforming it from a basic and often unreliable integration into a sophisticated and robust system. The rewrite focuses on comprehensive environment detection, multi-level caching, accurate derivation analysis, and proper handling of propagated and transitive dependencies. This significantly enhances xmake's ability to reliably find and utilize Nix-managed packages, improving build consistency and developer experience for Nix users. 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. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This is an impressive and comprehensive rewrite of the Nix package manager integration. The new implementation is far more robust and feature-rich than the original, with excellent additions like multi-level caching, proper derivation parsing, and extensive environment detection. The code is well-structured and demonstrates a deep understanding of the Nix ecosystem. I've identified a couple of correctness bugs, primarily related to caching and utility function usage, and have also suggested some refactoring for improved robustness and maintainability. Overall, this is a fantastic contribution that will significantly improve xmake's Nix support.
| local pattern = separator == ":" and "[^:]+" or "[^%s]+" | ||
| -- Create a hash-like key from the environment | ||
| local key_parts = {} | ||
| for k, v in pairs(env_data) do |
There was a problem hiding this comment.
use table.orderpairs() instead of table.sort
| local missing = {} | ||
| if not nix_store then table.insert(missing, "nix-store") end | ||
| if not nix then table.insert(missing, "nix") end | ||
| print("Nix: Required tools not found: " .. table.concat(missing, ", ")) |
|
|
||
| -- Get the derivation path | ||
| local drv_output = try {function() | ||
| return os.iorunv(nix_store.program, {"--query", "--valid-derivers", store_path}):trim() -- not "--deriver" because: |
There was a problem hiding this comment.
local outdata = os.iorunv
if outdata then
return outdata:trim()
end
return outdata| "derivation", "show", | ||
| drv_path, | ||
| "--extra-experimental-features", "nix-command flakes" | ||
| }):trim() |
| table.insert(paths, path) | ||
| end | ||
| -- Parse the JSON output | ||
| local derivation_data, parse_error = json.decode(derivation_json) |
There was a problem hiding this comment.
decode failed, it will raise errors.
so parse_error is nil
local derivation_data = json.decode(derivation_json)| end | ||
| -- Parse the JSON output | ||
| local derivation_data, parse_error = json.decode(derivation_json) | ||
| if not derivation_data or parse_error then |
| end | ||
|
|
||
| -- remove duplicates from array | ||
| local function remove_duplicates(arr) |
There was a problem hiding this comment.
use table.unique
arr = table.unique(arr)
| end | ||
|
|
||
| -- PackageInfo data | ||
| local PackageInfo = {} |
There was a problem hiding this comment.
use core.base.object to define object
xmake/xmake/modules/async/jobgraph.lua
Line 29 in 2bd081f
There was a problem hiding this comment.
and please use lower case + _
like: package_info
| local visited = {} | ||
|
|
||
| -- Add initial paths | ||
| for _, store_path in ipairs(store_paths) do |
There was a problem hiding this comment.
local all_paths = table.unique(store_paths)| -- get store paths from nix command output | ||
| local function get_store_paths_from_command(command, args, opt) | ||
| local output = try {function() | ||
| return os.iorunv(command, args):trim() |
| result.libfiles = {} | ||
| local user = os.getenv("USER") or "unknown" | ||
| local output = try {function() | ||
| return os.iorunv(nixos_option.program, {"users.users." .. user .. ".packages"}):trim() |
| end | ||
|
|
||
| local output = try {function() | ||
| return os.iorunv(nixos_option.program, {"environment.systemPackages"}):trim() |
| for _, dep_path in ipairs(all_deps) do | ||
| if not seen[dep_path] then | ||
| seen[dep_path] = true | ||
| table.insert(filtered_paths, dep_path) |
This PR completely rewrites the Nix package manager integration for xmake as the original version was too naive and unreliable. The rewrite introduces caching, derivation parsing, and multi-environment support.
Improvements:
Before: Only checked basic environment variables and nix-shell state
After: Discovers packages from all the standard Nix environments:
Nix shells (nix-shell, nix develop)
User profiles (nix profile, nix-env)
Home Manager (both tool and NixOS module)
NixOS system packages
NixOS user packages
Current system closure (as fallback for NixOS when nixos-option is not configured)
Session cache: Fast in-memory cache for repeated operations within the same build
Persistent cache: Disk-based cache that survives across xmake invocations
Environment-aware: Cache keys include environment state to detect changes
Selective invalidation: Only re-scans when the Nix environment actually changes
Before: Simple string matching against store path names
After: Uses nix derivation show to extract accurate package metadata:
Real package names (pname) instead of guessing from paths (unreliable)
Version information from derivation metadata
Multiple output support (dev, lib, bin, etc.)
Structured attribute handling
Automatically follows nix-support/propagated-build-inputs
Ensures transitive dependencies are available for linking
Cached traversal prevents redundant filesystem operations
pkg-config integration: Leverages existing .pc files when available
Multiple output awareness: Handles packages with separate dev/lib/bin outputs
Apologies for any deviations from xmake's coding conventions or API patterns. I'm still new to xmake source code and lua. I realize this packs a lot of functionality into one file, maybe I could move parts of the implementation to other dirs?