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

Added builtins.intern primop (WIP) #1502

Closed
wants to merge 1 commit into from
Closed

Conversation

taktoa
Copy link
Member

@taktoa taktoa commented Aug 5, 2017

NOTE: this is a work in progress; in particular, it is untested

This commit adds a new builtins.intern primop that allows a fixed-output Nix store path to be made from any given path, as long as it exists. In other words, this primop allows you to do something similar to Nix path references (like src = ./.;), except that the given path could already be in the Nix store.

Without an intensional store, this is necessary to make incremental builds work in Nix. You can make a hacky alternative without it -- (path: with builtins; toFile (basename path) (readFile path)) -- but since builtins.readFile fails on binary files, it cannot be used in all cases.

@copumpkin
Copy link
Member

Hmm, I don't quite get what this adds. Can't I get the same behavior as follows?

let
  pathAsString = "/path/to/some/file/computed/dynamically";
in "${/. + pathAsString}"

That evaluates to something like /nix/store/v8609p6dn3f42lbzfd45i3y8ymysrvni-dynamically without calling readFile and dealing with encodings and strings.

@taktoa
Copy link
Member Author

taktoa commented Aug 30, 2017

nix-repl> "${/. + haskellPackages.lens}"
error: a string that refers to a store path cannot be appended to a path, at (string):1:4

@copumpkin
Copy link
Member

Okay, after hashing it out with @taktoa on IRC, this makes a lot more sense to me and I'm 👍

My use case is for testing bootstrap tools. I currently have some hackery to plug bootstrap tools from one derivation into another stdenv for easier iteration, but after running a full bootstrap I then need to build everything again before I commit the final bootstrap. This should allow me to avoid that.

@taktoa
Copy link
Member Author

taktoa commented Aug 30, 2017

related discussion on #nixos-dev (with minor edits for clarity):

<taktoa>    copumpkin: oh btw do you now understand my builtins.intern primop
<taktoa>    or was that counterexample not elucidating
<copumpkin> taktoa: sort of, except that it still feels like it could be
            implemented in Nix, by checking if path is in /nix/store already.
            My takeaway from your blurb is that you didn't think it was possible
            to copy arbitrary binary files to store with existing primops
<taktoa>    if you can show me a way to do it in the currnet Nix 1.11, I'd love
            to see it, but I've tried a bunch of things and couldn't find
            another way to do it
<copumpkin> taktoa: well, your counterexample demonstrates that it barfs on
            store paths, so we check if it's a store path and then return it,
            otherwise use the snippset I included
<copumpkin> I guess it's harder for me to engage without a use case / goal
<taktoa>    oh I think you think the semantics are different than what I want
<copumpkin> ah, probably!
<taktoa>    I want something that given "/nix/store/…-foobar/baz/quux.json"
<taktoa>    will put it _in its own store path_ "/nix/store/…-quux.json"
<copumpkin> oh!
<copumpkin> I see
<copumpkin> what if I apply it to a top-level store path? it'll take a normal
            derivation and give me a FO version of it?
<taktoa>    yes
<copumpkin> awesome
<copumpkin> so I've wanted exactly that before :)
<copumpkin> many thumbs up
<copumpkin> well, as many as I have thumbs, so two
<taktoa>    disclaimer: haven't tested that PR much yet so it may not implement
            those semantics quite yet, but that is the goal
<copumpkin> that's the perfect thing for my stdenv bootstrap testing harness
<copumpkin> and various other forms of bootstrap harnesses

@shlevy
Copy link
Member

shlevy commented Aug 31, 2017

💯

@shlevy
Copy link
Member

shlevy commented Aug 31, 2017

Haven't looked at the impl yet, does this scan for references?

@zimbatm
Copy link
Member

zimbatm commented Nov 5, 2017

How does it compare to builtins.storePath? I admit that I am not entirely clear on how it would work. nevermind, got it

@zimbatm
Copy link
Member

zimbatm commented Nov 5, 2017

I was trying to implement something similar in Nix today, but it doesn't work with remote builders:

{ lib, stdenv, nix }:

# Creates a fixed derivation output that is just based on the
# content of the input.
name: input:

let
  drv = stdenv.mkDerivation {
    inherit name;
    phases = ["installPhase"];
    buildInputs = [ nix ];
    installPhase = ''
      # make a copy so that the name doesn't change
      cp -ar ${input} ${name}
      # then add to the store
      NIX_REMOTE=daemon nix-store --add ${name} > $out
    '';
  };
  casOut = lib.removeSuffix "\n" (builtins.readFile drv);
in
  builtins.storePath casOut

When using it I get:

error: path '/nix/store/66l3mpnhkv2s3csjs6a4iwmr9yan8wip-package.json' does not exist and cannot be created

It's like the reference to it has been lost

@taktoa
Copy link
Member Author

taktoa commented Nov 29, 2017

I just noticed this in pkgs/build-support/trivial-builders.nix:

