I know, heretic, but...
I also know that this is not perfect but it is a good start, I think. It
would be nice if this were part of the automatic "nixdoc" function
reference. I'd like guidance if this should be part of the rust section
or something else.
As it turns out Darwin does most of the things differently then "normal"
systems. They are using a different shared library extension and require
an obscure commandline parameter that has to be added to every build
system out there. That issue seems to be with clang on Darwin as on
Linux that flag isn't required to build the very same tests (when using
clang).
After adjusting these two details the tests are running fine on the
darwin box that I was able to obtain.
According to the Cargo documentation:
> The build script does not have access to the dependencies listed in
> the dependencies or dev-dependencies section (they’re not built
> yet!). Also, build dependencies are not available to the package
> itself unless also explicitly added in the [dependencies] table.
https://doc.rust-lang.org/cargo/reference/build-scripts.html
This change separates linkage of regular dependencies and build
dependencies.
* Make errors include the crate name and make them much more prominent.
* Move more code into lib.sh
* Already source generated logging code and lib.sh in configure
By overriding each dependency on every level of the dependency tree we
are creating a lot of unnecessary instances of the same derivation
Looking at the output size of `nix-instantiate --trace-function-calls
-vvvv …` and the execution time I got about a 10x improvement after
applying this change.
It was probably good intentions that lead to these overrides but in
practice no tooling (that I know of) really needs this. `carnix` and
`crate2nix` are fine without those overrides. Furthermore I believe that
it is the job of the tooling around `buildRustCrate` to provide a
coherent set of overrides. By not enforcing all of the overrides, debug
flags, verbosity, … to be the same throughout the closure we also allow
consumers to override specific aspects of the crates. Some (older?)
crates might need different `crateOverrides` then newer crates with the
same name. Currently such situations can not (easily) be implemented
with the override in-place.
Previously I did use `runCommand` to do the same. Using
releaseTools.aggregate seems a lot saner and we might get nicer hydra
output of the tests that are failing.
It used to be the case (ref missing) that cargo did treat
`src/$libName.rs` as an alternative to `src/lib.rs` when the latter
wasn't present. Recently I failed to reproduce that with vanilla cargo
and it started to cause pain with some crates of the form:
some_crate/
`- src
`- main.rs
`- some_crate.rs
We would build `src/some_crate.rs` and thing it is a library while that
might not be the actual case. This crate is a valid `bin` crate not a
`lib` crate as far as I can tell from the samples I took.
I removed support for the previously required heuristic and commented
out the test cases in case we will need them again. We could crawl in
the Git history but chances are that the next person looking into this
doesn't know about the history.
This helps us instruct rustc to build tests instead of binaries. The
actual build will then ONLY produce test executables. This is a first
step towards having rust crate tests within nixpkgs.
We default back to only a single output in test cases since that is the
only reasonable thing to do here.
Producing libraries or binaries in addition to tests would theoretically
be feasible but usually generates different dependency trees. It is very
common to have some libraries in `[dev-depdendencies]` within Cargo.toml
just for your tests. To not start mixing things up going with a
dedicated derivation for the test build sounds like the best choice for
now.
To use this you must provide a proper test dependency chain to
`buildRustCrate` (as you would usually do with your non-test inputs).
And then set the `buildTests` attribute to `true`. The derivation will
then contain all tests that were built in `$out/tests`. All common test
patterns and directories should be supported and tested by this change.
Below is an example how you would run a single test from the derivation.
This commit contains some more examples in the `buildRustCrateTests`
attribute set that might be helpful.
```
let
drv = buildRustCrate {
…
buildTests true;
};
in runCommand "test-my-crate" {} ''
touch $out
exec ${drv}/tests/my-test
''
```
While unifying most of the lib function calls I accidentially changed
the filterSource functions as well. Since there were no tests I ended
up forgetting about this case (even thought I ran into it…).
The previous lines were only different in the kind of dependencies but
otherwise exactly the same. It makes the entire thing a bit more
readable by moving this into a function that takes care of this.
We can get rid of a bunch of workarounds that were in the build script
before by just passing on the `crateBin` attribute.
Before we converted the list of attributes to a string only to convert
it back in bash during the build phase. We can do the entire looping
through builds in Nix and thus need no conversion and parsing of
attributes over and over again.
The big part that still remains bash is the heuristic that cargo
introduced and that we can't do at eval time.
That code had been in the derivation for a while but no explanation was
given why that is needed. It might be helpful to our future selfs to
document why things are done the way they are.
The expression is already long and confusing enough without the color
stuff sprinkled in. Moving it to a dedicated file makes sense.
I switched a bit of the color support code to pure Nix since there
wasn't much point in doing that in bash while we can just do it in Nix.
We can just use `lib` instead of `builtins` in all cases but the
`hashString` case. Also changed a few lines to make use of some optional
helpers from lib.
This cuts down the dependency tree on some rust builds where a crate not
just exposes a binary but also a library. `$out/lib` contained a bunch
of extra support files that among other information carry linker flags
(including the full path to link-time dependencies). Worst case this led
to some binary outputs depending on the full build closure of rust
crates.
Moving all the `$out/lib` files to `$lib/lib` solves this nicely.
`lib` might be a bit weird here as they are most of the time just rlib
files (rust libraries). Those are essential only required during
compilation but they can also be shared objects (like with traditional
C-style packages). Which is why I went with `lib` for the new output.
One of the caveats we are running into here is that we do not (always)
know ahead of time of a crate produces just a library or just a binary.
Cargo allows for some ambiguity regarding whether or not a crate
provides one, two, … binaries and libraries as it's outputs. Ideally we
would be able to rely on the `crateType` entirely but so far that isn't
the case. More work on that area might show how difficult that actually
is.
Quoting from the splitString docstring:
NOTE: this function is not performant and should never be used.
This replaces trivial uses of splitString for splitting version
strings with the (potentially builtin) splitVersion.
Before this change, buildRustCrate always called rustc with
--extern libName=[...]libName[...]
However, Cargo permits using a different name under which a dependency
is known to a crate. For example, rand 0.7.0 uses:
[dependencies]
getrandom_package = { version = "0.1.1", package = "getrandom", optional = true }
Which introduces the getrandom dependency such that it is known as
getrandom_package to the rand crate. In this case, the correct extern
flag is of the form
--extern getrandom_package=[...]getrandom[...]
which is currently not supported. In order to support such cases, this
change introduces a crateRenames argument to buildRustCrate. This
argument is an attribute set of dependencies that should be renamed. In
this case, crateRenames would be:
{
"getrandom" = "getrandom_package";
}
The extern options are then built such that if the libName occurs as
an attribute in this set, it value will be used as the local
name. Otherwise libName will be used as before.
buildRustCrate has a handy `include` helper, that only imports those whitelisted
files and folders to the store.
However, the function's matching logic is broken and includes all files,
regardless of whether or not they're whitelisted, as long as the whitelist
contains at least one name (regardless of whether that name exists). This is
because it doesn't take into account that
`lib.strings.removePrefix "foo" "bar" == "bar"` (that is, paths that don't match
the prefix are passed straight through).
