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

Correctly choose profile to use. #112

Merged

Conversation

mutexlox-signal
Copy link
Contributor

If, e.g., a user sets CARGO_PROFILE_RELEASE_DEBUG=1, the DEBUG environment variable will be set accordingly, and so the debug build will be used.

Instead, we should use the PROFILE env variable (see https://doc.rust-lang.org/cargo/reference/environment-variables.html#environment-variables-cargo-sets-for-build-scripts) to determine which profile to use.

This isn't 100% correct, because it won't pass through any other flags (in this case, we won't have debug information for the cubeb backend), but it is more correct to ensure the matching profile.

If, e.g., a user sets `CARGO_PROFILE_RELEASE_DEBUG=1`, the `DEBUG`
environment variable will be set accordingly, and so the debug build
will be used.

Instead, we should use the PROFILE env variable (see
https://doc.rust-lang.org/cargo/reference/environment-variables.html#environment-variables-cargo-sets-for-build-scripts)
to determine which profile to use.

This isn't 100% correct, because it won't pass through any other flags
(in this case, we won't have debug information for the cubeb backend),
but it is more correct to ensure the matching profile.
@mutexlox-signal
Copy link
Contributor Author

cc @kinetiknz for review

@mutexlox-signal
Copy link
Contributor Author

Not entirely sure what the build failures here are but they do look similar to https://github.com/mozilla/cubeb-rs/actions/runs/12285548106/job/34283783653#step:6:159. I don't think it's reproducing for me locally, though, and the error we do see in github isn't helpful...

@kinetiknz
Copy link
Contributor

Thanks!

The build failures appear to be caused by clippy warnings on the cubeb crates inside the vendored libcubeb. I'll look at changing libcubeb's CMakeLists.txt to report these errors to the console so we get legible errors in CI.

@kinetiknz kinetiknz merged commit ab34373 into mozilla:master Feb 3, 2025
3 of 7 checks passed
@kinetiknz
Copy link
Contributor

I'll look at changing libcubeb's CMakeLists.txt to report these errors to the console so we get legible errors in CI.

mozilla/cubeb@842a3c7 solves this particular issue.

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.

2 participants