foyer-rs / foyer

Hybrid in-memory and disk cache in Rust
https://foyer.rs
Apache License 2.0
291 stars 20 forks source link

fix: build on windows #684

Closed spector-9 closed 1 month ago

spector-9 commented 1 month ago

Currently foyer fails to compile on windows systems. This fixes the errors by adding some line that are cross platform or by adding crates that provide cross platform functionality.

close #263

MrCroxx commented 1 month ago

Hi, @spector-9 . Thank you very much for your contribution! It would be great to have foyer run on Windows. But I cannot get bandwidth to do it. This PR helps a lot!!

I'll review it tomorrow. Thank you again for this PR. 🥰

MrCroxx commented 1 month ago

the specific steps in the makefile are not compatible with windows. Ran cargo test though*

BTW, what build system is most widely used on Windows? Is powershell script okay to replace Makefile?

spector-9 commented 1 month ago

the specific steps in the makefile are not compatible with windows. Ran cargo test though*

BTW, what build system is most widely used on Windows? Is powershell script okay to replace Makefile?

Honestly I would suggest since this is a rust project use something like nurfile so script/shell is cross-platform. Atleast that's what I use for my projects. If not then I can take a look at makefile and see what can be done to make it work on windows, in a separate PR.

codecov[bot] commented 1 month ago

Codecov Report

Attention: Patch coverage is 92.30769% with 1 line in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
foyer-common/src/fs.rs 66.66% 1 Missing :warning:
Files with missing lines Coverage Δ
foyer-storage/src/device/direct_file.rs 85.94% <100.00%> (+0.23%) :arrow_up:
foyer-storage/src/device/direct_fs.rs 88.76% <100.00%> (+0.19%) :arrow_up:
foyer-storage/src/large/generic.rs 88.61% <ø> (ø)
foyer-common/src/fs.rs 25.92% <66.66%> (-5.11%) :arrow_down:

... and 1 file with indirect coverage changes

spector-9 commented 1 month ago

@MrCroxx Let me know if I need to do something else. Thanks for your help.

MrCroxx commented 1 month ago

@MrCroxx Let me know if I need to do something else. Thanks for your help.

Hi @spector-9 , Thank you a lot for the contribution. I'll update this branch to try to setup CI on Windows to see if it works.

I'm sorry that this may bring you some inconvenience. You need to update your local branch if needed.

If this CI passed, this PR will be merged ASAP.

Thank you again. 🥰 Wish you enjoy using and developing foyer ~

MrCroxx commented 1 month ago

https://x.com/CroxxMr/status/1831599811273093490

🥰 @spector-9