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

Impure derivations #6227

Merged
merged 12 commits into from
Mar 31, 2022
Merged

Impure derivations #6227

merged 12 commits into from
Mar 31, 2022

Conversation

edolstra
Copy link
Member

@edolstra edolstra commented Mar 10, 2022

This is a revived implementation of 18c512d.

Impure derivations are derivations that can produce a different result every time they're built. Example:

stdenv.mkDerivation {
  name = "impure";
  __impure = true; # marks this derivation as impure
  #outputHashAlgo = "sha256"; # optional, default is sha256
  #outputHashMode = "recursive"; # optional, default is recursive
  buildCommand = "date > $out";
};

Some important characteristics:

  • This requires the 'impure-derivations' experimental feature.

  • Impure derivations are not "cached". Thus, running "nix-build" on the example above multiple times will cause a rebuild every time.

  • They are implemented similar to CA derivations, i.e. the output is moved to a content-addressed path in the store. The difference is that we don't register a realisation in the Nix database.

  • Only impure or fixed-output derivations are allowed to depend on impure derivations directly. "Pure" (i.e. input-addressed or floating CA derivations) can depend on impure derivations indirectly, if there is a fixed-output derivation in between. Thus the fixed-output derivation forms an "impurity barrier" in the dependency graph.

  • When sandboxing is enabled, impure derivations can access the network in the same way as fixed-output derivations. In relaxed sandboxing mode, they can access the local filesystem.

