derivationStrict #554

layus · 2020-05-13T12:25:55Z

Implement derivationStrict primOp

Closes #364

jwiegley · 2020-05-16T00:48:01Z

This looks like great work, @kmicklas, I wonder if @shlevy and @ryantrinkle might have some time to take a look. If not, we can merge later next week and fix whatever is left to be fixed.

kmicklas · 2020-05-16T00:51:00Z

I think you meant to tag @layus 😄

jwiegley · 2020-05-16T00:57:23Z

Oh, right, thank you @kmicklas !

src/Nix/Effects.hs

sorki · 2020-05-19T13:25:06Z

src/Nix/Effects.hs

+    Right pathName -> do
+      -- todo: pass the hash explicitly
+      -- TODO: Handle the filter
+      res <- Store.runStore $ Store.addToStore pathName path recursive (Proxy :: Proxy Store.SHA256) (const False) repair


This also changed type to omit proxy so you can just do Store.addToStore @'Store.SHA256 pathName ..

sorki · 2020-05-19T13:30:51Z

haskell-nix/hnix-store#59 now also contains parser for Derivations but needs Gabriella439/Haskell-Nix-Derivation-Library#8

There's also support for building derivations now if you want to give it a try.

layus · 2020-05-21T07:23:06Z

Build failure due to the dependencies on newer versions of the hnix-store... We should realy integrate everything in one single repo.

sorki · 2020-05-21T12:27:26Z

Now buildable from hnix-overlay with GHC 8.10 using merged branches - sorki/hnix-overlay@4f3454b

Seems to work but eats all my ram 😄

Be careful as this does access real system store via daemon now (tests do as well).

layus · 2020-05-22T08:53:00Z

This PR is very inefficient. We need a way to cache hashDerivationModulo results. Otherwise we are doomed with an exponential computing time. It means that in practice computing nixpkgs.hello.drvPath does not complete in reasonable time on my machine.

It may also be related to the memory consumption issue a we load several times all the derivations from memory, parse everything and compute hashes. enforcing some strictness may also help.

From my point of view, this state should reside in the store itself. We should considr moving some of the code to hnix-store.

Anton-Latukha · 2020-05-23T16:35:01Z

@layus Above message may decerve to be an idea, so it is good to preseve it in Report.

layus · 2020-10-27T03:32:42Z

Hi, I think this has reached a mergeable state. 🎉 🎉 🎉

The main issue with exponential memory usage (in the size of the closure) is now addressed by adding a cache for hashDerivationModulo results, next to the cache of parsed nix files (see last commit). The bulk of the implementation resides in the second to last commit. Lots of code, but standalone for the most part.
The rest of the commits are refactoring, fixes and changes in various locations. Some of these changes are not strictly needed for this PR. If they are deemed undesirable, I can of course redact them.

There are no tests, but nixpkgs.hello gives the same hash with nix and hnix. Achieving this was quite a feat 🎉 and requires the fixes to hnix-store that are idling in haskell-nix/hnix-store#64. Also, tracking down the issue with the substring builtin was quite a pain.

Anyway, I think this is quite an achievement as it bridges the gap between hnix an nix. We can now correctly evaluate derivations ! 🥇

/cc @jwiegley, @shlevy, @ryantrinkle Would you mind having a look at this PR ?

layus · 2020-10-27T03:40:45Z

Pending questions to be resolved later on, in other PRs:

Consider moving hashDerivationModulo to hnix-store. It would also require to move the cache there.
Decide which store we want by default in hnix (readonly, standalone, daemon).
Implement proper back-and-forth communication between MonadStore (hnix-store) and MonadNix++ (hnix). @imalsogreg made a great job of tossing monads around for streaming nars in Stream to and from NAR format hnix-store#61. Maybe he could give some advice here too.
- Implement a proper path filter based on a nix function.
- Keep a single daemon socket for the whole hnix lifetime. Currently we open a new one on each store operation.
- I am more and more considering a handle-based scheme for the hnix-store API.

jwiegley · 2020-10-27T21:02:54Z

It's a fair bit going on, but I like it. What do you think @ryantrinkle?

Ericson2314 · 2020-10-27T21:06:03Z

Pending questions to be resolved later on, in other PRs

