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

Make NixStringContextElem::DrvDeep opt-in, and maybe deprecate it #7910

Open
3 tasks
Ericson2314 opened this issue Feb 27, 2023 · 4 comments
Open
3 tasks

Make NixStringContextElem::DrvDeep opt-in, and maybe deprecate it #7910

Ericson2314 opened this issue Feb 27, 2023 · 4 comments

Comments

@Ericson2314
Copy link
Member

Ericson2314 commented Feb 27, 2023

Currently, drvPath attributes have a very weird DrvDeep nix string context elem. This string context element makes refer to

all the outputs of all the derivations in the the closure of the path.

The "all the outputs part" is really weird! builtins.unsafeDiscardOutputDependency turns this back to a regular string context, but it is unfortunate to have to use an "unsafe" function to get back to the more normal behavior.

Instead, it would be nice if drvPath had a regular Opaque string context, and instead there was a primop to do the opposite: turn a string with a single such opaque string context elem to one with a single drv-deep string context elem.

See also #7330

TODO
  • Add a builtins.addDrvOutputDependencies primop
  • Wait for Nix on NixOS stable to support it
  • Add builtins.unsafeDiscardOutputDependency to Nixpkgs mkDerivation's drvPath.
@Ericson2314 Ericson2314 changed the title Make DrvDeep opt-in, and maybe deprecate it Make NixStringContextElem::DrvDeep opt-in, and maybe deprecate it Feb 27, 2023
@roberth
Copy link
Member

roberth commented May 11, 2023

We can achieve this goal for 99% without breaking the semantics of anyone's pinned expressions.

  • Add a builtins.addDrvOutputDependencies primop
  • Wait for Nixpkgs to increment its required Nix version to one that supports the new primop. (Or maybe not even)
  • Add builtins.unsafeDiscardOutputDependency to Nixpkgs mkDerivation's drvPath.

If we were not to wait, we might cause trouble for users who do need the old drvPath semantics. They'd have to upgrade their Nix sooner than the Nixpkgs min version would require them to. This might be rare enough that we could risk it.

Anyway, the point is that we can leverage Nixpkgs to deliver the improved behavior and cover basically everyone's use of derivations that way.

@Ericson2314
Copy link
Member Author

That sounds like a good plan to me!

@roberth
Copy link
Member

roberth commented May 12, 2023

Listed in the issue now.
I've changed the wait to "Wait for Nix on NixOS stable to support it". Seems like a more reasonable amount of time than minver. I feel like that one will take ages because of 2.4, whatever one may think of that. Maybe I'm wrong, but point is we should wait a reasonable amount of time, and this new wait condition sounds about right to me.

@Ericson2314
Copy link
Member Author

I have a slight variation: we could have an opt-in like __vanillaDrvPath = true so Nix will set drvPath in the right way. This is a very small variation from your idea, but seems like it might make it more "endorsed by Nix" fwiw.

