From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: passt.top; dmarc=pass (p=quarantine dis=none) header.from=redhat.com Authentication-Results: passt.top; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=ckhkgc4M; dkim-atps=neutral Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by passt.top (Postfix) with ESMTPS id C55375A061D for ; Sun, 03 May 2026 23:56:56 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1777845415; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=v6qaVj7Nd3mwrqhtVD+yRVjgEKQ+FwWpN/3i3MvibYo=; b=ckhkgc4MTJXkk1jqbq+F1vAWLecpjG9S/VNhn/veDN0rBu3WVfj19jRV3FpDGZGYmI6Xhv GsVXqgY3/Vj9ThwhryYQP/YKAfpxLpHW9kveeM/a57a2HL4RxjcXSJRwJyLTihEzymusRN lUhyldJZWM9PDLxqNVo+pfN0LzNm6Ns= Received: from mail-wr1-f71.google.com (mail-wr1-f71.google.com [209.85.221.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-277-4MJAKXJhPR2ot0OtPMZQWQ-1; Sun, 03 May 2026 17:56:54 -0400 X-MC-Unique: 4MJAKXJhPR2ot0OtPMZQWQ-1 X-Mimecast-MFC-AGG-ID: 4MJAKXJhPR2ot0OtPMZQWQ_1777845413 Received: by mail-wr1-f71.google.com with SMTP id ffacd0b85a97d-44dad1b938fso346421f8f.1 for ; Sun, 03 May 2026 14:56:53 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1777845411; x=1778450211; h=date:content-transfer-encoding:mime-version:organization:references :in-reply-to:message-id:subject:cc:to:from:x-gm-gg :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=v6qaVj7Nd3mwrqhtVD+yRVjgEKQ+FwWpN/3i3MvibYo=; b=UPuB663vR2uJxnbDI2CPEmNZ/8K0i0N/JXawkl9CwKF8vvfkpxkqZS4pRHBM2ea5qQ Kqid4kSR0MHiv/IaK0W4szHZBDf8wcICadSzzpX4UEgxZ4qNZJFzOYx/cxMSqydogBJJ xsNc0oApvXXaaxb5eAPp4r0OhjcdMzYW2sG1w3uPn35Mn4gZM5mfiZmwsxrzRjS6fRDr zuhEF/fitwNUlh6BYzUBcRi4x/AAad7b+9r39lMzmT8JbSwFIrKEMhqbtEeHDBfMhflB 4KncqP5nqHXdrwHwJwrouys+I0/5NIfO7sg2ft5n4mPKgSHSVJw5WXF76Kn+gGEkdZK5 K+Ig== X-Gm-Message-State: AOJu0YzWQa+fvOBmCkPeNqxsw751G+P40bYXQDQ5Z0vmgcD+GZG8lsgg p1QK8xlIyypPnXscJ78xOt4PJEZZDa1D13dRG3PCFt06gcaivCIEwDHqhft/A1FpW5elm0/pALZ XrRce/HZds7GaKzwcR75e6plIHZ5p7fWfPMkuOOI5hS/AJZYnqF+W2nxwOr86Sg== X-Gm-Gg: AeBDies3PZ147Wf5R8fvoor+V1d4XVAGc9T7fPgvvt3KAe1DwHVNuzZqNltVPrJbEQU gNBGkliUm7cx6jpzXKf8lYJonm9aR/CuECSqqYZzJuDC99R7ytywaoyeT3ZRq+MSFovf1fd9Xjx e2PUFIE7hMaj0fSjiAIGigRCjl2vqPfR8MMbJTBJiPvWX55npehgV0bf6Hc3RWSRr2uGw2kDi9N e4CAUAhsD3OtIfIsyUmSXYUN7rMMrcsK6y0Wczcw9XqcgoN8cWTgRDxVdNSrP3HJcpfktOdtEpz 3jzOD4Ja2gfW0ggWPxTpMDhypHsaCJ5IqWngDHS1/wzptBxwqUzbOiFvGpXi83JFYOB0f1tj8ca MwuNEblLW+ZganeqCMBQbOtrjbZp9FCqvdierY2XDKuQ= X-Received: by 2002:a05:600c:c082:b0:489:1f3e:5f69 with SMTP id 5b1f17b1804b1-48a98894fa7mr81103905e9.18.1777845410717; Sun, 03 May 2026 14:56:50 -0700 (PDT) X-Received: by 2002:a05:600c:c082:b0:489:1f3e:5f69 with SMTP id 5b1f17b1804b1-48a98894fa7mr81103735e9.18.1777845410282; Sun, 03 May 2026 14:56:50 -0700 (PDT) Received: from maya.myfinge.rs (ifcgrfdd.trafficplex.cloud. [2a10:fc81:a806:d6a9::1]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-48a8fee5046sm63283165e9.21.2026.05.03.14.56.48 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 03 May 2026 14:56:48 -0700 (PDT) From: Stefano Brivio To: David Gibson Subject: Re: [PATCH v2 04/13] Makefile: Remove non-standard $(FLAGS) variable Message-ID: <20260503235647.4aa1672c@elisabeth> In-Reply-To: References: <20260421032338.1909084-1-david@gibson.dropbear.id.au> <20260421032338.1909084-5-david@gibson.dropbear.id.au> <20260428091728.3b00b5be@elisabeth> Organization: Red Hat X-Mailer: Claws Mail 4.2.0 (GTK 3.24.49; x86_64-pc-linux-gnu) MIME-Version: 1.0 Date: Sun, 03 May 2026 23:56:47 +0200 (CEST) X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: C3jvpLDF_5RlaD10IbZH4kpD6Vl0AN-1aC2jK_IuBnc_1777845413 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Message-ID-Hash: G2VVRWFFXMQC6BRTVHFF54R37SCYFVJE X-Message-ID-Hash: G2VVRWFFXMQC6BRTVHFF54R37SCYFVJE X-MailFrom: sbrivio@redhat.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: passt-dev@passt.top X-Mailman-Version: 3.3.8 Precedence: list List-Id: Development discussion and patches for passt Archived-At: Archived-At: List-Archive: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: On Wed, 29 Apr 2026 13:47:13 +1000 David Gibson wrote: > On Tue, Apr 28, 2026 at 09:17:31AM +0200, Stefano Brivio wrote: > > On Tue, 21 Apr 2026 13:23:29 +1000 > > David Gibson wrote: > > > > > FLAGS was introduced over the more standard CFLAGS, because there are some > > > options we can't compile without, so overriding CFLAGS from the command > > > line wasn't practical. We've now better dealt with that using > > > BASE_CPPFLAGS, so there's no real need for FLAGS any more. I was about to add CFLAGS back to test/build/build.py (see below), but then I realised that this patch actually defeats the purpose of FLAGS, see commit 512f5b1aab2a ("Makefile: Allow define overrides by prepending, not appending, CFLAGS") for the actual reason behind it. That is, with this patch: > > > Replace it > > > with the more conventional CFLAGS, which now *can* be reasonable overridden > > > from the command line. ...passing CFLAGS completely overrides the default CFLAGS (instead of prepending them, that is, overriding single existing options). So you can't practically pass (add) "-Werror" anymore, as that will drop stuff like -std=c11, which means one can't use -Werror in build tests. It also drops bits like -pie -fPIE, so distributions (e.g. openSUSE) can't use that to override FORTIFY_SOURCE. When I committed 512f5b1aab2a, I realised that, with the "prepending" semantics, there's no way to completely override / clear CFLAGS, but: - it doesn't seem like a common use case anyway - what passing CFLAGS from the command line does isn't really standardised, and it seems to be common to use it as "extra" CFLAGS Now, it would be possible to introduce something like EXTRA_CFLAGS and ask users and distributions to switch to it, but it needs to happen in two steps: - add EXTRA_CFLAGS and ask distribution maintainers to switch to that - after a reasonable amount of time (I would say six months or so) make command-line CFLAGS replace the default CFLAGS I don't really see a strong motivation to do this though, at the moment. > > > > > > Signed-off-by: David Gibson > > > --- > > > Makefile | 21 ++++++++++----------- > > > test/build/build.py | 4 ++-- > > > 2 files changed, 12 insertions(+), 13 deletions(-) > > > > > > diff --git a/Makefile b/Makefile > > > index e89e5556..1e5f0282 100644 > > > --- a/Makefile > > > +++ b/Makefile > > > @@ -36,8 +36,8 @@ BASE_CPPFLAGS := -D_XOPEN_SOURCE=700 -D_GNU_SOURCE \ > > > -DVERSION=\"$(VERSION)\" > > > CPPFLAGS := $(FORTIFY_FLAG) -DDUAL_STACK_SOCKETS=$(DUAL_STACK_SOCKETS) > > > > > > -FLAGS := -Wall -Wextra -Wno-format-zero-length -Wformat-security > > > -FLAGS += -pedantic -std=c11 -O2 -pie -fPIE > > > +WARNINGS = -Wall -Wextra -Wno-format-zero-length -Wformat-security > > > +CFLAGS = -pedantic -std=c11 -O2 -pie -fPIE $(WARNINGS) > > > > > > PASST_SRCS = arch.c arp.c bitmap.c checksum.c conf.c dhcp.c dhcpv6.c \ > > > epoll_ctl.c flow.c fwd.c fwd_rule.c icmp.c igmp.c inany.c iov.c ip.c \ > > > @@ -66,7 +66,7 @@ ifeq ($(shell printf "$(C)" | $(CC) -S -xc - -o - >/dev/null 2>&1; echo $$?),0) > > > endif > > > > > > ifeq ($(shell :|$(CC) -fstack-protector-strong -S -xc - -o - >/dev/null 2>&1; echo $$?),0) > > > - FLAGS += -fstack-protector-strong > > > + CFLAGS += -fstack-protector-strong > > > endif > > > > > > prefix ?= /usr/local > > > @@ -85,7 +85,7 @@ endif > > > > > > all: $(BIN) $(MANPAGES) docs > > > > > > -static: FLAGS += -static > > > +static: CFLAGS += -static > > > static: CPPFLAGS += -DGLIBC_NO_STATIC_NSS > > > static: clean all > > > > > > @@ -96,12 +96,11 @@ seccomp_repair.h: seccomp.sh $(PASST_REPAIR_SRCS) > > > @ ARCH="$(TARGET_ARCH)" CC="$(CC)" ./seccomp.sh seccomp_repair.h $(PASST_REPAIR_SRCS) > > > > > > passt: $(PASST_SRCS) $(HEADERS) > > > - $(CC) $(FLAGS) $(CFLAGS) $(BASE_CPPFLAGS) $(CPPFLAGS) $(PASST_SRCS) -o passt $(LDFLAGS) > > > + $(CC) $(CFLAGS) $(BASE_CPPFLAGS) $(CPPFLAGS) $(PASST_SRCS) -o passt $(LDFLAGS) > > > > > > -passt.avx2: FLAGS += -Ofast -mavx2 -ftree-vectorize -funroll-loops > > > +passt.avx2: CFLAGS += -Ofast -mavx2 -ftree-vectorize -funroll-loops > > > passt.avx2: $(PASST_SRCS) $(HEADERS) > > > - $(CC) $(filter-out -O2,$(FLAGS)) $(CFLAGS) $(BASE_CPPFLAGS) $(CPPFLAGS) \ > > > - $(PASST_SRCS) -o passt.avx2 $(LDFLAGS) > > > + $(CC) $(CFLAGS) $(BASE_CPPFLAGS) $(CPPFLAGS) $(PASST_SRCS) -o passt.avx2 $(LDFLAGS) > > > > > > passt.avx2: passt > > > > > > @@ -109,16 +108,16 @@ pasta.avx2 pasta.1 pasta: pasta%: passt% > > > ln -sf $< $@ > > > > > > qrap: $(QRAP_SRCS) passt.h > > > - $(CC) $(FLAGS) $(CFLAGS) $(BASE_CPPFLAGS) $(CPPFLAGS) -DARCH=\"$(TARGET_ARCH)\" $(QRAP_SRCS) -o qrap $(LDFLAGS) > > > + $(CC) $(CFLAGS) $(BASE_CPPFLAGS) $(CPPFLAGS) -DARCH=\"$(TARGET_ARCH)\" $(QRAP_SRCS) -o qrap $(LDFLAGS) > > > > > > passt-repair: $(PASST_REPAIR_SRCS) seccomp_repair.h > > > - $(CC) $(FLAGS) $(CFLAGS) $(BASE_CPPFLAGS) $(CPPFLAGS) $(PASST_REPAIR_SRCS) -o passt-repair $(LDFLAGS) > > > + $(CC) $(CFLAGS) $(BASE_CPPFLAGS) $(CPPFLAGS) $(PASST_REPAIR_SRCS) -o passt-repair $(LDFLAGS) > > > > > > valgrind: EXTRA_SYSCALLS += rt_sigprocmask rt_sigtimedwait rt_sigaction \ > > > rt_sigreturn getpid gettid kill clock_gettime \ > > > mmap|mmap2 munmap open unlink gettimeofday futex \ > > > statx readlink > > > -valgrind: FLAGS += -g > > > +valgrind: CFLAGS += -g > > > valgrind: CPPFLAGS += -DVALGRIND > > > valgrind: all > > > > > > diff --git a/test/build/build.py b/test/build/build.py > > > index e3de8305..7c9cbb44 100755 > > > --- a/test/build/build.py > > > +++ b/test/build/build.py > > > @@ -60,7 +60,7 @@ def test_make(target: str, expected_files: list[str]) -> None: > > > with clone_sources(): > > > for p in ex_paths: > > > assert not p.exists(), f"{p} existed before make" > > > - sh(f'make {target} CFLAGS="-Werror"') > > > + sh(f'make {target}') > > > for p in ex_paths: > > > assert p.exists(), f"{p} wasn't made" > > > sh('make clean') > > > @@ -90,7 +90,7 @@ def test_install_uninstall() -> None: > > > progs = ['passt', 'pasta', 'qrap'] > > > > > > # Install > > > - sh(f'make install CFLAGS="-Werror" prefix={prefix}') > > > + sh(f'make install prefix={prefix}') > > > > Here, and above: I don't understand what (if anything) implies -Werror > > now. > > Ah, oops. I misread the -Werror in test/Makefile, thinking it was in > Makefile and applied to everything. I think the right fix is to put > -Werror in the default CFLAGS and remove it from here. That's not really doable as it would occasionally break distributions, where specific toolchain or architecture combinations lead quite often to harmless warnings. I can remember dozens of those, and not a single one that was actually a critical problem that made it preferable to have missing packages. It's also very annoying for developers, especially for myself as I often have to run quick tests with different compilers. I would rather add -Werror back to test/build/build.py for the moment being by dropping this patch, and then drop patches that non-trivially depend on this one (due to lack of time, not because I have anything against the other patches, which look good to me except for 13/13). That is, I would reduce this series to the bare minimum that's needed for the "RFC: Dynamic configuration update implementation" series, to avoid blocking progress there. I haven't quite figured out how to do that yet, but that's next on my list unless you get to that first. -- Stefano