haskell / bytestring

An efficient compact, immutable byte string type (both strict and lazy) suitable for binary or 8-bit character data.
http://hackage.haskell.org/package/bytestring
Other
290 stars 140 forks source link

0.12.1.0 fails to build on darwin aarch64 with GHC-8.10.7 #665

Open hasufell opened 7 months ago

hasufell commented 7 months ago

With 8.10.7:

bash-3.2$ cabal build -w ghc-8.10.7
Build profile: -w ghc-8.10.7 -O1
In order, the following will be built (use -v for more details):
 - data-array-byte-0.1.0.1 (lib) (requires build)
 - bytestring-0.12.1.0 (lib) (first run)
Starting     data-array-byte-0.1.0.1 (lib)
Building     data-array-byte-0.1.0.1 (lib)
Installing   data-array-byte-0.1.0.1 (lib)
Completed    data-array-byte-0.1.0.1 (lib)
Configuring library for bytestring-0.12.1.0..
Preprocessing library for bytestring-0.12.1.0..
Building library for bytestring-0.12.1.0..
[ 1 of 31] Compiling Data.ByteString.Builder.Prim.Internal ( Data/ByteString/Builder/Prim/Internal.hs, /Users/hetzner/bytestring-0.12.1.0/dist-newstyle/build/aarch64-osx/ghc-8.10.7/bytestring-0.12.1.0/build/Data/ByteString/Builder/Prim/Internal.o, /Users/hetzner/bytestring-0.12.1.0/dist-newstyle/build/aarch64-osx/ghc-8.10.7/bytestring-0.12.1.0/build/Data/ByteString/Builder/Prim/Internal.dyn_o )
[ 2 of 31] Compiling Data.ByteString.Builder.Prim.Internal.Base16 ( Data/ByteString/Builder/Prim/Internal/Base16.hs, /Users/hetzner/bytestring-0.12.1.0/dist-newstyle/build/aarch64-osx/ghc-8.10.7/bytestring-0.12.1.0/build/Data/ByteString/Builder/Prim/Internal/Base16.o, /Users/hetzner/bytestring-0.12.1.0/dist-newstyle/build/aarch64-osx/ghc-8.10.7/bytestring-0.12.1.0/build/Data/ByteString/Builder/Prim/Internal/Base16.dyn_o )

In file included from /var/folders/ds/4p3xcw1s33ldpmbxsvcsvllh0000gn/T/ghc44195_0/ghc_46.c:4:0: error:

/Users/hetzner/.ghcup/ghc/8.10.7/lib/ghc-8.10.7/include/ffi.h:436:5: error:
     error: 'FFI_GO_CLOSURES' is not defined, evaluates to 0 [-Werror,-Wundef]
    |
436 | #if FFI_GO_CLOSURES
    |     ^
#if FFI_GO_CLOSURES
    ^
1 error generated.
`gcc' failed in phase `C Compiler'. (Exit code: 1)
Error: cabal: Failed to build bytestring-0.12.1.0.

9.4.8 generates some warnings that seem to be non-fatal

cbits/aarch64/is-valid-utf8.c:30:13: error:
     warning: unknown pragma ignored [-Wunknown-pragmas]
   |
30 | #pragma GCC push_options
   |             ^
#pragma GCC push_options
            ^

cbits/aarch64/is-valid-utf8.c:31:13: error:
     warning: unknown pragma ignored [-Wunknown-pragmas]
   |
31 | #pragma GCC optimize("-O2")
   |             ^
#pragma GCC optimize("-O2")
            ^

cbits/aarch64/is-valid-utf8.c:284:13: error:
     warning: unknown pragma ignored [-Wunknown-pragmas]
    |
284 | #pragma GCC pop_options
    |             ^
#pragma GCC pop_options
            ^
3 warnings generated.

cbits/aarch64/is-valid-utf8.c:30:13: error:
     warning: unknown pragma ignored [-Wunknown-pragmas]
   |
30 | #pragma GCC push_options
   |             ^
#pragma GCC push_options
            ^

cbits/aarch64/is-valid-utf8.c:31:13: error:
     warning: unknown pragma ignored [-Wunknown-pragmas]
   |
31 | #pragma GCC optimize("-O2")
   |             ^
#pragma GCC optimize("-O2")
            ^

cbits/aarch64/is-valid-utf8.c:284:13: error:
     warning: unknown pragma ignored [-Wunknown-pragmas]
    |
284 | #pragma GCC pop_options
    |             ^
#pragma GCC pop_options
hasufell commented 7 months ago

llvm13 is used via brew for 8.10.7

clyring commented 7 months ago

Ugh. It doesn't make sense to fail because of -Wundef inside of compiler-supplied headers. But there's no obvious convenient way to say "just -Werror=undef for the code in this package."

Thanks for reporting. Does adding --ghc-options="-optP -Wno-error=undef -optc -Wno-error=undef" work around the problem?

Bodigrim commented 7 months ago

I think the immediate action is to make a revision for bytestring-0.12.1.0 prohibiting any builds with GHC < 9. While blunt, I don't believe it to hurt anyone and should resolve @hasufell's immediate issue (which is, I imagine, Cabal keen to use the latest and greatest releases).

clyring commented 7 months ago

I was thinking rather of making a revision that just removes the -Werror=undef bits. Or is that not allowed? I'm not too clear on the extent of what Hackage revisions can and can't do.

Bodigrim commented 7 months ago

No, you cannot change build flags, only dependency versions: https://hackage.haskell.org/package/bytestring-0.12.1.0/bytestring.cabal/edit

clyring commented 7 months ago

I see. It also looks possible to use our existing if (arch(aarch64)) block to only add the old-ghc-blocking base >= 4.15 constraint on that platform, which is a fair amount less blunt if it works. (But without having traced the bad included #if on a failing platform it's not a priori obvious exactly what the right condition is.)

Bodigrim commented 7 months ago

IIRC my experiments at https://github.com/haskell/bytestring/actions/runs/7791721116/job/21289850270, nothing before GHC 9.4 worked reliably on macOS aarch64.

clyring commented 7 months ago

It looks like this gets worked around on GHC's end since !10750 which appears starting with 9.4.6, 9.6.3, and 9.8.1.

clyring commented 7 months ago

Conveniently, both of the minor releases 9.4.6 and 9.6.3 bumped the base version. I've pushed a revision that checks for the corresponding base versions on aarch64. (It occurs to me now that a user could probably still get into trouble with -fpure-haskell on ghc-9.6.2. I'm inclined to not worry about that unless someone complains.)

hasufell commented 7 months ago

The defaults should be the most portable imo, not the most performant.

clyring commented 7 months ago

I'm not sure what you're getting at.

hasufell commented 7 months ago

I misread the flags, I was under the impression this is related to the standard C routines.

Then I'd suggest to make a new point release that removes Werror and manually enables it for CI.

clyring commented 7 months ago

I agree about removing -Werror=undef from the package configuration in the future to reduce the likelihood of this sort of problem re-occurring.

If there was an easy way to make -Werror=undef apply to only our CPP (and not stupid upstream CPP like this) I'd really like to keep it on, though, because if it fires in our code that is nearly 100% likely to be a bug. (And the flag has saved me a lot of debug time while refactoring already...)