Skip to content
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.

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

Reintroducing mkRustCrate #31150

Closed
wants to merge 48 commits into from
Closed
Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
48 commits
Select commit Hold shift + click to select a range
a20704c
Reintroducing mkRustCrate
P-E-Meunier Nov 2, 2017
7979239
mkRustCrate in all-packages.nix
P-E-Meunier Nov 2, 2017
2d85124
Handling packages that don't have a "build=" in their Cargo.toml
P-E-Meunier Nov 3, 2017
00e93d5
Correction after review
P-E-Meunier Nov 3, 2017
4b5aa82
Handling build dependencies for the latest generate-nix-pkg
P-E-Meunier Nov 4, 2017
08046ce
Adding the tool "carnix", compiled with rust-utils.nix
P-E-Meunier Nov 8, 2017
6088db3
Replacing fetchzip with fetchcrate
P-E-Meunier Nov 10, 2017
235a48e
Carnix 0.4.1
P-E-Meunier Nov 10, 2017
6846126
Factorise regular expression
P-E-Meunier Nov 10, 2017
1917e32
Correction after reviews
P-E-Meunier Nov 11, 2017
655f44a
Carnix 0.4.3 (including calls to buildRustCrate instead of mkRustCrate)
P-E-Meunier Nov 11, 2017
cf4ec94
Documenting buildRustCrate
P-E-Meunier Nov 11, 2017
a694f61
Adding myself in maintainers
P-E-Meunier Nov 11, 2017
d0a68ff
Run `cargo build` again after adding dependencies
P-E-Meunier Nov 12, 2017
ca81cad
Carnix 0.4.4
P-E-Meunier Nov 12, 2017
b4c5585
Carnix 0.4.5 + dependencies on sqlite and pkgconfig
P-E-Meunier Nov 12, 2017
4ce91ee
Replacing a rm $(find) with find | xargs rm
P-E-Meunier Nov 12, 2017
1f338c1
Carnix 0.4.6, which can now compile workspaces
P-E-Meunier Nov 14, 2017
4121d59
Overriding the Rust compiler now works.
P-E-Meunier Nov 15, 2017
428f66a
Carnix 0.4.7, updated for the latest Rust nightly
P-E-Meunier Nov 15, 2017
01645cf
rust-utils.nix: using the generic extension for shared objects (darwi…
P-E-Meunier Nov 15, 2017
2d2490a
Carnix 0.4.8, removing constants verbose and release (now overridable…
P-E-Meunier Nov 16, 2017
0387786
mkRustCrate Move rust libraries into a /rlibs dir
FlorentBecker Nov 17, 2017
6ffd5b4
mkRustCrate: correct transitive dependencies
FlorentBecker Nov 18, 2017
7b212b8
Replacing bash ifs with optionalString
P-E-Meunier Nov 18, 2017
f7bed04
runHook
P-E-Meunier Nov 18, 2017
95ed173
Merge branch 'mkRustCrate' of github.com:FlorentBecker/nixpkgs into m…
P-E-Meunier Nov 18, 2017
099e713
BuildInputs overrides
P-E-Meunier Nov 18, 2017
68b7867
Add CARGO_PKG_VERSION_* variables
P-E-Meunier Nov 20, 2017
50b984f
Merging FlorentBecker's recent refactoring
P-E-Meunier Nov 24, 2017
993d376
defaultCrateOverrides without { pkgs } argument
P-E-Meunier Nov 24, 2017
a99ef12
Crate name cannot contain '-'
P-E-Meunier Nov 27, 2017
7e6e3b5
Fixing the names of binaries containing a -
P-E-Meunier Nov 27, 2017
c82be14
Carnix 0.4.9
P-E-Meunier Nov 29, 2017
ea0cf4b
Correct SHA256 for carnix
P-E-Meunier Nov 29, 2017
4c788cc
Carnix 0.4.10
P-E-Meunier Nov 29, 2017
240fc52
Updates after the latest review
P-E-Meunier Nov 30, 2017
0e8ef4b
Fixing a typo
P-E-Meunier Nov 30, 2017
5555123
Carnix 0.4.13
P-E-Meunier Nov 30, 2017
457f9f5
Fixing the "[ integer" error when building binaries
P-E-Meunier Nov 30, 2017
c21a241
Documentation with examples that compile
P-E-Meunier Nov 30, 2017
59d16ef
Carnix 0.4.14
P-E-Meunier Nov 30, 2017
7d7fe21
Carnix src was wrong
P-E-Meunier Dec 1, 2017
fc84be3
Carnix 0.5, including a new way to resolve features
P-E-Meunier Dec 5, 2017
1cd1fca
Explain argument forwarding
P-E-Meunier Dec 11, 2017
cd0f359
rename rust-utils to build-rust-crate
Mic92 Dec 12, 2017
bf70097
rename defaultCrateOverrides to default-crate-overrides
Mic92 Dec 12, 2017
f84ad34
doc/rust: update documentation to reflect latest carnix
Mic92 Dec 12, 2017
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
270 changes: 270 additions & 0 deletions pkgs/build-support/rust/rust-utils.nix
Original file line number Diff line number Diff line change
@@ -0,0 +1,270 @@
# Copyright 2017 Pierre-Étienne Meunier
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

This would be then MIT, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed the copyright notice, it's indeed MIT. Is there a contributors list somewhere?

Copy link
Member

Choose a reason for hiding this comment

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

There is a maintainers list in lib/maintainers.nix, when you add yourself to a package as maintainer in meta.maintainers.

#
# This file is licensed under the Apache-2.0 license, and under the
# MIT license, at your convenience.

{ lib, buildPlatform, stdenv }:

let buildCrate = { crateName, crateVersion, dependencies, complete, crateFeatures, libName, build, release, libPath, crateType, metadata, crateBin, finalBins, verboseBuild }:

let depsDir = builtins.foldl' (deps: dep: deps + " " + dep.out) "" dependencies;
Copy link
Member

Choose a reason for hiding this comment

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

concatStringsSep " " dependencies

completeDepsDir = builtins.foldl' (deps: dep: deps + " " + dep.out) "" complete;
Copy link
Member

Choose a reason for hiding this comment

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

same as above.

deps =
builtins.foldl' (deps: dep:
let extern = lib.strings.replaceStrings ["-"] ["_"] dep.libName; in
deps + (if dep.crateType == "lib" then
" --extern ${extern}=${dep.out}/lib${extern}-${dep.metadata}.rlib"
else
" --extern ${extern}=${dep.out}/lib${extern}-${dep.metadata}.so")
) "" dependencies;
Copy link
Member

Choose a reason for hiding this comment

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

foldl' can be here also replaced by concatMapStringsSep " " (dep: ...) dependencies

optLevel = if release then 3 else 0;
rustcOpts = (if release then "-C opt-level=3" else "-C debuginfo=2");
rustcMeta = "-C metadata=" + metadata + " -C extra-filename=-" + metadata;
Copy link
Member

Choose a reason for hiding this comment

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

rustcMeta = "-C metadata=${metadata} -C extra-filename=-${metadata}";

in ''
Copy link
Member

Choose a reason for hiding this comment

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

buildRustCrate should also provide standard hooks in buildPhase:

  ''
    runHook preBuild
    # ...
    runHook postBuild
  '';


norm="$(printf '\033[0m')" #returns to "normal"
bold="$(printf '\033[0;1m')" #set bold
green="$(printf '\033[0;32m')" #set red
boldgreen="$(printf '\033[0;1;32m')" #set bold, and set red.
Copy link
Member

@Mic92 Mic92 Nov 3, 2017

Choose a reason for hiding this comment

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

It is better use tput here. Otherwise it might print garbage if stdout is not a terminal.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm trying to do this, but I keep getting the error

tput: No value for $TERM and no -T specified

When I set a TERM to my terminal, I get instead:

tput: terminal attributes: No such device or address

The command I'm trying to use is

         echo "`tput setaf 2``tput bold`Building $BUILD (${libName})`tput sgr0`"


mkdir -p target/deps
mkdir -p target/build
chmod uga+w target -R
for i in ${completeDepsDir}; do
ln -s -f $i/*.rlib target/deps #*/
ln -s -f $i/*.so target/deps #*/
if [ -e "$i/link" ]; then
cat $i/link >> target/link
cat $i/link >> target/link.final
fi
if [ -e $i/env ]; then
source $i/env
fi
done
if [ -e target/link ]; then
sort -u target/link > target/link.sorted
mv target/link.sorted target/link
sort -u target/link.final > target/link.final.sorted
mv target/link.final.sorted target/link.final
tr '\n' ' ' < target/link > target/link_
fi
EXTRA_BUILD=""
BUILD_OUT_DIR=""
export CARGO_PKG_NAME=${crateName}
export CARGO_PKG_VERSION=${crateVersion}
export CARGO_CFG_TARGET_ARCH=$(echo ${buildPlatform.system} | sed -e "s/\([^-]*\)-\([^-]*\)/\1/")
Copy link
Member

Choose a reason for hiding this comment

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

buildPlatform.parsed.cpu.name

export CARGO_CFG_TARGET_OS=$(echo ${buildPlatform.system} | sed -e "s/\([^-]*\)-\([^-]*\)/\2/")
Copy link
Member

Choose a reason for hiding this comment

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

buildPlatform.parsed.kernel.name

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Where is the documentation for this? If it doesn't exist, I'd like to add it somewhere.

Copy link
Member

Choose a reason for hiding this comment

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

I found out, while playing the nix-repl:

$ nix-repl
Welcome to Nix version 1.11.15. Type :? for help.
nix-repl> :l <nixpkgs>
Added 7885 variables.
nix-repl> buildPlatform.parsed
{ _type = "system"; abi = { ... }; cpu = { ... }; kernel = { ... }; vendor = { ... }; }
nix-repl>buildPlatform.parsed.kernel
{ _type = "kernel"; execFormat = { ... }; families = { ... }; name = "linux"; }

I think this was merged recently for cross compiling. It comes from lib/systems.

Copy link
Member

@Mic92 Mic92 Nov 3, 2017

Choose a reason for hiding this comment

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

This might belong to the nixpkgs documentation then, located in doc. If you are in hurry we also have a wiki to braindump knowledge: https://nixos.wiki/


export CARGO_CFG_TARGET_ENV="gnu"
export CARGO_MANIFEST_DIR="."
export DEBUG="${toString (!release)}"
export OPT_LEVEL="${toString optLevel}"
export TARGET="${buildPlatform.system}-gnu"
Copy link
Member

Choose a reason for hiding this comment

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

Maybe buildPlatform.config? I have: "x86_64-unknown-linux-gnu"

export HOST="${buildPlatform.system}-gnu"
Copy link
Member

@Mic92 Mic92 Nov 3, 2017

Choose a reason for hiding this comment

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

buildPlatform.config These variables will change later for cross compilation.

export PROFILE=${if release then "release" else "debug"}
export OUT_DIR=$(pwd)/target/build/${crateName}.out

BUILD=""
if [[ ! -z "${build}" ]] ; then
BUILD=${build}
elif [[ -e "build.rs" ]]; then
BUILD="build.rs"
fi
if [[ ! -z "$BUILD" ]] ; then
echo "$boldgreen" "Building $BUILD (${libName})" "$norm"
mkdir -p target/build/${crateName}
if [ -e target/link_ ]; then
rustc --crate-name build_script_build $BUILD --crate-type bin ${rustcOpts} ${crateFeatures} --out-dir target/build/${crateName} --emit=dep-info,link -L dependency=target/deps ${deps} --cap-lints allow $(cat target/link_)
Copy link
Member

@Mic92 Mic92 Nov 3, 2017

Choose a reason for hiding this comment

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

Can you wrap long lines like this for readability?

else
rustc --crate-name build_script_build $BUILD --crate-type bin ${rustcOpts} ${crateFeatures} --out-dir target/build/${crateName} --emit=dep-info,link -L dependency=target/deps ${deps} --cap-lints allow
fi
mkdir -p target/build/${crateName}.out
export RUST_BACKTRACE=1
BUILD_OUT_DIR="-L $OUT_DIR"
mkdir -p $OUT_DIR
target/build/${crateName}/build_script_build > target/build/${crateName}.opt
set +e
EXTRA_BUILD=$(grep "^cargo:rustc-flags=" target/build/${crateName}.opt | sed -e "s/cargo:rustc-flags=\(.*\)/\1/")
Copy link
Member

Choose a reason for hiding this comment

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

Why is both grep and sed required here?

EXTRA_FEATURES=$(grep "^cargo:rustc-cfg=" target/build/${crateName}.opt | sed -e "s/cargo:rustc-cfg=\(.*\)/--cfg \1/")

EXTRA_LINK=$(grep "^cargo:rustc-link-lib=" target/build/${crateName}.opt | sed -e "s/cargo:rustc-link-lib=\(.*\)/\1/")
EXTRA_LINK_SEARCH=$(grep "^cargo:rustc-link-search=" target/build/${crateName}.opt | sed -e "s/cargo:rustc-link-search=\(.*\)/\1/")
CRATENAME=$(echo ${crateName} | sed -e "s/\(.*\)-sys$/\1/" | tr '[:lower:]' '[:upper:]')
Copy link
Member

Choose a reason for hiding this comment

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

- CRATENAME=$(echo ${crateName} | sed -e "s/\(.*\)-sys$/\1/" | tr '[:lower:]' '[:upper:]')
+ CRATENAME=$(echo ${crateName} | sed -e "s/\(.*\)-sys$/\U\1/")


grep "^cargo:" target/build/${crateName}.opt | grep -v "^cargo:rustc-" | grep -v "^cargo:warning=" | grep -v "^cargo:rerun-if-changed=" | grep -v "^cargo:rerun-if-env-changed" | sed -e "s/cargo:\([^=]*\)=\(.*\)/export DEP_$(echo $CRATENAME)_\U\1\E=\2/" > target/env

set -e
if [ -n "$(ls target/build/${crateName}.out)" ]; then

if [ -e "${libPath}" ] ; then
cp -r target/build/${crateName}.out/* $(dirname ${libPath}) #*/
else
cp -r target/build/${crateName}.out/* src #*/
fi
fi
fi
# echo "Features: ${crateFeatures}" $EXTRA_FEATURES

EXTRA_LIB=""
CRATE_NAME=$(echo ${libName} | sed -e "s/-/_/g")
# echo "Libname" ${libName} ${libPath}
# echo "Deps: ${deps}"
if [ -e "${libPath}" ] ; then

echo "$boldgreen" "Building ${libPath}" "$norm"
if ${verboseBuild}; then
echo "$boldgreen" "Running" "$norm" "rustc --crate-name $CRATE_NAME ${libPath} --crate-type ${crateType} ${rustcOpts} ${rustcMeta} ${crateFeatures} --out-dir target/deps --emit=dep-info,link -L dependency=target/deps ${deps} --cap-lints allow $BUILD_OUT_DIR $EXTRA_BUILD $EXTRA_FEATURES"
fi
rustc --crate-name $CRATE_NAME ${libPath} --crate-type ${crateType} ${rustcOpts} ${rustcMeta} ${crateFeatures} --out-dir target/deps --emit=dep-info,link -L dependency=target/deps ${deps} --cap-lints allow $BUILD_OUT_DIR $EXTRA_BUILD $EXTRA_FEATURES
EXTRA_LIB=" --extern $CRATE_NAME=target/deps/lib$CRATE_NAME-${metadata}.rlib"
if [ -e target/deps/lib$CRATE_NAME-${metadata}.so ]; then
EXTRA_LIB="$EXTRA_LIB --extern $CRATE_NAME=target/deps/lib$CRATE_NAME-${metadata}.so"
fi
elif [ -e src/lib.rs ] ; then

echo "$boldgreen" "Building src/lib.rs (${libName})" "$norm"
if ${verboseBuild}; then
echo "$boldgreen" "Running" "$norm" "rustc --crate-name $CRATE_NAME src/lib.rs --crate-type ${crateType} ${rustcOpts} ${rustcMeta} ${crateFeatures} --out-dir target/deps --emit=dep-info,link -L dependency=target/deps ${deps} --cap-lints allow $BUILD_OUT_DIR $EXTRA_BUILD $EXTRA_FEATURES";
fi

rustc --crate-name $CRATE_NAME src/lib.rs --crate-type ${crateType} ${rustcOpts} ${rustcMeta} ${crateFeatures} --out-dir target/deps --emit=dep-info,link -L dependency=target/deps ${deps} --cap-lints allow $BUILD_OUT_DIR $EXTRA_BUILD $EXTRA_FEATURES
EXTRA_LIB=" --extern $CRATE_NAME=target/deps/lib$CRATE_NAME-${metadata}.rlib"
if [ -e target/deps/lib$CRATE_NAME-${metadata}.so ]; then
EXTRA_LIB="$EXTRA_LIB --extern $CRATE_NAME=target/deps/lib$CRATE_NAME-${metadata}.so"
fi

elif [ -e src/${libName}.rs ] ; then

echo "$boldgreen" "Building src/${libName}.rs" "$norm"

if ${verboseBuild}; then
echo "rustc --crate-name $CRATE_NAME src/${libName}.rs --crate-type ${crateType} ${rustcOpts} ${rustcMeta} ${crateFeatures} --out-dir target/deps --emit=dep-info,link -L dependency=target/deps ${deps} --cap-lints allow $BUILD_OUT_DIR $EXTRA_BUILD $EXTRA_FEATURES"
fi

rustc --crate-name $CRATE_NAME src/${libName}.rs --crate-type ${crateType} ${rustcOpts} ${rustcMeta} ${crateFeatures} --out-dir target/deps --emit=dep-info,link -L dependency=target/deps ${deps} --cap-lints allow $BUILD_OUT_DIR $EXTRA_BUILD $EXTRA_FEATURES
EXTRA_LIB=" --extern $CRATE_NAME=target/deps/lib$CRATE_NAME-${metadata}.rlib"
if [ -e target/deps/lib$CRATE_NAME-${metadata}.so ]; then
EXTRA_LIB="$EXTRA_LIB --extern $CRATE_NAME=target/deps/lib$CRATE_NAME-${metadata}.so"
fi

fi

echo "$EXTRA_LINK_SEARCH" | while read i; do
if [ ! -z "$i" ]; then
echo "-L $i" >> target/link
L=$(echo $i | sed -e "s#$(pwd)/target/build#$out#")
echo "-L $L" >> target/link.final
fi
done
echo "$EXTRA_LINK" | while read i; do
if [ ! -z "$i" ]; then
echo "-l $i" >> target/link
echo "-l $i" >> target/link.final
fi
done

if [ -e target/link ]; then
sort -u target/link.final > target/link.final.sorted
mv target/link.final.sorted target/link.final
sort -u target/link > target/link.sorted
mv target/link.sorted target/link

tr '\n' ' ' < target/link > target/link_
LINK=$(cat target/link_)
fi

mkdir -p target/bin
echo "${crateBin}" | sed -n 1'p' | tr ',' '\n' | while read BIN; do
Copy link
Member

@Mic92 Mic92 Nov 29, 2017

Choose a reason for hiding this comment

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

I was scratching my head, what this sed command does:

- echo "${crateBin}" | sed -n 1'p' | tr ',' '\n' | while read BIN; do
+ echo "${crateBin}" | head -n 1 | tr ',' '\n' | while read BIN; do

UPDATE the above does something else.

Copy link
Member

Choose a reason for hiding this comment

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

Why is this actually needed?

Copy link
Member

Choose a reason for hiding this comment

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

Could tr ',' '\n' also replaced by IFS=',' for the loop?

if [ ! -z "$BIN" ]; then
printf "%s" "$boldgreen"
echo "Building $BIN"
printf "%s" "$norm"
if ${verboseBuild}; then
echo "$boldgreen" "Running" "$norm" " rustc --crate-name $BIN --crate-type bin ${rustcOpts} ${crateFeatures} --out-dir target/bin --emit=dep-info,link -L dependency=target/deps $LINK ${deps}$EXTRA_LIB --cap-lints allow $BUILD_OUT_DIR $EXTRA_BUILD $EXTRA_FEATURES"
fi
rustc --crate-name $BIN --crate-type bin ${rustcOpts} ${crateFeatures} --out-dir target/bin --emit=dep-info,link -L dependency=target/deps $LINK ${deps}$EXTRA_LIB --cap-lints allow $BUILD_OUT_DIR $EXTRA_BUILD $EXTRA_FEATURES
fi
done
if [[ (-z "${crateBin}") && (-e src/main.rs) ]]; then
printf "%s" "$boldgreen"
echo "Building src/main.rs"
printf "%s" "$norm"
if ${verboseBuild}; then
echo "$boldgreen" "Running" "$norm" "rustc --crate-name $CRATE_NAME src/main.rs --crate-type bin ${rustcOpts} ${crateFeatures} --out-dir target/bin --emit=dep-info,link -L dependency=target/deps $LINK ${deps}$EXTRA_LIB --cap-lints allow $BUILD_OUT_DIR $EXTRA_BUILD $EXTRA_FEATURES"
fi
rustc --crate-name $CRATE_NAME src/main.rs --crate-type bin ${rustcOpts} ${crateFeatures} --out-dir target/bin --emit=dep-info,link -L dependency=target/deps $LINK ${deps}$EXTRA_LIB --cap-lints allow $BUILD_OUT_DIR $EXTRA_BUILD $EXTRA_FEATURES
fi
Copy link
Member

Choose a reason for hiding this comment

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

Cargo will also build any file in bin/*.rs as executable. The following code will enable that:

diff --git a/pkgs/build-support/rust/rust-utils.nix b/pkgs/build-support/rust/rust-utils.nix
index 0e9458bab11..5070186f07c 100644
--- a/pkgs/build-support/rust/rust-utils.nix
+++ b/pkgs/build-support/rust/rust-utils.nix
@@ -257,9 +257,14 @@ let buildCrate = { crateName, crateVersion, crateAuthors, buildDependencies,
            build_bin $BIN
          fi
       done
-      if [[ (-z "${crateBin}") && (-e src/main.rs) ]]; then
-         build_bin ${crateName} src/main.rs
-      fi
+      ${lib.optionalString (crateBin == "") ''
+        if [[ -e src/main.rs ]]; then
+          build_bin ${crateName} src/main.rs
+        fi
+        for i in src/bin/*.rs; do
+          build_bin "$(basename $i .rs)" "$i"
+        done
+      ''}
       # Remove object files to avoid "wrong ELF type"
       find target -type f -name "*.o" -print0 | xargs -0 rm -f
       runHook postBuild

tested with ofborg

Copy link
Contributor

@boxofrox boxofrox Nov 30, 2017

Choose a reason for hiding this comment

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

FYI, cargo doesn't always build bin/*.rs. rust-lang/cargo#4013

Copy link
Member

Choose a reason for hiding this comment

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

This is a nightmare. How do we handle this? Without good multiple-binary support, this patchset feels not useful.

Copy link
Contributor

@boxofrox boxofrox Nov 30, 2017

Choose a reason for hiding this comment

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

I mentioned the FYI only because I don't fully understand how this patch set works... yet, and wanted to make sure this wasn't overlooked.

It may already be handled. I have a Rust project I'm packaging to run on NixOS that has two [[bin]] sections in the Cargo.toml. After running my Cargo.lock through carnix, my nix package builds both binaries in the crate just fine.

The best I can figure is carnix emulates cargo's behavior here. Carnix does parse the Cargo.toml and if it finds [[bin]] sections, it provides a list of bin files as crateBin. Otherwise, it provides an empty list that will trigger Mic92's for loop. There's a bit of speculation on my part in there.

Copy link
Member

Choose a reason for hiding this comment

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

Oh I see, if I declared my binaries in the Cargo.toml it wouldn't require this patch? Great! Makes sense, thank you for the clarification.

Copy link
Contributor

@boxofrox boxofrox Nov 30, 2017

Choose a reason for hiding this comment

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

To better illustrate my example, here's a gist [1] of the carnix output for my project. Towards the bottom is a more condensed version with a layout of my project directory. The crateBin looks as I'd expect.

I'll run a few more tests and report back.

[1]: https://gist.github.com/boxofrox/59b0a7fe4cce97e9c5753aee50bd25d9

Copy link
Member

Choose a reason for hiding this comment

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

@boxofrox yes, you are right, we do the same as cargo and skip building main.rs and bin/*.rs in the presents of [[bin]] section.

Copy link
Contributor

Choose a reason for hiding this comment

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

Great. And the tests I ran--disabling some and all [[bin]] sections--produced nix files that reflect this behavior when ran through nix-build. This patch completes the imitation of cargo. Cheers.

'' + finalBins;

installCrate = crateName: ''
Copy link
Member

@Mic92 Mic92 Nov 17, 2017

Choose a reason for hiding this comment

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

same for installPhase:

installCrate = crateName: ''
  runHook preInstall
  ...
  runHook postInstall
'';

mkdir -p $out
if [ -s target/env ]; then
cp target/env $out/env
fi
if [ -s target/link.final ]; then
cp target/link.final $out/link
fi
cp target/deps/* $out # */
if [ "$(ls -A target/build)" ]; then # */
cp -r target/build/* $out # */
fi
if [ "$(ls -A target/bin)" ]; then # */
mkdir -p $out/bin
cp -P target/bin/* $out/bin # */
fi
'';
in

crate: rust: stdenv.mkDerivation rec {
Copy link
Member

Choose a reason for hiding this comment

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

I see why you have added these parameter (to use different rust versions). But should this not be also possible
by using `mkRustCrate.override ({rust = rust_foo;})?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't believe many Rust users use the default Rust from nixpkgs (of course, I might be wrong), since the overlay is much more convenient. What are the pros and cons of doing this?

Copy link
Member

@Mic92 Mic92 Nov 10, 2017

Choose a reason for hiding this comment

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

It is the documented way to override dependencies in derivations. If you want your own version of this function, you would do:

my-rust-crates = callPackage ../my-rust-crates {
  mkRustCrate = mkRustCrate.override ({rust = rustNightly;})
};

It would not limit what rust developer would like to use as a rust version, but still sets a default for nixpkgs and non-developer, who just want to build a rust application.

Copy link
Contributor

Choose a reason for hiding this comment

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

This would be useful as there are some features only available on rustNightly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done! It is actually more elegant (even though I had to modify all my projects' default.nix).


inherit (crate) crateName src;

release = if crate ? release then crate.release else false;
name = "rust_${crate.crateName}-${crate.version}";
buildInputs = [ rust ] ++ (lib.attrByPath ["buildInputs"] [] crate);
dependencies = builtins.map (dep: dep rust) (lib.attrByPath ["dependencies"] [] crate);

complete = builtins.foldl' (comp: dep: if lib.lists.any (x: x == comp) dep.complete then comp ++ dep.complete else comp) dependencies dependencies;

crateFeatures = if crate ? features then
builtins.foldl' (features: f: features + " --cfg feature=\\\"${f}\\\"") "" crate.features
Copy link
Member

Choose a reason for hiding this comment

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

Can be also replaced by concatStringsSep.

else "";

libName = if crate ? libName then crate.libName else crate.crateName;
libPath = if crate ? libPath then crate.libPath else "";

metadata = builtins.substring 0 10 (builtins.hashString "sha256" (crateName + "-" + crateVersion));

crateBin = if crate ? crateBin then
builtins.foldl' (bins: bin:
let name =
lib.strings.replaceStrings ["-"] ["_"]
(if bin ? name then bin.name else crateName);
path = if bin ? path then bin.path else "src/main.rs";
in
bins + (if bin == "" then "" else ",") + "${name} ${path}"

) "" crate.crateBin
else "";

finalBins = if crate ? crateBin then
builtins.foldl' (bins: bin:
let name = lib.strings.replaceStrings ["-"] ["_"]
(if bin ? name then bin.name else crateName);
new_name = if bin ? name then bin.name else crateName;
in
if name == new_name then bins else
(bins + "mv target/bin/${name} target/bin/${new_name};")

) "" crate.crateBin
else "";

build = if crate ? build then crate.build else "";
crateVersion = crate.version;
crateType =
if lib.attrByPath ["procMacro"] false crate then "proc-macro" else
if lib.attrByPath ["plugin"] false crate then "dylib" else "lib";
verboseBuild = if lib.attrByPath [ "verbose" ] false crate then "true" else "false";
buildPhase = buildCrate { inherit crateName dependencies complete crateFeatures libName build release libPath crateType crateVersion metadata crateBin finalBins verboseBuild; };
installPhase = installCrate crateName;
}
2 changes: 2 additions & 0 deletions pkgs/top-level/all-packages.nix
Original file line number Diff line number Diff line change
Expand Up @@ -6215,6 +6215,8 @@ with pkgs;
rust = callPackage ../development/compilers/rust { };
inherit (rust) cargo rustc;

mkRustCrate = callPackage ../build-support/rust/rust-utils.nix { };
Copy link
Member

@Mic92 Mic92 Nov 10, 2017

Choose a reason for hiding this comment

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

I usually try to avoid those discussion about naming things. Would like to stay with the name mkRustCrate to differentiate from buildRustPackage? Or would it not be better to name it buildRustCrate and become consistent with the other builders: buildGoPackage, buildRubyGem, buildPythonPackage, buildPerlPackage, buildNodePackage? (not that we are very consistent in nixpkgs as we could be).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I like buildRustCrate better, but don't have a strong opinion.

Copy link
Contributor

Choose a reason for hiding this comment

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

This solution only builds rust crates? There are other artefacts possible such as shared objects. by using buildRustPackage you gave a generic enough name to express intent.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ultimately, this solution should build anything cargo build can build. It already does, for many crates on crates.io (and my own projects).

Copy link
Contributor

Choose a reason for hiding this comment

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

yes indeed it does, I use your solution on my projects too. Though it's customized. I'll probably be removing that code in favour of this code. I renamed it to build-rust-package or to that effect and added an attribute type = "crate". I have a few other build types already. (see https://github.com/fractalide/fractalide/blob/master/modules/rs/crates/default.nix#L62-L72 for more details)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, changed the name to buildRustCrate. Nix's documentation is "light" enough that being at least consistent with other languages already provide some documentation. I'll add more documentation for this tool now, though.


rustPlatform = recurseIntoAttrs (makeRustPlatform rust);

makeRustPlatform = rust: lib.fix (self:
Expand Down