Add a way to get all the outputs of a derivation with their label #3678
Conversation
src/libstore/store-api.cc
Outdated
| return outputPaths; | ||
| } | ||
|
|
||
| StringSet Store::queryDerivationOutputNames(const StorePath & path) |
There was a problem hiding this comment.
Maybe add a comment saying these are obsolete / deprecated ?
src/libstore/path.hh
Outdated
| @@ -61,6 +61,7 @@ struct StorePath : rust::Value<3 * sizeof(void *) + 24, ffi_StorePath_drop> | |||
|
|
|||
| typedef std::set<StorePath> StorePathSet; | |||
| typedef std::vector<StorePath> StorePaths; | |||
| typedef std::map<string, StorePath> StorePathMap; | |||
There was a problem hiding this comment.
I might call this OutputPathMap instead, as StorePathMap sounds to me like std::map<StorePath, ???>.
There was a problem hiding this comment.
Indeed, that makes sense. Changed in 53edff2
src/libstore/daemon.cc
Outdated
| @@ -308,8 +308,7 @@ static void performOp(TunnelLogger * logger, ref<Store> store, | |||
|
|
|||
| case wopQueryReferences: | |||
| case wopQueryReferrers: | |||
| case wopQueryValidDerivers: | |||
| case wopQueryDerivationOutputs: { | |||
There was a problem hiding this comment.
Do you know what the policy is on supporting on protocol versions? I don't!
There was a problem hiding this comment.
The daemon should support older clients. So we can't remove APIs like this.
There was a problem hiding this comment.
Woops sorry, I'll restore them
|
Do we actually need these functions? The outputs of a derivation are also available by doing |
|
As a cleanup I've removed |
|
I agree having 3 of them is unnecessary, but this new on seams good to me? In #3523 (comment) you talk about not wanting to rely on derivations existing on disk as much. Avoiding |
Atm yes, but that won't be true anymore in a CA setting where the output paths don't match what's in the drv (which is the reason why I needed this in the first place) |
thufschmitt
force-pushed
the
remote-query-outputs
branch
from
4913e9e
to
53edff2
Compare
thufschmitt
force-pushed
the
remote-query-outputs
branch
from
53edff2
to
e23910e
Compare
|
I've rebased it on top of master. No idea what the OSX failure is. It seems totally unrelated |
|
Yes, we currently fetch the latest |
Generalize `queryDerivationOutputNames` and `queryDerivationOutputs` by adding a `queryDerivationOutputMap` that returns the map `outputName=>outputPath` (not that this is not equivalent to merging the results of `queryDerivationOutputs` and `queryDerivationOutputNames` as sets don't preserve the order, so we would end up with an incorrect mapping). squash! Add a way to get all the outputs of a derivation with their label Rename StorePathMap to OutputPathMap
thufschmitt
force-pushed
the
remote-query-outputs
branch
from
e23910e
to
d38f860
Compare
|
Thanks, merged! |
Generalize
queryDerivationOutputNamesandqueryDerivationOutputsby adding aqueryDerivationOutputMapthat returns the mapoutputName=>outputPath.This is needed by #3528 as it needs a way to get the path of a specific derivation output, which (afaik) isn't possible with the current API
(not that this is not equivalent to merging the results of
queryDerivationOutputsandqueryDerivationOutputNamesas sets don't preserve the order, so we would end up with an incorrect mapping).