Enable static builds #323
Enable static builds #323
Conversation
📝 WalkthroughWalkthroughThis change centralizes prelink configuration, makes prelink configurable across many Meson library targets, and introduces optional Wasmtime/WASM integration gated by Meson options and package flags. It also adjusts Darwin-static stdenv handling, adds signal handler startup, comments out a curl interrupt on static builds, and adds small packaging and doc updates. Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer / Packaging
participant Meson as Meson build
participant Build as Build system (compiler/linker)
participant Runtime as Nix runtime
participant Wasmtime as wasmtime lib
Dev->>Meson: enable `wasm` option / set `enableWasm`
Meson->>Build: emit compile flags (e.g., NIX_USE_WASMTIME) and link args
Build->>Wasmtime: link against wasmtime when enabled
Runtime->>Runtime: at runtime, code checks compile-time flag
alt NIX_USE_WASMTIME enabled
Runtime->>Runtime: select WasiDerivationBuilder for wasm32-wasip1
else disabled
Runtime->>Runtime: fallback to non-WASI builder
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
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 |
RossComputerGuy
force-pushed
the
tristanross/dsp-62-collab-figure-out-static-building-of-determinate-nix
branch
from
529dd02 to
dcb75d7
Compare
RossComputerGuy
force-pushed
the
tristanross/dsp-62-collab-figure-out-static-building-of-determinate-nix
branch
from
7053c04 to
769f599
Compare
RossComputerGuy
force-pushed
the
tristanross/dsp-62-collab-figure-out-static-building-of-determinate-nix
branch
3 times, most recently
from
878d282 to
37b33a6
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @.github/workflows/build.yml:
- Line 55: The step running "nix build .#packages.${{ inputs.system
}}.nix-cli-static --no-link -L" must be gated to Linux because nix-cli-static
relies on pkgsStatic which only works on Linux; add an if condition to the step
such as if: contains(inputs.system, 'linux') (or if: runner.os == 'Linux') so
the nix-cli-static build is only executed for Linux systems and skipped on macOS
(e.g., aarch64-darwin).
255efce to
1fee9a2
Compare
RossComputerGuy
force-pushed
the
tristanross/dsp-62-collab-figure-out-static-building-of-determinate-nix
branch
2 times, most recently
from
f8bfb37 to
512bac7
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packaging/dependencies.nix`:
- Around line 86-90: The curl override uses pkgs.stdenv to detect static builds
which can differ from the scoped stdenv in this file; change the checks in the
override to use the scoped stdenv available in the current scope (replace
pkgs.stdenv.hostPlatform.isStatic with stdenv.hostPlatform.isStatic) so
pslSupport and idnSupport are computed correctly for static/cross builds (update
references in the curl override block for pslSupport and idnSupport).
678239b to
a75a0c5
Compare
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Fix all issues with AI agents
In `@src/libfetchers-c/nix_api_fetchers.h`:
- Around line 24-35: Fix the doc comment for nix_libfetchers_init: correct the
grammar "it idempotent" to "it is idempotent" and change the `@ingroup` tag from
libfetcher_init to the existing group name libfetchers (or alternatively define
a libfetcher_init subgroup if that was intended) so the group reference matches
the file's `@defgroup`; update only the comment block above the
nix_libfetchers_init declaration.
In `@src/libfetchers/fetchers.cc`:
- Around line 546-557: initFetchers() currently registers every input scheme by
calling registerTarballInputScheme(), registerGitHubInputScheme(),
registerPathInputScheme(), etc., but each of those is also registered via
OnStartup wrappers in their individual modules (e.g., tarball.cc, github.cc,
path.cc), causing duplicate-registration errors from registerInputScheme() when
initFetchers() runs later; remove the OnStartup registration wrappers from the
individual source files (e.g., the OnStartup calls in tarball.cc, github.cc,
path.cc and any other register*InputScheme() OnStartup usages) so that
initFetchers() is the single place that calls all register*InputScheme()
functions and no duplicate registerInputScheme() calls occur.
In `@src/libfetchers/git.cc`:
- Around line 1238-1243: The duplicate registration happens because
rGitInputScheme (the OnStartup lambda) calls registerGitInputScheme() and
initFetchers() also calls the same function, causing registerInputScheme() to
throw on second registration; fix by making the registration idempotent or
removing the automatic startup registration: either delete the rGitInputScheme
OnStartup declaration so registerGitInputScheme() is only invoked from
initFetchers(), or change registerGitInputScheme() to guard against double
registration (for example with a static bool/atomic "registered" flag or by
checking if the scheme is already registered before calling
registerInputScheme()) so repeated calls are safe; update references to
rGitInputScheme, registerGitInputScheme, and registerInputScheme accordingly.
In `@src/libfetchers/github.cc`:
- Around line 662-664: These static variables use OnStartup wrappers which cause
duplicate registration when initFetchers() also registers schemes; remove the
OnStartup(...) wrapper and initialize the variables by directly calling the
registration functions instead (replace the lines for rGitHubInputScheme,
rGitLabInputScheme, and rSourceHutInputScheme so they call
registerGitHubInputScheme(), registerGitLabInputScheme(), and
registerSourceHutInputScheme() directly or remove the variables entirely and
rely on initFetchers() to perform registration).
In `@src/libfetchers/include/nix/fetchers/fetchers.hh`:
- Around line 281-282: The header declares nix::fetchers::initFetchers() but the
only implementation is nix::initFetchers() — causing a linker mismatch; either
remove the nix::fetchers::initFetchers() declaration from the header or add a
delegating implementation (nix::fetchers::initFetchers() { return
::nix::initFetchers(); }) so both symbols exist and link correctly; locate the
declaration in the nix::fetchers namespace and either delete it or implement the
wrapper to call the existing nix::initFetchers().
In `@src/libfetchers/indirect.cc`:
- Around line 142-147: The OnStartup static rIndirectInputScheme currently calls
registerIndirectInputScheme() but initFetchers() also calls
registerIndirectInputScheme(), causing double registration and a thrown error
from registerInputScheme(); fix by making registration idempotent: modify
registerIndirectInputScheme() to check for prior registration (e.g., a static
bool/guard or query existing schemes) and return early if already registered, or
alternatively remove the OnStartup rIndirectInputScheme registration so
initFetchers() is the sole initializer; update references to
rIndirectInputScheme, OnStartup, registerIndirectInputScheme(),
registerInputScheme(), and initFetchers() accordingly.
In `@src/libfetchers/mercurial.cc`:
- Around line 378-383: The Mercurial fetcher is being registered twice due to
the OnStartup static rMercurialInputScheme plus the initFetchers() path; remove
the duplicate registration by deleting the OnStartup usage and relying on
initFetchers() (or vice versa) so only one registration occurs. Specifically,
eliminate the rMercurialInputScheme = OnStartup(...) static or stop calling
registerMercurialInputScheme() from initFetchers(), ensuring only one of the
symbols (rMercurialInputScheme, registerMercurialInputScheme, or the
initFetchers registration entry) performs the
registerInputScheme(std::make_unique<MercurialInputScheme()) call.
In `@src/libfetchers/path.cc`:
- Around line 174-179: Static initialization via rPathInputScheme (OnStartup)
plus the explicit call to registerPathInputScheme() from initFetchers() causes
double registration and a thrown error from registerInputScheme; fix by making
registration idempotent or removing the automatic startup. Either remove
rPathInputScheme/OnStartup so registerPathInputScheme() is only invoked from
initFetchers(), or guard registerPathInputScheme() with a static bool (or check
registerInputScheme for existence) so repeated calls to
registerPathInputScheme() simply return without re-registering; reference
registerPathInputScheme, rPathInputScheme, OnStartup, registerInputScheme, and
initFetchers when applying the change.
In `@src/libfetchers/tarball.cc`:
- Around line 521-522: Remove the OnStartup wrappers to prevent duplicate
registration: replace the static initializers using OnStartup
(rTarballInputScheme and rFileInputScheme) with direct calls to
registerTarballInputScheme() and registerFileInputScheme() so the schemes are
registered exactly once (matching how initFetchers() performs registration);
update the lines referencing OnStartup to static auto rTarballInputScheme =
registerTarballInputScheme(); and static auto rFileInputScheme =
registerFileInputScheme().
src/libfetchers-c/nix_api_fetchers.h
Outdated
| /** | ||
| * @brief Initialize the Nix fetchers. | ||
| * @ingroup libfetcher_init | ||
| * | ||
| * This function must be called at least once, | ||
| * at some point before using a fetcher for the first time. | ||
| * This function can be called multiple times, and it idempotent. | ||
| * | ||
| * @param[out] context Optional, stores error information | ||
| * @return NIX_OK if the initialization was successful, an error code otherwise. | ||
| */ | ||
| nix_err nix_libfetchers_init(nix_c_context * context); |
There was a problem hiding this comment.
Minor documentation issues.
- Line 30: Grammar issue - "it idempotent" should be "it is idempotent"
- Line 26: The
@ingroup libfetcher_initreferences a group that doesn't appear to be defined. The file's@defgroupon line 3 useslibfetchers. Consider using@ingroup libfetchersfor consistency, or define alibfetcher_initsubgroup.
Suggested fix
/**
* `@brief` Initialize the Nix fetchers.
- * `@ingroup` libfetcher_init
+ * `@ingroup` libfetchers
*
* This function must be called at least once,
* at some point before using a fetcher for the first time.
- * This function can be called multiple times, and it idempotent.
+ * This function can be called multiple times, and it is idempotent.
*
* `@param`[out] context Optional, stores error information
* `@return` NIX_OK if the initialization was successful, an error code otherwise.
*/📝 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.
| /** | |
| * @brief Initialize the Nix fetchers. | |
| * @ingroup libfetcher_init | |
| * | |
| * This function must be called at least once, | |
| * at some point before using a fetcher for the first time. | |
| * This function can be called multiple times, and it idempotent. | |
| * | |
| * @param[out] context Optional, stores error information | |
| * @return NIX_OK if the initialization was successful, an error code otherwise. | |
| */ | |
| nix_err nix_libfetchers_init(nix_c_context * context); | |
| /** | |
| * `@brief` Initialize the Nix fetchers. | |
| * `@ingroup` libfetchers | |
| * | |
| * This function must be called at least once, | |
| * at some point before using a fetcher for the first time. | |
| * This function can be called multiple times, and it is idempotent. | |
| * | |
| * `@param`[out] context Optional, stores error information | |
| * `@return` NIX_OK if the initialization was successful, an error code otherwise. | |
| */ | |
| nix_err nix_libfetchers_init(nix_c_context * context); |
🤖 Prompt for AI Agents
In `@src/libfetchers-c/nix_api_fetchers.h` around lines 24 - 35, Fix the doc
comment for nix_libfetchers_init: correct the grammar "it idempotent" to "it is
idempotent" and change the `@ingroup` tag from libfetcher_init to the existing
group name libfetchers (or alternatively define a libfetcher_init subgroup if
that was intended) so the group reference matches the file's `@defgroup`; update
only the comment block above the nix_libfetchers_init declaration.
src/libfetchers/fetchers.cc
Outdated
| void initFetchers() | ||
| { | ||
| registerTarballInputScheme(); | ||
| registerFileInputScheme(); | ||
| registerGitInputScheme(); | ||
| registerGitHubInputScheme(); | ||
| registerGitLabInputScheme(); | ||
| registerSourceHutInputScheme(); | ||
| registerPathInputScheme(); | ||
| registerIndirectInputScheme(); | ||
| registerMercurialInputScheme(); | ||
| } |
There was a problem hiding this comment.
Double registration issue affects all input schemes.
This initFetchers() function calls all register*InputScheme() functions. However, each of those functions is also called via OnStartup in their respective source files (e.g., tarball.cc lines 521-522, github.cc lines 662-664, path.cc line 174).
Since registerInputScheme() throws on duplicate registration (line 29-30), calling initFetchers() after the OnStartup callbacks have already run will crash.
The OnStartup wrappers in all the individual files should be removed to let initFetchers() be the sole registration mechanism.
🤖 Prompt for AI Agents
In `@src/libfetchers/fetchers.cc` around lines 546 - 557, initFetchers() currently
registers every input scheme by calling registerTarballInputScheme(),
registerGitHubInputScheme(), registerPathInputScheme(), etc., but each of those
is also registered via OnStartup wrappers in their individual modules (e.g.,
tarball.cc, github.cc, path.cc), causing duplicate-registration errors from
registerInputScheme() when initFetchers() runs later; remove the OnStartup
registration wrappers from the individual source files (e.g., the OnStartup
calls in tarball.cc, github.cc, path.cc and any other register*InputScheme()
OnStartup usages) so that initFetchers() is the single place that calls all
register*InputScheme() functions and no duplicate registerInputScheme() calls
occur.
src/libfetchers/github.cc
Outdated
| static auto rGitHubInputScheme = OnStartup([] { registerGitHubInputScheme(); }); | ||
| static auto rGitLabInputScheme = OnStartup([] { registerGitLabInputScheme(); }); | ||
| static auto rSourceHutInputScheme = OnStartup([] { registerSourceHutInputScheme(); }); |
There was a problem hiding this comment.
Remove OnStartup wrappers to avoid double registration.
Same issue as in other fetcher files - these OnStartup wrappers will conflict with initFetchers() calls.
Proposed fix
-static auto rGitHubInputScheme = OnStartup([] { registerGitHubInputScheme(); });
-static auto rGitLabInputScheme = OnStartup([] { registerGitLabInputScheme(); });
-static auto rSourceHutInputScheme = OnStartup([] { registerSourceHutInputScheme(); });
-
void registerGitHubInputScheme()📝 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.
| static auto rGitHubInputScheme = OnStartup([] { registerGitHubInputScheme(); }); | |
| static auto rGitLabInputScheme = OnStartup([] { registerGitLabInputScheme(); }); | |
| static auto rSourceHutInputScheme = OnStartup([] { registerSourceHutInputScheme(); }); | |
| void registerGitHubInputScheme() |
🤖 Prompt for AI Agents
In `@src/libfetchers/github.cc` around lines 662 - 664, These static variables use
OnStartup wrappers which cause duplicate registration when initFetchers() also
registers schemes; remove the OnStartup(...) wrapper and initialize the
variables by directly calling the registration functions instead (replace the
lines for rGitHubInputScheme, rGitLabInputScheme, and rSourceHutInputScheme so
they call registerGitHubInputScheme(), registerGitLabInputScheme(), and
registerSourceHutInputScheme() directly or remove the variables entirely and
rely on initFetchers() to perform registration).
| void initFetchers(); | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find all occurrences of initFetchers in the header file
echo "=== Declarations in fetchers.hh ==="
rg -n "void initFetchers" src/libfetchers/include/nix/fetchers/fetchers.hh -B 2 -A 2
echo -e "\n=== Implementations in fetchers.cc ==="
rg -n "void initFetchers\|void.*fetchers.*initFetchers" src/libfetchers/fetchers.cc -B 2 -A 10Repository: DeterminateSystems/nix-src
Length of output: 341
🏁 Script executed:
#!/bin/bash
# Check namespace context for declarations
echo "=== Namespace context around line 281-282 in fetchers.hh ==="
sed -n '270,290p' src/libfetchers/include/nix/fetchers/fetchers.hh
echo -e "\n=== Namespace context around line 20-22 in fetchers.hh ==="
sed -n '15,30p' src/libfetchers/include/nix/fetchers/fetchers.hh
echo -e "\n=== Namespace context around implementations in fetchers.cc ==="
sed -n '540,560p' src/libfetchers/fetchers.ccRepository: DeterminateSystems/nix-src
Length of output: 1367
🏁 Script executed:
#!/bin/bash
# Search the entire codebase for all initFetchers implementations
echo "=== All initFetchers function definitions in the codebase ==="
rg -n "^\s*(void\s+)?initFetchers\s*\(" src/libfetchers/ --type cpp --type h -A 3Repository: DeterminateSystems/nix-src
Length of output: 904
Remove or implement the duplicate nix::fetchers::initFetchers() declaration.
The declaration at line 281 in the nix::fetchers namespace lacks a corresponding implementation. The only implementation (in fetchers.cc:546) is in the nix namespace. This will cause linker errors if the nix::fetchers::initFetchers() function is called. Either remove this declaration or provide an implementation. If both namespace versions are needed, have one delegate to the other.
🤖 Prompt for AI Agents
In `@src/libfetchers/include/nix/fetchers/fetchers.hh` around lines 281 - 282, The
header declares nix::fetchers::initFetchers() but the only implementation is
nix::initFetchers() — causing a linker mismatch; either remove the
nix::fetchers::initFetchers() declaration from the header or add a delegating
implementation (nix::fetchers::initFetchers() { return ::nix::initFetchers(); })
so both symbols exist and link correctly; locate the declaration in the
nix::fetchers namespace and either delete it or implement the wrapper to call
the existing nix::initFetchers().
src/libfetchers/tarball.cc
Outdated
| static auto rTarballInputScheme = OnStartup([] { registerTarballInputScheme(); }); | ||
| static auto rFileInputScheme = OnStartup([] { registerFileInputScheme(); }); |
There was a problem hiding this comment.
Remove OnStartup wrappers to avoid double registration.
Same issue as flagged in path.cc and fetchers.cc - these OnStartup wrappers will cause double registration when initFetchers() is called.
Proposed fix
-static auto rTarballInputScheme = OnStartup([] { registerTarballInputScheme(); });
-static auto rFileInputScheme = OnStartup([] { registerFileInputScheme(); });
-
void registerTarballInputScheme()🤖 Prompt for AI Agents
In `@src/libfetchers/tarball.cc` around lines 521 - 522, Remove the OnStartup
wrappers to prevent duplicate registration: replace the static initializers
using OnStartup (rTarballInputScheme and rFileInputScheme) with direct calls to
registerTarballInputScheme() and registerFileInputScheme() so the schemes are
registered exactly once (matching how initFetchers() performs registration);
update the lines referencing OnStartup to static auto rTarballInputScheme =
registerTarballInputScheme(); and static auto rFileInputScheme =
registerFileInputScheme().
a661669 to
b7fef3b
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/libfetchers/fetchers.cc`:
- Around line 546-567: The current initFetchers() uses a non-atomic static bool
guard which can race; replace that pattern with a thread-safe one-time
initialization using a static std::once_flag and std::call_once. Inside
initFetchers(), remove the static bool init and its check/assignment, declare a
static std::once_flag (e.g. fetchers_init_flag) and wrap the registration calls
(registerTarballInputScheme, registerFileInputScheme, registerGitInputScheme,
registerGitHubInputScheme, registerGitLabInputScheme,
registerSourceHutInputScheme, registerPathInputScheme,
registerIndirectInputScheme, registerMercurialInputScheme) in a
std::call_once(fetchers_init_flag, [] { ... }); to ensure registrations run
exactly once across threads.
♻️ Duplicate comments (1)
src/libfetchers-c/nix_api_fetchers.h (1)
24-35: Documentation issues persist from previous review.The past review flagged two issues that remain unaddressed:
- Line 26:
@ingroup libfetcher_initshould be@ingroup libfetchersto match the@defgroupon line 3- Line 30: Grammar error - "and it idempotent" should be "and it is idempotent"
Suggested fix
/** * `@brief` Initialize the Nix fetchers. - * `@ingroup` libfetcher_init + * `@ingroup` libfetchers * * This function must be called at least once, * at some point before using a fetcher for the first time. - * This function can be called multiple times, and it idempotent. + * This function can be called multiple times, and it is idempotent. * * `@param`[out] context Optional, stores error information * `@return` NIX_OK if the initialization was successful, an error code otherwise. */
src/libfetchers/fetchers.cc
Outdated
| namespace nix { | ||
|
|
||
| using namespace nix::fetchers; | ||
|
|
||
| void initFetchers() | ||
| { | ||
| static bool init; | ||
| if (init) | ||
| return; | ||
|
|
||
| init = true; | ||
|
|
||
| registerTarballInputScheme(); | ||
| registerFileInputScheme(); | ||
| registerGitInputScheme(); | ||
| registerGitHubInputScheme(); | ||
| registerGitLabInputScheme(); | ||
| registerSourceHutInputScheme(); | ||
| registerPathInputScheme(); | ||
| registerIndirectInputScheme(); | ||
| registerMercurialInputScheme(); | ||
| } |
There was a problem hiding this comment.
Make initFetchers() thread-safe (static bool causes a data race).
Line 552 uses a non-atomic static bool guard. If initFetchers() is reached concurrently (now possible via Input::fromURL / fromAttrs), two threads can pass the guard, causing double-registration or UB. Prefer std::once_flag + std::call_once.
🔧 Suggested fix (thread-safe one-time init)
+#include <mutex>
...
void initFetchers()
{
- static bool init;
- if (init)
- return;
-
- init = true;
-
- registerTarballInputScheme();
- registerFileInputScheme();
- registerGitInputScheme();
- registerGitHubInputScheme();
- registerGitLabInputScheme();
- registerSourceHutInputScheme();
- registerPathInputScheme();
- registerIndirectInputScheme();
- registerMercurialInputScheme();
+ static std::once_flag once;
+ std::call_once(once, [] {
+ registerTarballInputScheme();
+ registerFileInputScheme();
+ registerGitInputScheme();
+ registerGitHubInputScheme();
+ registerGitLabInputScheme();
+ registerSourceHutInputScheme();
+ registerPathInputScheme();
+ registerIndirectInputScheme();
+ registerMercurialInputScheme();
+ });
}📝 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.
| namespace nix { | |
| using namespace nix::fetchers; | |
| void initFetchers() | |
| { | |
| static bool init; | |
| if (init) | |
| return; | |
| init = true; | |
| registerTarballInputScheme(); | |
| registerFileInputScheme(); | |
| registerGitInputScheme(); | |
| registerGitHubInputScheme(); | |
| registerGitLabInputScheme(); | |
| registerSourceHutInputScheme(); | |
| registerPathInputScheme(); | |
| registerIndirectInputScheme(); | |
| registerMercurialInputScheme(); | |
| } | |
| namespace nix { | |
| using namespace nix::fetchers; | |
| void initFetchers() | |
| { | |
| static std::once_flag once; | |
| std::call_once(once, [] { | |
| registerTarballInputScheme(); | |
| registerFileInputScheme(); | |
| registerGitInputScheme(); | |
| registerGitHubInputScheme(); | |
| registerGitLabInputScheme(); | |
| registerSourceHutInputScheme(); | |
| registerPathInputScheme(); | |
| registerIndirectInputScheme(); | |
| registerMercurialInputScheme(); | |
| }); | |
| } |
🤖 Prompt for AI Agents
In `@src/libfetchers/fetchers.cc` around lines 546 - 567, The current
initFetchers() uses a non-atomic static bool guard which can race; replace that
pattern with a thread-safe one-time initialization using a static std::once_flag
and std::call_once. Inside initFetchers(), remove the static bool init and its
check/assignment, declare a static std::once_flag (e.g. fetchers_init_flag) and
wrap the registration calls (registerTarballInputScheme,
registerFileInputScheme, registerGitInputScheme, registerGitHubInputScheme,
registerGitLabInputScheme, registerSourceHutInputScheme,
registerPathInputScheme, registerIndirectInputScheme,
registerMercurialInputScheme) in a std::call_once(fetchers_init_flag, [] { ...
}); to ensure registrations run exactly once across threads.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/libexpr/include/nix/expr/primops.hh`:
- Around line 24-27: The member primOp is currently declared as an rvalue
reference (PrimOp && primOp) which can dangle; change the member to store PrimOp
by value (PrimOp primOp) and update the constructor initializer to move the
parameter into the member (primOp(std::move(primOp))) so that ensure() and
primOps().push_back(primOp) own a valid object; keep the existing registered
bool and ensure() usage unchanged.
In `@src/libexpr/primops.cc`:
- Around line 5205-5219: The ensure() method in RegisterPrimOp mutates
registered and primOps() without synchronization; wrap the body of
RegisterPrimOp::ensure() with a mutex to make the check-then-act atomic: add a
(likely static) std::mutex (or std::recursive_mutex if needed) used by
RegisterPrimOp::ensure(), acquire a std::lock_guard<std::mutex> at the start of
ensure(), then re-check registered and, if false, push primOp into primOps() and
set registered = true; include <mutex> and keep the lock scope minimal to cover
the check and mutation only.
| void ensure(); | ||
| private: | ||
| bool registered; | ||
| PrimOp && primOp; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find RegisterPrimOp class definition and constructor
rg -n "class RegisterPrimOp|struct RegisterPrimOp" -A 20 --type cpp --type hRepository: DeterminateSystems/nix-src
Length of output: 1537
🏁 Script executed:
#!/bin/bash
# Find RegisterPrimOp::ensure() implementation
rg -n "RegisterPrimOp::ensure|void ensure" -B 2 -A 10 --type cpp --type hRepository: DeterminateSystems/nix-src
Length of output: 6538
🏁 Script executed:
#!/bin/bash
# Search for RegisterPrimOp instantiation patterns to see if temporaries are used
rg -n "RegisterPrimOp\s*[({]|new RegisterPrimOp" -B 2 -A 2 --type cppRepository: DeterminateSystems/nix-src
Length of output: 1103
🏁 Script executed:
#!/bin/bash
# Check if registered member is initialized anywhere
rg -n "registered\s*=" -C 2 --type cpp --type h | head -30Repository: DeterminateSystems/nix-src
Length of output: 269
🌐 Web query:
C++ rvalue reference data members lifetime semantics constructor temporary binding
💡 Result:
In C++, binding a temporary to a reference data member (including an T&& rvalue-reference member) generally does not extend the temporary’s lifetime to the lifetime of the containing object. The temporary typically dies at the end of the full-expression that performed the construction, leaving the member dangling. [1]
The rule (what does and doesn’t get lifetime extension)
A temporary’s lifetime is extended when it is directly bound to a reference in certain contexts (e.g., a local const T& r = T{...}; or T&& r = T{...};). But there are important exceptions, including:
- A temporary bound to a reference in a member initializer (
mem-initializer) (e.g.S(T&& x) : r(std::move(x)) {}or: r(T{}) {}) lives only until the end of the full-expression containing the construction (i.e., through the;ofS s(...);), not as long asslives. [1]
Consequence for rvalue-reference data members
struct S {
std::string&& r;
S(std::string&& x) : r(std::move(x)) {}
};
S make() { return S(std::string("hi")); } // r will dangle after the full-expressionstd::string("hi") is a temporary; S::r ends up referring to it, but that temporary is destroyed at the end of the statement that creates the S. [1]
A subtle special case: aggregate initialization in C++20
For aggregates, C++20 introduced a distinction:
struct A { int&& r; };
A a1{7}; // lifetime *is* extended (braces) [1]
A a2(7); // well-formed, but r dangles (parens) [1]So brace-init aggregate can extend the lifetime in this specific scenario, but constructor-style init does not. [1]
Why expression-template code can “work”
Expression-template libraries avoid dangling by ensuring that:
- they only store references to operands that outlive the expression object, and/or
- they capture rvalues by value (owning storage) while referencing lvalues, often via perfect-forwarding wrappers.
If you store an rvalue-reference member to a temporary operand, you’ll reproduce the dangling behavior discussed in practice. [2]
Practical guidance
- Don’t use
T&&(orconst T&) data members to “hold” temporaries unless you prove the referred object outlives the containing object. - Prefer storing by value (possibly with move) for ownership:
struct S { std::string v; S(std::string x) : v(std::move(x)) {} };
Sources: cppreference on temporary lifetime extension and the “reference member in new-expression / construction” exception (and aggregate brace/parens difference) [1], plus a representative discussion showing the dangling behavior with rvalue-reference members in practice [2].
[1] Reference initialization & temporary lifetime notes (cppreference)
[2] “Data member and rvalue life-time” discussion (Stack Overflow)
Store PrimOp by value to prevent dangling references.
The member PrimOp && primOp binds to an rvalue reference parameter that may itself be bound to a temporary. Per C++ semantics, temporaries bound to reference data members in the initializer list do not have their lifetime extended—they are destroyed at the end of the constructor's full-expression. This leaves the member reference dangling when ensure() later accesses it via primOps().push_back(primOp).
The constructor correctly initializes registered to false, but storing primOp by value is necessary to own the object safely.
Suggested fix
- bool registered;
- PrimOp && primOp;
+ bool registered = false;
+ PrimOp primOp;Update the constructor initializer to primOp(std::move(primOp)), which will move-assign the parameter into the by-value member.
🤖 Prompt for AI Agents
In `@src/libexpr/include/nix/expr/primops.hh` around lines 24 - 27, The member
primOp is currently declared as an rvalue reference (PrimOp && primOp) which can
dangle; change the member to store PrimOp by value (PrimOp primOp) and update
the constructor initializer to move the parameter into the member
(primOp(std::move(primOp))) so that ensure() and primOps().push_back(primOp) own
a valid object; keep the existing registered bool and ensure() usage unchanged.
| RegisterPrimOp::RegisterPrimOp(PrimOp && primOp) | ||
| : registered(false) | ||
| , primOp(std::move(primOp)) | ||
| { | ||
| primOps().push_back(std::move(primOp)); | ||
| ensure(); | ||
| } | ||
|
|
||
| void RegisterPrimOp::ensure() | ||
| { | ||
| if (registered) | ||
| return; | ||
|
|
||
| primOps().push_back(primOp); | ||
| registered = true; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, locate the file and check the RegisterPrimOp class definition
find . -name "primops.cc" -o -name "primops.h" | head -20Repository: DeterminateSystems/nix-src
Length of output: 125
🏁 Script executed:
# Search for RegisterPrimOp class definition
rg "class RegisterPrimOp" -A 10 -B 2Repository: DeterminateSystems/nix-src
Length of output: 52
🏁 Script executed:
# Search for where initPrimOps is called
rg "initPrimOps\(" -nRepository: DeterminateSystems/nix-src
Length of output: 221
🏁 Script executed:
# Search for primOps function definition
rg "primOps\(\)" -B 3 -A 5 --max-count=10Repository: DeterminateSystems/nix-src
Length of output: 1983
🏁 Script executed:
# Check the context where initPrimOps() is called
sed -n '335,350p' src/libexpr/eval.ccRepository: DeterminateSystems/nix-src
Length of output: 595
🏁 Script executed:
# Find RegisterPrimOp class definition
rg "struct RegisterPrimOp|class RegisterPrimOp" -A 15Repository: DeterminateSystems/nix-src
Length of output: 1110
🏁 Script executed:
# Search for where RegisterPrimOp objects are instantiated
rg "RegisterPrimOp\{" -B 3 -A 3Repository: DeterminateSystems/nix-src
Length of output: 52
🏁 Script executed:
# Check the EvalState constructor to see if it's called from multiple threads
rg "EvalState::" src/libexpr/eval.cc | head -20Repository: DeterminateSystems/nix-src
Length of output: 1220
🏁 Script executed:
# Find initPrimOps() implementation
sed -n '5535,5600p' src/libexpr/primops.ccRepository: DeterminateSystems/nix-src
Length of output: 2001
🏁 Script executed:
# Search for RegisterPrimOp constructor calls more thoroughly
rg "RegisterPrimOp" src/libexpr/primops.cc | head -30Repository: DeterminateSystems/nix-src
Length of output: 1262
🏁 Script executed:
# Look for global RegisterPrimOp instances
rg "^.*RegisterPrimOp" src/libexpr/primops.cc | grep -v "//" | head -20Repository: DeterminateSystems/nix-src
Length of output: 866
🏁 Script executed:
# Check for any threading/concurrency related comments or patterns in eval.hh
rg "thread|concurrent|lock|mutex|atomic" src/libexpr/include/nix/expr/eval.hhRepository: DeterminateSystems/nix-src
Length of output: 592
🏁 Script executed:
# Look for any synchronization in the codebase for primop initialization
rg "primOps\|RegisterPrimOp" src/libexpr/primops.cc | grep -i "lock\|mutex\|atomic"Repository: DeterminateSystems/nix-src
Length of output: 52
Add synchronization to RegisterPrimOp::ensure() for thread-safe concurrent initialization.
The registered field and primOps() vector are mutated without synchronization. Since initPrimOps() is called in the EvalState constructor and EvalState can be constructed concurrently from multiple threads (as evidenced by concurrent data structures elsewhere in the codebase), this creates a data race. The check-then-act pattern (if (registered) return; ... registered = true;) without locking is unsafe under concurrent access.
🔒 Suggested fix (mutex-guarded registration)
+#include <mutex>
...
void RegisterPrimOp::ensure()
{
+ static std::mutex primOpsMutex;
+ std::lock_guard<std::mutex> lock(primOpsMutex);
if (registered)
return;
primOps().push_back(primOp);
registered = true;
}🤖 Prompt for AI Agents
In `@src/libexpr/primops.cc` around lines 5205 - 5219, The ensure() method in
RegisterPrimOp mutates registered and primOps() without synchronization; wrap
the body of RegisterPrimOp::ensure() with a mutex to make the check-then-act
atomic: add a (likely static) std::mutex (or std::recursive_mutex if needed)
used by RegisterPrimOp::ensure(), acquire a std::lock_guard<std::mutex> at the
start of ensure(), then re-check registered and, if false, push primOp into
primOps() and set registered = true; include <mutex> and keep the lock scope
minimal to cover the check and mutation only.
cole-h
force-pushed
the
tristanross/dsp-62-collab-figure-out-static-building-of-determinate-nix
branch
from
7c6e6ba to
5600f5f
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
nix-meson-build-support/default-system-cpu/meson.build (1)
13-18: Change'arm64'to'aarch64'in the cpu_family condition.Meson's canonical
cpu_familyvalue for Apple Silicon (and 64-bit ARM generally) is'aarch64', not'arm64'. The current condition checks for'arm64'which will never match, causing Apple Silicon machines to fall through to other branches and produce incorrect system strings. Update the condition tohost_machine.cpu_family() in [ 'arm', 'aarch64' ].
🤖 Fix all issues with AI agents
In `@src/nix/main.cc`:
- Around line 44-53: The current preprocessor blocks using __arm__ and __arm64__
unconditionally invoke `#error` (with macros STRINGIFY/TOSTRING) which breaks all
ARM/aarch64 builds; modify the blocks around the symbols __arm__ and __arm64__
to be conditional on an explicit opt-in debug macro (e.g., NIX_ARM_DIAGNOSTIC or
ENABLE_DEBUG_DIAGNOSTICS) or remove the `#error` entirely, and replace the
informal error text with a professional diagnostic via `#pragma` message (or log)
if the debug macro is set; ensure STRINGIFY and TOSTRING are still used for
printing NIX_LOCAL_SYSTEM, and keep the diagnostic gated so normal builds on ARM
succeed.
RossComputerGuy
force-pushed
the
tristanross/dsp-62-collab-figure-out-static-building-of-determinate-nix
branch
from
a291f7e to
37e288f
Compare
RossComputerGuy
force-pushed
the
tristanross/dsp-62-collab-figure-out-static-building-of-determinate-nix
branch
from
37e288f to
ddc66ad
Compare
RossComputerGuy
force-pushed
the
tristanross/dsp-62-collab-figure-out-static-building-of-determinate-nix
branch
from
ddc66ad to
4a22942
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/libstore-c/nix_api_store_internal.h`:
- Line 3: Remove the non-existent include "nix/store/async-path-writer.hh" and
eliminate or replace the associated nix::AsyncPathWriter member from the struct
in nix_api_store_internal.h; locate the struct that declares a
nix::AsyncPathWriter field, either remove that field entirely or swap it for an
available, correct type from existing nix headers (or forward-declare and add
the correct header if the type should exist), then update any code that
referenced that member to use the existing API/type.
In `@src/libstore/meson.build`:
- Around line 269-278: The Meson build snippet uses list concatenation
incorrectly: change the line that appends the wasmtime flag so the right-hand
operand is a list; specifically update the manipulation of link_args when
wasmtime_required.enabled() is true (the current link_args += '-lwasmtime') to
use list concatenation (e.g., wrap '-lwasmtime' in square brackets) so link_args
remains a list and Meson type-checking passes.
🧹 Nitpick comments (1)
flake.nix (1)
484-486: GuardpkgsStatic.nixComponents2access to prevent eval failures on systems that lack it.The static package definition is placed unconditionally within the
supportsCrossguard, butsupportsCrosscontrols cross-compilation support, not platform-specific availability ofpkgsStatic. Previous implementations explicitly excluded Darwin from static builds (referencing nixpkgs issue#320448), which was removed without verification that the underlying issue is resolved.For packages with
supportsCross = true(the default), this will attempt to accesspkgsStatic.nixComponents2on aarch64-darwin, which may not exist or be fully functional. Add an optional attributes check to prevent eval failures:Guard to safely access pkgsStatic.nixComponents2
// { - "${pkgName}-static" = nixpkgsFor.${system}.native.pkgsStatic.nixComponents2.${pkgName}; + "${pkgName}-static" = nixpkgsFor.${system}.native.pkgsStatic.nixComponents2.${pkgName}; }Replace with:
+ // lib.optionalAttrs + (nixpkgsFor.${system}.native ? pkgsStatic + && nixpkgsFor.${system}.native.pkgsStatic ? nixComponents2) { + "${pkgName}-static" = nixpkgsFor.${system}.native.pkgsStatic.nixComponents2.${pkgName}; + }
| @@ -1,12 +1,14 @@ | |||
| #ifndef NIX_API_STORE_INTERNAL_H | |||
| #define NIX_API_STORE_INTERNAL_H | |||
| #include "nix/store/async-path-writer.hh" | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for async-path-writer header file
echo "=== Searching for async-path-writer.hh in repository ==="
fd -type f -name '*async-path-writer*'
echo -e "\n=== Searching in nix/store directory ==="
fd -type f 'store' | head -20
echo -e "\n=== Checking all .hh files in repository ==="
fd -type f -name '*.hh' | grep -i 'async\|writer' || echo "No matching files found with grep"
echo -e "\n=== Looking for include directories ==="
fd -type d -name 'store' | head -10
echo -e "\n=== Checking file at src/libstore-c/nix_api_store_internal.h ==="
head -20 src/libstore-c/nix_api_store_internal.hRepository: DeterminateSystems/nix-src
Length of output: 1420
Add the missing header file or remove the reference.
The header file nix/store/async-path-writer.hh does not exist in the repository. The include statement on line 3 and the nix::AsyncPathWriter member in the struct will cause compilation failures. Either add the missing header file or remove this include and the associated struct member.
🧰 Tools
🪛 Clang (14.0.6)
[error] 3-3: 'nix/store/async-path-writer.hh' file not found
(clang-diagnostic-error)
🤖 Prompt for AI Agents
In `@src/libstore-c/nix_api_store_internal.h` at line 3, Remove the non-existent
include "nix/store/async-path-writer.hh" and eliminate or replace the associated
nix::AsyncPathWriter member from the struct in nix_api_store_internal.h; locate
the struct that declares a nix::AsyncPathWriter field, either remove that field
entirely or swap it for an available, correct type from existing nix headers (or
forward-declare and add the correct header if the type should exist), then
update any code that referenced that member to use the existing API/type.
| link_args = [] | ||
|
|
||
| wasmtime_required = get_option('wasm').disable_if( | ||
| get_option('default_library') == 'static', | ||
| error_message : 'Building with wasmtime and static linking is not supported', | ||
| ) | ||
|
|
||
| if wasmtime_required.enabled() | ||
| link_args += '-lwasmtime' | ||
| endif |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, check the actual state of the file
cat -n src/libstore/meson.build | sed -n '265,280p'Repository: DeterminateSystems/nix-src
Length of output: 562
🏁 Script executed:
# Search for similar link_args concatenation patterns in the codebase
rg 'link_args\s*\+=' src/libstore/meson.build -A 2 -B 2Repository: DeterminateSystems/nix-src
Length of output: 136
🏁 Script executed:
# Look for other list concatenation patterns with strings to understand Meson behavior
rg '\+=\s*["\']' src/libstore/meson.build | head -20Repository: DeterminateSystems/nix-src
Length of output: 138
🏁 Script executed:
# Find other meson.build files and search for list concatenation patterns
find . -name "meson.build" -type f | head -5Repository: DeterminateSystems/nix-src
Length of output: 255
🏁 Script executed:
# Search for other instances where lists are concatenated with strings or values
rg '\+=\s*[\[\]]' src/libstore/meson.build | head -20Repository: DeterminateSystems/nix-src
Length of output: 125
🏁 Script executed:
# Look for link_args or similar list operations in other meson files
rg 'link_args\s*=' . --type py --type txt 2>/dev/null | head -15Repository: DeterminateSystems/nix-src
Length of output: 52
🌐 Web query:
Meson list concatenation string operator += documentation
💡 Result:
Meson’s official docs don’t document a separate “string += operator”. Instead:
- String concatenation is documented with
+:
combined = str1 + '_' + str2[1] - Meson shows
+=as an augmented assignment that rebinds the variable (because “all objects are immutable”), e.g. with arrays:
var2 += [4](creates a new object and assigns it back) [1]
So, for strings the documented form is:
s = 'foo'
s = s + 'bar'And if you’re looking for where += is described at all in Meson’s DSL docs, the only explicit mention is that general “immutable objects / rebinding” example (with arrays), not a string-specific += section. [1]
Sources: [1] Meson Syntax documentation (mesonbuild.com)
Use list concatenation for link_args to avoid Meson type errors.
link_args += '-lwasmtime' is invalid in Meson because link_args is a list and the += operator for lists requires the right operand to be a list. Wrap the string in square brackets to fix the type mismatch.
🧩 Proposed fix
if wasmtime_required.enabled()
- link_args += '-lwasmtime'
+ link_args += [ '-lwasmtime' ]
endif📝 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.
| link_args = [] | |
| wasmtime_required = get_option('wasm').disable_if( | |
| get_option('default_library') == 'static', | |
| error_message : 'Building with wasmtime and static linking is not supported', | |
| ) | |
| if wasmtime_required.enabled() | |
| link_args += '-lwasmtime' | |
| endif | |
| link_args = [] | |
| wasmtime_required = get_option('wasm').disable_if( | |
| get_option('default_library') == 'static', | |
| error_message : 'Building with wasmtime and static linking is not supported', | |
| ) | |
| if wasmtime_required.enabled() | |
| link_args += [ '-lwasmtime' ] | |
| endif |
🤖 Prompt for AI Agents
In `@src/libstore/meson.build` around lines 269 - 278, The Meson build snippet
uses list concatenation incorrectly: change the line that appends the wasmtime
flag so the right-hand operand is a list; specifically update the manipulation
of link_args when wasmtime_required.enabled() is true (the current link_args +=
'-lwasmtime') to use list concatenation (e.g., wrap '-lwasmtime' in square
brackets) so link_args remains a list and Meson type-checking passes.
4a22942 to
ec47b7f
Compare
RossComputerGuy
force-pushed
the
tristanross/dsp-62-collab-figure-out-static-building-of-determinate-nix
branch
from
ec47b7f to
87fd294
Compare
RossComputerGuy
force-pushed
the
tristanross/dsp-62-collab-figure-out-static-building-of-determinate-nix
branch
from
87fd294 to
bb6e4c8
Compare
RossComputerGuy
force-pushed
the
tristanross/dsp-62-collab-figure-out-static-building-of-determinate-nix
branch
3 times, most recently
from
04421e3 to
40bad32
Compare
edolstra
force-pushed
the
tristanross/dsp-62-collab-figure-out-static-building-of-determinate-nix
branch
from
40bad32 to
b6a7958
Compare
| // FIXME(RossComputerGuy): this causes issues on static builds. | ||
| // In particular, it causes a segfault to happen at the end of the program running. | ||
| // auto callback = createInterruptCallback([&]() { stopWorkerThread(); }); |
There was a problem hiding this comment.
We should fix this properly, somehow. I don't think we want uninterruptible file transfers lol.
There was a problem hiding this comment.
Yes, I am just not sure what the fix is and how this is happening. I did try a couple different fixes and removing the offending code has been the only one to at least make things not crash on exit.
| try { | ||
| nix::initLibUtil(); | ||
| #ifndef _WIN32 | ||
| nix::unix::startSignalHandlerThread(); |
There was a problem hiding this comment.
Is this necessary? I seem to recall you saying you were just trying it out -- did it end up being load-bearing in the end?
There was a problem hiding this comment.
I believe it is to some degree since I saw it changed the stack trace in gdb.
Motivation
Enables static builds so static binaries can be made.
Context
Summary by CodeRabbit
New Features
Improvements
Bug Fixes / Tweaks