| Message ID | b2085612a223f59a2f7ef18a9bd97603226aa03d.1478784433.git.baruch@tkos.co.il |
|---|---|
| State | Accepted |
| Headers | show |
On 10-11-16 14:27, Baruch Siach wrote: > Rick Felker suggested[1] this hack as a workaround to musl libc conflict with > kernel headers: > > The problem is linux/libc-compat.h, which should fix this, only works > on glibc, by design. See: > > #ifndef _LIBC_COMPAT_H > #define _LIBC_COMPAT_H > > /* We have included glibc headers... */ > #if defined(__GLIBC__) > > /* Coordinate with glibc netinet/in.h header. */ > #if defined(_NETINET_IN_H) > > If you patch it like this: > > -#if defined(__GLIBC__) > +#if 1 > > then it should mostly work but it's still all a big hack. I think > that's what distros are doing. The problem is that the same header is > trying to do two different things: > > 1. Provide extra linux-kernel-API stuff that's not in the > libc/userspace headers. > > 2. Provide definitions of the standard types and constants for uClibc > and klibc, which don't have complete libc headers and rely on the > kernel headers for definitions. > > These two uses really should be separated out into separate headers so > that the latter only get included explicitly by uClibc and klibc and > otherwise remain completely unused. But that would require coordinated > changes/upgrades which are unlikely to happen. :( > > Upstream musl still evaluates[2][3] a permanent solution. > > With this in place we can revert (at least) commits a167081c5d (bridge-utils: > fix build with musl) and e74d4fc4932 (norm: add patch to fix musl build). > > [1] http://www.openwall.com/lists/musl/2015/10/08/2 > [2] http://git.musl-libc.org/cgit/musl/commit/?id=04983f2272382af92eb8f8838964ff944fbb8258 > [3] http://www.openwall.com/lists/musl/2016/11/09/2 > > Signed-off-by: Baruch Siach <baruch@tkos.co.il> Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be> As mentioned in response to v1, I really would prefer this to be part of musl-compat-headers, so that we have one place to collect all musl hacks. However, that would be a bit more intricate, because it would require musl-compat-headers to be built _after_ musl or the external toolchain, which makes for some complicated dependency chains. So I'm fine with this solution. However, if bridge-utils and norm are really the only packages that suffer from this problem, is it really worthwhile to apply this workaround? Regards, Arnout > --- > v2: > * Add Rick's explanation in the commit log (Arnout) > * Link to more recent upstream changes > --- > toolchain/toolchain/toolchain.mk | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) > > diff --git a/toolchain/toolchain/toolchain.mk b/toolchain/toolchain/toolchain.mk > index c22713bfe349..d317e917d032 100644 > --- a/toolchain/toolchain/toolchain.mk > +++ b/toolchain/toolchain/toolchain.mk > @@ -12,6 +12,20 @@ endif > > TOOLCHAIN_ADD_TOOLCHAIN_DEPENDENCY = NO > > +# Apply a hack that Rick Felker suggested[1] to avoid conflicts between libc > +# headers and kernel headers. This is a temporary measure until musl finds a > +# better solution. > +# > +# [1] http://www.openwall.com/lists/musl/2015/10/08/2 > +ifeq ($(BR2_TOOLCHAIN_USES_MUSL),y) > +define TOOLCHAIN_MUSL_KERNEL_HEADERS_COMPATIBILITY_HACK > + $(SED) 's/^#if defined(__GLIBC__)$$/#if 1/' \ > + $(STAGING_DIR)/usr/include/linux/libc-compat.h > +endef > +TOOLCHAIN_POST_INSTALL_STAGING_HOOKS += TOOLCHAIN_MUSL_KERNEL_HEADERS_COMPATIBILITY_HACK > +TOOLCHAIN_INSTALL_STAGING = YES > +endif > + > $(eval $(virtual-package)) > > toolchain: $(HOST_DIR)/usr/share/buildroot/toolchainfile.cmake >
Hello, On Sun, 13 Nov 2016 22:33:31 +0100, Arnout Vandecappelle wrote: > However, if bridge-utils and norm are really the only packages that suffer from > this problem, is it really worthwhile to apply this workaround? I'm sure there are more packages that we have patched for this reason, and possibly other build failures still unfixed that would get fixed (or at least partially). *However*, the musl folks have already committed a patch that apparently resolves the problem: http://git.musl-libc.org/cgit/musl/commit/?id=04983f2272382af92eb8f8838964ff944fbb8258 Baruch, could you investigate whether this musl commit fixes the header conflict problem for bridge-utils and norm? Thanks! Thomas
Hi Thomas, On Tue, Nov 15, 2016 at 11:33:47PM +0100, Thomas Petazzoni wrote: > On Sun, 13 Nov 2016 22:33:31 +0100, Arnout Vandecappelle wrote: > > > However, if bridge-utils and norm are really the only packages that suffer from > > this problem, is it really worthwhile to apply this workaround? > > I'm sure there are more packages that we have patched for this reason, > and possibly other build failures still unfixed that would get fixed > (or at least partially). > > *However*, the musl folks have already committed a patch that > apparently resolves the problem: > > http://git.musl-libc.org/cgit/musl/commit/?id=04983f2272382af92eb8f8838964ff944fbb8258 > > Baruch, could you investigate whether this musl commit fixes the header > conflict problem for bridge-utils and norm? The commit log links to this musl commit and to a mailing list report[1] that this fix is not enough on its own. As a result of this discussion Felix Janda posted a kernel patch[2] to address the issue. See also Rich's response[3]. Even if this patch is accepted, we'll most likely have to keep this workaround, or an equivalent, for as long as we support kernel headers v4.9 and older. [1] http://www.openwall.com/lists/musl/2016/11/09/2 [2] http://www.openwall.com/lists/musl/2016/11/11/1 [3] http://www.openwall.com/lists/musl/2016/11/11/2 baruch
Hello, On Wed, 16 Nov 2016 17:45:16 +0200, Baruch Siach wrote: > > Baruch, could you investigate whether this musl commit fixes the header > > conflict problem for bridge-utils and norm? > > The commit log links to this musl commit and to a mailing list report[1] that > this fix is not enough on its own. As a result of this discussion Felix Janda > posted a kernel patch[2] to address the issue. See also Rich's response[3]. > Even if this patch is accepted, we'll most likely have to keep this > workaround, or an equivalent, for as long as we support kernel headers v4.9 > and older. > > [1] http://www.openwall.com/lists/musl/2016/11/09/2 > [2] http://www.openwall.com/lists/musl/2016/11/11/1 > [3] http://www.openwall.com/lists/musl/2016/11/11/2 Thanks a lot for the explanation. So I believe we should apply your proposed workaround. However, I'm not sure I want to take the risk of applying this to the master branch (even though I agree it could fix some build failures). Are you OK with applying it to the next branch? Then, the next step (which also worries me a little bit) is: 1. Finding the packages that we have disabled for musl because of this header issue, and which should now be re-enabled in musl configurations. 2. Finding the packages that have patches to fix this musl-related issue, and remove those patches (beyond the two specific packages for which you already take care of this as part of your series). I'm not sure how to proceed to identify all the affected packages. Thanks, Thomas
Hi Thomas, On Wed, Nov 16, 2016 at 04:55:57PM +0100, Thomas Petazzoni wrote: > On Wed, 16 Nov 2016 17:45:16 +0200, Baruch Siach wrote: > > > > Baruch, could you investigate whether this musl commit fixes the header > > > conflict problem for bridge-utils and norm? > > > > The commit log links to this musl commit and to a mailing list report[1] that > > this fix is not enough on its own. As a result of this discussion Felix Janda > > posted a kernel patch[2] to address the issue. See also Rich's response[3]. > > Even if this patch is accepted, we'll most likely have to keep this > > workaround, or an equivalent, for as long as we support kernel headers v4.9 > > and older. > > > > [1] http://www.openwall.com/lists/musl/2016/11/09/2 > > [2] http://www.openwall.com/lists/musl/2016/11/11/1 > > [3] http://www.openwall.com/lists/musl/2016/11/11/2 > > Thanks a lot for the explanation. > > So I believe we should apply your proposed workaround. However, I'm not > sure I want to take the risk of applying this to the master branch > (even though I agree it could fix some build failures). Are you OK with > applying it to the next branch? Of course. This is definitely not for master. baruch
Hello, On Wed, 16 Nov 2016 18:25:20 +0200, Baruch Siach wrote: > > So I believe we should apply your proposed workaround. However, I'm not > > sure I want to take the risk of applying this to the master branch > > (even though I agree it could fix some build failures). Are you OK with > > applying it to the next branch? > > Of course. This is definitely not for master. And any thoughts about my concerns to identify the packages that should be re-enabled and patches that should be removed? Thomas
Hi Thomas, On Wed, Nov 16, 2016 at 05:36:05PM +0100, Thomas Petazzoni wrote: > On Wed, 16 Nov 2016 18:25:20 +0200, Baruch Siach wrote: > > > > So I believe we should apply your proposed workaround. However, I'm not > > > sure I want to take the risk of applying this to the master branch > > > (even though I agree it could fix some build failures). Are you OK with > > > applying it to the next branch? > > > > Of course. This is definitely not for master. > > And any thoughts about my concerns to identify the packages that should > be re-enabled and patches that should be removed? Over time as packages get version bumps we'll have the chance to revise musl patches. Patches that cleanly apply to newer versions should not bother us, since they don't add maintenance burden. As for !BR2_TOOLCHAIN_USES_MUSL dependencies, anyone interested in having a specific package building with musl may revise these dependences. Any typical package version bump sometimes makes dependencies removal possible, but it's not always apparent at bump time. That being said, there are some low hanging fruits here that I plan to pick as time allows. baruch
Hello, On Thu, 10 Nov 2016 15:27:11 +0200, Baruch Siach wrote: > Rick Felker suggested[1] this hack as a workaround to musl libc conflict with > kernel headers: > > The problem is linux/libc-compat.h, which should fix this, only works > on glibc, by design. See: > > #ifndef _LIBC_COMPAT_H > #define _LIBC_COMPAT_H > > /* We have included glibc headers... */ > #if defined(__GLIBC__) > > /* Coordinate with glibc netinet/in.h header. */ > #if defined(_NETINET_IN_H) > > If you patch it like this: > > -#if defined(__GLIBC__) > +#if 1 > > then it should mostly work but it's still all a big hack. I think > that's what distros are doing. The problem is that the same header is > trying to do two different things: > > 1. Provide extra linux-kernel-API stuff that's not in the > libc/userspace headers. > > 2. Provide definitions of the standard types and constants for uClibc > and klibc, which don't have complete libc headers and rely on the > kernel headers for definitions. > > These two uses really should be separated out into separate headers so > that the latter only get included explicitly by uClibc and klibc and > otherwise remain completely unused. But that would require coordinated > changes/upgrades which are unlikely to happen. :( > > Upstream musl still evaluates[2][3] a permanent solution. > > With this in place we can revert (at least) commits a167081c5d (bridge-utils: > fix build with musl) and e74d4fc4932 (norm: add patch to fix musl build). > > [1] http://www.openwall.com/lists/musl/2015/10/08/2 > [2] http://git.musl-libc.org/cgit/musl/commit/?id=04983f2272382af92eb8f8838964ff944fbb8258 > [3] http://www.openwall.com/lists/musl/2016/11/09/2 > > Signed-off-by: Baruch Siach <baruch@tkos.co.il> > --- > v2: > * Add Rick's explanation in the commit log (Arnout) > * Link to more recent upstream changes > --- > toolchain/toolchain/toolchain.mk | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) Series applied to the next branch. Thanks! Thomas
Hello Baruch, I'm reviving this old thread, because my colleague Florent (in Cc) has found an issue with this patch. On Thu, 10 Nov 2016 15:27:11 +0200, Baruch Siach wrote: > +# [1] http://www.openwall.com/lists/musl/2015/10/08/2 > +ifeq ($(BR2_TOOLCHAIN_USES_MUSL),y) > +define TOOLCHAIN_MUSL_KERNEL_HEADERS_COMPATIBILITY_HACK > + $(SED) 's/^#if defined(__GLIBC__)$$/#if 1/' \ > + $(STAGING_DIR)/usr/include/linux/libc-compat.h > +endef The problem with this is that libc-compat.h was introduced only in kernel 3.12 (it seems). Therefore, trying to build a musl toolchain with kernel headers 3.10 currently fails, with this $(SED) command failing because $(STAGING_DIR)/usr/include/linux/libc-compat.h does not exist. So, we could add a kernel headers >= 3.12 to the condition to make the build work, but then we wouldn't have the fix that these modifications provide to avoid conflicts between libc and kernel headers. Do you know if kernel headers < 3.12 are unaffected by the libc/kernel headers conflict issue? If they are, do you know what the solution is for kernel headers < 3.12 ? Thanks! Thomas
diff --git a/toolchain/toolchain/toolchain.mk b/toolchain/toolchain/toolchain.mk index c22713bfe349..d317e917d032 100644 --- a/toolchain/toolchain/toolchain.mk +++ b/toolchain/toolchain/toolchain.mk @@ -12,6 +12,20 @@ endif TOOLCHAIN_ADD_TOOLCHAIN_DEPENDENCY = NO +# Apply a hack that Rick Felker suggested[1] to avoid conflicts between libc +# headers and kernel headers. This is a temporary measure until musl finds a +# better solution. +# +# [1] http://www.openwall.com/lists/musl/2015/10/08/2 +ifeq ($(BR2_TOOLCHAIN_USES_MUSL),y) +define TOOLCHAIN_MUSL_KERNEL_HEADERS_COMPATIBILITY_HACK + $(SED) 's/^#if defined(__GLIBC__)$$/#if 1/' \ + $(STAGING_DIR)/usr/include/linux/libc-compat.h +endef +TOOLCHAIN_POST_INSTALL_STAGING_HOOKS += TOOLCHAIN_MUSL_KERNEL_HEADERS_COMPATIBILITY_HACK +TOOLCHAIN_INSTALL_STAGING = YES +endif + $(eval $(virtual-package)) toolchain: $(HOST_DIR)/usr/share/buildroot/toolchainfile.cmake
Rick Felker suggested[1] this hack as a workaround to musl libc conflict with kernel headers: The problem is linux/libc-compat.h, which should fix this, only works on glibc, by design. See: #ifndef _LIBC_COMPAT_H #define _LIBC_COMPAT_H /* We have included glibc headers... */ #if defined(__GLIBC__) /* Coordinate with glibc netinet/in.h header. */ #if defined(_NETINET_IN_H) If you patch it like this: -#if defined(__GLIBC__) +#if 1 then it should mostly work but it's still all a big hack. I think that's what distros are doing. The problem is that the same header is trying to do two different things: 1. Provide extra linux-kernel-API stuff that's not in the libc/userspace headers. 2. Provide definitions of the standard types and constants for uClibc and klibc, which don't have complete libc headers and rely on the kernel headers for definitions. These two uses really should be separated out into separate headers so that the latter only get included explicitly by uClibc and klibc and otherwise remain completely unused. But that would require coordinated changes/upgrades which are unlikely to happen. :( Upstream musl still evaluates[2][3] a permanent solution. With this in place we can revert (at least) commits a167081c5d (bridge-utils: fix build with musl) and e74d4fc4932 (norm: add patch to fix musl build). [1] http://www.openwall.com/lists/musl/2015/10/08/2 [2] http://git.musl-libc.org/cgit/musl/commit/?id=04983f2272382af92eb8f8838964ff944fbb8258 [3] http://www.openwall.com/lists/musl/2016/11/09/2 Signed-off-by: Baruch Siach <baruch@tkos.co.il> --- v2: * Add Rick's explanation in the commit log (Arnout) * Link to more recent upstream changes --- toolchain/toolchain/toolchain.mk | 14 ++++++++++++++ 1 file changed, 14 insertions(+)