Make --repair-path also repair corrupt optimised links

There already existed a smoke test for the link content length,
but it appears that there exists some corruptions pernicious enough
to replace the file content with zeros, and keeping the same length.

--repair-path now goes as far as checking the content of the link,
making it true to its name and actually repairing the path for such
coruption cases.
This commit is contained in:
Guillaume Maudoux 2022-01-11 11:57:45 +01:00
parent c260640dec
commit e9a4abdb5d
6 changed files with 75 additions and 20 deletions

View file

@ -2460,7 +2460,7 @@ void LocalDerivationGoal::registerOutputs()
} }
if (curRound == nrRounds) { if (curRound == nrRounds) {
localStore.optimisePath(actualPath); // FIXME: combine with scanForReferences() localStore.optimisePath(actualPath, false); // FIXME: combine with scanForReferences()
worker.markContentsGood(newInfo.path); worker.markContentsGood(newInfo.path);
} }

View file

@ -1307,7 +1307,7 @@ void LocalStore::addToStore(const ValidPathInfo & info, Source & source,
canonicalisePathMetaData(realPath, -1); canonicalisePathMetaData(realPath, -1);
optimisePath(realPath); // FIXME: combine with hashPath() optimisePath(realPath, repair); // FIXME: combine with hashPath()
registerValidPath(info); registerValidPath(info);
} }
@ -1419,7 +1419,7 @@ StorePath LocalStore::addToStoreFromDump(Source & source0, const string & name,
canonicalisePathMetaData(realPath, -1); // FIXME: merge into restorePath canonicalisePathMetaData(realPath, -1); // FIXME: merge into restorePath
optimisePath(realPath); optimisePath(realPath, repair);
ValidPathInfo info { dstPath, narHash.first }; ValidPathInfo info { dstPath, narHash.first };
info.narSize = narHash.second; info.narSize = narHash.second;
@ -1463,7 +1463,7 @@ StorePath LocalStore::addTextToStore(const string & name, const string & s,
dumpString(s, sink); dumpString(s, sink);
auto narHash = hashString(htSHA256, *sink.s); auto narHash = hashString(htSHA256, *sink.s);
optimisePath(realPath); optimisePath(realPath, repair);
ValidPathInfo info { dstPath, narHash }; ValidPathInfo info { dstPath, narHash };
info.narSize = sink.s->size(); info.narSize = sink.s->size();

View file

@ -172,8 +172,9 @@ public:
void optimiseStore() override; void optimiseStore() override;
/* Optimise a single store path. */ /* Optimise a single store path. Optionally, test the encountered
void optimisePath(const Path & path); symlinks for corruption. */
void optimisePath(const Path & path, bool repair);
bool verifyStore(bool checkContents, RepairFlag repair) override; bool verifyStore(bool checkContents, RepairFlag repair) override;
@ -253,7 +254,7 @@ private:
InodeHash loadInodeHash(); InodeHash loadInodeHash();
Strings readDirectoryIgnoringInodes(const Path & path, const InodeHash & inodeHash); Strings readDirectoryIgnoringInodes(const Path & path, const InodeHash & inodeHash);
void optimisePath_(Activity * act, OptimiseStats & stats, const Path & path, InodeHash & inodeHash); void optimisePath_(Activity * act, OptimiseStats & stats, const Path & path, InodeHash & inodeHash, bool repair);
// Internal versions that are not wrapped in retry_sqlite. // Internal versions that are not wrapped in retry_sqlite.
bool isValidPath_(State & state, const StorePath & path); bool isValidPath_(State & state, const StorePath & path);

View file

@ -88,7 +88,7 @@ Strings LocalStore::readDirectoryIgnoringInodes(const Path & path, const InodeHa
void LocalStore::optimisePath_(Activity * act, OptimiseStats & stats, void LocalStore::optimisePath_(Activity * act, OptimiseStats & stats,
const Path & path, InodeHash & inodeHash) const Path & path, InodeHash & inodeHash, bool repair)
{ {
checkInterrupt(); checkInterrupt();
@ -110,7 +110,7 @@ void LocalStore::optimisePath_(Activity * act, OptimiseStats & stats,
if (S_ISDIR(st.st_mode)) { if (S_ISDIR(st.st_mode)) {
Strings names = readDirectoryIgnoringInodes(path, inodeHash); Strings names = readDirectoryIgnoringInodes(path, inodeHash);
for (auto & i : names) for (auto & i : names)
optimisePath_(act, stats, path + "/" + i, inodeHash); optimisePath_(act, stats, path + "/" + i, inodeHash, repair);
return; return;
} }
@ -151,7 +151,20 @@ void LocalStore::optimisePath_(Activity * act, OptimiseStats & stats,
/* Check if this is a known hash. */ /* Check if this is a known hash. */
Path linkPath = linksDir + "/" + hash.to_string(Base32, false); Path linkPath = linksDir + "/" + hash.to_string(Base32, false);
retry: /* Maybe delete the link, if it has been corrupted. */
if (pathExists(linkPath)) {
auto stLink = lstat(linkPath);
if (st.st_size != stLink.st_size
|| (repair && hash != hashPath(htSHA256, linkPath).first))
{
// XXX: Consider overwriting linkPath with our valid version.
warn("removing corrupted link '%s'", linkPath);
warn("There may be more corrupted paths."
"\nYou should run `nix-store --verify --check-contents --repair` to fix them all");
unlink(linkPath.c_str());
}
}
if (!pathExists(linkPath)) { if (!pathExists(linkPath)) {
/* Nope, create a hard link in the links directory. */ /* Nope, create a hard link in the links directory. */
if (link(path.c_str(), linkPath.c_str()) == 0) { if (link(path.c_str(), linkPath.c_str()) == 0) {
@ -187,12 +200,6 @@ void LocalStore::optimisePath_(Activity * act, OptimiseStats & stats,
return; return;
} }
if (st.st_size != stLink.st_size) {
warn("removing corrupted link '%s'", linkPath);
unlink(linkPath.c_str());
goto retry;
}
printMsg(lvlTalkative, format("linking '%1%' to '%2%'") % path % linkPath); printMsg(lvlTalkative, format("linking '%1%' to '%2%'") % path % linkPath);
/* Make the containing directory writable, but only if it's not /* Make the containing directory writable, but only if it's not
@ -260,7 +267,7 @@ void LocalStore::optimiseStore(OptimiseStats & stats)
if (!isValidPath(i)) continue; /* path was GC'ed, probably */ if (!isValidPath(i)) continue; /* path was GC'ed, probably */
{ {
Activity act(*logger, lvlTalkative, actUnknown, fmt("optimising path '%s'", printStorePath(i))); Activity act(*logger, lvlTalkative, actUnknown, fmt("optimising path '%s'", printStorePath(i)));
optimisePath_(&act, stats, realStoreDir + "/" + std::string(i.to_string()), inodeHash); optimisePath_(&act, stats, realStoreDir + "/" + std::string(i.to_string()), inodeHash, false);
} }
done++; done++;
act.progress(done, paths.size()); act.progress(done, paths.size());
@ -278,12 +285,12 @@ void LocalStore::optimiseStore()
stats.filesLinked); stats.filesLinked);
} }
void LocalStore::optimisePath(const Path & path) void LocalStore::optimisePath(const Path & path, bool repair)
{ {
OptimiseStats stats; OptimiseStats stats;
InodeHash inodeHash; InodeHash inodeHash;
if (settings.autoOptimiseStore) optimisePath_(nullptr, stats, path, inodeHash); if (settings.autoOptimiseStore) optimisePath_(nullptr, stats, path, inodeHash, repair);
} }

View file

@ -40,7 +40,7 @@ struct LoggerSettings : Config
Setting<bool> showTrace{ Setting<bool> showTrace{
this, false, "show-trace", this, false, "show-trace",
R"( R"(
Where Nix should print out a stack trace in case of Nix Whether Nix should print out a stack trace in case of Nix
expression evaluation errors. expression evaluation errors.
)"}; )"};
}; };

View file

@ -74,3 +74,50 @@ if [ "$(nix-hash $path2)" != "$hash" -o -e $path2/bad ]; then
echo "path not repaired properly" >&2 echo "path not repaired properly" >&2
exit 1 exit 1
fi fi
# Check that --repair-path also checks content of optimised symlinks (1/2)
nix-store --verify-path $path2
if (! nix-store --optimize); then
echo "nix-store --optimize failed to optimize the store" >&2
exit 1
fi
chmod u+w $path2/bar
echo 'rabrab' > $path2/bar # different length
if nix-store --verify-path $path2; then
echo "nix-store --verify-path did not detect .links file corruption" >&2
exit 1
fi
nix-store --repair-path $path2 --option auto-optimise-store true
if [ "$(nix-hash $path2)" != "$hash" -o "BAR" != "$(< $path2/bar)" ]; then
echo "path not repaired properly" >&2
exit 1
fi
# Check that --repair-path also checks content of optimised symlinks (2/2)
nix-store --verify-path $path2
if (! nix-store --optimize); then
echo "nix-store --optimize failed to optimize the store" >&2
exit 1
fi
chmod u+w $path2
chmod u+w $path2/bar
sed -e 's/./X/g' < $path2/bar > $path2/tmp # same length, different content.
cp $path2/tmp $path2/bar
rm $path2/tmp
if nix-store --verify-path $path2; then
echo "nix-store --verify-path did not detect .links file corruption" >&2
exit 1
fi
nix-store --repair-path $path2 --substituters "file://$cacheDir" --no-require-sigs --option auto-optimise-store true
if [ "$(nix-hash $path2)" != "$hash" -o "BAR" != "$(< $path2/bar)" ]; then
echo "path not repaired properly" >&2
exit 1
fi