builder: split nix-instantiate & nix-build #117

Profpatsch · 2019-07-25T11:31:50Z

A bit of refactoring, to finally split the instantiation and build (see last commit).

Please review commit-by-commit.

Add wrappers around PathBuf to distinguish drv files and final build results
Use struct OutputPaths to keep results of logged-evalatuion.nix & move errors from use time to parse time
remove lorri shell
Fix an issue with nix output parsing
and finally split instantiate and build

(Part of the fix for #58)

Closes #143
Closes #50

grahamc · 2019-08-08T13:39:11Z

src/nix.rs

    attribute: Option<String>,
+    result_gc_root_path: Option<PathBuf>,


Right off the bat feedback: this isn't workable. We need to GC root everything Nix evaluates and builds, and enforce it at the type system level.

One improvement on the way it works on master is if a store path result from the Nix evaluator carries with it its tempdir handle, forcing the tempdir to stay alive until the Nix value is garbage collected.

So, what effectively happened is: Once I removed the lorri shell subcommand, no code whatsoever is using this stuff anymore. The only part that creates a gc root is the store path generated by logged-evaluation.nix, and that is handled by roots. I’m thinking about completely removing this from the builder for now.

Okay, the problem with creating the roots after a build is that there is a small period of time (race condition) where a GC could delete the store path. Thus we must pass the gc root to the nix build.

This is resolved by basing this branch on #131. PTAL

Profpatsch · 2019-08-08T14:24:33Z

Rebased & ready for review.

Profpatsch · 2019-08-13T18:22:15Z

I split this PR up in multiple PRs for easier review:

Part 1 nix, builder: Fix nix output parsing #133
Part 2 Builder: restrict named drvs to struct #134
Part 3 Builder: use OutputPaths for root names #135

And the rest of the commits are in this PR.

The build is split into a two-step process: - nix-instantiate, which is called with -vv and parses its stderr (and produces drv files) - nix-build, which builds the drv file The idea behind this is that we can’t easily write tests which use `nix-build` (nix-in-nix problem), so we are only gonna use the first step for tests (which is enough to check the stderr parsing stuff). Another problem this solves is that the stderr of nix-build is now separated from the stderr of nix-instantiate, and nix-build is not called with -vv, meaning we can show it to the user while it’s generated, making it possible to fix the silence problem lorri has. Closes #126

lorri would crash if the instantiation works but the build has an error; because we didn’t convert the build errors into `Info::Success`. Closes #121

We don’t capture the build output of nix-build at the moment, so let’s use `nix-store --read-log` to check whether the non-utf8 sequence is there.

The older version of Nix on darwin (2.0) had a bug where it wouldn't interpret a symlink as a `.drv`. The upstream Travis support for newer Nixes on Darwin is a bit broken, so this is working around that for now. The default language changes to `minimal`, but the Linux version overrides it to `nix`.

Profpatsch · 2019-10-18T19:59:15Z

holy cow, I can’t believe it’s merged

Profpatsch force-pushed the builder-add-transitive-source-detection-tests branch from 498e81f to 252d0e7 Compare August 1, 2019 10:33

grahamc suggested changes Aug 8, 2019

View reviewed changes

Profpatsch force-pushed the builder-add-transitive-source-detection-tests branch 4 times, most recently from 64a980a to 07e4e96 Compare August 8, 2019 14:17

Profpatsch changed the title ~~WIP Builder add transitive source detection tests~~ builder: split nix-instantiate & nix-build Aug 8, 2019

Profpatsch marked this pull request as ready for review August 8, 2019 14:22

Profpatsch force-pushed the builder-add-transitive-source-detection-tests branch from 07e4e96 to c712e01 Compare August 8, 2019 14:24

Profpatsch force-pushed the builder-add-transitive-source-detection-tests branch from e285e19 to a54af4a Compare August 9, 2019 12:28

Profpatsch mentioned this pull request Aug 12, 2019

nix/CallOpts: return gc root handle #131

Merged

Profpatsch force-pushed the builder-add-transitive-source-detection-tests branch 2 times, most recently from b279440 to 4e020e5 Compare August 13, 2019 17:10

Profpatsch requested a review from grahamc August 13, 2019 17:13

This was referenced Aug 13, 2019

nix, builder: Fix nix output parsing #133

Merged

Builder: restrict named drvs to struct #134

Merged

Builder: use OutputPaths for root names #135

Merged

grahamc added the P1 critical: next release label Aug 14, 2019

Profpatsch mentioned this pull request Aug 16, 2019

Graham/fix nix output parsing #137

Merged

Profpatsch changed the base branch from master to builder-use-OutputPaths-for-root-names August 16, 2019 16:21

grahamc force-pushed the builder-use-OutputPaths-for-root-names branch from 630502b to 27afeba Compare August 20, 2019 16:30

Profpatsch force-pushed the builder-add-transitive-source-detection-tests branch from 65cb5b9 to a7e1184 Compare August 28, 2019 15:43

Profpatsch force-pushed the builder-use-OutputPaths-for-root-names branch from 27afeba to c576b95 Compare August 28, 2019 15:45

grahamc force-pushed the builder-add-transitive-source-detection-tests branch from a7e1184 to 5292d7e Compare August 29, 2019 14:44

grahamc force-pushed the builder-use-OutputPaths-for-root-names branch 3 times, most recently from 26543c5 to 90aebe2 Compare August 29, 2019 15:22

grahamc mentioned this pull request Sep 23, 2019

CI: fix the ci_check return code accumulation #155

Merged

grahamc force-pushed the builder-add-transitive-source-detection-tests branch 2 times, most recently from eccd66f to ab6535e Compare September 23, 2019 19:25

grahamc changed the base branch from builder-use-OutputPaths-for-root-names to master September 23, 2019 19:25

Profpatsch force-pushed the builder-add-transitive-source-detection-tests branch from 24d0305 to 8ae8699 Compare October 4, 2019 18:33

Profpatsch changed the base branch from master to builder-default.nix-logic October 4, 2019 18:34

Profpatsch mentioned this pull request Oct 4, 2019

builder: add test for default.nix logic #166

Merged

Profpatsch force-pushed the builder-default.nix-logic branch from 1f73f45 to fa64be3 Compare October 7, 2019 13:43

Profpatsch force-pushed the builder-add-transitive-source-detection-tests branch from 8ae8699 to fc9141a Compare October 7, 2019 13:58

Profpatsch force-pushed the builder-default.nix-logic branch from fa64be3 to e7acd51 Compare October 7, 2019 14:01

grahamc changed the base branch from builder-default.nix-logic to master October 7, 2019 16:20

Profpatsch force-pushed the builder-add-transitive-source-detection-tests branch from fc9141a to a37fd0a Compare October 7, 2019 16:21

grahamc and others added 5 commits October 16, 2019 12:40

lib: introduce DrvFile

8cfa426

builder/tests: extract drv helper

87593f3

builder: Catch build errors thrown by CallOpts

d9fed3b

lorri would crash if the instantiation works but the build has an error; because we didn’t convert the build errors into `Info::Success`. Closes #121

builder/tests: check output of utf8-drv via nix log

f57b79b

We don’t capture the build output of nix-build at the moment, so let’s use `nix-store --read-log` to check whether the non-utf8 sequence is there.

grahamc force-pushed the builder-add-transitive-source-detection-tests branch from a37fd0a to f57b79b Compare October 16, 2019 16:40

grahamc mentioned this pull request Oct 16, 2019

Grahamc/builder add transitive source detection tests #178

Merged

grahamc and others added 7 commits October 18, 2019 13:41

builder: run: split build bits in to a build() fn

e4f5970

eval/build split: Introduce simpler types

dbcf209

builder: drop unused types

aa8ad0f

build_loop/builder: collect logs from instantiation and building

9da8f06

builder test: disregard FailedAtRealize

1edc8cb

builder: Add stderr back to UTF-8 test panic message

904b9b0

grahamc merged commit f20a323 into master Oct 18, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

builder: split nix-instantiate & nix-build #117

builder: split nix-instantiate & nix-build #117

Profpatsch commented Jul 25, 2019 •

edited

Loading

grahamc Aug 8, 2019

Profpatsch Aug 8, 2019

Profpatsch Aug 9, 2019

Profpatsch Aug 13, 2019

Profpatsch commented Aug 8, 2019

Profpatsch commented Aug 13, 2019

Profpatsch commented Oct 18, 2019

		attribute: Option<String>,
		result_gc_root_path: Option<PathBuf>,

builder: split nix-instantiate & nix-build #117

builder: split nix-instantiate & nix-build #117

Conversation

Profpatsch commented Jul 25, 2019 • edited Loading

grahamc Aug 8, 2019

Choose a reason for hiding this comment

Profpatsch Aug 8, 2019

Choose a reason for hiding this comment

Profpatsch Aug 9, 2019

Choose a reason for hiding this comment

Profpatsch Aug 13, 2019

Choose a reason for hiding this comment

Profpatsch commented Aug 8, 2019

Profpatsch commented Aug 13, 2019

Profpatsch commented Oct 18, 2019

Profpatsch commented Jul 25, 2019 •

edited

Loading