-
Notifications
You must be signed in to change notification settings - Fork 27
Fix compatibilities with unix >=2.8 package #60
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
base: master
Are you sure you want to change the base?
Conversation
Since version 2.8, the unix package changed `UserEntry` and `GroupEntry` into the `ByteString` based implementation instead of `String` as a breaking change. We can compile this package with unix >=2.8 if just re-exports the two of new data types, but it's probably uncomfortable for users because types of fields of the two isn't the same between prior 2.8 and since 2.8. So we need to define and export compatible data types for `UserEntry` and `GroupEntry` to resolve this problem.
mmhat
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just got hit by that myself. Thank you for fixing that! 👍
src/System/PosixCompat/User.hsc
Outdated
|
|
||
| #include "HsUnixCompat.h" | ||
|
|
||
| #ifdef UNIX_2_8 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe just use MIN_VERSION_unix(2, 8, 0) here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your review. I didn't know the MIN_VERSION_* macro. It seems that It's used at other points in this package. I'll use it. No need to use an extra flag if use it.
unix-compat.cabal
Outdated
| description: build against old-time package | ||
| default: False | ||
|
|
||
| flag unix28 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the reasoning behind introducing an extra flag?
Use MIN_VERSION* macro to branch code by versions of unix package.
|
Friendly bump. Btw, another option to work around this incompatibility, @a5ob7r, is to release a new version of |
|
I think the right fix for this is to just delete |
|
Thanks for the PR, but we've deleted |
Since version 2.8, the unix package changed
UserEntryandGroupEntryinto theByteStringbased implementation instead ofStringas a breaking change. We can compile this package with unix >=2.8 if just re-exports the two of new data types, but it's probably uncomfortable for users because types of fields of the two isn't the same between prior 2.8 and since 2.8. So we need to define and export compatible data types forUserEntryandGroupEntryto resolve this problem.This patch provides API compatibilities not between platforms but between versions of library, so the
compatwhich this patch provide isn't probably the same as thecompatwhich this package provides.Fix #57.