build: remove unnecessary allocator-api2 dep.#28
Merged
kyren merged 1 commit intodjc:masterfrom May 7, 2024
Merged
Conversation
5 tasks
Collaborator
That's probably the friendliest introduction to a PR I've ever gotten haha. Nice to see you again! 👋 Yeah, this makes complete sense and I'm gonna merge it. Do you need me to do another hashlink release to get everyone lined up? Just let me know! |
Contributor
Author
|
@kyren: Yes please! A release would definitely facilitate, but is not hard-blocking for us. Thank you so much! 😊 |
Collaborator
|
Okay, release is done! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Hey there @kyren! 👋🏻 Positive heckler at 0:29 of your RustConf 2018 talk here. 🙂 We (sorta) meet again!
This time, I come representing some dependency management effort at Mozilla, where I've been auditing dependencies of crates like
libsqlite0.31, which includeshashlink0.9.0. We've used previous versions ofhashlinkvia this transitive dependency, buthashlink's update tohashbrown0.14 has an interesting wrinkle in it: it includes theallocator-api2feature/dependency combo by default. But…it doesn't seem like it's being meaningfully consumed by most crates usinghashbrown, includinghashlink's case. 😖Turns out, this feature probably doesn't make sense unless a consumer-of-a-consumer crate (i.e., something using
hashlink) usesallocator-api2directly and has access to the allocators thathashlink's underlyinghashbrowncollections use (which isn't currently exposed). So, I disable this feature with this PR for Mozilla's sake. End-user applications can still re-enable this with a direct dependency onhashbrown, with which they can request thehashbrown/allocator-api2themselves.I don't know why
hashbrownmade this adefaultfeature, TBH. 🫤 It's been annoying (but doable) to combat this unnecessary dependency in other crates likegpu-descriptor(see zakarumych/gpu-descriptor#40).