A motivating example for impure derivations is the problem of using fetchurl on a dynamically generated tarball whose contents are deterministic, but where the tarball does not have a canonical form. Previously, this required fetchurl to do the unpacking in the same derivation. (That's what fetchzip does.) But now we can separate these two steps:

tarball = stdenv.mkDerivation {
  __impure = true;
  name = "tarball";
  buildInputs = [ curl ];
  buildCommand =
    "curl --fail -Lk https://github.com/NixOS/patchelf/tarball/c1f89c077e44a495c62ed0dcfaeca21510df93ef > $out";
};

unpacked = stdenv.mkDerivation {
  name = "unpacked";
  outputHashAlgo = "sha256";
  outputHashMode = "recursive";
  outputHash = "1jl8n1n36w63wffkm56slcfa7vj9fxkv4ax0fr0mcfah55qj5l8s";
  buildCommand =
    "mkdir $out; tar xvf ${tarball} -C $out";
};

Another application could be impure (final) steps in CA pipelines like Hydra jobsets, like a job that uploads a build result somewhere.

Comment on lines +47 to +57
/* Impure output which is moved to a content-addressed location (like
CAFloating) but isn't registered as a realization.
*/
struct DerivationOutputImpure
{
/* information used for expected hash computation */
FileIngestionMethod method;
HashType hashType;
};

Copy link
Member

Choose a reason for hiding this comment

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

I don't think we need this? I didn't think we did, and I went through and indeed it seems this always handled the same way as a CA output.

@Ericson2314
Copy link
Member

Ericson2314 commented Mar 11, 2022

Pure derivations are not allowed to depend on impure derivations. In the future fixed-output derivations will be allowed to depend on impure derivations, thus forming an "impurity barrier" in the dependency graph.

Actually, even that not the only way to get back to purity. Once a pure derivation is resolved, we don't where any of it's CA imputs came from --- since they are "just" data! --- and that means we don't care whether those imputs came from CA derivations. What this means is that pure unresolved derivations perhaps can only be impurely resolved, so we can use our in-memory resolution cache to finish the job once pure resolution gets stuck, at the cost of "tainting" any downstream resolutions.

If we squint and imagine a pure derivation is a function a -> b, what the above means is that impurity is a functor, and we are in effect mapping our derivation function to impure a -> impure b.

All that said, for impure a -> b, yes a fixed output derivation is needed. No way to "map" a derivation to that type.

@edolstra edolstra marked this pull request as ready for review March 11, 2022 12:32
Copy link
Member

@thufschmitt thufschmitt left a comment

Choose a reason for hiding this comment

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

That’s great 😎

I managed to break a number of things (which is expected given that this adds a whole new derivation type, but better be aware of them). I don’t think these should be blockers for this PR (afaik, nothing breaks as long as there’s no impure derivation involved), just something to keep in mind for later:

  • nix-build
    $ nix-build impure-derivations.nix -A impure
    warning: unknown experimental feature 'ca-references'
    this derivation will be built:
      /tmp/nix-test/default/store/hhrsdj65kvf1n0d5y1yj6qc21dqr0gki-impure.drv
    warning: unknown experimental feature 'ca-references'
    building '/tmp/nix-test/default/store/hhrsdj65kvf1n0d5y1yj6qc21dqr0gki-impure.drv'...
    impure
    nix-build: src/nix-build/nix-build.cc:585: void main_nix_build(int, char**): Assertion `maybeOutputPath' failed.
    [1]    4184961 IOT instruction (core dumped)  nix-build impure-derivations.nix -A impure
  • nix-shell
    $ nix-shell impure-derivations.nix -A impureOnImpure
    warning: unknown experimental feature 'ca-references'
    this derivation will be built:
      /tmp/nix-test/default/store/hhrsdj65kvf1n0d5y1yj6qc21dqr0gki-impure.drv
    warning: unknown experimental feature 'ca-references'
    building '/tmp/nix-test/default/store/hhrsdj65kvf1n0d5y1yj6qc21dqr0gki-impure.drv'...
    impure
    warning: output 'out' of input '/tmp/nix-test/default/store/hhrsdj65kvf1n0d5y1yj6qc21dqr0gki-impure.drv' missing, aborting the resolving
    nix-shell: src/nix-build/nix-build.cc:410: void main_nix_build(int, char**): Assertion `resolvedDrv && "Successfully resolved the derivation"' failed.
    [1]    7110 IOT instruction (core dumped)  nix-shell impure-derivations.nix -A impureOnImpure
  • nix develop
    $ nix develop -f impure-derivations.nix impureOnImpure
    warning: unknown experimental feature 'ca-references'
    error: pure derivation '/tmp/nix-test/default/store/gw8z2q3qnc6bx1ap3s8v81fbvd5qxlrr-impure-on-impure-env.drv' depends on impure derivation '/tmp/nix-test/default/store/hhrsdj65kvf1n0d5y1yj6qc21dqr0gki-impure.drv'
  • Trying to copy an impure derivation will cause Nix to segfault:
    $ nix copy --to /tmp/nix --file impure-derivations.nix impure
    [1/0/1 built] building impure: impurenix: src/libstore/derived-path.cc:106: nix::BuiltPath::toRealisedPaths(nix::Store&) const::<lambda(const Built&)>: Assertion `thisRealisation' failed.
    [1]    4183600 IOT instruction (core dumped)  nix copy --to /tmp/nix --file impure-derivations.nix impure
  • nix why-depends is broken too:
    $ nix why-depends --file impure-derivations.nix inputAddressedAfterCA impure
    warning: unknown experimental feature 'ca-references'
    this derivation will be built:
      /tmp/nix-test/default/store/hhrsdj65kvf1n0d5y1yj6qc21dqr0gki-impure.drv
    error: cannot operate on an output of unbuilt content-addressed derivation 'sha256:ddffc26775f61118da8597d1f2f62c544ec92461b31b5b1075ed8689a7ee8292!out'
  • import-from-derivation (though I’m not sure that’s something that we want to support):
    $ nix eval --impure --expr 'import (import ./impure-derivations.nix).impure'
    error: output 'out' of derivation '/tmp/nix-test/default/store/hhrsdj65kvf1n0d5y1yj6qc21dqr0gki-impure.drv' has no store path mapped to it
    
          … while realising the context of path '/135sba7ahqvizkd4xq2cbpwm2l4dy8dqli0ym1jmrsxjn975pz5w'
    
          at «string»:1:1:
    
                1| import (import ./impure-derivations.nix).impure
                | ^

(but as I said, these are just stuff to keep in mind for later, definitely not a criticism of the PR)

src/libstore/build/derivation-goal.cc Outdated Show resolved Hide resolved
tests/impure-derivations.sh Outdated Show resolved Hide resolved
@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/tweag-nix-dev-update-26/18252/1

@Ericson2314
Copy link
Member

I am fixing the conflicts in this.

@Ericson2314
Copy link
Member

Ericson2314 commented Mar 18, 2022

I must say, while the BuildResult returning is convenient, the fact that we using it to drift away from the good "Command Query Responsibility Segregation" that we had before makes me nervous.

I think I would rather create a notion of transactions.

struct LayeredTrustMapStore: Store {
// some trust map functions,
// letting us know whether the results came from this store
// of the underlying store.
}

ref<LayeredTrustMapStore> Store::createTransacation();

The idea is that

  1. All the commands @thufschmitt mentioned should work again.
  2. Thanks to virtual we have the option of e.g. persisting impure trust maps in DB but segregated.
  3. Interesting overlap with https://gist.github.com/edolstra/afa5a41d4acbc0d6c8cccfede7fd4792
  4. Impure and pure flows re trust maps should be more similar: same how just different where the mapping goes.

@Ericson2314
Copy link
Member

Ericson2314 commented Mar 18, 2022

master...Ericson2314:impure-derivations-ng Here is the rebase. Note I didn't (intentionally) change any design --- my code review comments are separate from this just, which is just fixing the conflicts I caused recent on master.

Impure derivations are derivations that can produce a different result
every time they're built. Example:

  stdenv.mkDerivation {
    name = "impure";
    __impure = true; # marks this derivation as impure
    outputHashAlgo = "sha256";
    outputHashMode = "recursive";
    buildCommand = "date > $out";
  };

Some important characteristics:

* This requires the 'impure-derivations' experimental feature.

* Impure derivations are not "cached". Thus, running "nix-build" on
  the example above multiple times will cause a rebuild every time.

* They are implemented similar to CA derivations, i.e. the output is
  moved to a content-addressed path in the store. The difference is
  that we don't register a realisation in the Nix database.

* Pure derivations are not allowed to depend on impure derivations. In
  the future fixed-output derivations will be allowed to depend on
  impure derivations, thus forming an "impurity barrier" in the
  dependency graph.

* When sandboxing is enabled, impure derivations can access the
  network in the same way as fixed-output derivations. In relaxed
  sandboxing mode, they can access the local filesystem.
@edolstra edolstra changed the title Add support for impure derivations Impure derivations Mar 31, 2022
@edolstra edolstra merged commit c9a29d0 into master Mar 31, 2022
@edolstra edolstra deleted the impure-derivations-ng branch March 31, 2022 17:58
> _DerivationTypeRaw;

struct DerivationType : _DerivationTypeRaw {
using Raw = _DerivationTypeRaw;
using Raw::Raw;
using InputAddressed = DerivationType_InputAddressed;
using ContentAddressed = DerivationType_ContentAddressed;

using Impure = DerivationType_Impure;
Copy link
Member

@Ericson2314 Ericson2314 Mar 31, 2022

Choose a reason for hiding this comment

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

@edolstra I am a bit annoyed you added this and didn't wait for code review. A huge point of my refactor of the derivation type was that we can represent all pure/sandboxing combinations in the content addressed case, rather than having another tacked on variant ad-hoc-ly. In addition to commenting to this effect prior, the merge conflict resolution I linked in the PR thread implemented all this in the code so you could see exactly what I am talking about.

I know you are BDFL and don't have to care about what others write on your PRs, but it would be nice even if you disagree to at least acknowledge you read and understood the comments others leave.

Copy link
Member

Choose a reason for hiding this comment

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

(I was about to compare this to the BuildResult issue, but I see in #6311 (comment) you did in fact reply again, and I missed that and didn't link the PR I had opened (until now), so that is my fault for not following up again there more promptly.)

[](const ContentAddressed & ca) {
return !ca.pure;
return true;
Copy link
Member

Choose a reason for hiding this comment

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

A ContentAddressed that is neither fixed-output or sandboxed is impure.

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/tweag-nix-dev-update-27/18433/1

@ghost
Copy link

ghost commented Sep 16, 2022

Is there a timeline for de-experimentalizing this feature?

Along with NixOS/nixpkgs#188587 and #7052 it will allow us to eliminate all those "remember not to use fetchpatch here" comments all over the place in nixpkgs.

# Note: this package is used for bootstrapping fetchurl, and thus
# cannot use fetchpatch! All mutable patches (generated by GitHub or
# cgit) that are needed here should be included directly in Nixpkgs as
# files.

Some of these banners are extraneous, others are missing, and all of them are unchecked. It would be great if we could just remove the restriction instead.

@thufschmitt
Copy link
Member

Is there a timeline for de-experimentalizing this feature?

Afaik no, but one blocker (at least) for Nixpkgs is that hydra currently doesn't support it, and it's probably not a trivial change

@ghost
Copy link

ghost commented Sep 16, 2022

hydra currently doesn't support it, and it's probably not a trivial change

Hrm, what version of Nix does Hydra run (and is that documented anywhere)?

I assume from your comment that it must be running something older than 2.8, because since Nix 2.8 enabling impure derivations has been a trivial change (--extra-experimental-features impure-derivations).

@thufschmitt
Copy link
Member

Hydra is generally pretty close from Nix master, but it's not that simple unfortunately, because Hydra actually re-implements part of the Nix build loop, which means that features like that requires changing hydra.

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

5 participants