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

Patch page_size to point to a fork that upgrades its dependency to winapi crate 0.3.8 #948

Closed
wants to merge 1 commit into from

Conversation

hrydgard
Copy link

Until Elzair merges our PR, Elzair/page_size_rs#1. Does not seem active.

For build perf, old winapi (<= 2.8) is super slow to build since it by defaults includes everything. Cargo.lock shrinks a little too.

Saves about 30 seconds of CPU during build (though with enough cores there's little difference in total built time, but if wasmer is built as part of a larger project, the difference appears).

@syrusakbary
Copy link
Member

@hrydgard I think if we merge this change, we will not be able to publish the crate to crates.io.

We'd be happy depending on a published fork on crates.io though, if you want to do a fork, publish it to crates.io and update the PR we will merge it promptly :)

@hrydgard
Copy link
Author

@syrusakbary In my opinion it's kind of a odd little crate. Anytime you care about what the page size is, you're likely also using other platform-specific functionality anyway, so the few extra lines to determine the page size using platform-specific code should be no big deal and not enough to motivate adding an external dependency.

So instead of us taking over maintenance of such a strange little crate, it might a better idea for you guys to get rid of the dependency on this crate, and simply take the contents and merge it into your code directly at the call sites, or just grab the lib.rs and rename it page_size.rs and just put it next to your other files?

@hrydgard
Copy link
Author

hrydgard commented Dec 20, 2019

The maintainer woke up, https://github.com/Elzair/page_size_rs has now been bumped to 0.4.2 which fixes this issue. Since you guys specify the dependency as "0.4", you can probably just close this after the next time you run cargo update.

@syrusakbary
Copy link
Member

We just merged #1093 ...things should be good now in master :)

@hrydgard hrydgard deleted the page_size_switch branch January 8, 2020 08:43
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