These things do further my suspicion that it's time to move to a monorepo so it's easier to experiment without how to integrate the various libraries while keeping things modular.

jwiegley · 2020-10-27T21:13:25Z

I'm generally a fan of monorepos.

layus · 2020-11-01T22:02:20Z

Short update about performance:

$ NIX_DATA_DIR=$PWD/hnix/data command time ./dist-newstyle/build/x86_64-linux/ghc-8.10.1/hnix-0.10.1/x/hnix/build/hnix/hnix --eval -E '"${(import  { }).hello}"'
"/nix/store/w9yy7v61ipb5rx6i35zq1mvc2iqfmps1-hello-2.10"
22.21user 0.67system 0:22.95elapsed 99%CPU (0avgtext+0avgdata 1615464maxresident)k
0inputs+0outputs (0major+400263minor)pagefaults 0swaps
$ command time nix-instantiate --eval -E '"${(import  {}).hello}"'                      (nix)
"/nix/store/w9yy7v61ipb5rx6i35zq1mvc2iqfmps1-hello-2.10"
0.22user 0.03system 0:00.25elapsed 100%CPU (0avgtext+0avgdata 111252maxresident)k
0inputs+0outputs (0major+25171minor)pagefaults 0swaps

22 each time :-). And yet 100 times slower. Good news is that we have some room for improvement 😂

jwiegley · 2020-11-01T22:27:09Z

@layus Parsing and evaluating are each still very slow.

layus · 2020-11-01T23:33:08Z

@jwiegley Any known culprit for that ?

M default.nix M hnix.cabal M src/Nix/Effects/Derivation.hs default.nix: fx overlay call

Anton-Latukha · 2020-12-31T04:53:38Z

Reshuffled the history, cleaned it up, and minimized transitions as much as I could.

Figured-out and fixed a couple of things in config changes.

Now, it looks strangely familiar:

On macOS compilations fails due to non-existent IPC calls

command was: /nix/store/0nlwbj5qbdf7r58x2iqz0a17az2j5fdn-clang-wrapper-7.1.0/bin/clang -c dist/build/System/Linux/Namespaces_hsc_make.c -o dist/build/System/Linux/Namespaces_hsc_make.o -D__GLASGOW_HASKELL__=810 -Ddarwin_BUILD_OS=1 -Dx86_64_BUILD_ARCH=1 -Ddarwin_HOST_OS=1 -Dx86_64_HOST_ARCH=1 -I/nix/store/0jj05cyma647rywsqpq3darnzl6a520s-libc++-7.1.0/include -I/nix/store/ns7f13gldnk83l2fgjlzbx9bnfvc2g72-libc++abi-7.1.0/include -I/nix/store/6snmf1z43ay6m1b8jv8dr4h8im056a62-compiler-rt-7.1.0-dev/include -I/nix/store/q8xcp8hsa35wjki8mad0sbk4nzqrdkb5-libiconv-osx-10.12.6/include -I/nix/store/0jj05cyma647rywsqpq3darnzl6a520s-libc++-7.1.0/include -I/nix/store/ns7f13gldnk83l2fgjlzbx9bnfvc2g72-libc++abi-7.1.0/include -I/nix/store/6snmf1z43ay6m1b8jv8dr4h8im056a62-compiler-rt-7.1.0-dev/include -I/nix/store/q8xcp8hsa35wjki8mad0sbk4nzqrdkb5-libiconv-osx-10.12.6/include -Idist/build/autogen -Idist/build/global-autogen -include dist/build/autogen/cabal_macros.h -I/nix/store/lj34zl1kfjd2556aslrn8hsyn0ng1hzb-ghc-8.10.3/lib/ghc-8.10.3/unix-2.7.2.2/include -I/nix/store/lj34zl1kfjd2556aslrn8hsyn0ng1hzb-ghc-8.10.3/lib/ghc-8.10.3/time-1.9.3/include -I/nix/store/lj34zl1kfjd2556aslrn8hsyn0ng1hzb-ghc-8.10.3/lib/ghc-8.10.3/bytestring-0.10.12.0/include -I/nix/store/q8xcp8hsa35wjki8mad0sbk4nzqrdkb5-libiconv-osx-10.12.6/include -I/nix/store/lj34zl1kfjd2556aslrn8hsyn0ng1hzb-ghc-8.10.3/lib/ghc-8.10.3/base-4.14.1.0/include -I/nix/store/9m9q7bmn937d15p8q9m26fpxypi8vg92-gmp-6.2.1-dev/include -I/nix/store/lj34zl1kfjd2556aslrn8hsyn0ng1hzb-ghc-8.10.3/lib/ghc-8.10.3/integer-gmp-1.0.3.0/include -I/nix/store/lj34zl1kfjd2556aslrn8hsyn0ng1hzb-ghc-8.10.3/lib/ghc-8.10.3/include -I/nix/store/jikzfsnc21fsfzn991md0vi8j14p02sm-libffi-3.3-dev/include -I/nix/store/lj34zl1kfjd2556aslrn8hsyn0ng1hzb-ghc-8.10.3/lib/ghc-8.10.3/include/
error: Namespaces.hsc:79:16: error: use of undeclared identifier 'CLONE_NEWIPC'
    hsc_const (CLONE_NEWIPC);
               ^
