Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Build script failure on platform detection #508

Closed
semenov-vladyslav opened this issue Mar 7, 2023 · 5 comments · Fixed by #510
Closed

Build script failure on platform detection #508

semenov-vladyslav opened this issue Mar 7, 2023 · 5 comments · Fixed by #510

Comments

@semenov-vladyslav
Copy link
Contributor

Automatic backend (DalekBits) detection in curve25519-dalek-4.0.0-rc.1 build.rs fails at unwrap platforms::Platform::find(&target_triplet).unwrap() while building a risc0 guest method using ed25519-dalek crate. The actual target_triplet is riscv32im-risc0-zkvm-elf. I was unable to configure curve25519_dalek_bits externally, risc0-build doesn't allow it yet, and .cargo/config didn't work either. As a workaround we could return DalekBits::Dalek32 if the platform is not found since this is the default value returned for weird target_pointer_width.

@pinkforest
Copy link
Contributor

pinkforest commented Mar 9, 2023

Thanks for catching this 😅

This shouldn't happen if curve25519_dalek_bits= 32 | 64 was used

#[cfg(all(not(curve25519_dalek_bits = "64"), not(curve25519_dalek_bits = "32")))]
  let curve25519_dalek_bits = lotto_curve25519_dalek_bits();

How was this configured as this code point would not trigger given the above ?

e.g. what build command was used / where the cfg was defined ?

Note cfg() cannot be controlled by the ransient dependency - see https://github.com/pinkforest/cfg_lottery_demo

It's best to erorr out for build if platforms crate misses target definition instead of trying to default u32 as it would be sudden change of backend if there is switch from default to something else when platforms crate recognises it.

Also in this case I understand this is non-supported custom target that Rust makes no opinion of otherwise so in this case the best behaviour would be to insist on specifying it explicitly instead of guessing one behind the scenes on custom target.

Nonetheless I need to clean out that error making it more explicit.

@semenov-vladyslav
Copy link
Contributor Author

#510 didn't actually fix the issue. Now I get:

Error: Unknown Rust target platform.
Please set cfg(curve25519_dalek_bits) explicitly either as 32 or 64.

But I had this issue because I couldn't set curve25519_dalek_bits in the first place.

@pinkforest
Copy link
Contributor

pinkforest commented Mar 11, 2023

It fixed the correct issue - the error that is

The build here is using a custom target which I suspect is using tooling which does not signal cfg to rustc which seems to be the problem here.

If that error arises then it is a build system error which does not signal cfg correctly to rustc - more than happy to help but I don't have details of the custom build system being used - which doesn't seem to align with the standard rustc build tooling or be part of the standard rust toolchain that is able to signal the cfg flags if these were set correctly.

Could please share the details how it is being built exactly e.g. which commands with cargo I assume ?

@tarcieri
Copy link
Contributor

@semenov-vladyslav can you not set the RUSTFLAGS environment variable?

@pinkforest
Copy link
Contributor

pinkforest commented Mar 12, 2023

Raised a follow-up issue:

But would be good to know more details re: build command e.g. RUSTENV which rustc uses to set the cfg flag.

env RUSTFLAGS='--cfg curve25519_dalek_bits="32"' <build command> should work with rustc unless the custom build command that does exec() on rustc doesn't pass it on ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants