INF-629: Update common to get split builds by nmattia · Pull Request #200 · dfinity/sdk

nmattia · 2019-11-22T15:58:05Z

Updates the common repo to have split builds:

shell build
format build
lint + doc build
debug build
release build

Includes the following commits:

commit b35ba23941c5f3b555eba33c144f2cf9e5036cfb
Author: Joachim Breitner mail@joachim-breitner.de
Date: Wed Nov 20 11:57:28 2019 +0100

Do not use lib.inNixShell (#69)

* Do not use lib.inNixShell

this does not just mean “is the curren derivation the target of
`nix-shell`”, but it also turns true when this is a _dependency_ of
some `nix-shell` (e.g. Motoko’s). This is bad, because it changes
the derivation of `drun`, so although `drun` in in the nix cache, when
trying to enter `nix-shell`, it now depends on a _different_ `drun` (one
built with `-D warnings`), so our poor people have to watch rust compile
stuff.

Debuggig this was also tricky, and required help from the nix IRC
channel. They said that using `lib.inNixShell` in a derivation like this
is a bad idea.

So we now omit `-D warnings` simply for the `debug` and `shell` build,
where that makes more sense, and do not use `lib.inNixShell`.

commit b4e92e6010ad7caee863cb8e74b8c1426f92794a
Merge: d862d5d f19329c
Author: Nicolas Mattia nicolas@dfinity.org
Date: Thu Nov 14 18:16:18 2019 +0100

Merge pull request #67 from dfinity-lab/nm-always-touch

Touch rust files in preBuild

commit f19329c71b275d58f19109ac923110963bfbc2d3
Author: Nicolas Mattia nicolas@dfinity.org
Date: Thu Nov 14 18:15:09 2019 +0100

Run preBuild and postBuild in lint

commit 639866f9ec57523c1f4a53c14205841503f4f8ae
Author: Nicolas Mattia nicolas@dfinity.org
Date: Thu Nov 14 17:49:31 2019 +0100

s/preBuild/preCheck

commit 5dfef852592e35e88c86d96280584f4d631d3269
Author: Nicolas Mattia nicolas@dfinity.org
Date: Thu Nov 14 17:36:10 2019 +0100

Fix standalone test evaluation

commit 3df1411493bb6edad6e54664fcc819e28715ed31
Author: Nicolas Mattia nicolas@dfinity.org
Date: Thu Nov 14 17:21:00 2019 +0100

Touch rust files in preBuild

commit d862d5d1c492a2077364d3771645c72e0233ce93
Merge: 8387b5b 0367c94
Author: Nicolas Mattia nicolas@dfinity.org
Date: Thu Nov 14 16:51:43 2019 +0100

Merge pull request #66 from dfinity-lab/nm-fix-standalone-builder

Port standalone builder to new rust builder

commit 0367c948b70603b29b0f4ceff45df2c29045fd80
Author: Nicolas Mattia nicolas@dfinity.org
Date: Thu Nov 14 16:31:21 2019 +0100

Port standalone builder to new rust builder

Before this commit, building standalone rust on Linux leads to evaluation errors.

commit 8387b5bfd0cd8df0acccdca53feb1c38010eecd3
Merge: 68ddd96 bc868d9
Author: Nicolas Mattia nicolas@dfinity.org
Date: Thu Nov 14 16:04:10 2019 +0100

Merge pull request #65 from dfinity-lab/nm-split-dfn-further

INF-484: INF-588: INF-581: Split rust jobs

commit bc868d9fc02230a5e6a0833833b9af26edcd7325
Author: Nicolas Mattia nicolas@dfinity.org
Date: Thu Nov 14 16:02:03 2019 +0100

Create empty directory for lint job

commit 68ddd96f7c6171cb5316b6ca51e137ff8d478e06
Merge: a9d886e d8b3e93
Author: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Date: Thu Nov 14 14:46:30 2019 +0000

Merge pull request #33 from dfinity-lab/update-niv

Update niv

commit d8b3e93d9bc8ad7ebe4eacea2bed47bd39c41f93
Author: Nikola Knezevic nikola@dfinity.org
Date: Tue Oct 29 10:36:01 2019 +0100

Update niv

This is to help with a problem in other repos where we had an infinite
recursion due to setting up nixpkgs in nix-shell shebang line.

commit 765754201b3b899899af232a8c6f8fd4a01d7594
Author: Nicolas Mattia nicolas@dfinity.org
Date: Thu Nov 14 15:23:36 2019 +0100

Update naersk for faster builds

commit 0b7f62cad98e0a6374af1e2496ca86dd7868f485
Author: Nicolas Mattia nicolas@dfinity.org
Date: Thu Nov 14 15:22:52 2019 +0100

Make doc overridable

commit 5d40a3800d3eb2591330884784c2f3e6ac384494
Author: Nicolas Mattia nicolas@dfinity.org
Date: Thu Nov 14 15:22:38 2019 +0100

Remove unnecessary phase

commit 8385c76857644474749f454d032029b556133265
Author: Nicolas Mattia nicolas@dfinity.org
Date: Thu Nov 14 15:22:18 2019 +0100

Use releaseTools.aggregate to aggregate

commit 54e2e13973d1603500185936212bdb78ab4d837b
Author: Nicolas Mattia nicolas@dfinity.org
Date: Thu Nov 14 14:39:33 2019 +0100

INF-484: INF-588: INF-581: split rust jobs

This splits the rust jobs further, exporting a single interface for rust packages, exporting the following:

* `build`: the release build
* `debug`: the debug build
* `doc`: the built documentation
* `lint`: the lint checks
* `fileChecks`: extra file checks
* `fmt`: the formatting checks
* `shell`: a shell derivation

Moreover a target called `allChecks` is added, which builds all of the above.

commit 548f4e78dd896ac1cef990a20daf00fbb93a7655
Author: Nicolas Mattia nicolas@dfinity.org
Date: Thu Nov 14 14:38:10 2019 +0100

lib: inline moreutils

commit a9d886e463e3df12439c4e6888d26e4a6902e757
Author: Akhi Singhania akhi@dfinity.org
Date: Thu Nov 14 12:30:49 2019 +0100

Add libfuse

commit 2f236bd89793c2816bed76832f996ec80173d620
Merge: f77e818 a73dfb9
Author: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Date: Wed Nov 13 16:02:53 2019 +0000

Merge pull request #63 from dfinity-lab/nm-split-checks

INF-581: INF-588: Split buildDfinityRustPackage jobs

commit a73dfb98a9a9e1d477a0f8ac05a0613a0f79e465
Author: Nicolas Mattia nicolas@dfinity.org
Date: Wed Nov 13 15:55:02 2019 +0100

INF-581: INF-588: Extract fmt check from rust workspace

commit 99b261dcc45c25d50b5721bd3d52747ebbb39303
Author: Nicolas Mattia nicolas@dfinity.org
Date: Wed Nov 13 13:25:20 2019 +0100

INF-581: INF-588: Extract file checks from rust workspace

commit f77e818eb07ca5731fe97004c4b7524132429128
Merge: bbf39c3 63bd2a5
Author: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Date: Wed Nov 13 10:45:50 2019 +0000

Merge pull request #62 from dfinity-lab/nm-refactor-build-dfn-rs-pkg

INF-581: INF-588: Refactor buildDfinityRustPackage

commit 63bd2a5d6ebfb2885a66bfde431de157057fee6f
Author: Nicolas Mattia nicolas@dfinity.org
Date: Wed Nov 13 10:55:45 2019 +0100

INF-581: INF-588: Refactor buildDfinityRustPackage

paulyoung · 2019-11-22T19:04:56Z

nix/rust-workspace.nix

-      $doc/nix-support/hydra-build-products
-  '';
-})
+# override all derivations and add DFX_ASSETS as an environment variable


