refactor: add more errors and better details to the build command #69

hansl · 2019-10-09T21:45:48Z

And add a negative test for the build command.

eftychis · 2019-10-09T22:25:06Z

dfx/src/config/cache.rs

@@ -85,9 +85,7 @@ pub fn get_binary_path_from_version(version: &str, binary_name: &str) -> Result<

 pub fn binary_command_from_version(version: &str, name: &str) -> Result<std::process::Command> {
    let path = get_binary_path_from_version(version, name)?;
-    let mut cmd = std::process::Command::new(path);
-    cmd.stdout(std::process::Stdio::inherit());


what was this printout for?

We showed the output of the command to stdout, but really there's no need. If we need to pipe it to STDOUT we should do so in the command.

dfx/src/lib/error.rs

Co-Authored-By: eftychis <eftychis@users.noreply.github.com>

dfx/src/main.rs

eftychis · 2019-10-09T22:32:17Z

dfx/src/main.rs

+            ::std::process::exit(255)
+        }
+        Err(err) => {
+            eprintln!("An error occured:\n{:#?}", err);


Also nitpick I think you can drop the ? (errors implement Display)

DfxError (the generic one) does not implement display. Only BuildErrorKind.

oh weird -- we are breaking protocol 😛

eftychis · 2019-10-09T22:32:58Z

e2e/utils/assertions.bash

@@ -34,11 +34,11 @@ assert_command_fail() {
 #    $2 - The string to match against (output). By default it will use
 #         $output.
 assert_match() {
-    regex=$1
+    regex="$1"


maybe {} around?

Doesn't matter, and we don't do it if there's no need elsewhere. Separate PR?

yeah nvm it is $1.

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

* INF-629: Update common to get split builds 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 * Simplify rust-shell.nix * Don't build shells twice

refactor: add more errors and better details to the build command

dee1fc7

And add a negative test for the build command.

hansl requested a review from a team as a code owner October 9, 2019 21:45

eftychis reviewed Oct 9, 2019

View reviewed changes

dfx/src/lib/error.rs Outdated Show resolved Hide resolved

Update dfx/src/lib/error.rs

2f2bb74

Co-Authored-By: eftychis <eftychis@users.noreply.github.com>

eftychis reviewed Oct 9, 2019

View reviewed changes

dfx/src/main.rs Outdated Show resolved Hide resolved

eftychis reviewed Oct 9, 2019

View reviewed changes

dfx/src/main.rs Outdated Show resolved Hide resolved

fixup

5c92aef

eftychis reviewed Oct 9, 2019

View reviewed changes

eftychis approved these changes Oct 9, 2019

View reviewed changes

hansl merged commit 97da4a0 into master Oct 9, 2019

hansl deleted the hansl/error-messages branch October 9, 2019 22:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: add more errors and better details to the build command #69

refactor: add more errors and better details to the build command #69

hansl commented Oct 9, 2019

eftychis Oct 9, 2019

hansl Oct 9, 2019

eftychis Oct 9, 2019

hansl Oct 9, 2019

eftychis Oct 9, 2019

eftychis Oct 9, 2019

hansl Oct 9, 2019

eftychis Oct 9, 2019

refactor: add more errors and better details to the build command #69

refactor: add more errors and better details to the build command #69

Conversation

hansl commented Oct 9, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment