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

Put loom behind cfg(oneshot_loom) again #45

Merged
merged 1 commit into from
Jun 13, 2024
Merged

Conversation

faern
Copy link
Owner

@faern faern commented Jun 11, 2024

Followup to #43. Loom is still optional, taking it out of the dependency graph and not polluting Cargo.lock for downstream users. But also "hide" the feature a bit more. It's still exposed in cargo metadata sadly. And building with cargo build --all-features will enable the loom feature. However just enabling the feature does nothing.

This puts the dependency also behind [target.'cfg(oneshot_loom)'.dependencies] and all the code that is conditional on relying on loom vs std now use cfg(oneshot_loom) instead of cfg(feature = "loom"). This (again) makes it way harder for downstream users to accidentally compile oneshot with loom integration turned on.

So we are back to how things were before #43 but with the following differences:

  • Due to being optional, loom is taken out of the default dependency tree
  • The cfg flag is renamed from loom to oneshot_loom to not collide with other loom-enabled crates, since RUSTFLAGS are global.

Ping @Wuelle :)

@faern faern force-pushed the improve-loom-integration branch 2 times, most recently from 8897491 to 1f06d79 Compare June 11, 2024 06:56
@faern faern force-pushed the improve-loom-integration branch from 1f06d79 to 26066b2 Compare June 13, 2024 20:24
@faern faern merged commit 0a824d5 into main Jun 13, 2024
12 checks passed
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.

1 participant