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

Add support for Windows 7 and 8 #146

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

helgoboss
Copy link

@helgoboss helgoboss commented Apr 16, 2023

This implements #145.

@robbert-vdh
Copy link
Member

robbert-vdh commented Apr 16, 2023

Does this require adding a dependency on libloading? I would just use LoadLibrary and GetProcAddress since we already depend on the winapi crate anyways (which should probably be replaced with the windows crate at some point), and then store the function pointers in a OnceCell (which is now in the standard library) so they're always there without having to look them up again each time you call them.

I proposed doing this in the Discord a couple months back but no one seemed to be particularly enthusiastic about having to maintain WIndows 7 support since it may break over time if you don't periodically test on it, and Windows 7 and 8 are end of life.

EDIT: I thought OnceCell in the standard library was already stabilized, but it won't be until Rust 1.70/June 1st.

@helgoboss
Copy link
Author

It doesn't strictly need libloading, I just thought it's easier to rely on the abstraction. But yes, I guess I could change it to use LoadLibrary and GetProcAddress directly.

I'm also not really inclined to test this regularly. But eagerly linking to the library functions completely rules out the possiblity to run this on older Windows versions ... I find this a bit restrictive for a general-purpose lib, especially when it's only because of 2 functions and the solution is rather simple. Maybe it's okay if it's not regularly tested as long as the the README doesn't actively advertise that it supports old Windows versions?

Some audio folks are quite conservative when it comes to the OS. There was a user wondering why ReaLearn doesn't work on his syststem (helgoboss/helgobox#791). That's why I added it in the first place.

@BillyDM BillyDM mentioned this pull request Mar 18, 2024
27 tasks
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