Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mergify merged 3 commits into from
Nov 25, 2019
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f247d12
INF-629: Update common to get split builds
nmattia Nov 22, 2019
d85325f
Simplify rust-shell.nix
nmattia Nov 25, 2019
5536aa6
Don't build shells twice
nmattia Nov 25, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion nix/default.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ let
else builtins.fetchGit {
name = "common-sources";
url = "ssh://git@github.com/dfinity-lab/common";
rev = "bbf39c31e08b4ab7db82f799a3e3c693a0fdced6";
rev = "b35ba23941c5f3b555eba33c144f2cf9e5036cfb";
};
in import commonSrc {
inherit system crossSystem config;
Expand Down
24 changes: 11 additions & 13 deletions nix/overlays/dfinity-sdk.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,23 +1,21 @@
self: super:
let
mkRelease = super.callPackage ./mk-release.nix {};

rust-package' = super.callPackage ../rust-workspace.nix {};
# remove some stuff leftover by callPackage
rust-package = removeAttrs rust-package'
[ "override" "overrideDerivation" ];
rust-workspace = rust-package.build;
in {
dfinity-sdk = rec {
packages = rec {
packages =
# remove the shell since it's being built below in "shells"
removeAttrs rust-package [ "shell" ] // rec {
inherit rust-workspace;
rust-workspace-debug = rust-package.debug;
js-user-library = super.callPackage ../../js-user-library/package.nix {
inherit (self) napalm;
};
rust-workspace = super.callPackage ../rust-workspace.nix {};
rust-workspace-debug = (rust-workspace.override (_: {
release = false;
doClippy = true;
doFmt = true;
doDoc = true;
})).overrideAttrs (oldAttrs: {
name = "${oldAttrs.name}-debug";
});
rust-workspace-doc = rust-workspace-debug.doc;

rust-workspace-standalone = super.lib.standaloneRust
{ drv = rust-workspace;
Expand Down Expand Up @@ -136,7 +134,7 @@ in {
# of all the shells here.
shells = {
js-user-library = import ../../js-user-library/shell.nix { pkgs = self; };
rust-workspace = import ../rust-shell.nix { pkgs = self; };
rust-workspace = import ../rust-shell.nix { pkgs = self; shell = rust-package.shell; };
};

licenses = {
Expand Down
18 changes: 3 additions & 15 deletions nix/rust-shell.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,21 +1,9 @@
{ pkgs ? (import ./. {}).pkgs }:
{ pkgs ? (import ./. {}).pkgs, shell }:
pkgs.mkCompositeShell {
name = "dfinity-sdk-rust-env";
buildInputs = [pkgs.rls];
inputsFrom = [

(pkgs.dfinity-sdk.packages.rust-workspace-debug.overrideAttrs (oldAttrs: {
# _oldAttrs.configurePhase refers to the dfinity-application-and-others-deps
# derivation which is the build of all 3rd-party Rust dependencies. Since in this
# nix-shell we use cargo locally to build all dependencies we don't need to depend
# on this derivation saving a lot of time downloading/building.
configurePhase = "";

# for some odd reason this is needed in the shell:
# https://dfinity.atlassian.net/browse/INF-542
nativeBuildInputs = oldAttrs.nativeBuildInputs ++ [ pkgs.stdenv.cc ];
})) ];

nativeBuildInputs = [ pkgs.stdenv.cc ];
inputsFrom = [ shell ];
shellHook = ''
# Set CARGO_HOME to minimize interaction with any environment outside nix
export CARGO_HOME=${pkgs.lib.dfinityRoot}/.cargo-home
Expand Down
72 changes: 38 additions & 34 deletions nix/rust-workspace.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,4 @@
{ release ? true
, doClippy ? false
, doFmt ? false
, doDoc ? false
, motoko
{ motoko
, buildDfinityRustPackage
, cargo-graph
, darwin
Expand All @@ -15,7 +11,7 @@
, dfinity-sdk
}:
let
drv = buildDfinityRustPackage {
workspace = buildDfinityRustPackage {
name = "dfinity-sdk-rust";
srcDir = ../.;
regexes = [
Expand All @@ -26,35 +22,43 @@ let
".*Cargo\.lock$"
"^.cargo/config$"
];
inherit release doClippy doFmt doDoc;
};
in
drv.overrideAttrs (oldAttrs: {
DFX_ASSETS = runCommandNoCC "dfx-assets" {} ''
mkdir -p $out
cp ${dfinity.nodemanager}/bin/nodemanager $out
cp ${dfinity.ic-client}/bin/client $out
cp ${motoko.moc-bin}/bin/moc $out
cp ${motoko.mo-ide}/bin/mo-ide $out
cp ${motoko.didc}/bin/didc $out
cp ${motoko.rts}/rts/mo-rts.wasm $out
mkdir $out/stdlib && cp -R ${motoko.stdlib}/. $out/stdlib
mkdir $out/js-user-library && cp -R ${dfinity-sdk.packages.js-user-library}/. $out/js-user-library
'';
workspace' = (workspace //
{ lint = workspace.lint.overrideAttrs (oldAttrs: {
nativeBuildInputs = oldAttrs.nativeBuildInputs ++ [
cargo-graph
graphviz
Copy link
Contributor

@eftychis eftychis Nov 25, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

?

Copy link
Contributor Author

@nmattia nmattia Nov 25, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

];

nativeBuildInputs = oldAttrs.nativeBuildInputs ++ lib.optionals doDoc [
cargo-graph
graphviz
];
postDoc = oldAttrs.postDoc + ''
pushd dfx
cargo graph | dot -Tsvg > ../target/doc/dfx/cargo-graph.svg
popd
'';

postDoc = oldAttrs.postDoc + ''
pushd dfx
cargo graph | dot -Tsvg > ../target/doc/dfx/cargo-graph.svg
popd
'';
postInstall = oldAttrs.postInstall + ''
echo "report cargo-graph-dfx $doc dfx/cargo-graph.svg" >> \
$doc/nix-support/hydra-build-products
'';
});
});
in

postInstall = oldAttrs.postInstall + lib.optionalString doDoc ''
echo "report cargo-graph-dfx $doc dfx/cargo-graph.svg" >> \
$doc/nix-support/hydra-build-products
'';
})
# override all derivations and add DFX_ASSETS as an environment variable
Copy link
Contributor

@paulyoung paulyoung Nov 22, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

@paulyoung paulyoung Nov 22, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

@hansl hansl Nov 25, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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

@eftychis eftychis Nov 25, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

@nmattia nmattia Nov 25, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

cp ${dfinity.nodemanager}/bin/nodemanager $out
cp ${dfinity.ic-client}/bin/client $out
cp ${motoko.moc-bin}/bin/moc $out
cp ${motoko.mo-ide}/bin/mo-ide $out
cp ${motoko.didc}/bin/didc $out
cp ${motoko.rts}/rts/mo-rts.wasm $out
mkdir $out/stdlib && cp -R ${motoko.stdlib}/. $out/stdlib
mkdir $out/js-user-library && cp -R ${dfinity-sdk.packages.js-user-library}/. $out/js-user-library
'';
})
) workspace')
(throw "this argument is used to trigger the functor and shouldn't actually be evaluated.")