-
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.
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
@@ -37,7 +40,78 @@ module System.PosixCompat.User ( | |||
|
|||
#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
@@ -26,6 +26,9 @@ flag old-time | |||
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
UserEntry
andGroupEntry
into theByteString
based implementation instead ofString
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 forUserEntry
andGroupEntry
to resolve this problem.This patch provides API compatibilities not between platforms but between versions of library, so the
compat
which this patch provide isn't probably the same as thecompat
which this package provides.Fix #57.