Namespaces.hsc:79:16: error: use of undeclared identifier 'CLONE_NEWIPC'
Namespaces.hsc:79:16: error: use of undeclared identifier 'CLONE_NEWIPC'
Namespaces.hsc:80:16: error: use of undeclared identifier 'CLONE_NEWNET'

In Nix builds fail because they use network access in tests

  Eval comparison tests
    builtins.toJSON.nix:                                             FAIL
      Exception: Network.Socket.connect: <socket: 13>: does not exist (No such file or directory)
    builtins.lessThan-01.nix:                                        FAIL
      Exception: Network.Socket.connect: <socket: 14>: does not exist (No such file or directory)
    builtins.getContext.nix:                                         FAIL
      Exception: Network.Socket.connect: <socket: 13>: does not exist (No such file or directory)
    builtins.replaceStrings-01.nix:                                  FAIL
      Exception: Network.Socket.connect: <socket: 13>: does not exist (No such file or directory)
      
  Nix (upstream) language tests
    eval okay
      eval-okay-eq-derivations:                                      FAIL
        Exception: Network.Socket.connect: <socket: 13>: does not exist (No such file or directory)
      eval-okay-context:                                             FAIL
        Exception: Network.Socket.connect: <socket: 13>: does not exist (No such file or directory)

On GHC 8.10 GitHub decided no matter what PR is - to take a New Year break after 10 minutes of work

10 minutes...

Error: The operation was canceled.

And what really rings true is the Cabal build

   Eval comparison tests
    nixpkgs.hello.nix:                                               FAIL
      Exception: /nix/store/q5xckc93ra0fahpj6z01ffrxj3pa0qa8-source: openBinaryFile: does not exist (No such file or directory)

What would you do - every build is familiar in its own way. But only the last one is, as always, being true. If GitHub was not lazy today - the GHC 8.10 build would've told us the exact same story (since it tells it locally).

It compiles, but fails on that little test.

Test group defined as:

genEvalCompareTests = do
    td <- D.listDirectory testDir

    let unmaskedFiles = filter ((==".nix") . takeExtension) td
    let files = unmaskedFiles \\ maskedFiles

    pure $ testGroup "Eval comparison tests" $ map (mkTestCase testDir) files
  where
    mkTestCase td f = testCase f $ assertEvalFileMatchesNix (td </> f)

What we essentially have is a 1 test to fix. And that is a test introduced here: nixpkgs.hello.nix(open):

hnix/tests/eval-compare/nixpkgs.hello.nix

Lines 1 to 22 in ac7450a

    
           /* 
        
           FIXME: TODO: This is a bad test because: 
        
           1. We may not have access to the network in test environments (e.g. Nix :-)) 
        
           2. fetchTarball is not yet implemented in hnix and call the real nix 
        
           3. We want all the tests that rely on nixpkgs to use the same fetched path 
        
           4. We want a single place to update the nixpkgs revision we test. 
        
           5. Currently hnix tests are not sandboxed as the ones in hnix-store-remote, 
        
              so all the .drv we build en up in the host store. 
        
            XXX: NIXPKGS_TESTS are broken for now. Fix that too when fixing this. 
        
           */ 
        
           let  
        
             nixpkgs = builtins.fetchTarball { 
        
               url = "https://github.com/NixOS/nixpkgs/archive/1dc37370c489b610f8b91d7fdd40633163ffbafd.tar.gz"; 
        
             }; 
        
           in 
        
           with import nixpkgs {}; 
        
           "${pkgs.hello.drvPath}"

