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
Remove storetype delegate reg store -- contains #3736 #3829
Remove storetype delegate reg store -- contains #3736 #3829
Conversation
to each Store implementation. The generic regStore implementation will only be for the ambiguous shorthands, like "" and "auto". This also could get us close to simplifying the daemon command.
src/nix-daemon/nix-daemon.cc
Outdated
@@ -277,7 +278,9 @@ static int _main(int argc, char * * argv) | |||
initPlugins(); | |||
|
|||
if (stdio) { | |||
if (getStoreType() == tDaemon) { | |||
if (openUncachedStore().dynamic_pointer_cast<UDSRemoteStore>()) { |
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.
Doesn't this cause the store to be opened twice in the non-UDSRemoteStore case? Maybe it's better to do store = openUncachedStore(); if (store.dynamic_pointer_cast ...) ...
.
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 Ah I remember now: I wanted to do that, but I wasn't sure how to destroy the opened store in the UDSRemoteStore
before reopening it, since nix::ref
is always non-null.
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.
The connections are opened on demand, so this was never a problem, but the new way also uses the store that was opened/downcast so there is no duplicate work.
…type-delegate-regStore
Removes duplicate websocket opening code, and also means we should be able to to ssh-ssh-... daemon relays, not just uds-uds-... ones.
It seems like the diff still contains changes from #3736 (merged). Rebasing on master will make the review easier. |
…delegate-regStore
Thanks for catching @roberth! I can't edit the commit message but I did fix the diff. |
…delegate-regStore
@edolstra OK fixed up post-merge so this is ready again. |
Thanks!! |
to each Store implementation. The generic regStore implementation will
only be for the ambiguous shorthands, like "" and "auto".
This also could get us close to simplifying the daemon command.