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

Support bytestring-0.10 #1

Merged
merged 1 commit into from
Apr 10, 2024
Merged

Support bytestring-0.10 #1

merged 1 commit into from
Apr 10, 2024

Conversation

edsko
Copy link
Collaborator

@edsko edsko commented Apr 10, 2024

We were previously explicitly using on BS constructor, which changed in bytestring-0.11. Fortunately, it is not too difficult to avoid this.

On the pattern matching side, instead of

compress (BS sfp slen) = 
    ..
    withForeignPtr sfp $ \sptr ->
      ..

we can do

compress bs =  
    ..
    unsafeUseAsCStringLen bs $ \(sptr, slen) ->
      ..

which does the same thing:

unsafeUseAsCStringLen :: ByteString -> (CStringLen -> IO a) -> IO a
unsafeUseAsCStringLen (BS ps l) action = withForeignPtr ps $ \p -> action (castPtr p, l)

On the construction side, instead of

dfp <- mallocForeignPtrBytes ..
withForeignPtr dfp $ \dptr ->
  ..
    return $ BS dfp ..

we can instead do

dptr <- mallocBytes ..
..
unsafePackMallocCStringLen (dptr, ..)

which then adds the finalizer (which was previously added by
mallocForeignPtrBytes):

unsafePackMallocCStringLen :: CStringLen -> IO ByteString
unsafePackMallocCStringLen (cstr, len) = do
    fp <- newForeignPtr c_free_finalizer (castPtr cstr)
    return $! BS fp len

Supporting bytestring-0.10 makes install plans a lot easier, especially on CI (upgrading bytestring is possible, but bytestring has a lot of dependencies, all of which then need to be updated also). Accordingly, this simplifies the cabal.haskell-ci file.

/cc @FinleyMcIlwaine

@edsko edsko force-pushed the edsko/support-older-bytestring branch from f1ed45f to 952228d Compare April 10, 2024 09:01
@edsko edsko marked this pull request as draft April 10, 2024 09:07
@edsko edsko force-pushed the edsko/support-older-bytestring branch from 952228d to b446ce9 Compare April 10, 2024 09:30
@edsko edsko force-pushed the edsko/support-older-bytestring branch from b446ce9 to 4d143d8 Compare April 10, 2024 09:47
@edsko
Copy link
Collaborator Author

edsko commented Apr 10, 2024

@FinleyMcIlwaine I made an independent fork of snappy at https://github.com/edsko/snappy/tree/edsko/bytestring-0.11 , because your fork required bytestring-0.11, which initially confused me (why would such an old and unmaitained package require a new version of bytestring?) until I realized you needed that only to hide the BS constructor to avoid the name clash. I instead renamed the local BS constructor to WrapBS; this appears to be the only change actually required to build it.

I guess I can submit a PR against your fork, or we can drop your fork in favour of mine, I don't mind either way (but GitHub doesn't recognize your fork as being a fork, for some reason..? 🤔 ).

@edsko edsko marked this pull request as ready for review April 10, 2024 09:50
@edsko edsko merged commit 7b37b14 into main Apr 10, 2024
10 checks passed
@edsko edsko deleted the edsko/support-older-bytestring branch April 10, 2024 09:53
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.

1 participant