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

remove no_std hashbrown dep #104

Merged
merged 3 commits into from
May 30, 2024
Merged

Conversation

knickish
Copy link
Collaborator

@knickish knickish commented May 27, 2024

Hashbrown added several recursive deps, time to rip it out

@knickish knickish requested a review from not-fl3 May 27, 2024 16:33
@not-fl3
Copy link
Owner

not-fl3 commented May 27, 2024

Fun fact: hashbrown now depends on syn and whatnot, which kind of eleminates the whole purpose of nanoserde

│   ├── hashbrown v0.13.2
│   │   └── ahash v0.8.11
│   │       ├── cfg-if v1.0.0
│   │       ├── once_cell v1.19.0
│   │       └── zerocopy v0.7.34
│   │           └── zerocopy-derive v0.7.34 (proc-macro)
│   │               ├── proc-macro2 v1.0.82
│   │               │   └── unicode-ident v1.0.12
│   │               ├── quote v1.0.36
│   │               │   └── proc-macro2 v1.0.82
│   │               │       └── unicode-ident v1.0.12
│   │               └── syn v2.0.63
│   │                   ├── proc-macro2 v1.0.82
│   │                   │   └── unicode-ident v1.0.12
│   │                   ├── quote v1.0.36
│   │                   │   └── proc-macro2 v1.0.82
│   │                   │       └── unicode-ident v1.0.12
│   │                   └── unicode-ident v1.0.12
│   │       [build-dependencies]
│   │       └── version_check v0.9.4

never trust a crates.io dependency, they can bring a full rust parser into your dependency tree at literally any moment :D

(not a no-std user here, no opinion on what to do about it)

@knickish
Copy link
Collaborator Author

Gah thats not cool. Hmmm. Maybe we should just drop hashmap impl when doing no_std?

@knickish
Copy link
Collaborator Author

I guess that wouldn't be additive though :/

@knickish knickish force-pushed the update_hashbrown_dep branch from 7f28eb8 to f6d3303 Compare May 27, 2024 20:06
@knickish knickish force-pushed the update_hashbrown_dep branch from f6d3303 to db0d90f Compare May 27, 2024 20:22
@knickish
Copy link
Collaborator Author

@not-fl3 looks like that was optional, got it down to

cargo +nightly tree --features "no_std, binary"

nanoserde v0.2.0
├── hashbrown v0.14.5
│   └── ahash v0.8.11
│       ├── cfg-if v1.0.0
│       ├── once_cell v1.19.0
│       └── zerocopy v0.7.34
│       [build-dependencies]
│       └── version_check v0.9.4
└── nanoserde-derive v0.2.0 (proc-macro)

@not-fl3
Copy link
Owner

not-fl3 commented May 27, 2024

--target=all still got syn etc, and Cargo alwas downloads it now, even when no-std is off.

@not-fl3
Copy link
Owner

not-fl3 commented May 27, 2024

Actually, it was not the case for 0.12 (tho 0.12 use getrandom/wasi...)
Do we really need 0.14? Is it worth downloading 6 extra fairly heavy dependencies?

--target=all do happen on the CI and cargo downloads all those dependencies no matter what features are used, so I would rather not have hashbrown from now on at all to be honest, but I am not sure if this would be very easy to do

@knickish
Copy link
Collaborator Author

I guess it makes more sense to make a PR to add nanoserde support for their types in hashbrown itself at this point. They already have features for serde + rykv, so seems likely they would accept it

@knickish knickish changed the title update no_std hashbrown dep remove no_std hashbrown dep May 28, 2024
@knickish knickish force-pushed the update_hashbrown_dep branch from 334b573 to 0a1fd3f Compare May 28, 2024 23:42
@knickish knickish force-pushed the update_hashbrown_dep branch from 0a1fd3f to f5f2470 Compare May 28, 2024 23:43
@knickish
Copy link
Collaborator Author

Alright, ripped it all out now and switched to a BTreeMap for the toml parser internal representation of arrays so that it can be used with no_std

@knickish knickish requested a review from not-fl3 May 28, 2024 23:48
@not-fl3
Copy link
Owner

not-fl3 commented May 29, 2024

Wow, such a clever solution, a huge fan of this PR! Somehow I totally overlooked that alloc got collections!

still totally usable with no_std and we still have std::HashMap support for jsons with std!

Looks super good to me!

@knickish knickish merged commit 7cd011f into not-fl3:master May 30, 2024
16 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.

2 participants