(Came up with that when responding to #7087, got this and that confused. Tangent idea over there is relevant to the above.)

Ericson2314 added a commit to obsidiansystems/nix that referenced this issue Jun 27, 2023
The new primop is fairly simple. What is far more weird is the test
plan.

The test plan is taken by Robert's comment in
NixOS#7910 (comment)
describing how we could migrate *Nixpkgs* without a breaking change in
Nix. The Nix testsuite has its own `mkDerivation`, and we so we do the
same thing with it:

 - `drvPath` is now overridden to not have the funky `DrvDeep` string
   context anymore.

 - Tests that previously needed to
   `builtins.unsafeDiscardOutputDependency` a `drvPath` no don't.

 - Tests that previous did *not* need to
   `builtins.unsafeDiscardOutputDependency` a `drvPath` now *do*.

Also, there is a regular language test testing the `derivation` primop
in isolation (again, no breaking change to it!) which has been extended.
Ericson2314 added a commit to obsidiansystems/nix that referenced this issue Jun 27, 2023
The new primop is fairly simple. What is far more weird is the test
plan.

The test plan is taken by Robert's comment in
NixOS#7910 (comment)
describing how we could migrate *Nixpkgs* without a breaking change in
Nix. The Nix testsuite has its own `mkDerivation`, and we so we do the
same thing with it:

 - `drvPath` is now overridden to not have the funky `DrvDeep` string
   context anymore.

 - Tests that previously needed to
   `builtins.unsafeDiscardOutputDependency` a `drvPath` no don't.

 - Tests that previous did *not* need to
   `builtins.unsafeDiscardOutputDependency` a `drvPath` now *do*.

Also, there is a regular language test testing the `derivation` primop
in isolation (again, no breaking change to it!) which has been extended.
Ericson2314 added a commit to obsidiansystems/nix that referenced this issue Jun 27, 2023
The new primop is fairly simple. What is far more weird is the test
plan.

The test plan is taken by Robert's comment in
NixOS#7910 (comment)
describing how we could migrate *Nixpkgs* without a breaking change in
Nix. The Nix testsuite has its own `mkDerivation`, and we so we do the
same thing with it:

 - `drvPath` is now overridden to not have the funky `DrvDeep` string
   context anymore.

 - Tests that previously needed to
   `builtins.unsafeDiscardOutputDependency` a `drvPath` no don't.

 - Tests that previous did *not* need to
   `builtins.unsafeDiscardOutputDependency` a `drvPath` now *do*.

Also, there is a regular language test testing the `derivation` primop
in isolation (again, no breaking change to it!) which has been extended.
Ericson2314 added a commit to obsidiansystems/nix that referenced this issue Jul 17, 2023
The new primop is fairly simple. What is far more weird is the test
plan.

The test plan is taken by Robert's comment in
NixOS#7910 (comment)
describing how we could migrate *Nixpkgs* without a breaking change in
Nix. The Nix testsuite has its own `mkDerivation`, and we so we do the
same thing with it:

 - `drvPath` is now overridden to not have the funky `DrvDeep` string
   context anymore.

 - Tests that previously needed to
   `builtins.unsafeDiscardOutputDependency` a `drvPath` no don't.

 - Tests that previous did *not* need to
   `builtins.unsafeDiscardOutputDependency` a `drvPath` now *do*.

Also, there is a regular language test testing the `derivation` primop
in isolation (again, no breaking change to it!) which has been extended.

Finally, we will need (TODO ACTUALLY MAKE!) unit tests catching failures
to demonstrate when `builtins.addDrvOutputDependencies` intentionally
does *not* work.

Co-authored-by: Robert Hensing <roberth@users.noreply.github.com>
Ericson2314 added a commit to obsidiansystems/nix that referenced this issue Oct 13, 2023
The new primop is fairly simple. What is far more weird is the test
plan.

The test plan is taken by Robert's comment in
NixOS#7910 (comment)
describing how we could migrate *Nixpkgs* without a breaking change in
Nix. The Nix testsuite has its own `mkDerivation`, and we so we do the
same thing with it:

 - `drvPath` is now overridden to not have the funky `DrvDeep` string
   context anymore.

 - Tests that previously needed to
   `builtins.unsafeDiscardOutputDependency` a `drvPath` no don't.

 - Tests that previous did *not* need to
   `builtins.unsafeDiscardOutputDependency` a `drvPath` now *do*.

Also, there is a regular language test testing the `derivation` primop
in isolation (again, no breaking change to it!) which has been extended.

Co-authored-by: Robert Hensing <roberth@users.noreply.github.com>
Ericson2314 added a commit to obsidiansystems/nix that referenced this issue Oct 16, 2023
The new primop is fairly simple. What is far more weird is the test
plan.

The test plan is taken by Robert's comment in
NixOS#7910 (comment)
describing how we could migrate *Nixpkgs* without a breaking change in
Nix. The Nix testsuite has its own `mkDerivation`, and we so we do the
same thing with it:

 - `drvPath` is now overridden to not have the funky `DrvDeep` string
   context anymore.

 - Tests that previously needed to
   `builtins.unsafeDiscardOutputDependency` a `drvPath` no don't.

 - Tests that previous did *not* need to
   `builtins.unsafeDiscardOutputDependency` a `drvPath` now *do*.

Also, there is a regular language test testing the `derivation` primop
in isolation (again, no breaking change to it!) which has been extended.

Co-authored-by: Robert Hensing <roberth@users.noreply.github.com>
Co-authored-by: Théophane Hufschmitt <7226587+thufschmitt@users.noreply.github.com>
Ericson2314 added a commit to obsidiansystems/nix that referenced this issue Oct 16, 2023
The new primop is fairly simple. What is far more weird is the test
plan.

The test plan is taken by Robert's comment in
NixOS#7910 (comment)
describing how we could migrate *Nixpkgs* without a breaking change in
Nix. The Nix testsuite has its own `mkDerivation`, and we so we do the
same thing with it:

 - `drvPath` is now overridden to not have the funky `DrvDeep` string
   context anymore.

 - Tests that previously needed to
   `builtins.unsafeDiscardOutputDependency` a `drvPath` no don't.

 - Tests that previous did *not* need to
   `builtins.unsafeDiscardOutputDependency` a `drvPath` now *do*.

Also, there is a regular language test testing the `derivation` primop
in isolation (again, no breaking change to it!) which has been extended.

Co-authored-by: Robert Hensing <roberth@users.noreply.github.com>
Co-authored-by: Théophane Hufschmitt <7226587+thufschmitt@users.noreply.github.com>
Ericson2314 added a commit to obsidiansystems/nix that referenced this issue Oct 16, 2023
The new primop is fairly simple. What is far more weird is the test
plan.

The test plan is taken by Robert's comment in
NixOS#7910 (comment)
describing how we could migrate *Nixpkgs* without a breaking change in
Nix. The Nix testsuite has its own `mkDerivation`, and we so we do the
same thing with it:

 - `drvPath` is now overridden to not have the funky `DrvDeep` string
   context anymore.

 - Tests that previously needed to
   `builtins.unsafeDiscardOutputDependency` a `drvPath` no don't.

 - Tests that previous did *not* need to
   `builtins.unsafeDiscardOutputDependency` a `drvPath` now *do*.

Also, there is a regular language test testing the `derivation` primop
in isolation (again, no breaking change to it!) which has been extended.

Co-authored-by: Robert Hensing <roberth@users.noreply.github.com>
Co-authored-by: Théophane Hufschmitt <7226587+thufschmitt@users.noreply.github.com>
Ericson2314 added a commit to obsidiansystems/nix that referenced this issue Oct 16, 2023
The new primop is fairly simple. What is far more weird is the test
plan.

The test plan is taken by Robert's comment in
NixOS#7910 (comment)
describing how we could migrate *Nixpkgs* without a breaking change in
Nix. The Nix testsuite has its own `mkDerivation`, and we so we do the
same thing with it:

 - `drvPath` is now overridden to not have the funky `DrvDeep` string
   context anymore.

 - Tests that previously needed to
   `builtins.unsafeDiscardOutputDependency` a `drvPath` no don't.

 - Tests that previous did *not* need to
   `builtins.unsafeDiscardOutputDependency` a `drvPath` now *do*.

Also, there is a regular language test testing the `derivation` primop
in isolation (again, no breaking change to it!) which has been extended.

Co-authored-by: Robert Hensing <roberth@users.noreply.github.com>
Co-authored-by: Théophane Hufschmitt <7226587+thufschmitt@users.noreply.github.com>
Ericson2314 added a commit to obsidiansystems/nix that referenced this issue Oct 17, 2023
The new primop is fairly simple. What is far more weird is the test
plan.

The test plan is taken by Robert's comment in
NixOS#7910 (comment)
describing how we could migrate *Nixpkgs* without a breaking change in
Nix. The Nix testsuite has its own `mkDerivation`, and we so we do the
same thing with it:

 - `drvPath` is now overridden to not have the funky `DrvDeep` string
   context anymore.

 - Tests that previously needed to
   `builtins.unsafeDiscardOutputDependency` a `drvPath` no don't.

 - Tests that previous did *not* need to
   `builtins.unsafeDiscardOutputDependency` a `drvPath` now *do*.

Also, there is a regular language test testing the `derivation` primop
in isolation (again, no breaking change to it!) which has been extended.

Co-authored-by: Robert Hensing <roberth@users.noreply.github.com>
Co-authored-by: Théophane Hufschmitt <7226587+thufschmitt@users.noreply.github.com>
Ericson2314 added a commit to obsidiansystems/nix that referenced this issue Oct 20, 2023
The new primop is fairly simple. What is far more weird is the test
plan.

The test plan is taken by Robert's comment in
NixOS#7910 (comment)
describing how we could migrate *Nixpkgs* without a breaking change in
Nix. The Nix testsuite has its own `mkDerivation`, and we so we do the
same thing with it:

 - `drvPath` is now overridden to not have the funky `DrvDeep` string
   context anymore.

 - Tests that previously needed to
   `builtins.unsafeDiscardOutputDependency` a `drvPath` no don't.

 - Tests that previous did *not* need to
   `builtins.unsafeDiscardOutputDependency` a `drvPath` now *do*.

Also, there is a regular language test testing the `derivation` primop
in isolation (again, no breaking change to it!) which has been extended.

Co-authored-by: Robert Hensing <roberth@users.noreply.github.com>
Co-authored-by: Théophane Hufschmitt <7226587+thufschmitt@users.noreply.github.com>
Ericson2314 added a commit to obsidiansystems/nix that referenced this issue Oct 20, 2023
The new primop is fairly simple. What is far more weird is the test
plan.

The test plan is taken by Robert's comment in
NixOS#7910 (comment)
describing how we could migrate *Nixpkgs* without a breaking change in
Nix. The Nix testsuite has its own `mkDerivation`, and we so we do the
same thing with it:

 - `drvPath` is now overridden to not have the funky `DrvDeep` string
   context anymore.

 - Tests that previously needed to
   `builtins.unsafeDiscardOutputDependency` a `drvPath` no don't.

 - Tests that previous did *not* need to
   `builtins.unsafeDiscardOutputDependency` a `drvPath` now *do*.

Also, there is a regular language test testing the `derivation` primop
in isolation (again, no breaking change to it!) which has been extended.

Co-authored-by: Robert Hensing <roberth@users.noreply.github.com>
Co-authored-by: Théophane Hufschmitt <7226587+thufschmitt@users.noreply.github.com>
Ericson2314 added a commit to obsidiansystems/nix that referenced this issue Oct 20, 2023
The new primop is fairly simple. What is far more weird is the test
plan.

The test plan is taken by Robert's comment in
NixOS#7910 (comment)
describing how we could migrate *Nixpkgs* without a breaking change in
Nix. The Nix testsuite has its own `mkDerivation`, and we so we do the
same thing with it:

 - `drvPath` is now overridden to not have the funky `DrvDeep` string
   context anymore.

 - Tests that previously needed to
   `builtins.unsafeDiscardOutputDependency` a `drvPath` no don't.

 - Tests that previous did *not* need to
   `builtins.unsafeDiscardOutputDependency` a `drvPath` now *do*.

Also, there is a regular language test testing the `derivation` primop
in isolation (again, no breaking change to it!) which has been extended.

Co-authored-by: Robert Hensing <roberth@users.noreply.github.com>
Co-authored-by: Théophane Hufschmitt <7226587+thufschmitt@users.noreply.github.com>
Ericson2314 added a commit to obsidiansystems/nix that referenced this issue Oct 23, 2023
The new primop is fairly simple. What is far more weird is the test
plan.

The test plan is taken by Robert's comment in
NixOS#7910 (comment)
describing how we could migrate *Nixpkgs* without a breaking change in
Nix. The Nix testsuite has its own `mkDerivation`, and we so we do the
same thing with it:

 - `drvPath` is now overridden to not have the funky `DrvDeep` string
   context anymore.

 - Tests that previously needed to
   `builtins.unsafeDiscardOutputDependency` a `drvPath` no don't.

 - Tests that previous did *not* need to
   `builtins.unsafeDiscardOutputDependency` a `drvPath` now *do*.

Also, there is a regular language test testing the `derivation` primop
in isolation (again, no breaking change to it!) which has been extended.

Co-authored-by: Robert Hensing <roberth@users.noreply.github.com>
Co-authored-by: Théophane Hufschmitt <7226587+thufschmitt@users.noreply.github.com>

addDrvOutputDependencies: Print outpath in error message
Ericson2314 added a commit to obsidiansystems/nix that referenced this issue Oct 23, 2023
The new primop is fairly simple. What is far more weird is the test
plan.

The test plan is taken by Robert's comment in
NixOS#7910 (comment)
describing how we could migrate *Nixpkgs* without a breaking change in
Nix. The Nix testsuite has its own `mkDerivation`, and we so we do the
same thing with it:

 - `drvPath` is now overridden to not have the funky `DrvDeep` string
   context anymore.

 - Tests that previously needed to
   `builtins.unsafeDiscardOutputDependency` a `drvPath` no don't.

 - Tests that previous did *not* need to
   `builtins.unsafeDiscardOutputDependency` a `drvPath` now *do*.

Also, there is a regular language test testing the `derivation` primop
in isolation (again, no breaking change to it!) which has been extended.

Co-authored-by: Robert Hensing <roberth@users.noreply.github.com>
Co-authored-by: Théophane Hufschmitt <7226587+thufschmitt@users.noreply.github.com>
Co-authored-by: Valentin Gagarin <valentin.gagarin@tweag.io>
Ericson2314 added a commit to obsidiansystems/nix that referenced this issue Oct 23, 2023
The new primop is fairly simple. What is far more weird is the test
plan.

The test plan is taken by Robert's comment in
NixOS#7910 (comment)
describing how we could migrate *Nixpkgs* without a breaking change in
Nix. The Nix testsuite has its own `mkDerivation`, and we so we do the
same thing with it:

 - `drvPath` is now overridden to not have the funky `DrvDeep` string
   context anymore.

 - Tests that previously needed to
   `builtins.unsafeDiscardOutputDependency` a `drvPath` no don't.

 - Tests that previous did *not* need to
   `builtins.unsafeDiscardOutputDependency` a `drvPath` now *do*.

Also, there is a regular language test testing the `derivation` primop
in isolation (again, no breaking change to it!) which has been extended.

Co-authored-by: Robert Hensing <roberth@users.noreply.github.com>
Co-authored-by: Théophane Hufschmitt <7226587+thufschmitt@users.noreply.github.com>
Co-authored-by: Valentin Gagarin <valentin.gagarin@tweag.io>
Ericson2314 added a commit to obsidiansystems/nix that referenced this issue Oct 23, 2023
The new primop is fairly simple. What is far more weird is the test
plan.

The test plan is taken by Robert's comment in
NixOS#7910 (comment)
describing how we could migrate *Nixpkgs* without a breaking change in
Nix. The Nix testsuite has its own `mkDerivation`, and we so we do the
same thing with it:

 - `drvPath` is now overridden to not have the funky `DrvDeep` string
   context anymore.

 - Tests that previously needed to
   `builtins.unsafeDiscardOutputDependency` a `drvPath` no don't.

 - Tests that previous did *not* need to
   `builtins.unsafeDiscardOutputDependency` a `drvPath` now *do*.

Also, there is a regular language test testing the `derivation` primop
in isolation (again, no breaking change to it!) which has been extended.

Co-authored-by: Robert Hensing <roberth@users.noreply.github.com>
Co-authored-by: Théophane Hufschmitt <7226587+thufschmitt@users.noreply.github.com>
Co-authored-by: Valentin Gagarin <valentin.gagarin@tweag.io>
Ericson2314 added a commit to obsidiansystems/nix that referenced this issue Oct 23, 2023
The test plan is taken by Robert's comment in
NixOS#7910 (comment)
describing how we could migrate *Nixpkgs* without a breaking change in
Nix. The Nix testsuite has its own `mkDerivation`, and we so we do the
same thing with it:

 - `drvPath` is now overridden to not have the funky `DrvDeep` string
   context anymore.

 - Tests that previously needed to
   `builtins.unsafeDiscardOutputDependency` a `drvPath` no don't.

 - Tests that previous did *not* need to
   `builtins.unsafeDiscardOutputDependency` a `drvPath` now *do*.
Ericson2314 added a commit to obsidiansystems/nix that referenced this issue Oct 23, 2023
The new primop is fairly simple. What is far more weird is the test
plan.

The test plan is taken by Robert's comment in
NixOS#7910 (comment)
describing how we could migrate *Nixpkgs* without a breaking change in
Nix. The Nix testsuite has its own `mkDerivation`, and we so we do the
same thing with it:

 - `drvPath` is now overridden to not have the funky `DrvDeep` string
   context anymore.

 - Tests that previously needed to
   `builtins.unsafeDiscardOutputDependency` a `drvPath` no don't.

 - Tests that previous did *not* need to
   `builtins.unsafeDiscardOutputDependency` a `drvPath` now *do*.

Also, there is a regular language test testing the `derivation` primop
in isolation (again, no breaking change to it!) which has been extended.

Co-authored-by: Robert Hensing <roberth@users.noreply.github.com>
Co-authored-by: Théophane Hufschmitt <7226587+thufschmitt@users.noreply.github.com>
Co-authored-by: Valentin Gagarin <valentin.gagarin@tweag.io>
Ericson2314 added a commit to obsidiansystems/nix that referenced this issue Oct 23, 2023
The test plan is taken by Robert's comment in
NixOS#7910 (comment)
describing how we could migrate *Nixpkgs* without a breaking change in
Nix. The Nix testsuite has its own `mkDerivation`, and we so we do the
same thing with it:

 - `drvPath` is now overridden to not have the funky `DrvDeep` string
   context anymore.

 - Tests that previously needed to
   `builtins.unsafeDiscardOutputDependency` a `drvPath` no don't.

 - Tests that previous did *not* need to
   `builtins.unsafeDiscardOutputDependency` a `drvPath` now *do*.
Ericson2314 added a commit to obsidiansystems/nix that referenced this issue Dec 15, 2023
The test plan is taken by Robert's comment in
NixOS#7910 (comment)
describing how we could migrate *Nixpkgs* without a breaking change in
Nix. The Nix testsuite has its own `mkDerivation`, and we so we do the
same thing with it:

 - `drvPath` is now overridden to not have the funky `DrvDeep` string
   context anymore.

 - Tests that previously needed to
   `builtins.unsafeDiscardOutputDependency` a `drvPath` no don't.

 - Tests that previous did *not* need to
   `builtins.unsafeDiscardOutputDependency` a `drvPath` now *do*.
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

No branches or pull requests

2 participants