I would think and also wait on thoughts on what is the thing with this test.

Anton-Latukha · 2020-12-31T08:08:49Z

This happens because:

fetch 'nix/fetchurl.nix` was not found in the Nix search path

Dolstra recently moves/reduces the helper .nix files.
Recently afair he reduced the derivation.nix, which we internally use.
In derivation it now lives in the ./share/nix/corepkgs/fetchurl.nix.

Anton-Latukha · 2020-12-31T08:39:43Z

It is somewhere between functions fetchTarball, it's fetch and instantiateExpr.

Anton-Latukha · 2020-12-31T08:55:54Z

Why and how it looks into it.
corepkgs/fetchurl.nix needs inclusion into cabal file.
...
Maybe it is foldNixPath.

layus · 2020-12-31T15:27:19Z

That's a lot of small issues bundled together. Let's try them one at a time:

fetch 'nix/fetchurl.nix` was not found in the Nix search path comes is an issue I am unsure how to solve. I ran all my tests with
NIX_DATA_DIR=/path/to/hnix/data env var. The tests expect to have it in the same location. (See https://github.com/layus/hnix/blob/2ec374d19e4a4090abed5513faf6ad6644f0ff8b/tests/Main.hs#L99 ). If it fails, it may be because we do not bundle these files. Or something else. Not quite sure.

For this PR, we need to bundle the files correctly so that hnix invocations can find them by default in the hnix installation.

As for the

Dolstra recently moves/reduces the helper .nix files.
Recently afair he reduced the derivation.nix, which we internally use.
In derivation it now lives in the ./share/nix/corepkgs/fetchurl.nix.

This can wait for a future PR. It is not part of the nix interface. It's implementation specific. We could also decide to bundle it in a single value just like we do for builtins.derivation

In nix, the build fails because we need network access. That was already the case before. Worth to be fixed, but not necessarily here.

But they also fail because the need a working nix daemon. In hnix-store @sorki make a great work at both i) hiding these tests behind a flag and ii) wrapping everything in namespaces to provide a sandboxed nix daemon.

We could import either. And possibly both. So we would have flags tho disable tests that require networking, and provide a sandboxed nix daemon for tests.

It is a bit sad that the current implementation relies on the remote version of the hnix-store. a read-only version would fit the tests better. But it does not exist yet. Among other things because it would require some internal state that needs some architectural decisions about where/how to keep it. So for now we need the real nix daemon somewhere.

I guess that already some stuff to grind :-)

layus · 2020-12-31T15:39:07Z

As for <nix/fetchurl.nix> it is called from nixpkgs @ https://github.com/NixOS/nixpkgs/blob/1dc37370c489b610f8b91d7fdd40633163ffbafd/pkgs/build-support/fetchurl/boot.nix#L11

The link references the same nixpkgs commit as the one we fetch with fetchTarball.

The solution is a proper NIX_DATA_DIR being set because that gets used as the <nix/...> prefix in the default nix path: https://github.com/layus/hnix/blob/2ec374d19e4a4090abed5513faf6ad6644f0ff8b/src/Nix/Builtins.hs#L313-L326

layus · 2020-12-31T15:44:49Z

Or rather a preperly filled getDataDir which is generated automatically in Paths_hnix.hs and gets filled by Data-Files: cabal directive.

layus · 2020-12-31T16:35:48Z

builtins.fetchTarball is broken because we have https://github.com/layus/hnix/blob/77859172494967a9de6292b4021844efe3b547ab/tests/Main.hs#L98 which downloads to $PWD while still printing a path in /nix/store/... . I wonder if it is a new nix feature. local?root= seems a hack that could be bo more supported in new nix builds.

layus · 2020-12-31T16:38:11Z

Indeed. local?root=... is now local?store=... 🐪

Anton-Latukha · 2020-12-31T17:33:44Z

Great read. I never thought that it might be the Nixpkgs call of the file, automatically assumed it is or Haskell code or nix-instantiate invocations, so dug there.