We only need DFX_ASSETS for dfx. Do we need to change anything here so that we're not affecting our other Rust packages?

I realize that this is separate to the motivation for this PR, but the comment made me think about this.

Agreed with Paul, we should only add DFX_ASSETS to the rust derivations.

eftychis · 2019-11-25T08:05:02Z

nix/rust-workspace.nix

+    { lint = workspace.lint.overrideAttrs (oldAttrs: {
+      nativeBuildInputs = oldAttrs.nativeBuildInputs ++ [
+        cargo-graph
+        graphviz


@eftychis can you clarify? The SDK seem to use cargo graph, which requires graphviz.

eftychis · 2019-11-25T08:06:41Z

nix/rust-workspace.nix

+  if !lib.isDerivation drv then drv else
+    drv.overrideAttrs (_: {
+      DFX_ASSETS = runCommandNoCC "dfx-assets" {} ''
+        mkdir -p $out


can we get a bit more context here? I would rather not have to rebuild everything.

what do you mean? this is the same as before.

Updates the `common` repo to have split builds: * shell build * format build * lint + doc build * debug build * release build Includes the following commits: commit b35ba23941c5f3b555eba33c144f2cf9e5036cfb Author: Joachim Breitner <mail@joachim-breitner.de> Date: Wed Nov 20 11:57:28 2019 +0100 Do not use lib.inNixShell (#69) * Do not use lib.inNixShell this does not just mean “is the curren derivation the target of `nix-shell`”, but it also turns true when this is a _dependency_ of some `nix-shell` (e.g. Motoko’s). This is bad, because it changes the derivation of `drun`, so although `drun` in in the nix cache, when trying to enter `nix-shell`, it now depends on a _different_ `drun` (one built with `-D warnings`), so our poor people have to watch rust compile stuff. Debuggig this was also tricky, and required help from the nix IRC channel. They said that using `lib.inNixShell` in a derivation like this is a bad idea. So we now omit `-D warnings` simply for the `debug` and `shell` build, where that makes more sense, and do not use `lib.inNixShell`. commit b4e92e6010ad7caee863cb8e74b8c1426f92794a Merge: d862d5d f19329c Author: Nicolas Mattia <nicolas@dfinity.org> Date: Thu Nov 14 18:16:18 2019 +0100 Merge pull request #67 from dfinity-lab/nm-always-touch Touch rust files in preBuild commit f19329c71b275d58f19109ac923110963bfbc2d3 Author: Nicolas Mattia <nicolas@dfinity.org> Date: Thu Nov 14 18:15:09 2019 +0100 Run preBuild and postBuild in lint commit 639866f9ec57523c1f4a53c14205841503f4f8ae Author: Nicolas Mattia <nicolas@dfinity.org> Date: Thu Nov 14 17:49:31 2019 +0100 s/preBuild/preCheck commit 5dfef852592e35e88c86d96280584f4d631d3269 Author: Nicolas Mattia <nicolas@dfinity.org> Date: Thu Nov 14 17:36:10 2019 +0100 Fix standalone test evaluation commit 3df1411493bb6edad6e54664fcc819e28715ed31 Author: Nicolas Mattia <nicolas@dfinity.org> Date: Thu Nov 14 17:21:00 2019 +0100 Touch rust files in preBuild commit d862d5d1c492a2077364d3771645c72e0233ce93 Merge: 8387b5b 0367c94 Author: Nicolas Mattia <nicolas@dfinity.org> Date: Thu Nov 14 16:51:43 2019 +0100 Merge pull request #66 from dfinity-lab/nm-fix-standalone-builder Port standalone builder to new rust builder commit 0367c948b70603b29b0f4ceff45df2c29045fd80 Author: Nicolas Mattia <nicolas@dfinity.org> Date: Thu Nov 14 16:31:21 2019 +0100 Port standalone builder to new rust builder Before this commit, building standalone rust on Linux leads to evaluation errors. commit 8387b5bfd0cd8df0acccdca53feb1c38010eecd3 Merge: 68ddd96 bc868d9 Author: Nicolas Mattia <nicolas@dfinity.org> Date: Thu Nov 14 16:04:10 2019 +0100 Merge pull request #65 from dfinity-lab/nm-split-dfn-further INF-484: INF-588: INF-581: Split rust jobs commit bc868d9fc02230a5e6a0833833b9af26edcd7325 Author: Nicolas Mattia <nicolas@dfinity.org> Date: Thu Nov 14 16:02:03 2019 +0100 Create empty directory for lint job commit 68ddd96f7c6171cb5316b6ca51e137ff8d478e06 Merge: a9d886e d8b3e93 Author: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> Date: Thu Nov 14 14:46:30 2019 +0000 Merge pull request #33 from dfinity-lab/update-niv Update niv commit d8b3e93d9bc8ad7ebe4eacea2bed47bd39c41f93 Author: Nikola Knezevic <nikola@dfinity.org> Date: Tue Oct 29 10:36:01 2019 +0100 Update niv This is to help with a problem in other repos where we had an infinite recursion due to setting up nixpkgs in nix-shell shebang line. commit 765754201b3b899899af232a8c6f8fd4a01d7594 Author: Nicolas Mattia <nicolas@dfinity.org> Date: Thu Nov 14 15:23:36 2019 +0100 Update naersk for faster builds commit 0b7f62cad98e0a6374af1e2496ca86dd7868f485 Author: Nicolas Mattia <nicolas@dfinity.org> Date: Thu Nov 14 15:22:52 2019 +0100 Make doc overridable commit 5d40a3800d3eb2591330884784c2f3e6ac384494 Author: Nicolas Mattia <nicolas@dfinity.org> Date: Thu Nov 14 15:22:38 2019 +0100 Remove unnecessary phase commit 8385c76857644474749f454d032029b556133265 Author: Nicolas Mattia <nicolas@dfinity.org> Date: Thu Nov 14 15:22:18 2019 +0100 Use releaseTools.aggregate to aggregate commit 54e2e13973d1603500185936212bdb78ab4d837b Author: Nicolas Mattia <nicolas@dfinity.org> Date: Thu Nov 14 14:39:33 2019 +0100 INF-484: INF-588: INF-581: split rust jobs This splits the rust jobs further, exporting a single interface for rust packages, exporting the following: * `build`: the release build * `debug`: the debug build * `doc`: the built documentation * `lint`: the lint checks * `fileChecks`: extra file checks * `fmt`: the formatting checks * `shell`: a shell derivation Moreover a target called `allChecks` is added, which builds all of the above. commit 548f4e78dd896ac1cef990a20daf00fbb93a7655 Author: Nicolas Mattia <nicolas@dfinity.org> Date: Thu Nov 14 14:38:10 2019 +0100 lib: inline moreutils commit a9d886e463e3df12439c4e6888d26e4a6902e757 Author: Akhi Singhania <akhi@dfinity.org> Date: Thu Nov 14 12:30:49 2019 +0100 Add libfuse commit 2f236bd89793c2816bed76832f996ec80173d620 Merge: f77e818 a73dfb9 Author: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> Date: Wed Nov 13 16:02:53 2019 +0000 Merge pull request #63 from dfinity-lab/nm-split-checks INF-581: INF-588: Split buildDfinityRustPackage jobs commit a73dfb98a9a9e1d477a0f8ac05a0613a0f79e465 Author: Nicolas Mattia <nicolas@dfinity.org> Date: Wed Nov 13 15:55:02 2019 +0100 INF-581: INF-588: Extract fmt check from rust workspace commit 99b261dcc45c25d50b5721bd3d52747ebbb39303 Author: Nicolas Mattia <nicolas@dfinity.org> Date: Wed Nov 13 13:25:20 2019 +0100 INF-581: INF-588: Extract file checks from rust workspace commit f77e818eb07ca5731fe97004c4b7524132429128 Merge: bbf39c3 63bd2a5 Author: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> Date: Wed Nov 13 10:45:50 2019 +0000 Merge pull request #62 from dfinity-lab/nm-refactor-build-dfn-rs-pkg INF-581: INF-588: Refactor buildDfinityRustPackage commit 63bd2a5d6ebfb2885a66bfde431de157057fee6f Author: Nicolas Mattia <nicolas@dfinity.org> Date: Wed Nov 13 10:55:45 2019 +0100 INF-581: INF-588: Refactor buildDfinityRustPackage

hansl

Overall, LGTM, but the DFX_ASSETS seem overkill.

nmattia requested a review from a team as a code owner November 22, 2019 15:58

paulyoung reviewed Nov 22, 2019

View reviewed changes

eftychis reviewed Nov 25, 2019

View reviewed changes

nmattia force-pushed the nm-update-common-multi-build branch from d4749a8 to f247d12 Compare November 25, 2019 10:40

nmattia added 2 commits November 25, 2019 12:09

Simplify rust-shell.nix

d85325f

Don't build shells twice

5536aa6

nmattia force-pushed the nm-update-common-multi-build branch from bcbe822 to 5536aa6 Compare November 25, 2019 15:11

hansl approved these changes Nov 25, 2019

View reviewed changes

nmattia added the automerge-squash label Nov 25, 2019

mergify bot merged commit 824280a into master Nov 25, 2019

mergify bot deleted the nm-update-common-multi-build branch November 25, 2019 16:12

mergify bot removed the automerge-squash label Nov 25, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Comments

INF-629: Update common to get split builds#200

INF-629: Update common to get split builds#200
mergify[bot] merged 3 commits intomasterfrom
nm-update-common-multi-build

nmattia commented Nov 22, 2019

Uh oh!

paulyoung Nov 22, 2019

Uh oh!

paulyoung Nov 22, 2019

Uh oh!

hansl Nov 25, 2019

Uh oh!

eftychis Nov 25, 2019

Uh oh!

nmattia Nov 25, 2019

Uh oh!

eftychis Nov 25, 2019

Uh oh!

nmattia Nov 25, 2019

Uh oh!

hansl left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Comments

Conversation

nmattia commented Nov 22, 2019

Uh oh!

paulyoung Nov 22, 2019

Choose a reason for hiding this comment

Uh oh!

paulyoung Nov 22, 2019

Choose a reason for hiding this comment

Uh oh!

hansl Nov 25, 2019

Choose a reason for hiding this comment

Uh oh!

eftychis Nov 25, 2019

Choose a reason for hiding this comment

Uh oh!

nmattia Nov 25, 2019

Choose a reason for hiding this comment

Uh oh!

eftychis Nov 25, 2019

Choose a reason for hiding this comment

Uh oh!

nmattia Nov 25, 2019

Choose a reason for hiding this comment

Uh oh!

hansl left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants