feature | start-date | author | co-authors | related-issues |
---|---|---|---|---|
ergonomic_cmakeFlags |
2016-08-20 |
Aneesh Agrawal |
(find a buddy later to help out with the RFC) |
Make usage of cmakeFlags
in Nixpkgs more ergonomic
by representing them as structured data in the form of attribute sets,
and passing them more intelligently to the builder.
Improvements:
- Enable first-class Nix handling of
cmakeFlags
, including easier extension and overriding - Handle spaces, newlines and other whitespace correctly
- Make it possible to reference build-time environment variables directly
There are two main parts to this change:
- Using attribute sets for
cmakeFlags
in Nix - Passing
cmakeFlags
to the builder more intelligently
Almost all flags passed to cmakeFlags
are setting CMake cache options
with "-D<name>=<value>"
or "-D<name>:<type>=<value>"
.
Because the CMake cache is essentially a set of keys and values,
model cmakeFlags
as an attribute set in Nix as well.
mkDerivation
will take an incoming cmakeFlags
set and convert each
key/value pair to an appropriate "-D"
option for CMake. String values
are used as is, while integers are converted to strings and booleans are
converted to "ON"
and "OFF"
. This presents the following benefits:
- Removal of
"-D"
visual noise everywhere for ease of reading. - The
=
operater is literal, allowing extra whitespace for more ease of reading. - CMake options are now first class Nix values that can be operated on
directly with the full strength of the Nix language. For example,
boolean values can be used directly because the serialization logic is
consolidated into
mkDerivation
, getting rid of helpers likeedf
and making it easier to re-use feature flags likepythonSupport
. - Sets automatically sort options, improving cache re-use.
- An override which updates a CMake flag option will only incur
(amortized) O(1) cost with a set, as opposed to O(n) cost with an
(unsorted) list, which would require a full linear search. It's also
shorter to write: just use
//
.
A few other CMake flag types are supported:
- Setting a value to null will cause its key to be unset via
"-U<name>"
. - The
generator
key is mapped to the"-G<value>"
option. - The
extraArgs
key is an escape hatch; any flags in this list of strings are not pre-processed, but concatenated to the generated list.
Because cmakeFlags
will now be processed by mkDerivation
,
overrideAttrs
should be used instead of overrideDerivation
when overriding cmakeFlags
.
TODO: Add example(s)
Nix's regular conversion of lists to an environment variable
simply concatenates the contents with spaces,
which breaks if any of the flags themselves contain spaces.
cmakeFlagsArray
was added as a way to get around this limitation,
but is unsatisfactory to use because items must be appended via bash
in a preConfigure hook.
Instead, pass the list to the builder in a smarter way:
instead of relying on Nix's conversion to a string,
perform our own conversion by escaping double quotes,
double quoting each item, and passing the flags as a Bash array,
which is hydrated via eval
.
This handles flags with spaces, newlines and other whitespace,
as well as double quotes, faithfully.
We already use eval
heavily in Nixpkgs builders,
so this does not pose any additional security risks.
Additionally, the use of eval makes it possible to reference build-time
environment variables such as $out
directly when setting cmakeFlags
,
instead of needing to set up these flags in preConfigure
.
Make the list available during preConfigure as a bash array, so any dynamic modifications to the CMake flags can be done there.
These changes make also cmakeFlagsArray
redundant,
so remove and replace it in all cases with cmakeFlags
.
TODO: Add example(s)
There is already a WIP PR in Nixpkgs for an earlier version of these changes,
at NixOS/nixpkgs#17886.
Locally I have a version of this patch that implements this updated behavior.
If accepted, there are currently 319 files in Nixpkgs containing cmakeFlags
.
My local patch already has updates for some of these packages;
if this RFC is accepted I will update the rest.
This is a small enough number to do by hand in a few days.
This complicates mkDerivation
with code used only by a subset of packages,
which will increase evaluation time (TODO: quantify impact)
and decrease modularity due to special casing cmakeFlags
.
Finding a way around this is one of the unresolved questions.
This causes a fair amount of code churn and may cause regressions. Careful review and testing, plus potentially a temporary Hydra jobset, should hopefully minimize any regressions.
Keep the existing mechanism, which works (but is awkward).
I would like to find a way to move
the cmakeFlags
related code into the CMake-related derivations
so that it is automatically included when cmake
is in buildInputs
,
instead of putting this code in mkDerivation
.
This should remove any evaluation overhead if CMake is not being used
and make for a cleaner implementation.
If this is successful, I would hope to see similar approaches taken for
e.g. configureFlags
, and in general more uses of structured data passing.