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

Shuffle ParseSink code in preparation for git hashing support #9024

Merged
merged 4 commits into from
Sep 25, 2023
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
67 changes: 0 additions & 67 deletions src/libutil/archive.cc
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,6 @@ struct ArchiveSettings : Config
#endif
"use-case-hack",
"Whether to enable a Darwin-specific hack for dealing with file name collisions."};
Setting<bool> preallocateContents{this, false, "preallocate-contents",
"Whether to preallocate files when writing objects with known size."};
};

static ArchiveSettings archiveSettings;
Expand Down Expand Up @@ -302,71 +300,6 @@ void parseDump(ParseSink & sink, Source & source)
}


struct RestoreSink : ParseSink
{
Path dstPath;
AutoCloseFD fd;

void createDirectory(const Path & path) override
{
Path p = dstPath + path;
if (mkdir(p.c_str(), 0777) == -1)
throw SysError("creating directory '%1%'", p);
};

void createRegularFile(const Path & path) override
{
Path p = dstPath + path;
fd = open(p.c_str(), O_CREAT | O_EXCL | O_WRONLY | O_CLOEXEC, 0666);
if (!fd) throw SysError("creating file '%1%'", p);
}

void closeRegularFile() override
{
/* Call close explicitly to make sure the error is checked */
fd.close();
}

void isExecutable() override
{
struct stat st;
if (fstat(fd.get(), &st) == -1)
throw SysError("fstat");
if (fchmod(fd.get(), st.st_mode | (S_IXUSR | S_IXGRP | S_IXOTH)) == -1)
throw SysError("fchmod");
}

void preallocateContents(uint64_t len) override
{
if (!archiveSettings.preallocateContents)
return;

#if HAVE_POSIX_FALLOCATE
if (len) {
errno = posix_fallocate(fd.get(), 0, len);
/* Note that EINVAL may indicate that the underlying
filesystem doesn't support preallocation (e.g. on
OpenSolaris). Since preallocation is just an
optimisation, ignore it. */
if (errno && errno != EINVAL && errno != EOPNOTSUPP && errno != ENOSYS)
throw SysError("preallocating file of %1% bytes", len);
}
#endif
}

void receiveContents(std::string_view data) override
{
writeFull(fd.get(), data);
}

void createSymlink(const Path & path, const std::string & target) override
{
Path p = dstPath + path;
nix::createSymlink(target, p);
}
};


