Skip to content

xtask/kimchi-stubs: add option to build stubs w or w/o optim #3248

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

dannywillems
Copy link
Member

@dannywillems dannywillems commented Jun 4, 2025

This aims to replace commands used in Mina/crypto/kimchi_bindings/stubs/dune. It has been reported a few times that the proof-systems repository should be responsible to define its own build targets.

The user can now generate the bindings with:

cargo run -p xtask -- build-kimchi-stubs

Optimisations are activated by default, but can be disabled with

RUST_TARGET_FEATURE_OPTIMISATIONS=n cargo run -p xtask -- \
  build-kimchi-stubs \
  --offline \ # optional argument
  --target-dir /tmp/hello-world

@svv232 can you check that the warnings you do have on Mina/compatible now does not appear when running either
RUST_TARGET_FEATURE_OPTIMISATIONS=y cargo run -p xtask -- build-kimchi-stubs or RUST_TARGET_FEATURE_OPTIMISATIONS=n cargo run -p xtask -- build-kimchi-stubs?

@richardpringle can I have your opinion on the code?

EDIT:
This patch, as a consequence, will remove the following warnings MacOS users might see:

'+adx' is not a recognized feature for this target (ignoring feature)
'+bmi2' is not a recognized feature for this target (ignoring feature)
'+adx' is not a recognized feature for this target (ignoring feature)
'+bmi2' is not a recognized feature for this target (ignoring feature)
'+adx' is not a recognized feature for this target (ignoring feature)
'+bmi2' is not a recognized feature for this target (ignoring feature)
'+adx' is not a recognized feature for this target (ignoring feature)
'+bmi2' is not a recognized feature for this target (ignoring feature)
'+adx' is not a recognized feature for this target (ignoring feature)
'+bmi2' is not a recognized feature for this target (ignoring feature)
'+adx' is not a recognized feature for this target (ignoring feature)

richardpringle
richardpringle previously approved these changes Jun 4, 2025
Copy link
Contributor

@richardpringle richardpringle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't need to apply any of my comments, but you can if you would like 😉

They can also be done in a follow up. This looks good overall

None
};

let target_dir = target_dir.unwrap_or("target/kimchi_stubs_build");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm. Might want to get the crate-directory (absolute path) and put it in front.

But then again, it might be fine. I don't like relative paths, I never remember where they are relative to

This aims to replace commands used in Mina/crypto/kimchi_bindings/stubs/dune.
It has been reported a few times that the proof-systems repository should be
responsible to define its own build targets.

The user can now generate the bindings with:
```
cargo xtask build-kimchi-stubs
```

Optimisations are activated by default, but can be disabled with
```
RUST_TARGET_FEATURE_OPTIMISATIONS=n cargo xtask \
  build-kimchi-stubs \
  --offline \ # optional argument
  --target-dir /tmp/hello-world
```
@dannywillems dannywillems force-pushed the dw/add-xtask-kimchi-stubs branch from f1d5216 to 79f9d5c Compare July 16, 2025 14:23
@dannywillems dannywillems requested a review from Trivo25 July 16, 2025 14:23
@Trivo25
Copy link
Member

Trivo25 commented Jul 17, 2025

Getting this on my Mac:

error[E0599]: no function or associated item named `new` found for struct `CpuId` in the current scope
   --> xtask/src/main.rs:91:28
    |
91  |         let cpuid = CpuId::new();
    |                            ^^^ function or associated item not found in `CpuId<_>`
    |

looks like that's not supported when targeting ARM chips?

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 this pull request may close these issues.

3 participants