Re: [gentoo-dev] [PATCH 1/2] eclass/cargo.eclass: add cargo_src_configure

2020-06-13 Thread Kent Fredric
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

2020-06-12 Thread Georgy Yakovlev
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

2020-06-12 Thread Luca Barbato

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

2020-06-12 Thread Georgy Yakovlev
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

2020-06-12 Thread Georgy Yakovlev
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

2020-06-12 Thread Kent Fredric
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

2020-06-12 Thread Luca Barbato

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

2020-06-12 Thread Georgy Yakovlev
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