From owner-dev-commits-src-all@freebsd.org Sun Jan 3 14:47:18 2021 Return-Path: Delivered-To: dev-commits-src-all@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id 4519B4D3F81; Sun, 3 Jan 2021 14:47:18 +0000 (UTC) (envelope-from uspoerlein@gmail.com) Received: from mail-yb1-xb32.google.com (mail-yb1-xb32.google.com [IPv6:2607:f8b0:4864:20::b32]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "smtp.gmail.com", Issuer "GTS CA 1O1" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4D81nL1Dmqz3FxV; Sun, 3 Jan 2021 14:47:17 +0000 (UTC) (envelope-from uspoerlein@gmail.com) Received: by mail-yb1-xb32.google.com with SMTP id u203so23630753ybb.2; Sun, 03 Jan 2021 06:47:17 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=zOcnqKrQpvFMDJU0JWPg61U0S1J3h8MdJoIZMpJcQgc=; b=nLMZ6fLnqGvM4btnGQuqXUc35d5AbCZ20FPSgy6Zt4XDa+4z6SRsahD2sgtlsy3FG+ zTEDi+FSiCGCVt0GPvFP4wprOPt15Yy0dQ+4P7gnTdcP7cRXWi6P/ZXnck0M52VJbTg8 2MLd3KLJCO3lS8hB3Y9HZJgDI5KijScS16jYDZCPLhPBg/Y7HpMYUk9x8qGqY+O7V+dV qEicmSJwN56U7dZJ/9PzZ5VffJKtbqNEKq0W8nSH63xu40SX1KVsWPxzt5czjOd/Vhlq NIaNmB8rJG6DTJ1h6Em781Ud2o7x0ypoovcS6P62+v/4zmJPAeBcabcuAfwom+NQrMr0 YNww== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=zOcnqKrQpvFMDJU0JWPg61U0S1J3h8MdJoIZMpJcQgc=; b=YsjoQxT0DbpRz0ivpP7zvQxhg9ZLNj8yVRG7HWUG29qtcgKBuVKTqBs47JMsLF6dAD NGK8bjSRZYxcBWgcTayL0f/LqdXSALGo8wDYQJz8BlU/3H+PsW7eT6L8ohimtfdsdrKw 1QFCGWkoGgSFQtRHpiU1XBZ+4/ET+bW7wWBWhGDtP3s5Yg7Dsb0j2tg/sqMl8u33poKd n6dQj0XBbb6KkUPi6Wfo537BvnM6SPqaK3pUC+qwSJ5FxnQ3L0bBtZMJ12BqqvC0xqm/ KjmxxXjyvVqAXJzsLHhy1/P4v5vr7KhTb/CjiSNbqD/J6ZH3jr6pLiZJWU8yJ42fSHZE pcQw== X-Gm-Message-State: AOAM532R83in8W1MjcBJI5mUQBLMtTbKxNAIol+MxXQVdyuPwvBd9mT7 xQZZpl1Tf8bPLiUvqlq2TpzcT7M2Zf4TVCKSPNsdYFsD X-Google-Smtp-Source: ABdhPJxlYtbkozLCQQHL/9+JN5Dm1ZmkpjemM3H7QssePJga2lTvf8/06o5+6bcCE7mxD/CSWYc3tz1+eFNxdLB55bc= X-Received: by 2002:a25:9b84:: with SMTP id v4mr50490911ybo.83.1609685236754; Sun, 03 Jan 2021 06:47:16 -0800 (PST) MIME-Version: 1.0 References: <202012251556.0BPFu9l7029902@gitrepo.freebsd.org> In-Reply-To: From: =?UTF-8?Q?Ulrich_Sp=C3=B6rlein?= Date: Sun, 3 Jan 2021 15:47:07 +0100 Message-ID: Subject: Re: git: f7cd7fe51c41 - sys/contrib/zstd: Import zstd 1.4.8 To: cem@freebsd.org Cc: src-committers , dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org X-Rspamd-Queue-Id: 4D81nL1Dmqz3FxV X-Spamd-Bar: ---- Authentication-Results: mx1.freebsd.org; none X-Spamd-Result: default: False [-4.00 / 15.00]; REPLY(-4.00)[] Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Content-Filtered-By: Mailman/MimeDel 2.1.34 X-BeenThere: dev-commits-src-all@freebsd.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Commit messages for all branches of the src repository List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 03 Jan 2021 14:47:18 -0000 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, wrote: > On Sun, Jan 3, 2021 at 4:37 AM Ulrich Sp=C3=B6rlein > 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=3Dhidden, 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/f2ac2b7bcf6294f1c3a8d62b5f2ca78cfc6= fb980/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=3Dhidden 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 >