{
  # Copy a path to the Nix store.
  # Nix automatically copies files to the store before stringifying paths.
  # If you need the store path of a file, ${copyPathToStore <path>} can be
  # shortened to ${<path>}.
  copyPathToStore = builtins.filterSource (p: t: true);
}

This makes me wonder if builtins.filterSource is sufficient to implement builtins.intern.
I will need to experiment.

@spacekitteh
Copy link

Any updates fam?

@edolstra
Copy link
Member

Maybe it's because I don't understand what the word "intern" means in this context, but I don't really understand the use case here. You already can copy paths as fixed-output (content-addressable) paths in various ways:

$ nix-instantiate --eval -E '"${./Papers}"'
"/nix/store/imaak9saxmr2hryi7vg6cmr1n85a9qq7-Papers"

$ nix-instantiate --eval -E 'builtins.path { path = ./Papers; }'
"/nix/store/imaak9saxmr2hryi7vg6cmr1n85a9qq7-Papers"

$ nix hash-path --type sha256 ./Papers/       
ecc01911c725c05ada5899baeeaad4deab7c0b4dbb03aaf13e026bbdb27ba2d8

$ tar cvf /tmp/Papers.tar Papers/

$ nix-build -E 'with import <nixpkgs> {}; fetchzip { name = "Papers"; url = /tmp/Papers.tar; sha256 = "ecc01911c725c05ada5899baeeaad4deab7c0b4dbb03aaf13e026bbdb27ba2d8"; }'
/nix/store/imaak9saxmr2hryi7vg6cmr1n85a9qq7-Papers

@vcunat
Copy link
Member

vcunat commented Jul 26, 2018

Is the point to make a copy if and only if the path doesn't refer within store already? @edolstra: "intern" seems a reference to a technique used in some languages.

@taktoa
Copy link
Member Author

taktoa commented Jul 26, 2018

@edolstra @vcunat No, the point is to copy even if the path you are interning is inside another store path. All the ways you list are about files not in the store; what I want is something that turns any path to a file into a top-level fixed output store path. This is pretty essential to making incremental builds work properly in Nix.

@vcunat
Copy link
Member

vcunat commented Jul 26, 2018

Ah, right, I see now, you need per-file nix store paths for incremental builds, and that's hard if they come from a directory in nix store.

@ocharles
Copy link

ocharles commented Jul 26, 2018

It seems that the following does the same as intern, but requires IFD and evaluation time "building". Is this the same as intern, and would intern make this any "better"?

/*
 * Run `cabal sdist` on a directory and import that directory into the Nix
 * store. This allows you to clean a directory to just what is depended on
 * by a .cabal file, without any dependency on the original directory tree.
 */

dir:
let
  pkgs = import ./pkgs {};

  src-tree = pkgs.stdenv.mkDerivation {
    name = "src-sdist";
    buildCommand = ''
      mkdir -p $out/src
      cd ${dir}
      cabal sdist --output-directory=$out/src
    '';
    buildInputs = [ pkgs.cabal-install ];
  };

in
import
  ( pkgs.stdenv.mkDerivation {
      name = "src.nix";
      buildCommand = ''echo "${src-tree}"/src > $out'';
    }
  )

Assuming this is in cabal-sdist.nix, I use this as:

{
  src = import ./cabal-sdist.nix ./.;
}

@taktoa
Copy link
Member Author

taktoa commented Jul 26, 2018

@ocharles the store path that generates won't be a fixed output path, so all semblance of incremental building will be lost (everything will rebuilt if any one file changes).

@vcunat
Copy link
Member

vcunat commented Jul 26, 2018

I think that's still doable with readFile from some derivation that computes the hash, but probably it wouldn't be nice.

@taktoa
Copy link
Member Author

taktoa commented Jul 26, 2018

@vcunat
From the original PR text:

You can make a hacky alternative without it -- (path: with builtins; toFile (basename path) (readFile path)) -- but since builtins.readFile fails on binary files, it cannot be used in all cases.

@vcunat
Copy link
Member

vcunat commented Jul 26, 2018

I meant something else. A derivation that uses sha256sum or something to compute and write the hash of file "F" into a text file, then readFile that into the parameter needed for another fixed-output derivation that just copies the file "F" into the output.

@ocharles
Copy link

ocharles commented Jul 26, 2018

@taktoa The result of the final import does produce something that only depends on the contents of cabal sdist --output-directory - isn't that what intern would also do? The final import breaks any prior dependencies. I can, for example, add a random extra file and run a build again, and it won't cause a rebuild (because it won't be part of cabal sdist).

Note that the trick is that the final expression is: ${cabal-sdist-result}/src -- that is, the import ./cabal-sdist.nix ./. expression normalises to /nix/store/hash-src. The hash is entirely calculated from the contents of src and the literal string "src", but nothing else.

@stale
Copy link

stale bot commented Feb 12, 2021

I marked this as stale due to inactivity. → More info

@stale stale bot added the stale label Feb 12, 2021
@stale
Copy link

stale bot commented Apr 16, 2022

I closed this issue due to inactivity. → More info

@stale stale bot closed this Apr 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants