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

Make Posix instances conditional for non-standard platforms #86

Merged
merged 1 commit into from
Nov 30, 2023

Conversation

amesgen
Copy link
Contributor

@amesgen amesgen commented Apr 2, 2023

From this note in the System.Posix.Types Haddocks:

This module contains platform specific information about types. As such the types presented on this page reflect the platform on which the documentation was generated and may not coincide with the types on your platform.

This e.g. fixes compilation via the new WASM backend in 9.6. I didn't adapt the TH-based boilerplate for pre-DerivingVia GHCs (< 8.6) because I don't see a use case, but happy to also add the necessary CPP there if that is desired.


A simpler alternative to this PR would be to drop these instances; this would be a breaking change, but I would be very surprised if anyone is actually using them.

@dhess
Copy link

dhess commented Nov 15, 2023

Hi, we've just run into this, as well. Can we get this merged?

Copy link
Contributor

@konsumlamm konsumlamm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just ran into this as well, @chessai could this be merged?

@@ -141,10 +141,51 @@ import qualified Prelude as P
#endif

#if !HOST_OS_WINDOWS
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe the HOST_OS_WINDOWS part can be removed, it was added in #35 as a bandaid for #34, but this PR should be a proper fix.

@chessai chessai merged commit 2aaec35 into chessai:master Nov 30, 2023
@amesgen amesgen deleted the posix-types-ifdefs branch November 30, 2023 19:34
@dhess
Copy link

dhess commented Apr 11, 2024

Thanks for merging this! Any chance you can upload a new release with this change to Hackage?

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.

4 participants