Handle multivalued target_family

[madsmtm]

target_family / CARGO_CFG_TARGET_FAMILY is documented to be multi-valued.

This was found by a crater run in rust-lang/rust#124355 (comment). The concrete-csprng crate may fail to compile in the future if Rust decides to add further target families.

The commit history since v0.4.0 shows only cosmetic changes, so I've bumped the version to v0.4.1 to make it easier for you to release this as a patch version.

Check-list:

• Tests for the changes have been added (for bug fixes / features)
  • Can't really test this yet, it's more of making sure that this works in the future.
• Docs have been added / updated (for bug fixes / features)
• Relevant issues are marked as resolved/closed, related issues are linked in the description
• Check for breaking changes (including serialization changes) and add them to commit message following the conventional commit specification

[cla-bot]

Thank you for your pull request. We require contributors to sign our Contributor License Agreement / Terms and Conditions, and we don't seem to have the users @madsmtm on file. In order for us to review and merge your code, please sign:

• For individual contribution: our CLA
• for Bounty submission, if you are an individual: our T&C
• for Bounty submission, if you are a company: our T&C
  to get yourself added.

If you already signed one of this document, just wait to be added to the bot config.

[IceTDrinker]

Hello, thanks a lot for the report of the crater run, as you can see there is a CLA for outside contributors, in any case we’ll look to get this fixed as I believe the current use of the build.rs is an antipattern in concrete-csprng

[madsmtm]

I've signed the CLA, but not sure if that's enough. I don't care about attribution or anything for this PR, feel free to license these changes under whatever license you prefer, now or in the future. And you may freely rebase my name away from appearing in any commits if that's easier for your automated systems.

[IceTDrinker]

@madsmtm let us know if you are willing to sign the CLA or if we should fix this ourselves given your report

[IceTDrinker]

Ah let me check on our end then !

[aquint-zama]

@cla-bot check

[cla-bot]

The cla-bot has been summoned, and re-checked this pull request!

[IceTDrinker]

@madsmtm thanks a lot for fixing this, very much appreciated 🙏

I see there is a formatting issue, could you run make fmt and push the corrected commit here amending the first one ?

one note however reading the docs on the topic really does not make it clear (in my opinion) that it can be multi valued, and no such multi valued example is provided (I'm guessing because it can't happen today).

Perhaps a "in the future CARGO_CFG_TARGET_FAMILY may contain several target families in a comma separated list e.g. CARGO_CFG_TARGET_FAMILY=unix,some_other_family and build.rs scripts are expected to properly parse those" or having a sort of build utils available from cargo that parses those env variables automatically and that can be loaded by the person writing the build.rs script would be useful as the proper behavior would always be enforced (or at least more easily enforced if build.rs writers just use the provided utils).

Let me know if there is a location where we can give this feedback to perhaps update some documentation

Cheers

[IceTDrinker]

Small logic bug in the any closure should be a != I believe

[IceTDrinker]

if you prefer we do the fix to avoid back and forth @madsmtm let us know, we'll add a co-authored tag on the commit as you were the one to bring this to our attention, proper attribution is the least we can do !

[madsmtm]

I see there is a formatting issue

Ah, sorry about this, I was somewhat lazy, and did this PR without fully setting up my editor :/. Have fixed it now, and actually tested that it works.

one note however reading the docs on the topic really does not make it clear (in my opinion) that it can be multi valued, and no such multi valued example is provided (I'm guessing because it can't happen today).

It says:

Any number of target_family key-value pairs can be set.

But I agree that having no example of this is confusing, I've opened rust-lang/cargo#14165 and rust-lang/reference#1518 to resolve this.

having a sort of build utils available from cargo

There is some recent work on this in rust-lang/cargo#12432.

[IceTDrinker]

I see there is a formatting issue

Ah, sorry about this, I was somewhat lazy, and did this PR without fully setting up my editor :/. Have fixed it now, and actually tested that it works.

one note however reading the docs on the topic really does not make it clear (in my opinion) that it can be multi valued, and no such multi valued example is provided (I'm guessing because it can't happen today).

It says:

Any number of target_family key-value pairs can be set.

But I agree that having no example of this is confusing, I've opened rust-lang/cargo#14165 and rust-lang/reference#1518 to resolve this.

having a sort of build utils available from cargo

There is some recent work on this in rust-lang/cargo#12432.

Thanks for the links, as for the docs, it says multiple pairs but I don’t see comma separated list as pairs 😅

in any cas thanks, merged and 0.4.1 is live wont’ hamper future crater runs