I wanted to propose simple separation of consciences: cherry-pick the test into the separate PR and merge current. And deal with fixing the envocation in the test separately.

bqv · 2020-12-31T17:40:35Z

As for the

Dolstra recently moves/reduces the helper .nix files.
Recently afair he reduced the derivation.nix, which we internally use.
In derivation it now lives in the ./share/nix/corepkgs/fetchurl.nix.

This can wait for a future PR. It is not part of the nix interface. It's implementation specific. We could also decide to bundle it in a single value just like we do for builtins.derivation

If they're needed, and it's not going to leave this PR in purgatory for another seven months, I think this is the most sensible option

layus · 2020-12-31T21:04:05Z

I wanted to propose simple separation of consciences: cherry-pick the test into the separate PR and merge current. And deal with fixing the invocation in the test separately.

Ok, let's go with that. HNix does not yet support chroot stores, but runs the tests with it. It worked while we were only using the real nix for real store operations. Now it breaks.

This reverts commit ac7450a for the test it contains requires support for chroot stores in HNix, and it is not yet implemented. See comments starting at haskell-nix#554 (comment) for details.

bqv · 2020-12-31T21:46:12Z

Hooray, we might be merged before the new year!

Happy holidays, friends.

layus · 2020-12-31T21:53:16Z

@Anton-Latukha I think these two new changes are enough, even if not entirely satisfactory. It would be fun to merge this in late 2020. But getting the first merge of 2021 would be an achievement too.

I would appreciate if you could merge it yourself after running the checks you consider necessary :-D.

Cheers ! And happy new year :-)

Anton-Latukha · 2021-01-01T16:11:17Z

Yep, I'd already cleaned-up and merged it as #554 (comment) when I wrote it, but I actually wrote that comment from the TERM==linux, I had a hard a severe rare case of unsolvable by me hardware problem because of Linux Kerne team doing the right things with NVidia's "GPL condom" and those overall. I made test&code assessment in the Linux console, Emacs is a terminal OS, that saves the ball every time.
Now everything works fine, just a minute.

default.nix: cosmetic: fx align

Anton-Latukha · 2021-01-01T16:55:59Z

I formed that test into a separate PR and dropped its revert here.

Cabal builds already worked - which is a minimal and enough requirement for the PR merges for me. There is no sense in breaking the master for everyone because some of us want to merge the patch that can not under the most basic conditions.

I am not an authority, but I recommend to think about our pathways - all our work and infrastructure, Nix & Nixpkgs Haskell, Stack also - are still, in the end, depend and use Hackage and its Cabal descriptions, and so - our Hackage community releases which are Cabal builds and descriptions. That is why it is the main thing for me - to keep Cabal builds working. On top of it - that keeps the history and the code open for any person in the community, it is a minimal requirement and enough to do Haskell work.

If I ever merge my or other PRs that had the Cabal builds broken but I merged them - you can beat my hands with a bamboo stick.

I wish one day to be as productive as you, @layus

bqv · 2021-01-01T18:38:46Z

Thank you all!

Anton-Latukha · 2021-01-02T00:26:07Z

That is not all, do not put a cart before a horse.

We have a big API. And a big API change.
For the release and to respect people that use HNix - the changelog needs to be populated first-class. Putting in the short human form the cliffnotes to guide fellows what happened from what they may need to be guided in a human for to update to new HNix. There is a big list of changes to be parsed and simply explained in the changelog. It is late and I would not finish it today.
If our API is as big and open - we would need to do this guidance all the time for every function we add, and for any function type change - that is additionally also a major release breakage level. Since almost all of them are made public - so we must support what we provide and promise to support to the public.
That is also why I want for us to do an API clean at some point. We need to expose only what downstream needs, and put a disclaimer that if something is additionally needed - for people to open reports or PRs opening it, and issuing those minor releases fast.
The changelog effort and a number of breaking major releases are linear to the API size of the project.
Changelog is populated in: #788

jwiegley requested review from ryantrinkle and kmicklas and removed request for ryantrinkle May 16, 2020 00:47

sorki reviewed May 19, 2020

View reviewed changes

src/Nix/Effects.hs Show resolved Hide resolved

sorki reviewed May 19, 2020

View reviewed changes

