On Tue, Aug 13, 2019 at 03:52:31PM +0200, Thomas Deutschmann wrote: > On 2019-08-13 03:04, Dave Chinner wrote: > >> Normally, configure will get the value and the Makefiles will use the > >> value _from_ configure... but using configure _and_ reading _and adding_ > >> values from environment _in addition_ seems to be wrong. > > > > <sigh> > > > > xfsprogs-2.7.18 (16 May 2006) > > - Fixed a case where xfs_repair was reporting a valid used > > block as a duplicate during phase 4. > > - Fixed a case where xfs_repair could incorrectly flag extent > > b+tree nodes as corrupt. > > - Portability changes, get xfs_repair compiling on IRIX. > > - Parent pointer updates in xfs_io checker command. > > - Allow LDFLAGS to be overridden, for Gentoo punters. > > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > > > > Back in 2006 we added the ability for LDFLAGS to be overriden > > specifically because Gentoo users wanted it. > > Well, you got us :-) > > Sorry, I wasn't around 2006 and don't know all details. Looks > like commit https://git.kernel.org/pub/scm/fs/xfs/xfsprogs-dev.git/commit/?id=4d32d744f07ce74abac7029c3ee7c6f5e4238d25 caused that > changelog entry. I was hoping you'd look at the change and then notice and understand that the override provided was for LDFLAGS passed by configure, not the make environment. And that, having looked at this hunk of the patch: -LDFLAGS = $(LLDFLAGS) +LDFLAGS += $(LOADERFLAGS) $(LLDFLAGS) then, perhaps, you'd see the obvious bug introduced by that change that resulted in LDFLAGS no longer being initialised correctly and so could be accidentally initialised from the external environment.... > Back to xfsprogs: When you pass your default CFLAGS/LDFLAGS > to configure and different to make, i.e. > > CFLAGS="-O2 -pipe -march=ivybridge -mtune=ivybridge -mno-xsaveopt -frecord-gcc-switches" \ > LDFLAGS="-Wl,-O1 -Wl,--as-needed" \ > ./configure > > and > > CFLAGS="-O2 -pipe -march=native" \ > LDFAGS="-Wl,-fno-lto" \ > make V=1 > > you will see > > > /bin/bash ../libtool --quiet --tag=CC --mode=link gcc -o mkfs.xfs -Wl,-fno-lto -Wl,-O1 -Wl,--as-needed -Wl,-O1 -Wl,--as-needed -Wl,-O1 -Wl,--as-needed -static-libtool-libs proto.o xfs_mkfs.o ../libxfs/libxfs.la ../libxcmd/libxcmd.la ../libfrog/libfrog.la -lrt -lpthread -lblkid -luuid I also note that you haven't observed that CFLAGS did not get overridden in this test, despite it being specified in the environment like LDFLAGS. So we correctly initialise CFLAGS so it can't be overriden from make, but we don't initialise LDFLAGS. Yeah, I noticed this a couple of days ago, and I've been waiting for you to send the "you forgot to initialise a varaible 13 years ago" patch after I pointed you right at it... Really, I should have just sent this patch when I wrote it couple of days ago, rather than thinking that with enough information you'd find it yourself and get credit and cudos for fixing it for us. Gesta non verba. -Dave. -- Dave Chinner david@xxxxxxxxxxxxx xfsprogs: LDFLAGS comes from configure, not the make environment From: Dave Chinner <dchinner@xxxxxxxxxx> When doing: $ LDFLAGS=foo make bad things happen because we don't initialise LDFLAGS to an empty string in include/builddefs.in and hence make takes wahtever is in the environment and runs with it. This causes problems even when the correct linker options are specified correctly through configure. We don't support overriding build flags (like CFLAGS) though the make environment, so it was an oversight 13 years ago to allow LDFLAGS to be overridden when adding support to custom LDFLAGS being passed from the the configure script. This ensures we only ever use linker flags from configure, not the make environment. Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx> --- include/builddefs.in | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/include/builddefs.in b/include/builddefs.in index c5b38b073e1f..e0f0ad94976e 100644 --- a/include/builddefs.in +++ b/include/builddefs.in @@ -16,6 +16,10 @@ LTLDFLAGS = @LDFLAGS@ CFLAGS = @CFLAGS@ -D_FILE_OFFSET_BITS=64 BUILD_CFLAGS = @BUILD_CFLAGS@ -D_FILE_OFFSET_BITS=64 +# make sure we don't pick up whacky LDFLAGS from the make environment and +# only use what we calculate from the configured options above. +LDFLAGS = + LIBRT = @librt@ LIBUUID = @libuuid@ LIBPTHREAD = @libpthread@
