Allow bytestring-0.12

[Bodigrim]

Could this please be backported to whatever release is to accompany GHC 9.8?

Cf. https://gitlab.haskell.org/ghc/ghc/-/issues/23926#note_524594

[Kleidukos]

@Bodigrim Thanks a lot!

[Bodigrim]

Seems stuck on CI, could someone please unstuck it?

[geekosaur]

Did I make a mess?

/home/runner/work/cabal/cabal/dist-newstyle-validate-ghc-9.4.4/build/x86_64-linux/ghc-9.4.4/cabal-testsuite-3/build/global-autogen: checkForInteriorNuls: invalid argument (POSIX filepaths must not contain internal NUL octets.)

The diff from the rebase doesn't look like it could have caused this.

[Bodigrim]

No, it's not you, it's a new unix-2.8.2.0: https://hackage.haskell.org/package/unix-2.8.2.0/changelog
This is unrelated to bytestring-0.12, but also needs fixing before release for GHC 9.8: if something inside Cabal test suite (rogue QuickCheck instance?) generates file names with \NUL, we are in trouble. (Such filenames used to be truncated silently before)

[Mikolaj]

@geekosaur: I can't see this NUL error message in CI logs. Where is it? How do we reproduce?

[Mikolaj]

If this is reproduced, we need a new ticket for it and I'm sure @Kleidukos would want to block 3.10.2.0 on it.

[geekosaur]

Configuring test suite 'check-tests' for Cabal-tests-3..
Preprocessing test suite 'check-tests' for Cabal-tests-3..
Building test suite 'check-tests' for Cabal-tests-3..
[1 of 1] Compiling Main             ( tests/CheckTests.hs, /mnt/data1/cabal-source/dist-newstyle-validate-ghc-9.4.7/build/x86_64-linux/ghc-9.4.7/Cabal-tests-3/t/check-tests/build/check-tests/check-tests-tmp/Main.o )

tests/CheckTests.hs:6:1: warning: [-Wunused-imports]
    The import of ‘Test.Tasty.ExpectedFailure’ is redundant
      except perhaps to import instances from ‘Test.Tasty.ExpectedFailure’
    To import instances alone, use: import Test.Tasty.ExpectedFailure()
  |
6 | import Test.Tasty.ExpectedFailure
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[2 of 2] Linking /mnt/data1/cabal-source/dist-newstyle-validate-ghc-9.4.7/build/x86_64-linux/ghc-9.4.7/Cabal-tests-3/t/check-tests/build/check-tests/check-tests
[1 of 2] Compiling Main             ( /mnt/data1/cabal-source/dist-newstyle-validate-ghc-9.4.7/build/x86_64-linux/ghc-9.4.7/cabal-testsuite-3/setup/setup.hs, /mnt/data1/cabal-source/dist-newstyle-validate-ghc-9.4.7/build/x86_64-linux/ghc-9.4.7/cabal-testsuite-3/setup/Main.o )
[2 of 2] Linking /mnt/data1/cabal-source/dist-newstyle-validate-ghc-9.4.7/build/x86_64-linux/ghc-9.4.7/cabal-testsuite-3/setup/setup
Configuring cabal-testsuite-3...
/usr/bin/tar: getPermissions:checkForInteriorNuls: invalid argument (POSIX filepaths must not contain internal NUL octets.)
Error: cabal: Failed to build cabal-testsuite-3. The failure occurred during
the configure step.

[geekosaur]

Am I misreading the above, or is this happening while verifying permissions (presumably exec) on /usr/bin/tar? I tried looking at that but saw nothing obvious. It does make me wonder if something is spuriously NUL-terminating paths, though.

[gbaz]

I concluded there's an off-by-one logic bug somewhere between filepath, unix-2.8.2.0, and bytestring's short-bytestring interpretation.

This gist shows it. https://gist.github.com/gbaz/f068beb5da3acab08444452b1a421fd8

For cabal I suggest we enforce unix < 2.8.2.0 for now and force upstream to sort it out.

[gbaz]

@hasufell see above ^^

[Mikolaj]

@Bodigrim: would you alert the Upstream?

@Kleidukos: do you approve unix < 2.8.2.0 as proposed by @gbaz? If so, perhaps we could add it as yet another commit to this PR?

Anything else we are missing?

[geekosaur]

I don't think we've sorted out which upstream to notify yet. It could be bytestring if it's an off-by-one in length, or it could be a bug in encodeFP from filepath.

[hasufell]

This looks like something worse than off by one:

ghci> :set -XOverloadedStrings
ghci> import qualified Data.ByteString.Short as BSS
ghci> import Data.ByteString.Internal (c_strlen)
ghci> BSS.useAsCStringLen "ScriptEnv0.hs" $ \(ptr, len) -> c_strlen ptr >>= \clen -> print (clen, len)
(13,13)
ghci> BSS.useAsCStringLen "ScriptEnv0.hs" $ \(ptr, len) -> c_strlen ptr >>= \clen -> print (clen, len)
(38,13)
ghci> BSS.useAsCStringLen "ScriptEnv0.hs" $ \(ptr, len) -> c_strlen ptr >>= \clen -> print (clen, len)
(29,13)
ghci> BSS.useAsCStringLen "ScriptEnv0.hs" $ \(ptr, len) -> c_strlen ptr >>= \clen -> print (clen, len)
(38,13)

[hasufell]

Actually it's not. You're just using c_strlen wrong. useAsCStringLen does not null-terminate the CString, so you're relying on the memory location to be sane: https://hackage.haskell.org/package/bytestring-0.12.0.2/docs/src/Data.ByteString.Short.Internal.html#useAsCStringLen

This is not a bug.

[geekosaur]

We're not the ones using it; that code is buried in directory.

[hasufell]

So where exactly does this issue surface? Do you use c_strlen in your test? Does something in filepath/unix use it wrongly? Which codepath is failing?

[hasufell]

Ok, I found it: https://github.com/haskell/unix/blob/fadc6e1b07c212a388bc08f1f8b32873edf9d164/System/Posix/PosixPath/FilePath.hsc#L149-L166

[geekosaur]

Right, I just landed there after retracing my steps earlier this afternoon, which started with directory.

[hasufell]

haskell/unix#294

[hasufell]

Please try the fix by adjusting your cabal.project pointing it to commit 2f4b1cf136c09c176a83617d5a20fc5c4b2d5264 in unix.

[Mikolaj]

@Bodigrim says "In the meantime I've revised unix-2.8.2.0 with base < 0" so we can probably resume the release of 3.10.2.0. Let me restart the failing tests on CI.

[Mikolaj]

@mergify backport 3.10

[mergify]

backport 3.10

✅ Backports have been created

• #9257 Allow bytestring-0.12 (backport #9241) has been created for branch 3.10 but encountered conflicts