Anton-Latukha added the challenging May require somewhat complex changes; not recommend if you aren't familiar with the codebase label May 25, 2020

sorki mentioned this pull request Jun 14, 2020

Can NixString be the NAtom? #534

Closed

This was referenced Jul 13, 2020

Please, fix HNix build inside Nixpkgs #641

Closed

Remote store haskell-nix/hnix-store#59

Merged

layus mentioned this pull request Jul 24, 2020

Improve digests generation and parsing haskell-nix/hnix-store#64

Closed

layus force-pushed the derivationStrict branch 2 times, most recently from 33815b6 to d38db1c Compare October 27, 2020 03:20

expipiplus1 mentioned this pull request Nov 1, 2020

Next release #714

Closed

layus force-pushed the derivationStrict branch from 90f8e79 to b1f6ef2 Compare November 1, 2020 22:48

use & support recent hnix-store master == 0.4

1927b42

M default.nix M hnix.cabal M src/Nix/Effects/Derivation.hs default.nix: fx overlay call

Anton-Latukha force-pushed the derivationStrict branch from 0b9c888 to 7785917 Compare December 31, 2020 03:58

Anton-Latukha mentioned this pull request Jan 1, 2021

(needs "Chroot store support" #813) Add an integration test for derivationStrict builtin #789

Draft

layus added 3 commits January 1, 2021 18:18

Add references, docs and minor improvements

9b478f5

default.nix: cosmetic: fx align

Update chroot store syntax to new nix standards

1a81b65

install required data files with the binary

43be29e

Anton-Latukha force-pushed the derivationStrict branch from 5bbcea1 to 43be29e Compare January 1, 2021 16:19

Anton-Latukha merged commit 8a6ff07 into haskell-nix:master Jan 1, 2021

This was referenced Jan 2, 2021

What strategic steps can be used to clean-up the API #779

Open

Please, make the CI green again #806

Closed

Anton-Latukha mentioned this pull request Mar 21, 2021

Derivation: Please, define undefined #896

Closed

derivationStrict #554

derivationStrict #554

Conversation

layus commented May 13, 2020 • edited Loading

jwiegley commented May 16, 2020

kmicklas commented May 16, 2020

jwiegley commented May 16, 2020

sorki May 19, 2020

Choose a reason for hiding this comment

sorki commented May 19, 2020

layus commented May 21, 2020

sorki commented May 21, 2020

layus commented May 22, 2020

Anton-Latukha commented May 23, 2020

layus commented Oct 27, 2020 • edited Loading

layus commented Oct 27, 2020

jwiegley commented Oct 27, 2020

Ericson2314 commented Oct 27, 2020 • edited Loading

jwiegley commented Oct 27, 2020 • edited Loading

layus commented Nov 1, 2020

jwiegley commented Nov 1, 2020

layus commented Nov 1, 2020

Anton-Latukha commented Dec 31, 2020 • edited Loading

Anton-Latukha commented Dec 31, 2020

Anton-Latukha commented Dec 31, 2020

Anton-Latukha commented Dec 31, 2020 • edited Loading

layus commented Dec 31, 2020

layus commented Dec 31, 2020 • edited Loading

layus commented Dec 31, 2020

layus commented Dec 31, 2020

layus commented Dec 31, 2020

Anton-Latukha commented Dec 31, 2020 • edited Loading

bqv commented Dec 31, 2020

layus commented Dec 31, 2020

bqv commented Dec 31, 2020

layus commented Dec 31, 2020

Anton-Latukha commented Jan 1, 2021

Anton-Latukha commented Jan 1, 2021

bqv commented Jan 1, 2021

Anton-Latukha commented Jan 2, 2021 • edited Loading

layus commented May 13, 2020 •

edited

Loading

layus commented Oct 27, 2020 •

edited

Loading

Ericson2314 commented Oct 27, 2020 •

edited

Loading

jwiegley commented Oct 27, 2020 •

edited

Loading

Anton-Latukha commented Dec 31, 2020 •

edited

Loading

Anton-Latukha commented Dec 31, 2020 •

edited

Loading

layus commented Dec 31, 2020 •

edited

Loading

Anton-Latukha commented Dec 31, 2020 •

edited

Loading

Anton-Latukha commented Jan 2, 2021 •

edited

Loading