void restorePath(const Path & path, Source & source)
{
RestoreSink sink;
Expand Down
17 changes: 1 addition & 16 deletions src/libutil/archive.hh
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

#include "types.hh"
#include "serialise.hh"
#include "fs-sink.hh"


namespace nix {
Expand Down Expand Up @@ -72,22 +73,6 @@ time_t dumpPathAndGetMtime(const Path & path, Sink & sink,
*/
void dumpString(std::string_view s, Sink & sink);

/**
* \todo Fix this API, it sucks.
*/
struct ParseSink
{
virtual void createDirectory(const Path & path) { };

virtual void createRegularFile(const Path & path) { };
virtual void closeRegularFile() { };
virtual void isExecutable() { };
virtual void preallocateContents(uint64_t size) { };
virtual void receiveContents(std::string_view data) { };

virtual void createSymlink(const Path & path, const std::string & target) { };
};

/**
* If the NAR archive contains a single file at top-level, then save
* the contents of the file to `s`. Otherwise barf.
Expand Down
77 changes: 77 additions & 0 deletions src/libutil/fs-sink.cc
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps this should be filesystem.cc, together with other fs utils (to be moved later), considering per-compilation-unit overhead as mentioned in the meeting today.

Copy link
Member Author

@Ericson2314 Ericson2314 Sep 22, 2023

Choose a reason for hiding this comment

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

Actually I forgot before during the meeting, I really disagree with combining files because it is worse for incremental builds. Latency is most important during development when we are doing incremental builds.

Optimizing non-incremental building is IMO quite a smell. (And we can still have the automatic way of combining files based on number of CPUs if we like.)

Copy link
Member

Choose a reason for hiding this comment

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

Is that something Meson can do?

optimized for extremely fast full and incremental builds without sacrificing correctness

Couldn't find details about how that is achieved. Maybe you know the right search term?

Copy link
Member Author

Choose a reason for hiding this comment

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

Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
#include <fcntl.h>

#include "config.hh"
#include "fs-sink.hh"

namespace nix {

struct RestoreSinkSettings : Config
{
Setting<bool> preallocateContents{this, false, "preallocate-contents",
"Whether to preallocate files when writing objects with known size."};
};

static RestoreSinkSettings restoreSinkSettings;

static GlobalConfig::Register r1(&restoreSinkSettings);


void RestoreSink::createDirectory(const Path & path)
{
Path p = dstPath + path;
if (mkdir(p.c_str(), 0777) == -1)
throw SysError("creating directory '%1%'", p);
};

void RestoreSink::createRegularFile(const Path & path)
{
Path p = dstPath + path;
fd = open(p.c_str(), O_CREAT | O_EXCL | O_WRONLY | O_CLOEXEC, 0666);
if (!fd) throw SysError("creating file '%1%'", p);
}

void RestoreSink::closeRegularFile()
{
/* Call close explicitly to make sure the error is checked */
fd.close();
}

void RestoreSink::isExecutable()
{
struct stat st;
if (fstat(fd.get(), &st) == -1)
throw SysError("fstat");
if (fchmod(fd.get(), st.st_mode | (S_IXUSR | S_IXGRP | S_IXOTH)) == -1)
throw SysError("fchmod");
}

void RestoreSink::preallocateContents(uint64_t len)
{
if (!restoreSinkSettings.preallocateContents)
return;

#if HAVE_POSIX_FALLOCATE
if (len) {
errno = posix_fallocate(fd.get(), 0, len);
/* Note that EINVAL may indicate that the underlying
filesystem doesn't support preallocation (e.g. on
OpenSolaris). Since preallocation is just an
optimisation, ignore it. */
if (errno && errno != EINVAL && errno != EOPNOTSUPP && errno != ENOSYS)
throw SysError("preallocating file of %1% bytes", len);
}
#endif
}

void RestoreSink::receiveContents(std::string_view data)
{
writeFull(fd.get(), data);
}

void RestoreSink::createSymlink(const Path & path, const std::string & target)
{
Path p = dstPath + path;
nix::createSymlink(target, p);
}

}
42 changes: 42 additions & 0 deletions src/libutil/fs-sink.hh
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
#pragma once
///@file

#include "types.hh"
#include "serialise.hh"

namespace nix {

/**
* \todo Fix this API, it sucks.
*/
struct ParseSink
{
virtual void createDirectory(const Path & path) { };

virtual void createRegularFile(const Path & path) { };
virtual void closeRegularFile() { };
virtual void isExecutable() { };
virtual void preallocateContents(uint64_t size) { };
virtual void receiveContents(std::string_view data) { };

virtual void createSymlink(const Path & path, const std::string & target) { };
};

struct RestoreSink : ParseSink
{
Path dstPath;
AutoCloseFD fd;


void createDirectory(const Path & path) override;

void createRegularFile(const Path & path) override;
void closeRegularFile() override;
void isExecutable() override;
void preallocateContents(uint64_t size) override;
void receiveContents(std::string_view data) override;

void createSymlink(const Path & path, const std::string & target) override;
};

}
10 changes: 7 additions & 3 deletions src/libutil/hash.hh
Original file line number Diff line number Diff line change
Expand Up @@ -145,13 +145,17 @@ std::string printHash16or32(const Hash & hash);
Hash hashString(HashType ht, std::string_view s);

/**
* Compute the hash of the given file.
* Compute the hash of the given file, hashing its contents directly.
*
* (Metadata, such as the executable permission bit, is ignored.)
*/
Hash hashFile(HashType ht, const Path & path);

/**
* Compute the hash of the given path. The hash is defined as
* (essentially) hashString(ht, dumpPath(path)).
* Compute the hash of the given path, serializing as a Nix Archive and
* then hashing that.
*
* The hash is defined as (essentially) hashString(ht, dumpPath(path)).
*/
typedef std::pair<Hash, uint64_t> HashResult;
HashResult hashPath(HashType ht, const Path & path,
Expand Down
Loading