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

Pointer width detection in build.rs may break for obscure platforms #610

Closed
Shnatsel opened this issue Dec 28, 2023 · 1 comment · Fixed by #636
Closed

Pointer width detection in build.rs may break for obscure platforms #610

Shnatsel opened this issue Dec 28, 2023 · 1 comment · Fixed by #636

Comments

@Shnatsel
Copy link

The build.rs file currently uses the platforms crate to map the rustc target triples to pointer width. I am the author of the platforms crate 👋

The platforms crate adds and removes targets following the target support in the latest release of rustc. This means that the list of targets it knows about may be different from the list of targets that your local rustc knows about.

It would be better to query the Rust compiler directly instead of relying on the platforms crate. You can do it with rustc --print=cfg --target=TRIPLE, e.g. rustc --print=cfg --target x86_64-unknown-uefi will print the pointer width, among other things. By sourcing it from the compiler that performs the build, there is no chance of the pointer width data to be missing.

@pinkforest
Copy link
Contributor

pinkforest commented Feb 26, 2024

Thanks - Yeah thought about that but platforms was just ergonomic at the time.

I also thought we could expand to rustc-metadata but then there was already rustc-cfg which we could add and could be suited for the task either with modifications or directly.

But we've used environment variables for this which even rustc-cfg recommends over that create - also saving dep as a bonus.

So I guess we can just read the environment variable CARGO_CFG_TARGET_POINTER_WIDTH instead and drop the dep indeed despite it being slightly ergonomic and giving better idea about obscure platforms as you've noted 👍

EDIT: One downside is that we need to deal with string match when figuring out whether a platform default override is in future - and it could be helpful to turn this into type proper if / when there is decision to set any wasm/aarch64 target_pointer_width as default 64 over 32 - nonetheless ideally these should be set explciitly via curve25519_dalek_bits

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