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

Reserve the __contentAddressed derivation parameter #3710

Merged
merged 2 commits into from Jun 17, 2020

Conversation

thufschmitt
Copy link
Member

Not implementing anything here, just throwing an error if a derivation sets __contentAddressed = true without --experimental-features content-addressed-paths (and also with it as there's nothing implemented yet)

Not implementing anything here, just throwing an error if a derivation
sets `__contentAddressed = true` without
`--experimental-features content-addressed-paths`
(and also with it as there's nothing implemented yet)
@@ -1195,6 +1198,14 @@ void DerivationGoal::haveDerivation()

parsedDrv = std::make_unique<ParsedDerivation>(drvPath, *drv);

contentAddressed = parsedDrv->contentAddressed();

if (this->contentAddressed) {
Copy link
Member

Choose a reason for hiding this comment

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

Don't need this-> here.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, fixed

@@ -809,6 +809,9 @@ class DerivationGoal : public Goal
/* Whether this is a fixed-output derivation. */
bool fixedOutput;

/* Whether this is a content adressed derivation */
bool contentAddressed = false;
Copy link
Member

Choose a reason for hiding this comment

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

Maybe not add this and use a temporary below in hopes that #3418 is merged soon?

Copy link
Member Author

Choose a reason for hiding this comment

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

Mh right. I've removed it in favor of directly calling parsedDrv->contentAddressed()

@edolstra edolstra merged commit 2f51cd8 into NixOS:master Jun 17, 2020
@edolstra
Copy link
Member

BTW shouldn't content-addressed-paths be content-addressed-derivations (or ca-derivations for consistency)? Since content-addressed paths are already partially supported, and there is a ca-references feature for full support.

@edolstra edolstra deleted the reserve_ca_derivations branch June 17, 2020 16:29
@thufschmitt
Copy link
Member Author

BTW shouldn't content-addressed-paths be content-addressed-derivations (or ca-derivations for consistency)? Since content-addressed paths are already partially supported, and there is a ca-references feature for full support.

You're definitely right. I didn't know about ca-references. I'll align with it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants