Stream to and from NAR format #61
Conversation
hnix-store-core/tests/NarFormat.hs
Outdated
| nar <- localPackNar narEffectsIO bigFileName | ||
| BSL.writeFile narFileName . runPut . put $ nar | ||
| -- nar <- localPackNar narEffectsIO bigFileName | ||
| -- BSL.writeFile narFileName . runPut . putNar $ nar |
hnix-store-core/tests/NarFormat.hs
Outdated
| Right _ -> | ||
| bracket (return ()) (\_ -> P.runCommand "rm -rf testfile nixstorenar.nar hnix.nar") $ \_ -> do | ||
| Right _ -> Temp.withSystemTempDirectory "hnix-store" $ \baseDir -> do | ||
| -- bracket (return ()) (\_ -> cleanup) $ \_ -> do |
hnix-store-core/tests/NarFormat.hs
Outdated
|
|
||
| run = do | ||
| filesPrecount <- countProcessFiles | ||
| -- nar <- localPackNar narEffectsIO narFile |
|
Ugh - for expectation setting, I'm less confident now that the streaming is working as well as I thought. Limiting the RTS to 10M, I can easily get tests to fail by NARing files > 10M. Still working on it... |
|
@sorki @shlevy @domenkozar this is ready for review, if you have time to give it a look. I'm happy to polish it further. Quickie view of how I'm testing: @domenkozar I'm especially interested in seeing if this branch solves the issues you were seeing, if you can point that project at this branch and adapt for the changed API. |
|
@imalsogreg great! I'll try to revive the branch using hnix-store-core and see if it works. |
|
@imalsogreg thanks again for this awesome work. I'm a bit behind on my reviews, will try my best to test this soon. |
|
Thanks @domenkozar, no rush :) |
| @@ -0,0 +1,97 @@ | |||
| -- | | |||
|
I've realized that Path filter has a wrong type of Looks like repair flag is only used for local store so that can be dropped for now I think. |
|
@sorki any chance your comment was meant for another PR? @domenkozar any luck with your test case? |
|
@imalsogreg it was meant for this one. It is related to storepath and addToStore though. |
|
I get the following compilation errors on LTS-15.14: |
|
@sorki @shlevy or @domenkozar , seems to be working. I rebased the PR on |
|
Happy to test once I'm back from vacations :) |
|
FWIW C++ Nix now streams files much better with the latest daemon protocol. I would definitely look to use this improvement to implement that next. |
|
Bump @domenkozar (not too sure of course, but you should approximately be back from vacations now ;-) As the core and remote versions of hnix-store live in the same repo, It would make sense to also update hnix-store-remote to work with these changes. I made a quick prototype. Is is supposed to work like that ? https://gist.github.com/layus/bcb1cec3051773f67620c1e8ac189267 We will also have to decide if we want to re-export the Nar datatype, or cleanup some functions in hnix-store-remote that use it. |
|
Feel free to merge this, it's going to take some time before I can give it a try. |
Addresses #51
System.Nix.Narchanges its API - the main functions become roughly (not exactly)buildNarIO :: FilePath -> Handle -> IO ()Create a NAR from a regular filesystem object, stream it out on the Handle.unpackNarIO :: Handle -> FilePath -> IO ()Recreate filesystem object from a NAR file accessed by the HandleThe types
NarandFileSystemObjectare no longer parts of the external API, although they still exist and help the implementation somewhat. RemovingFileSystemObjectwas necessary to let NAR construction/consumption work in constant space, since parsing one to completion implies holding it all in memory. In the future we could bring back this return type by stripping out the files themselves. And it would certainly be useful to have that as a parse result, so thathnix-storecould analyze the spine of the Nar.Test tests all pass with
+RTS -M10m(limiting heap size to 10Mb), and several of them produce NARs larger than 100Mb. So, we have streaming. There is also a test that packs/unpacks a directory with 1000 files into a NAR and ensures that we do not hold onto file descriptors for longer than needed (as #51 discovered formaster).