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

Thoughts on removing the windows-sys dependency? #58

Closed
RazrFalcon opened this issue Dec 10, 2022 · 6 comments
Closed

Thoughts on removing the windows-sys dependency? #58

RazrFalcon opened this issue Dec 10, 2022 · 6 comments

Comments

@RazrFalcon
Copy link

Right now, if one try to vendor this crate, the vendor directory will end up being 283MB big. This feels a bit insane.
Especially since you use essentially a single winapi method - MoveFileExW.
Have you thought about dropping the the windows-sys dependency and calling MoveFileExW directly?
I'm sure winapi and windows crates have a reason to be that big, but maybe it can be avoided in this particular case?

@untitaker
Copy link
Owner

What I have right now seems like the sanctioned way to do winapi stuff. How likely is it that an application on windows doesn't need one of those two crates?

I'm open to make the backend swappable while winapi is (still?) around, or to expand version ranges, those seem like no-brainers to me.

I however also have no idea what it would require to directly make the syscall.

@RazrFalcon
Copy link
Author

Yes, this is what worries me. winapi and windows crates are huge and feel like an overkill, but there should be a reason behind this.

Afaik, this is one of the reasons: danburkert/memmap-rs#89 (comment)
But I'm not sure if it affects this crate or not, since you don't need that many functions to begin with.

PS: I thought about making something like atomicwrites-lite, calling system API directly and hoping for the best, but maybe it can be incorporated directly.

@untitaker
Copy link
Owner

OK, I read up on that thread and I am not sure why you need to improve vendored archive size. Is this affecting clone speed of your repo in CI (just a guess), or something else?

I am open to adding features to disable windows support, or move windows support in a separate crate. Idk if that materially improves the situation for you?

@RazrFalcon
Copy link
Author

If you want your library to be included in most Linux distros, you have to provide a vendored archive (cargo doesn't support os/target-specific vendored archives afaik). And winapi/windows crates add like 25MB even in a compressed form. And all I what to call is a single method. This is just pure bloat.

Also, the compilation speed will be much faster, because those crates are huge.

I understand that this is me problem and it's fine for me to simply fork it, not the first time, but I thought that you might be interested in this as well.

@untitaker
Copy link
Owner

Yeah sorry, I don't think I'm going to make adjustments for that reason. I don't have much interest in working around distro things from the get-go since I have bad previous experiences with Linux distro maintainers as an application developer, and reducing bloat for bloat's sake is I think not compelling enough.

If you came to me, for example, with the argument that you need to deploy the application for some embedded usecase, and for some reason LTO doesn't work right (and I guess for some reason you're using Windows???), I think I'd want to figure it out. Idk, totally made up scenario but it's something where every MB on the end user device actually counts.

It seems to me that your energy is better spent in figuring out a story for target-specific vendoring archives if this is really a big deal on Linux distros, since it would reduce bloat across the board for absolutely all Rust packages. But I have absolutely zero knowledge about it.

@RazrFalcon
Copy link
Author

Got it. No worries.

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

No branches or pull requests

2 participants