Re: LFS_CFLAGS on 32bits architectures
Hi, On Mon, May 30, 2022 at 8:29 PM Steve Langasek wrote: > > Hi Andreas, > > On Mon, May 30, 2022 at 02:16:59PM +, Andreas Hasenack wrote: > > I experimented a few things, and this got it building again: > > --- a/debian/rules > > +++ b/debian/rules > > @@ -10,6 +10,10 @@ export DH_GOLANG_INSTALL_ALL := 1 > > # Skip integration tests when building package: they need docker images. > > export ADSYS_SKIP_INTEGRATION_TESTS=1 > > > > +# be sure to set LFS_CFLAGS if needed, required for libsmbclient on 32bits > > +CGO_CFLAGS := $(shell getconf LFS_CFLAGS) > > +export CGO_CFLAGS > > + > > %: > > dh $@ --buildsystem=golang --with=golang,apport > > Would be interesting to know if 'export DEB_BUILD_MAINT_OPTIONS=future=+lfs' > also fixed the build, as conceptually that's the same level of abstraction > as fixing dpkg-buildflags per the above. That works for the build, but fails later override_dh_auto_install when d/rules does this: override_dh_auto_install: dh_auto_install -- --no-source ... # run go generate to install assets, but don’t regenerate them GENERATE_ONLY_INSTALL_TO_DESTDIR=$(CURDIR)/debian/adsys go generate -tags tools $(GOFLAGS) ./... with: # run go generate to install assets, but don’t regenerate them GENERATE_ONLY_INSTALL_TO_DESTDIR=/home/ubuntu/git/packages/adsys/adsys/debian/adsys go generate -tags tools -ldflags=-X=github.com/ubuntu/adsys/internal/consts.Version=0.8.6~ppa5 --mod=vendor -buildmode=pie ./... # github.com/mvo5/libsmbclient-go In file included from ../../vendor/github.com/mvo5/libsmbclient-go/libsmbclient.go:17: /usr/include/samba-4.0/libsmbclient.h:84:13: error: size of array 'smbc_off_t_should_be_at_least_64bits_use_LFS_CFLAGS' is too large 84 | typedef int smbc_off_t_should_be_at_least_64bits_use_LFS_CFLAGS[sizeof(off_t)-7]; | ^~~ cmd/adsysd/main.go:17: running "go": exit status 2 I'm not sure what this step is doing, but it looks like the lfs flags are lost. My other approach (with CFO_FLAGS) worked. Isn't there a place in the golang buildsystem to inject this $(getconf LFS_CFLASG)? Imagine users who just git clone that repo and build adsys manually on armhf, against upstream samba (without the LFS patch in libsmbclient.h). They would get a build with a 32bit off_t type which would be broken. A plain upstream samba build will expect an off_t of 64 bits[1]. Given [1], that samba will always be built with LFS, then I think the LFS_CFLAGS change should be in adsys, somewhere in their build system. 1. https://github.com/samba-team/samba/blob/ac16351ff5a0c5b46f461c26516b85e8483bba83/buildtools/wafsamba/wscript#L611 -- ubuntu-devel mailing list ubuntu-devel@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/ubuntu-devel
Re: LFS_CFLAGS on 32bits architectures
Hi Andreas, On Mon, May 30, 2022 at 02:16:59PM +, Andreas Hasenack wrote: > on a pi3 (armhf): > $ getconf LFS_CFLAGS > -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 > This only returns a value where it's needed, aka, 32bits (AFAIK). > Shouldn't this be part of our default set of CFLAGS on 32bits architectures? In principle, yes. The problem is, it is ABI-breaking to add this to the default set of CFLAGS on an architecture after it has been bootstrapped. There are libraries which expose either C/Linux types or their own types in function prototypes, where those types are of different sizes depending on the LFS defines in use. IIRC a classic example of this is zlib. So because i386 and armhf (and likewise all other 32-bit Debian architectures to date) have been bootstrapped without LFS_CFLAGS exported to dpkg-buildflags by default, we can't add it now without introducing forced ABI changes on an indeterminate number of libraries. Those libraries SHOULD all be made to transition to LFS_CFLAGS, but we have to know which libraries have their ABIs changing and plan transitions for them rather than just flipping the switch. If someone were able to do the necessary static analysis over the archive to identify all of the affected libraries, then I think there's a strong case to be made that we should do them all at once and then change the default flags, because that would then fix the problem for any new libraries that are introduced in the future. FYI this issue is mentioned in the dpkg-buildflags manpage: FEATURE AREAS [...] future Several compile-time options (detailed below) can be used to enable features that should be enabled by default, but cannot due to backwards compatibility reasons. lfs This setting (disabled by default) enables Large File Support on 32-bit architectures where their ABI does not include LFS by default, by adding -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 to CPPFLAGS. > I experimented a few things, and this got it building again: > --- a/debian/rules > +++ b/debian/rules > @@ -10,6 +10,10 @@ export DH_GOLANG_INSTALL_ALL := 1 > # Skip integration tests when building package: they need docker images. > export ADSYS_SKIP_INTEGRATION_TESTS=1 > > +# be sure to set LFS_CFLAGS if needed, required for libsmbclient on 32bits > +CGO_CFLAGS := $(shell getconf LFS_CFLAGS) > +export CGO_CFLAGS > + > %: > dh $@ --buildsystem=golang --with=golang,apport Would be interesting to know if 'export DEB_BUILD_MAINT_OPTIONS=future=+lfs' also fixed the build, as conceptually that's the same level of abstraction as fixing dpkg-buildflags per the above. > I'm trying a rebuild of all reverse dependencies of libsmbclient to > see if there are any other failures, but I am wondering if this is the > current approach we want to take, or revert to our old-and-tried patch > that just defines it for all apps including libsmbclient.h? I think that if there's a solution that doesn't require patching upstream samba, while guarding against miscompilation of either libraries or their consumers and avoiding accidentally introducing any ABI changes, we should prefer that. -- Steve Langasek Give me a lever long enough and a Free OS Debian Developer to set it on, and I can move the world. Ubuntu Developer https://www.debian.org/ slanga...@ubuntu.com vor...@debian.org signature.asc Description: PGP signature -- ubuntu-devel mailing list ubuntu-devel@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/ubuntu-devel
LFS_CFLAGS on 32bits architectures
Hi,
on a pi3 (armhf):
$ getconf LFS_CFLAGS
-D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64
This only returns a value where it's needed, aka, 32bits (AFAIK).
Shouldn't this be part of our default set of CFLAGS on 32bits architectures?
The reason I ask is this old samba bug[1] ("64bits prototype not
precised")], which iterated over a few fixes:
- force-refine _LARGEFILE64_SOURCE and _FILE_OFFSET_BIGS in
libsmbclient.h[2] (it's been like this since 2011 in the package)
- it was replaced with [3], which uses a `static_assert(sizeof(off_t)
>= 8` check and fails the build on 32bit without LFS
- then it was replaced again with [4], which uses this one liner:
`typedef int
smbc_off_t_should_be_at_least_64bits_use_LFS_CFLAGS[sizeof(off_t)-7];`,
which also fails the build because it becomes a very large array on
32bits if off_t is not 64bits.
In Ubuntu builds, only the first original patch was ever used. The
other iterations happened in debian unstable and I encountered them
while starting a new merge from debian for the kinetic cycle.
So far, only adsys is failing to build:
# github.com/ubuntu/adsys/vendor/github.com/mvo5/libsmbclient-go
In file included from
src/github.com/ubuntu/adsys/vendor/github.com/mvo5/libsmbclient-go/libsmbclient.go:17:
/usr/include/samba-4.0/libsmbclient.h:84:13: error: size of array
'smbc_off_t_should_be_at_least_64bits_use_LFS_CFLAGS' is too large
84 | typedef int
smbc_off_t_should_be_at_least_64bits_use_LFS_CFLAGS[sizeof(off_t)-7];
| ^~~
I experimented a few things, and this got it building again:
--- a/debian/rules
+++ b/debian/rules
@@ -10,6 +10,10 @@ export DH_GOLANG_INSTALL_ALL := 1
# Skip integration tests when building package: they need docker images.
export ADSYS_SKIP_INTEGRATION_TESTS=1
+# be sure to set LFS_CFLAGS if needed, required for libsmbclient on 32bits
+CGO_CFLAGS := $(shell getconf LFS_CFLAGS)
+export CGO_CFLAGS
+
%:
dh $@ --buildsystem=golang --with=golang,apport
I'm trying a rebuild of all reverse dependencies of libsmbclient to
see if there are any other failures, but I am wondering if this is the
current approach we want to take, or revert to our old-and-tried patch
that just defines it for all apps including libsmbclient.h?
1. http://bugs.debian.org/221618
2.
https://git.launchpad.net/ubuntu/+source/samba/tree/debian/patches/bug_221618_precise-64bit-prototype.patch?h=import/2%254.13.2%2bdfsg-1
3.
https://salsa.debian.org/samba-team/samba/-/blob/b141e7178961404a9f9cca92d04e780c5f504878/debian/patches/libsmbclient-ensure-lfs-221618.patch
4.
https://salsa.debian.org/samba-team/samba/-/blob/d0d8db5c6d6e8b5e26944031f6786031c1a389ca/debian/patches/libsmbclient-ensure-lfs-221618.patch
--
ubuntu-devel mailing list
ubuntu-devel@lists.ubuntu.com
Modify settings or unsubscribe at:
https://lists.ubuntu.com/mailman/listinfo/ubuntu-devel
