Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inline the usage of nix::readDirectory and remove it #10684

Merged
merged 3 commits into from
May 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/libcmd/repl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,7 @@ StringSet NixRepl::completePrefix(const std::string & prefix)
try {
auto dir = std::string(cur, 0, slash);
auto prefix2 = std::string(cur, slash + 1);
for (auto & entry : readDirectory(dir == "" ? "/" : dir)) {
for (auto & entry : std::filesystem::directory_iterator{dir == "" ? "/" : dir}) {
auto name = entry.path().filename().string();
if (name[0] != '.' && hasPrefix(name, prefix2))
completions.insert(prev + entry.path().string());
Expand Down
4 changes: 2 additions & 2 deletions src/libstore/builtins/buildenv.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,10 @@ struct State
/* For each activated package, create symlinks */
static void createLinks(State & state, const Path & srcDir, const Path & dstDir, int priority)
{
std::vector<std::filesystem::directory_entry> srcFiles;
std::filesystem::directory_iterator srcFiles;

try {
srcFiles = readDirectory(srcDir);
srcFiles = std::filesystem::directory_iterator{srcDir};
} catch (std::filesystem::filesystem_error & e) {
if (e.code() == std::errc::not_a_directory) {
warn("not including '%s' in the user environment because it's not a directory", srcDir);
Expand Down
4 changes: 2 additions & 2 deletions src/libstore/gc.cc
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ void LocalStore::findTempRoots(Roots & tempRoots, bool censor)
{
/* Read the `temproots' directory for per-process temporary root
files. */
for (auto & i : readDirectory(tempRootsDir)) {
for (auto & i : std::filesystem::directory_iterator{tempRootsDir}) {
auto name = i.path().filename().string();
if (name[0] == '.') {
// Ignore hidden files. Some package managers (notably portage) create
Expand Down Expand Up @@ -228,7 +228,7 @@ void LocalStore::findRoots(const Path & path, std::filesystem::file_type type, R
type = std::filesystem::symlink_status(path).type();

if (type == std::filesystem::file_type::directory) {
for (auto & i : readDirectory(path))
for (auto & i : std::filesystem::directory_iterator{path})
findRoots(i.path().string(), i.symlink_status().type(), roots);
}

Expand Down
2 changes: 1 addition & 1 deletion src/libstore/globals.cc
Original file line number Diff line number Diff line change
Expand Up @@ -345,7 +345,7 @@ void initPlugins()
for (const auto & pluginFile : settings.pluginFiles.get()) {
std::vector<std::filesystem::path> pluginFiles;
try {
auto ents = readDirectory(pluginFile);
auto ents = std::filesystem::directory_iterator{pluginFile};
for (const auto & ent : ents)
pluginFiles.emplace_back(ent.path());
} catch (std::filesystem::filesystem_error & e) {
Expand Down
2 changes: 1 addition & 1 deletion src/libstore/local-binary-cache-store.cc
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ class LocalBinaryCacheStore : public virtual LocalBinaryCacheStoreConfig, public
{
StorePathSet paths;

for (auto & entry : readDirectory(binaryCacheDir)) {
for (auto & entry : std::filesystem::directory_iterator{binaryCacheDir}) {
auto name = entry.path().filename().string();
if (name.size() != 40 ||
!hasSuffix(name, ".narinfo"))
Expand Down
4 changes: 2 additions & 2 deletions src/libstore/local-store.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1406,7 +1406,7 @@ bool LocalStore::verifyStore(bool checkContents, RepairFlag repair)

printInfo("checking link hashes...");

for (auto & link : readDirectory(linksDir)) {
for (auto & link : std::filesystem::directory_iterator{linksDir}) {
auto name = link.path().filename();
printMsg(lvlTalkative, "checking contents of '%s'", name);
PosixSourceAccessor accessor;
Expand Down Expand Up @@ -1498,7 +1498,7 @@ LocalStore::VerificationResult LocalStore::verifyAllValidPaths(RepairFlag repair
database and the filesystem) in the loop below, in order to catch
invalid states.
*/
for (auto & i : readDirectory(realStoreDir)) {
for (auto & i : std::filesystem::directory_iterator{realStoreDir.to_string()}) {
try {
storePathsInStoreDir.insert({i.path().filename().string()});
} catch (BadStorePath &) { }
Expand Down
3 changes: 1 addition & 2 deletions src/libstore/posix-fs-canonicalise.cc
Original file line number Diff line number Diff line change
Expand Up @@ -144,8 +144,7 @@ static void canonicalisePathMetaData_(
#endif

if (S_ISDIR(st.st_mode)) {
std::vector<std::filesystem::directory_entry> entries = readDirectory(path);
for (auto & i : entries)
for (auto & i : std::filesystem::directory_iterator{path})
canonicalisePathMetaData_(
i.path().string(),
#ifndef _WIN32
Expand Down
2 changes: 1 addition & 1 deletion src/libstore/profiles.cc
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ std::pair<Generations, std::optional<GenerationNumber>> findGenerations(Path pro
std::filesystem::path profileDir = dirOf(profile);
auto profileName = std::string(baseNameOf(profile));

for (auto & i : readDirectory(profileDir.string())) {
for (auto & i : std::filesystem::directory_iterator{profileDir}) {
if (auto n = parseName(profileName, i.path().filename().string())) {
auto path = i.path().string();
gens.push_back({
Expand Down
9 changes: 6 additions & 3 deletions src/libstore/unix/builtins/unpack-channel.cc
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,13 @@ void builtinUnpackChannel(

unpackTarfile(src, out);

auto entries = readDirectory(out);
if (entries.size() != 1)
auto entries = std::filesystem::directory_iterator{out};
auto fileName = entries->path().string();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe this way around it actually works.

auto fileCount = std::distance(std::filesystem::begin(entries), std::filesystem::end(entries));

if (fileCount != 1)
throw Error("channel tarball '%s' contains more than one file", src);
std::filesystem::rename(entries[0].path().string(), (out + "/" + channelName));
std::filesystem::rename(fileName, (out + "/" + channelName));
}

}
14 changes: 0 additions & 14 deletions src/libutil/file-system.cc
Original file line number Diff line number Diff line change
Expand Up @@ -222,20 +222,6 @@ Path readLink(const Path & path)
}


std::vector<fs::directory_entry> readDirectory(const Path & path)
{
std::vector<fs::directory_entry> entries;
entries.reserve(64);

for (auto & entry : fs::directory_iterator{path}) {
checkInterrupt();
entries.push_back(std::move(entry));
}

return entries;
}


std::string readFile(const Path & path)
{
AutoCloseFD fd = toDescriptor(open(path.c_str(), O_RDONLY
Expand Down
6 changes: 0 additions & 6 deletions src/libutil/file-system.hh
Original file line number Diff line number Diff line change
Expand Up @@ -122,12 +122,6 @@ Path readLink(const Path & path);
*/
Descriptor openDirectory(const std::filesystem::path & path);

/**
* Read the contents of a directory. The entries `.` and `..` are
* removed.
*/
std::vector<std::filesystem::directory_entry> readDirectory(const Path & path);

/**
* Read the contents of a file into a string.
*/
Expand Down
2 changes: 1 addition & 1 deletion src/libutil/linux/cgroup.cc
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ static CgroupStats destroyCgroup(const std::filesystem::path & cgroup, bool retu

/* Otherwise, manually kill every process in the subcgroups and
this cgroup. */
for (auto & entry : readDirectory(cgroup)) {
for (auto & entry : std::filesystem::directory_iterator{cgroup}) {
if (entry.symlink_status().type() != std::filesystem::file_type::directory) continue;
destroyCgroup(cgroup / entry.path().filename(), false);
}
Expand Down
2 changes: 1 addition & 1 deletion src/libutil/posix-source-accessor.cc
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ SourceAccessor::DirEntries PosixSourceAccessor::readDirectory(const CanonPath &
{
assertNoSymlinks(path);
DirEntries res;
for (auto & entry : nix::readDirectory(makeAbsPath(path).string())) {
for (auto & entry : std::filesystem::directory_iterator{makeAbsPath(path)}) {
auto type = [&]() -> std::optional<Type> {
std::filesystem::file_type nativeType;
try {
Expand Down
2 changes: 1 addition & 1 deletion src/libutil/unix/file-descriptor.cc
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ void closeMostFDs(const std::set<int> & exceptions)
{
#if __linux__
try {
for (auto & s : readDirectory("/proc/self/fd")) {
for (auto & s : std::filesystem::directory_iterator{"/proc/self/fd"}) {
auto fd = std::stoi(s.path().filename());
if (!exceptions.count(fd)) {
debug("closing leaked FD %d", fd);
Expand Down
2 changes: 1 addition & 1 deletion src/nix-collect-garbage/nix-collect-garbage.cc
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ void removeOldGenerations(std::string dir)

bool canWrite = access(dir.c_str(), W_OK) == 0;

for (auto & i : readDirectory(dir)) {
for (auto & i : std::filesystem::directory_iterator{dir}) {
checkInterrupt();

auto path = i.path().string();
Expand Down
2 changes: 1 addition & 1 deletion src/nix/flake.cc
Original file line number Diff line number Diff line change
Expand Up @@ -866,7 +866,7 @@ struct CmdFlakeInitCommon : virtual Args, EvalCommand
{
createDirs(to);

for (auto & entry : readDirectory(from)) {
for (auto & entry : std::filesystem::directory_iterator{from}) {
auto from2 = entry.path().string();
auto to2 = to + "/" + entry.path().filename().string();
auto st = lstat(from2);
Expand Down
7 changes: 4 additions & 3 deletions src/nix/prefetch.cc
Original file line number Diff line number Diff line change
Expand Up @@ -115,9 +115,10 @@ std::tuple<StorePath, Hash> prefetchFile(

/* If the archive unpacks to a single file/directory, then use
that as the top-level. */
auto entries = readDirectory(unpacked);
if (entries.size() == 1)
tmpFile = entries[0].path();
auto entries = std::filesystem::directory_iterator{unpacked};
auto file_count = std::distance(entries, std::filesystem::directory_iterator{});
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This broke the prefetch command if unpack is used. Fixed in #11051

if (file_count == 1)
tmpFile = entries->path();
else
tmpFile = unpacked;
}
Expand Down
2 changes: 1 addition & 1 deletion src/nix/run.cc
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@ void chrootHelper(int argc, char * * argv)
if (mount(realStoreDir.c_str(), (tmpDir + storeDir).c_str(), "", MS_BIND, 0) == -1)
throw SysError("mounting '%s' on '%s'", realStoreDir, storeDir);

for (auto entry : readDirectory("/")) {
for (auto entry : std::filesystem::directory_iterator{"/"}) {
auto src = entry.path().string();
Path dst = tmpDir + "/" + entry.path().filename().string();
if (pathExists(dst)) continue;
Expand Down
Loading