Thanks for the quick fix. Could you push that commit to your own freebsd clone on GitHub? That should give you the CI results after a few minutes.
If not, I can get to it later and confirm whether that works. On Sun, 3 Jan 2021, 15:30 Conrad Meyer, <[email protected]> wrote: > On Sun, Jan 3, 2021 at 4:37 AM Ulrich Spörlein <[email protected]> > wrote: > > This broke the kernel-toolchain CI on linux and macos systems, most > > importantly the Github CI actions. They were nerfed due to the master > > -> main switch, but I've bisected this to the merge commit that > > introduces the build failure. > > Thanks for investigating it. > > > sys/contrib/zstd/lib/compress/zstd_compress_internal.h ends like so > > with this import: > > +/** ZSTD_cycleLog() : > > + * condition for correct operation : hashLog > 1 */ > > +#ifdef __FreeBSD__ /* This symbol is needed by dll-linked > > CLI zstd(1). */ > > +ZSTDLIB_API U32 ZSTD_cycleLog(U32 hashLog, ZSTD_strategy strat); > > +#endif > > > > Why would this be specific to FreeBSD as the host OS? > > In short: it isn't (wasn't). The intention was to mark the > FreeBSD-specific modification. (The modification was not the > definition itself, which is present in 1.4.5, but the ZSTDLIB_API > annotation.) At the time, the tree was not built on non-FreeBSD > platforms. > > It was introduced in the v1.4.5 import: 37f1f2684f26 (r361426), not > v1.4.8. The intention was to expose the symbol in our > libprivatezstd.so, so that zstd(1) could link it. Upstream libzstd > builds with -fvisibility=hidden, and ZSTDLIB_API is > __attribute__((visibility("default"))) on GCC-like platforms (i.e., > Clang). > > Why does this show up in 1.4.8? I'm not sure. In 1.4.5, the symbol > was only defined in libzstd, but never used. However, it was used in > zstd(1). In 1.4.8, it is also used in libzstd: both in > zstdmt_compress.c and in zstd_compress.c (though, after the > definition). So apparently our non-FreeBSD CI build either skips > zstd(1), or allows invoking implicit undeclared functions in zstd(1). > But not in libzstd. > > Despite it being a private symbol, zstd(1) uses it for the flag > mentioned in the 1.4.5 commit (and in v1.4.5: *only* in zstd(1)): > > $ nm -D /usr/bin/zstd | grep cycleLog > U ZSTD_cycleLog > > I.e., upstream zstd build broke shared-linked zstd(1) in 1.4.5 (or > maybe earlier, but this was a new breakage for us). See > https://github.com/facebook/zstd/issues/1680 , > https://github.com/facebook/zstd/issues/1854 . See also: > > https://github.com/facebook/zstd/blob/f2ac2b7bcf6294f1c3a8d62b5f2ca78cfc6fb980/programs/Makefile#L291-L302 > > Now, we do not actually appear to build libprivatezstd with -fprivate, > so in fact symbols declared in zstd_compress_intenal.h without > ZSTDLIB_API are all visible anyway. E.g., > > $ nm -D /usr/lib/libprivatezstd.so.5 | grep ZSTD_writeLastEmptyBlock > 000000000001ca60 T ZSTD_writeLastEmptyBlock > > But maybe it would be nice to restore -fvisibility=hidden to match > upstream eventually. > > In the short term, I think this should fix CI: > > --- a/sys/contrib/zstd/lib/compress/zstd_compress_internal.h > +++ b/sys/contrib/zstd/lib/compress/zstd_compress_internal.h > @@ -1198,8 +1198,9 @@ > > /** ZSTD_cycleLog() : > * condition for correct operation : hashLog > 1 */ > -#ifdef __FreeBSD__ /* This symbol is needed by dll-linked CLI > zstd(1). */ > -ZSTDLIB_API U32 ZSTD_cycleLog(U32 hashLog, ZSTD_strategy strat); > -#endif > +/* Begin FreeBSD - This symbol is needed by dll-linked CLI zstd(1). */ > +ZSTDLIB_API > +/* End FreeBSD */ > +U32 ZSTD_cycleLog(U32 hashLog, ZSTD_strategy strat); > > #endif /* ZSTD_COMPRESS_H */ > > Best, > Conrad > _______________________________________________ [email protected] mailing list https://lists.freebsd.org/mailman/listinfo/dev-commits-src-all To unsubscribe, send any mail to "[email protected]"
