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
Overhaul wopAddToStore #4030
Overhaul wopAddToStore #4030
Conversation
0c9db88
to
6ecfebd
Compare
8670128
to
d9c3d49
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@edolstra This is ready for review
@@ -240,6 +239,18 @@ struct ClientSettings | |||
} | |||
}; | |||
|
|||
static void writeValidPathInfo(ref<Store> store, unsigned int clientVersion, Sink & to, std::shared_ptr<const ValidPathInfo> info) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved, unchanged.
to << store->printStorePath(pathInfo->path); | ||
writeValidPathInfo(store, clientVersion, to, pathInfo); | ||
|
||
} else { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
else
branch is the unchanged previous protocol implementation.
@@ -419,11 +421,27 @@ void RemoteStore::querySubstitutablePathInfos(const StorePathCAMap & pathsMap, S | |||
} | |||
|
|||
|
|||
ref<const ValidPathInfo> RemoteStore::readValidPathInfo(ConnectionHandle & conn, const StorePath & path) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved, unchanged except improved to use ref
over shared_ptr
@@ -992,6 +961,49 @@ std::exception_ptr RemoteStore::Connection::processStderr(Sink * sink, Source * | |||
return nullptr; | |||
} | |||
|
|||
void | |||
ConnectionHandle::withFramedSink(std::function<void(Sink &sink)> fun) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved, unchanged.
StorePath addToStore(const string & name, const Path & srcPath, | ||
FileIngestionMethod method = FileIngestionMethod::Recursive, HashType hashAlgo = htSHA256, | ||
PathFilter & filter = defaultPathFilter, RepairFlag repair = NoRepair) override; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Default implementation is used.
Use with FramedSink, which also allows the logical stream to be terminated | ||
in the event of an exception. | ||
*/ | ||
struct FramedSource : Source |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated comment, implemention moved and unchanged.
The exception_ptr reference can be used to terminate the stream when you | ||
detect that an error has occurred on the remote end. | ||
*/ | ||
struct FramedSink : nix::BufferedSink |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
New comment, interface changed to take a BufferedSink
instead of the whole Connection
. Other than that, implementation unchanged.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Baring those 2 little things, I cannot recommend this enough!
CC @edolstra
Co-authored-by: John Ericson <git@JohnEricson.me>
Co-authored-by: John Ericson <git@JohnEricson.me>
A ValidPathInfo is created anyway. By returning it we can save a roundtrip and we have a nicer interface.
Also checked that all usages satisfy the requirement and removed dead code.
52f21a8
to
ca30abb
Compare
Thanks! |
After merging this, commands like
I think |
Fixed in 980edd1. |
RemoteStore::addToStoreFromDump
. Closes Implement RemoteStore.addToStoreFromDump #4009addToStore
in daemon. Closes WIP: Constant space daemonwopAddToStore
-- contains #3801 #3802ContentAddressMethod
which represents the labels that precede CA hashes but not the hashwopAddTextToStore
intowopAddToStore
. Not super necessary but was easy with the above in place and fits well with the newContentAddress
* stuffFramedSource
andFramedSink
are now reusable