Re: [gentoo-dev] [PATCH 1/2] eclass/cargo.eclass: add cargo_src_configure
On Fri, 12 Jun 2020 09:24:11 -0700 Georgy Yakovlev wrote: > Not sure if --features=default will activate default set and how it > will react to being passed along. > > --no-default-features --features default > IDK, looks kinda unnatural. > > I have a feeling that we'll get more boilerplate if we pass > --no-default-features than if we don't. > > We can re-evaluate as time goes by, but for now I see no major benefit > and only downsides. One problem I stumbled onto is that while --no-default-features does work generically regardless of defined features, --features default does _NOT_ work generically, as it fails if there is no defined "default" feature. Another is some crates simply don't work with --no-default-features, and they need --features=std or similar in order to work. Hence, I don't think --no-default-features makes for a great default mechanism for rust. Its something you should only reach for when you know better. pgpgZywecqaW5.pgp Description: OpenPGP digital signature
Re: [gentoo-dev] [PATCH 1/2] eclass/cargo.eclass: add cargo_src_configure
On 6/12/20 11:59 AM, Luca Barbato wrote: > On 12/06/2020 18:24, Georgy Yakovlev wrote: >> On 6/12/20 4:16 AM, Luca Barbato wrote: >>> On 12/06/2020 11:04, Georgy Yakovlev wrote: +# cargo_src_configure --no-default-features >>> >>> Shall we default in not-defaulting so we can spare some boilerplate? >> I don't think so. Let me explain. >> >> First of all, this will force to explicitly micro-manage all the >> features for all the packages in the tree. >> > > The idea is: > - if myfeatures is empty, do not pass --no-default-features. > - if myfeatures has content, automatically pass --no-default-features. > I realized that was the intention after sending the email. That makes more sense, I'll scout toml files a bit and probably will implement it. I certainly remember scenarios where turning off default features is highly undesirable, but toggling extras makes sense. > --no-default-features --features default seems working as intended btw. > in that case local myfeatures=( default $(usex flagX featureX '') ) -> --no-default-features --features default --features featureX looks quite good actually. just need to make sure we pass --no-default-features first in order in that scenario. > lu
Re: [gentoo-dev] [PATCH 1/2] eclass/cargo.eclass: add cargo_src_configure
On 12/06/2020 18:24, Georgy Yakovlev wrote: On 6/12/20 4:16 AM, Luca Barbato wrote: On 12/06/2020 11:04, Georgy Yakovlev wrote: +# cargo_src_configure --no-default-features Shall we default in not-defaulting so we can spare some boilerplate? I don't think so. Let me explain. First of all, this will force to explicitly micro-manage all the features for all the packages in the tree. The idea is: - if myfeatures is empty, do not pass --no-default-features. - if myfeatures has content, automatically pass --no-default-features. --no-default-features --features default seems working as intended btw. lu
Re: [gentoo-dev] [PATCH 1/2] eclass/cargo.eclass: add cargo_src_configure
On 6/12/20 6:03 AM, Kent Fredric wrote: I've replied privately by mistake, replying to the list again. > On Fri, 12 Jun 2020 02:04:51 -0700 > Georgy Yakovlev wrote: > >> +# cargo_src_configure --no-default-features > > Looking at the source, I don't see how this is passed from this command to > anything. yeah I caught it later, will just pass it as readonly ECARGO_FEATURES=( ${myfeatures[@]/#/--features } ${@} ) > >> +# transform array from simple feature list >> +# to multiple cargo args: >> +# --features feature1 --features feature2 ... >> +readonly ECARGO_FEATURES=( ${myfeatures[@]/#/--features } ) > > Is this really necessary? > >> cargo --features "feature1 feature2" > > is perfectly legal. > cargo is very picky about how multiple features are quoted in your example. like REALLY picky, and it breaks depending on how array is passed. I had that initially but found that just not worth it because it may break. multiple toggles is perfectly legal way to pass features as well and is more robust in this situation IMO.
Re: [gentoo-dev] [PATCH 1/2] eclass/cargo.eclass: add cargo_src_configure
On 6/12/20 4:16 AM, Luca Barbato wrote: > On 12/06/2020 11:04, Georgy Yakovlev wrote: >> +# cargo_src_configure --no-default-features > > Shall we default in not-defaulting so we can spare some boilerplate? I don't think so. Let me explain. First of all, this will force to explicitly micro-manage all the features for all the packages in the tree. Like 90% of crates I've seen ship working choice and there's no benefit of micro-management, and no benefit of enabling non-default features at all. Not all features need to be exposed as use-flags, and this was quite consistent situation so far. I know it may sound a bit strange but automagic in this case works with no downsides, unlike other languages/frameworks/build-systems. Also it will break all the tree packages which do not manage features directly if merged as is. Not sure if --features=default will activate default set and how it will react to being passed along. --no-default-features --features default IDK, looks kinda unnatural. I have a feeling that we'll get more boilerplate if we pass --no-default-features than if we don't. We can re-evaluate as time goes by, but for now I see no major benefit and only downsides. > > lu >
Re: [gentoo-dev] [PATCH 1/2] eclass/cargo.eclass: add cargo_src_configure
On Fri, 12 Jun 2020 02:04:51 -0700 Georgy Yakovlev wrote: > +#cargo_src_configure --no-default-features Looking at the source, I don't see how this is passed from this command to anything. > + # transform array from simple feature list > + # to multiple cargo args: > + # --features feature1 --features feature2 ... > + readonly ECARGO_FEATURES=( ${myfeatures[@]/#/--features } ) Is this really necessary? > cargo --features "feature1 feature2" is perfectly legal. pgpryNPG16yA7.pgp Description: OpenPGP digital signature
Re: [gentoo-dev] [PATCH 1/2] eclass/cargo.eclass: add cargo_src_configure
On 12/06/2020 11:04, Georgy Yakovlev wrote: +# cargo_src_configure --no-default-features Shall we default in not-defaulting so we can spare some boilerplate? lu
[gentoo-dev] [PATCH 1/2] eclass/cargo.eclass: add cargo_src_configure
simple src_configure implementation inspired by cmake.eclass Closes: https://bugs.gentoo.org/721936 Signed-off-by: Georgy Yakovlev --- eclass/cargo.eclass | 51 ++--- 1 file changed, 44 insertions(+), 7 deletions(-) diff --git a/eclass/cargo.eclass b/eclass/cargo.eclass index ad90a0c7dd8..b084e082730 100644 --- a/eclass/cargo.eclass +++ b/eclass/cargo.eclass @@ -23,7 +23,7 @@ esac inherit multiprocessing toolchain-funcs -EXPORT_FUNCTIONS src_unpack src_compile src_install src_test +EXPORT_FUNCTIONS src_unpack src_configure src_compile src_install src_test IUSE="${IUSE} debug" @@ -35,6 +35,24 @@ ECARGO_VENDOR="${ECARGO_HOME}/gentoo" # Allows overriding the default cwd to run cargo install from : ${CARGO_INSTALL_PATH:=.} +# @VARIABLE: myfeatures +# @DEFAULT_UNSET +# @DESCRIPTION: +# Optional cargo features defined as bash array. Should be defined before calling +# src_configure. +# Example for package that has x11 and wayland as features. +# Also disables default features, as optional parameter is passed. +# @CODE +# src_configure() { +# local myfeatures=( +# $(usex X x11 '') +# $(usev wayland) +# ) +# +# cargo_src_configure --no-default-features +# } +# @CODE + # @FUNCTION: cargo_crate_uris # @DESCRIPTION: # Generates the URIs to put in SRC_URI to help fetch dependencies. @@ -113,6 +131,7 @@ cargo_live_src_unpack() { mkdir -p "${S}" || die pushd "${S}" > /dev/null || die + # need to specify CARGO_HOME before cargo_gen_config fired CARGO_HOME="${ECARGO_HOME}" cargo fetch || die CARGO_HOME="${ECARGO_HOME}" cargo vendor "${ECARGO_VENDOR}" || die popd > /dev/null || die @@ -152,6 +171,26 @@ cargo_gen_config() { EOF # honor NOCOLOR setting [[ "${NOCOLOR}" = true || "${NOCOLOR}" = yes ]] && echo "color = 'never'" >> "${ECARGO_HOME}/config" + + export CARGO_HOME="${ECARGO_HOME}" +} + +# @FUNCTION: cargo_src_configure +# @DESCRIPTION: +# Configure cargo package features +cargo_src_configure() { + debug-print-function ${FUNCNAME} "$@" + + [[ -z ${myfeatures} ]] && declare -a myfeatures=() + local myfeaturestype=$(declare -p myfeatures 2>&-) + if [[ "${myfeaturestype}" != "declare -a myfeatures="* ]]; then + die "myfeatures must be declared as array" + fi + + # transform array from simple feature list + # to multiple cargo args: + # --features feature1 --features feature2 ... + readonly ECARGO_FEATURES=( ${myfeatures[@]/#/--features } ) } # @FUNCTION: cargo_src_compile @@ -160,11 +199,9 @@ cargo_gen_config() { cargo_src_compile() { debug-print-function ${FUNCNAME} "$@" - export CARGO_HOME="${ECARGO_HOME}" - tc-export AR CC - cargo build $(usex debug "" --release) "$@" \ + cargo build $(usex debug "" --release) ${ECARGO_FEATURES[@]} "$@" \ || die "cargo build failed" } @@ -174,8 +211,8 @@ cargo_src_compile() { cargo_src_install() { debug-print-function ${FUNCNAME} "$@" - cargo install --path ${CARGO_INSTALL_PATH} \ - --root="${ED}/usr" $(usex debug --debug "") "$@" \ + cargo install --path ${CARGO_INSTALL_PATH} --root="${ED}/usr" \ + $(usex debug --debug "") ${ECARGO_FEATURES[@]} "$@" \ || die "cargo install failed" rm -f "${ED}/usr/.crates.toml" rm -f "${ED}/usr/.crates2.json" @@ -189,7 +226,7 @@ cargo_src_install() { cargo_src_test() { debug-print-function ${FUNCNAME} "$@" - cargo test $(usex debug "" --release) "$@" \ + cargo test $(usex debug "" --release) ${ECARGO_FEATURES[@]} "$@" \ || die "cargo test failed" } -- 2.27.0