WIP/RFC: stdenv: deprecate passing a string for configureFlags #15799
Conversation
|
It's currently very hard to do deprecation warnings in Nix (we don't have a good way), for example those will show up in |
|
See discussion in #15357 |
Currently, configureFlags can be passed as a string or a list.
Passing a string works OK for small cases but it becomes hard to manage
interspersing spaces between the flags with many options, and doesn't
match the semantics of each flag being a separate item.
Passing a list (of strings) is a better fit for the type of data, but
it currently relies on Nix's regular conversion to an environment
variable, which simply concatenates the contents with spaces.
This breaks if any of the configure flags themselves contain spaces.
configureFlagsArray 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.
So, update configureFlags to be more ergonomic:
- Deprecate passing a string for configureFlags (require a list),
and include helpful trace/abort messages.
- 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
interspersing tabs. Tabs should be fairly rare in configureFlags;
in case a literal tab needs to be passed, it must be given extra
escapes:
configureFlags = [ "--with-some-arg-with-a-\\\t-character" ]
(The only alternative would require usage of eval.)
This also fixes passing flags that contain spaces.
- Make the list available during preConfigure as a bash array, so any
dynamic modifications to the configure flags can be done there.
There are roughly 1500 uses of configureFlags at present, which is too
many to change all at one time, and it is a commonly used flag (e.g.
in packages outside of nixpkgs). So, use a generous deprecation
timetable: start warning in the next release (16.09), and convert to a
hard error in the release after that (17.03). Since we are mid-way
between releases, this gives 9 months and 1.5 releases for users to
upgrade; switching from strings to lists is not too hard.
These changes make also configureFlagsArray redundant. There are roughly
70 uses of configureFlagsArray at present, which is enough to remove all
at once (in a future patchset).
This is a WIP that I'm posting as an RFC; I've updated enough files
to be able to build bash successfully. More to come if this is
welcomed, but I'd like to get help with converting all the uses.
I'd also like to know which docs need to be updated.
aneeshusa
force-pushed
the
make-configureFlags-more-ergonomic
branch
from
3a71bc3
to
9e7f629
Compare
The general ideaWhen I think of it, most of list-type (The attrsets suggested in another PR would best be converted to a list of strings in the nix language already.) Transition periodTo avoid necessity of converting every single package at once, |
|
@vcunat to be clear - in the other PR (#17886), mkDerivation converts attrset -> list of strings ( No problem with waiting for 16.09 branch-off. This PR needs to be updated (e.g. ad automatic tab escaping), and I've realized after #17886 this approach works much better with I agree that throwing warnings at end-users is not the most helpful, but there should be some way of notifying maintainers of these packages to update them. In this case since both strings and lists are supported now we could ask maintainers to start changing |
|
Passing an array of strings is equivalent to passing a concatenated string interspersed with spaces. So deprecating strings doesn't really accomplish anything except a huge amount of code churn. The reason for However, it would be great if Nix had a way to pass real arrays (and even simple attribute sets) to builders. But that would require some not entirely trivial changes (e.g. the .drv file format would have to support arrays in environment variables) |
|
@edolstra: you would have to read more carefully, as the point of this PR is to side-step the default way of how lists are passed to the builder (and it uses tabs instead of spaces ATM). |
|
It's possible the number of non-list uses of |
|
I like this a lot, what's left to discuss/fix? |
|
I think if we rebase a lot of this will shrink dramatically. That's what I remember from my |
|
Closing in favor of #45886. |
Motivation for this change
See below.
Things done
(nix.useSandbox on NixOS,
or option
build-use-sandboxinnix.confon non-NixOS)
nix-shell -p nox --run "nox-review wip"./result/bin/)Currently, configureFlags can be passed as a string or a list.
Passing a string works OK for small cases but it becomes hard to manage
interspersing spaces between the flags with many options, and doesn't
match the semantics of each flag being a separate item.
Passing a list (of strings) is a better fit for the type of data, but
it currently relies on Nix's regular conversion to an environment
variable, which simply concatenates the contents with spaces.
This breaks if any of the configure flags themselves contain spaces.
configureFlagsArray 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.
So, update configureFlags to be more ergonomic:
and include helpful trace/abort messages.
on Nix's conversion to a string, perform our own conversion by
interspersing tabs. Tabs should be fairly rare in configureFlags;
in case a literal tab needs to be passed, it must be given extra
escapes:
configureFlags = [ "--with-some-arg-with-a-\t-character" ](The only alternative would require usage of eval.)
This also fixes passing flags that contain spaces.
dynamic modifications to the configure flags can be done there.
There are roughly 1500 uses of configureFlags at present, which is too
many to change all at one time, and it is a commonly used flag (e.g.
in packages outside of nixpkgs). So, use a generous deprecation
timetable: start warning in the next release (16.09), and convert to a
hard error in the release after that (17.03). Since we are mid-way
between releases, this gives 9 months and 1.5 releases for users to
upgrade; switching from strings to lists is not too hard.
These changes make also configureFlagsArray redundant. There are roughly
70 uses of configureFlagsArray at present, which is enough to remove all
at once (in a future patchset).
This is a WIP that I'm posting as an RFC; I've updated enough files
to be able to build bash successfully. More to come if this is
welcomed, but I'd like to get help with converting all the uses.
I'd also like to know which docs need to be updated.