-
Notifications
You must be signed in to change notification settings - Fork 92
Backports for 2.7.3 #235
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
Backports for 2.7.3 #235
Conversation
haskell#218 Error was: uncaught exception: IOException of type InvalidArgument semTrywait: invalid argument (Bad file descriptor)
And fix free-bug in 'putEnv'. Fixes haskell#68
This commit introduces a CPP guard to take care of the difference between OSes of the errors of posix_fallocate. On Linux and NetBSD, posix_fallocate reports error by returning an error number. errno is not set. On the other hand, on FreeBSD, posix_fallocate returns -1 and sets errno. The existing code could handle FreeBSD-style errors only.
The FreeBSD-style error (return -1, set errno) is obsolete as of FreeBSD 11.0; it now reports errors as same as the other OSes.
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.
To be honest I'd recommend to leave unix-2.7
in peace. At the very least, this is not 2.7.2.3
but rather 2.7.3
.
System/Posix/ByteString/FilePath.hsc
Outdated
-- | ||
throwErrnoTwoPathsIfMinus1_ :: (Eq a, Num a) => String -> RawFilePath -> RawFilePath -> IO a -> IO () | ||
throwErrnoTwoPathsIfMinus1_ loc path1 path2 = | ||
throwErrnoIfMinus1_ (loc <> " '" <> BC.unpack path1 <> "' to '" <> BC.unpack path2 <> "'") |
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.
This is strictly speaking a breaking change: someone could reasonably catch the error message and analyse it.
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.
Well, not per PVP, because the API is defined in terms of types only, not values.
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.
This can reasonably break a customer, so it is a breaking change, even if PVP technically allows me to replace every function call with undefined
.
That's probably overzealous, but I have extremely limited appetite to mess things up in 2.7 series.
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.
Sure, I can remove this patchset, since it's not strictly speaking a bugfix.
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.
technically allows me to replace every function call with undefined.
You're giving me ideas 😈
This previously failed to build with a warning on FreeBSD 11.2
CI does not check for GHC < 8.2, so I suggest we bump the lower bound for |
Ah, there is no CI at all in |
Needed for test suite
Added. |
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.
The code changes LGTM. Note I have not closely examined all the changes in the test tree. I don't know why they're needed. If the rest of the team is cool with that, go ahead and merge...
@bgamari
Backports: