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

Introduce `ahash-compile-time-rng` feature. #125

Merged
merged 2 commits into from Oct 31, 2019

Conversation

@0ndorio
Copy link
Contributor

0ndorio commented Oct 30, 2019

Content

Disables the default features of ahash and reintroduces them
through a new feature called ahash-compile-time-rng, which is
enabled by default.

The new feature makes it possible for depended crates to rely on
hashbrown with ahash as default hasher and to disable the
compile-time-rng at the same time.

This might be useful for any depended crate targeting no_std,
which contains rand or rand_core somewhere inside the dependency
tree as a bug in cargo accidentally enables the underlying getrandom
feature if compile-time-rng is enabled [1].

... fixes #124

[1] rust-lang/cargo#5760


Warnings

(1) Compiling ahash with disabled compile-time-rng feature is currently broken and requires tkaitchuck/aHash#25 to be merged in advance.

(2) This introduces a hidden behavior change for all dependent crates, using hashbrown with default-features = false and features = 'ahash'. This happens as the ahash feature no longer implicitly enables the compile-time-rng feature of ahash.


Is the naming of the feature okay? Do I need to add any additional changes?

@0ndorio 0ndorio force-pushed the 0ndorio:allow_to_disable_ahash_feature branch from 3fe2350 to 2907d9b Oct 30, 2019
@Amanieu

This comment has been minimized.

Copy link
Collaborator

Amanieu commented Oct 30, 2019

Clippy seems to be failing due to a new lint. Could you add clippy::must_use_candidate to the whitelist in lib.rs?

Also, could you document the feature in README.md (there's a feature list near the bottom).

Disables the default features of `ahash` and reintroduces them
through a new feature called `ahash-compile-time-rng`, which is
enabled by default.

The new feature makes it possible for depended crates to rely on
`hashbrown` with `ahash` as default hasher and to disable the
`compile-time-rng` at the same time.

This might be useful for any dependent crate targeting `no_std`,
which contains `rand` or `rand_core` somewhere inside the dependency
tree as a bug in cargo accidentally enables the underlying `getrandom`
feature if `compile-time-rng` is enabled [1].

... fixes #124

[1] rust-lang/cargo#5760
@0ndorio 0ndorio force-pushed the 0ndorio:allow_to_disable_ahash_feature branch from 2907d9b to 43911e5 Oct 30, 2019
@0ndorio

This comment has been minimized.

Copy link
Contributor Author

0ndorio commented Oct 30, 2019

Clippy seems to be failing due to a new lint. Could you add clippy::must_use_candidate to the whitelist in lib.rs?

Added new commit to fix this.

Also, could you document the feature in README.md (there's a feature list near the bottom).

Amended the documentation to the original commit.

@0ndorio 0ndorio force-pushed the 0ndorio:allow_to_disable_ahash_feature branch from 43911e5 to 388287c Oct 30, 2019
@Amanieu

This comment has been minimized.

Copy link
Collaborator

Amanieu commented Oct 30, 2019

You need to run cargo fmt to fix CI.

@0ndorio 0ndorio force-pushed the 0ndorio:allow_to_disable_ahash_feature branch from 388287c to 92bc9f8 Oct 31, 2019
@Amanieu

This comment has been minimized.

Copy link
Collaborator

Amanieu commented Oct 31, 2019

@bors r+

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Oct 31, 2019

📌 Commit 92bc9f8 has been approved by Amanieu

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Oct 31, 2019

⌛️ Testing commit 92bc9f8 with merge 592137a...

bors added a commit that referenced this pull request Oct 31, 2019
Introduce `ahash-compile-time-rng` feature.

**Content**

Disables the default features of `ahash` and reintroduces them
through a new feature called `ahash-compile-time-rng`, which is
enabled by default.

The new feature makes it possible for depended crates to rely on
`hashbrown` with `ahash` as default hasher and to disable the
`compile-time-rng` at the same time.

This might be useful for any depended crate targeting `no_std`,
which contains `rand` or `rand_core` somewhere inside the dependency
tree as a bug in cargo accidentally enables the underlying `getrandom`
feature if `compile-time-rng` is enabled [1].

... fixes #124

[1] rust-lang/cargo#5760

---

**Warnings**

 (1) Compiling `ahash` with disabled `compile-time-rng` feature is currently broken and requires tkaitchuck/aHash#25 to be merged in advance.

 (2) This introduces a hidden behavior change for all dependent crates, using hashbrown with `default-features = false` and `features = 'ahash'`. This happens as the `ahash` feature no longer implicitly enables the `compile-time-rng` feature of `ahash`.

---

Is the naming of the feature okay?  Do I need to add any additional  changes?
@bors

This comment has been minimized.

Copy link
Contributor

bors commented Oct 31, 2019

☀️ Test successful - checks-travis
Approved by: Amanieu
Pushing 592137a to master...

@bors bors merged commit 92bc9f8 into rust-lang:master Oct 31, 2019
2 checks passed
2 checks passed
Travis CI - Pull Request Build Passed
Details
homu Test successful
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.