Make direct winapi deps optional #767
Conversation
jhartzell42
changed the title
|
Not sure how I feel about that if you disable a feature flag you can break compilation. We should likely also update the places in code that use the windows crate and program some logic for what happens if it is disabled. I had this idea for some time, and its how it used to be in the past, but to put input/events behind a feature flag. Because on all > windows 10 systems that is one of the main user of the windows API. Maybe we also have to do something with getting cursor pos, and terminal size. |
|
As this feature isn't designed to be disabled on Windows, perhaps it should just use the compile_error macro to explain that this feature is mandatory on Windows, giving a more explicit error rather than just failing to compile more haphazardly. The idea would be that the feature flag would only be disabled on platforms that know they're only going to run on Unix-based platforms, as a hack to work around the Cargo constraint. I can't build a project that depends on this in Yocto without a change to make the crossterm_winapi and windows deps optional in some way. If you would still rather a feature mapping that accomplishes this but leaves a binary able to build in Windows, I'm honestly not sure why -- I'd always want to build on Windows with these crates included -- but I can code that up too. I'd just need a little more detail on what you're thinking. |
|
Im fine with the first suggestion of adding a cleaner message when disabling 'windows' flag on windows. I am only concerned about devs disabling default features and then ending up with a bunch of compile errors without direct instructions on what causes it. |
|
I added the appropriate error messages. I don't have a Windows machine to test generation of these errors, however. Could you help me confirm that these errors are in fact triggered if these features are left out on Windows? I'm obviously also open to suggestions on how to phrase the errors. |
|
It seems to work correctly! Also its correctly enabled by default. |
|
@TimonPost I made the formatting fix that the CI wanted, but now it apparently needs approval to run the workflow again. |
|
@TimonPost I had to make a CI change because it was failing testing Windows with no features. Instead, I test it with no default features, but the Windows feature still enabled. Please re-run the work-flow! I hope I got it right! |
In #766 / #767, a windows feature flag was added to solve an issue with the way that Yocto package generation was working. This is not actually a problem with Crossterm, but with the Yocto package generation tooling which adds all the dependencies for all features, even if they are not relevant to the target platform. This is a bug in the Yocto / meta-rust / bitbake tooling and not in Crossterm. For more information, see <meta-rust/cargo-bitbake#58>. The fix for this on the Yocto side is to remove dependencies that are conditional on windows. This commit removes the windows feature flag as it's not needed. This is a breaking change for any apps which were specifically using the windows feature flag. If you were using this feature flag, you will need to update your Cargo.toml to remove it. The necessary dependencies are still included in the Cargo.toml file, so you should not need to make any other changes.
This is to make building easier on platforms like Yocto so we can actually remove it from the calculated dependencies. Would address #766 .