libutil: use makeTempPath in createTempSubdir
This makes the paths more nondeterministic, but more reliably unique, and lets us remove the retry loop. Note that this adds random entropy to the build directory visible inside derivations on Darwin and unsandboxed Linux. It was already non‐deterministic in the presence of concurrent builds and similar, but now we can reliably expect it to be different every time. On the whole I think that’s a good thing, as it is impossible to ensure a single consistent build directory and derivation outputs should not depend on it. Package reproducibility isn’t great on Darwin to begin with, though, and the reproducibility bugs this will turn up in packages will be more urgent to fix than when the build directory was mostly consistent. A quick survey of my local store shows that many C, C++, and Rust binaries contain build directory references, likely due to use of `__FILE__` and its equivalents; non‐binary offenders include: * Install logs included in the Rust and Cargo bootstrap compilers * Example errors in the Rust documentation referencing build paths * Configuration information installed with CPython itself * Python 2 metadata from resholve’s closure * Cython metadata * Generated headers in Facebook libraries referencing source paths * Generated CMake files in Facebook libraries referencing source paths I haven’t built that much in this store since the last GC, so this is probably only a small sample of the problems across the tree. These are all instances of <https://reproducible-builds.org/docs/build-path/>, though, and should probably just be treated as general reproducibility bugs outside of contexts like the Linux sandbox where we can normalize them away entirely. I have implemented away build directory paths for C/C++, applied some additional fixes for non‐`__FILE__`‐related issues in binaries from ATF and LLVM, and fixed the derivation bug causing the CPython 3 issue, and will work on upstreaming these changes. Rust is working on the problem upstream, with some temporary workarounds we can potentially apply in Nixpkgs for now. The rest will require some distributed effort. Change-Id: I6a6a69648f74d85c6fca86cc52f38fd957e4f9ad
This commit is contained in:
@@ -447,7 +447,7 @@ try {
|
|||||||
/* Create a temporary directory where the build will take
|
/* Create a temporary directory where the build will take
|
||||||
place. */
|
place. */
|
||||||
tmpDirRoot =
|
tmpDirRoot =
|
||||||
createTempDir(buildDir, "nix-build-" + std::string(drvPath.name()), false, false, 0700);
|
createTempDir(buildDir, "nix-build-" + std::string(drvPath.name()), 0700);
|
||||||
} catch (SysError & e) {
|
} catch (SysError & e) {
|
||||||
/*
|
/*
|
||||||
* Fallback to the global tmpdir and create a safe space there
|
* Fallback to the global tmpdir and create a safe space there
|
||||||
@@ -466,7 +466,7 @@ try {
|
|||||||
constexpr int toplevelDirMode = 0700;
|
constexpr int toplevelDirMode = 0700;
|
||||||
#endif
|
#endif
|
||||||
auto nixBuildsTmp = createTempDir(
|
auto nixBuildsTmp = createTempDir(
|
||||||
"", fmt("nix-builds-%s", geteuid()), false, false, toplevelDirMode
|
"", fmt("nix-builds-%s", geteuid()), toplevelDirMode
|
||||||
);
|
);
|
||||||
warn(
|
warn(
|
||||||
"Failed to use the system-wide build directory '%s', falling back to a temporary "
|
"Failed to use the system-wide build directory '%s', falling back to a temporary "
|
||||||
@@ -475,7 +475,7 @@ try {
|
|||||||
nixBuildsTmp
|
nixBuildsTmp
|
||||||
);
|
);
|
||||||
tmpDirRoot = createTempDir(
|
tmpDirRoot = createTempDir(
|
||||||
nixBuildsTmp, "nix-build-" + std::string(drvPath.name()), false, false, 0700
|
nixBuildsTmp, "nix-build-" + std::string(drvPath.name()), 0700
|
||||||
);
|
);
|
||||||
worker.buildDirOverride = nixBuildsTmp;
|
worker.buildDirOverride = nixBuildsTmp;
|
||||||
}
|
}
|
||||||
|
|||||||
+1
-1
@@ -25,7 +25,7 @@ SSH::SSH(const std::string & host, const std::optional<uint16_t> port, const std
|
|||||||
throw Error("invalid SSH host name '%s'", host);
|
throw Error("invalid SSH host name '%s'", host);
|
||||||
|
|
||||||
auto state(state_.lock());
|
auto state(state_.lock());
|
||||||
state->tmpDir = std::make_unique<AutoDelete>(createTempDir("", "nix", true, true, 0700));
|
state->tmpDir = std::make_unique<AutoDelete>(createTempDir("", "nix", 0700));
|
||||||
}
|
}
|
||||||
|
|
||||||
void SSH::addCommonSSHOpts(Strings & args)
|
void SSH::addCommonSSHOpts(Strings & args)
|
||||||
|
|||||||
@@ -5,10 +5,9 @@
|
|||||||
|
|
||||||
namespace nix {
|
namespace nix {
|
||||||
|
|
||||||
Path createTempDir(const Path & tmpRoot, const Path & prefix,
|
Path createTempDir(const Path & tmpRoot, const Path & prefix, mode_t mode)
|
||||||
bool includePid, bool useGlobalCounter, mode_t mode)
|
|
||||||
{
|
{
|
||||||
return createTempSubdir(tmpRoot.empty() ? defaultTempDir() : tmpRoot, prefix, includePid, useGlobalCounter, mode);
|
return createTempSubdir(tmpRoot.empty() ? defaultTempDir() : tmpRoot, prefix, mode);
|
||||||
}
|
}
|
||||||
|
|
||||||
std::pair<AutoCloseFD, Path> createTempFile(const Path & prefix)
|
std::pair<AutoCloseFD, Path> createTempFile(const Path & prefix)
|
||||||
|
|||||||
@@ -8,8 +8,7 @@ namespace nix {
|
|||||||
/**
|
/**
|
||||||
* Create a temporary directory.
|
* Create a temporary directory.
|
||||||
*/
|
*/
|
||||||
Path createTempDir(const Path & tmpRoot = "", const Path & prefix = "nix",
|
Path createTempDir(const Path & tmpRoot = "", const Path & prefix = "nix", mode_t mode = 0755);
|
||||||
bool includePid = true, bool useGlobalCounter = true, mode_t mode = 0755);
|
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Create a temporary file, returning a file handle and its path.
|
* Create a temporary file, returning a file handle and its path.
|
||||||
|
|||||||
@@ -646,26 +646,11 @@ void AutoDelete::reset(const Path & p, bool recursive) {
|
|||||||
|
|
||||||
//////////////////////////////////////////////////////////////////////
|
//////////////////////////////////////////////////////////////////////
|
||||||
|
|
||||||
static Path tempName(PathView parent, const Path & prefix, bool includePid,
|
|
||||||
std::atomic<unsigned int> & counter)
|
|
||||||
{
|
|
||||||
auto tmpRoot = canonPath(parent, true);
|
|
||||||
if (includePid)
|
|
||||||
return fmt("%1%/%2%-%3%-%4%", tmpRoot, prefix, getpid(), counter++);
|
|
||||||
else
|
|
||||||
return fmt("%1%/%2%-%3%", tmpRoot, prefix, counter++);
|
|
||||||
}
|
|
||||||
|
|
||||||
Path createTempSubdir(const Path & parent, const Path & prefix,
|
Path createTempSubdir(const Path & parent, const Path & prefix,
|
||||||
bool includePid, bool useGlobalCounter, mode_t mode)
|
mode_t mode)
|
||||||
{
|
{
|
||||||
static std::atomic<unsigned int> globalCounter = 0;
|
|
||||||
std::atomic<unsigned int> localCounter = 0;
|
|
||||||
auto & counter(useGlobalCounter ? globalCounter : localCounter);
|
|
||||||
|
|
||||||
while (1) {
|
|
||||||
checkInterrupt();
|
checkInterrupt();
|
||||||
Path tmpDir = tempName(parent, prefix, includePid, counter);
|
Path tmpDir = makeTempPath(parent + "/", prefix);
|
||||||
if (mkdir(tmpDir.c_str(), mode) == 0) {
|
if (mkdir(tmpDir.c_str(), mode) == 0) {
|
||||||
#if __FreeBSD__
|
#if __FreeBSD__
|
||||||
/* Explicitly set the group of the directory. This is to
|
/* Explicitly set the group of the directory. This is to
|
||||||
@@ -676,14 +661,13 @@ Path createTempSubdir(const Path & parent, const Path & prefix,
|
|||||||
will be owned by "wheel"; but if the user is not in
|
will be owned by "wheel"; but if the user is not in
|
||||||
"wheel", then "tar" will fail to unpack archives that
|
"wheel", then "tar" will fail to unpack archives that
|
||||||
have the setgid bit set on directories. */
|
have the setgid bit set on directories. */
|
||||||
if (chown(tmpDir.c_str(), (uid_t) -1, getegid()) != 0)
|
if (chown(tmpDir.c_str(), (uid_t) -1, getegid()) != 0) {
|
||||||
throw SysError("setting group of directory '%1%'", tmpDir);
|
throw SysError("setting group of directory '%1%'", tmpDir);
|
||||||
|
}
|
||||||
#endif
|
#endif
|
||||||
return tmpDir;
|
return tmpDir;
|
||||||
}
|
}
|
||||||
if (errno != EEXIST)
|
|
||||||
throw SysError("creating directory '%1%'", tmpDir);
|
throw SysError("creating directory '%1%'", tmpDir);
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
Path makeTempPath(const Path & root, const Path & prefix)
|
Path makeTempPath(const Path & root, const Path & prefix)
|
||||||
|
|||||||
@@ -308,7 +308,7 @@ typedef std::unique_ptr<DIR, DIRDeleter> AutoCloseDir;
|
|||||||
* Create a temporary directory in a given parent directory.
|
* Create a temporary directory in a given parent directory.
|
||||||
*/
|
*/
|
||||||
Path createTempSubdir(const Path & parent, const Path & prefix = "nix",
|
Path createTempSubdir(const Path & parent, const Path & prefix = "nix",
|
||||||
bool includePid = true, bool useGlobalCounter = true, mode_t mode = 0755);
|
mode_t mode = 0755);
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Return temporary path constructed by appending to a root path.
|
* Return temporary path constructed by appending to a root path.
|
||||||
|
|||||||
Reference in New Issue
Block a user