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

AVRO-3800: [Rust] profile section should be declared in the root package #2354

Merged
merged 1 commit into from
Jul 17, 2023

Conversation

sarutak
Copy link
Member

@sarutak sarutak commented Jul 17, 2023

AVRO-3800

What is the purpose of the change

This PR fixes an issue that [profile.release] declared in wasm-demo/Cargo.toml is ignored.
In that section, opt-level = "s" is specified but it's ignored.

$ cargo test -v --test demos --release
...
     Running `rustc --crate-name hello_wasm ... opt-level=3 ...`
     Running `rustc --crate-name demos ... opt-level=3 ...`

To fix this issue, this change moves the section to Cargo.toml of the root package.

Verifying this change

After this change is applied, we can confirm that opt-level is changed as expected.

$ cargo test -v --test demos --release
...
     Running `rustc --crate-name hello_wasm ... opt-level=s ...`
     Running `rustc --crate-name demos ... opt-level=s ...`

Documentation

No new docs as this PR introduces no new features.

@sarutak sarutak changed the title AVRO-3800: [Rust] profile section should be declared in the root package. AVRO-3800: [Rust] profile section should be declared in the root package Jul 17, 2023
@github-actions github-actions bot added the Rust label Jul 17, 2023
@martin-g
Copy link
Member

Do you have some reference to Cargo documentation explaining that this is the correct way ?
To me this looks like a bug in cargo that it ignores the package specific settings.

@sarutak
Copy link
Member Author

sarutak commented Jul 17, 2023

This doc says like as follows.

Cargo only looks at the profile settings in the Cargo.toml manifest at the root of the workspace. Profile settings defined in dependencies will be ignored.

@martin-g martin-g merged commit dc81b35 into apache:master Jul 17, 2023
14 checks passed
martin-g pushed a commit that referenced this pull request Jul 17, 2023
@martin-g
Copy link
Member

Thank you, @sarutak !

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

Successfully merging this pull request may close these issues.

2 participants