Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Low level
<drvPath>^<outputName>
installable syntax to match existing<highLevelInstallable>^<outputNames>
syntax #4543Low level
<drvPath>^<outputName>
installable syntax to match existing<highLevelInstallable>^<outputNames>
syntax #4543Changes from 26 commits
8499f32
1ef88da
e5c42bb
0966532
9c6be01
6951b26
5c1f2e0
fda2224
41e755b
6b61d77
b18720e
49ad315
b585548
6cafe30
f3262bc
8735f55
279ecf7
0e4ec98
12461e2
13f2a6f
26534f1
1879c7c
dc075dc
c7cce3e
d8c1c24
c886b18
dabb03b
32ae715
5273cf4
f61d575
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Note it previously read the derivation and returned all its output names explicitly here, but that didn't work e.g. when the drv file itself needs to be downloaded and so we cannot yet read it.
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.
and then use
parseOutputsSpec()
to parse the^
and the outputs. And thenSourceExprCommand::parseInstallables()
doesn't need to split on^
.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.
See #6815 which I made so this deduplication can happen. Note that part of the complication is
!
is still used in the remote store protocol forDerivedPath
, so some trickiness is needed so^
and!
are used for different use-cases, but we can do better de-duplicating than we do currently.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 can we return to this after this PR? #6815 to deduplicate things has prooved to be much bigger than this. I would really appreciate being able to make this change and then do the clean up afterwards.
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.
A simpler patch is not available because we still need to parse with with both
!
and^
, because the former is still used internally e.g. in the protocols.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.
#6815 now works! But I'll still keep it separately because that is much bigger than this.