Created attachment 227693 [details] v1 (use "git am") security/suricata needs to respect CC per https://docs.freebsd.org/en/books/porters-handbook/porting-dads/#dads-cc similar to https://cgit.freebsd.org/ports/commit/?id=e6d430445 Reproduce by deleting 'cc' from jail and overriding CC to another value. Package contents did change; PORTREVISION is bumped for security/suricata: 11.4/amd64: https://codeberg.org/ei/misc/commit/716325ef210 11.4/i386: https://codeberg.org/ei/misc/commit/cdbc47d98c2 12.2/amd64: https://codeberg.org/ei/misc/commit/858a4cc1217 12.2/i386: https://codeberg.org/ei/misc/commit/07437db86d8 13.0/amd64: https://codeberg.org/ei/misc/commit/14fc2ac3966 Log diff with the patch applied: 11.4/amd64: https://codeberg.org/ei/misc/commit/0a380048f17 11.4/i386: https://codeberg.org/ei/misc/commit/097fe22f3a6 12.2/amd64: https://codeberg.org/ei/misc/commit/ae58a9c4a3c 12.2/i386: https://codeberg.org/ei/misc/commit/a8e2c57bab2 13.0/amd64: https://codeberg.org/ei/misc/commit/7d711387079 Testport logs with the patch applied: 11.4/amd64: https://codeberg.org/ei/misc/commit/474602e619c 11.4/i386: https://codeberg.org/ei/misc/commit/01696078a4a 12.2/amd64: https://codeberg.org/ei/misc/commit/b2656aaadd3 12.2/i386: https://codeberg.org/ei/misc/commit/001aa252999 13.0/amd64: https://codeberg.org/ei/misc/commit/642ddf45993 13.0/aarch64 QA via qemu-user-static is omitted because the dependency 'lang/rust' can not build per https://cgit.freebsd.org/ports/commit/lang/rust/Makefile?id=ea6c7641aa
Are environment/user CFLAGS and other flags (LDFLAGS, etc) honoured? Honoured meaning 'appended', not just included
(In reply to Kubilay Kocak from comment #1) > Are environment/user CFLAGS and other flags (LDFLAGS, etc) honoured? Yes. Checked via build log: [...] #### /usr/local/etc/poudriere.d/130-amd64-make.conf #### -CFLAGS+=-O0 -LDFLAGS+=-Wl,--as-needed +CFLAGS+=-O3 +LDFLAGS+=-Wl,--no-as-needed [...] - CFLAGS -O2 -pipe -O0 -fstack-protector-strong -isystem /usr/local/include -fno-strict-aliasing -DOS_FREEBSD -std=c11 -I${srcdir}/../rust/gen -I${srcdir}/../rust/dist + CFLAGS -O2 -pipe -O3 -fstack-protector-strong -isystem /usr/local/include -fno-strict-aliasing -DOS_FREEBSD -std=c11 -I${srcdir}/../rust/gen -I${srcdir}/../rust/dist [...] -/bin/sh ../../libtool --tag=CC --mode=link cc -I../.. -D_GNU_SOURCE -g -Wall -Wextra -std=gnu99 -pedantic -Wextra -Wno-missing-field-initializers -Wshadow -Wpointer-arith -Wstrict-prototypes -Wmissing-prototypes -Wno-unused-parameter -O2 -pipe -O0 -fstack-protector-strong -isystem /usr/local/include -fno-strict-aliasing -O2 -Wstrict-overflow=1 -fstack-protector -D_FORTIFY_SOURCE=2 -Wformat -Wformat-security -fPIC -Wl,--as-needed -fstack-protector-strong -L/usr/local/lib -o liblzma-c.la LzFind.lo LzmaDec.lo -lz -L/usr/local/lib +/bin/sh ../../libtool --tag=CC --mode=link cc -I../.. -D_GNU_SOURCE -g -Wall -Wextra -std=gnu99 -pedantic -Wextra -Wno-missing-field-initializers -Wshadow -Wpointer-arith -Wstrict-prototypes -Wmissing-prototypes -Wno-unused-parameter -O2 -pipe -O3 -fstack-protector-strong -isystem /usr/local/include -fno-strict-aliasing -O2 -Wstrict-overflow=1 -fstack-protector -D_FORTIFY_SOURCE=2 -Wformat -Wformat-security -fPIC -Wl,--no-as-needed -fstack-protector-strong -L/usr/local/lib -o liblzma-c.la LzFind.lo LzmaDec.lo -lz -L/usr/local/lib [...] I didn't expose rust related build log via cargo.mk verbosity due to unfamiliarity with cargo, though the difference can be observed via build speed (-O0 vs -O3): [...] Compiling suricata v6.0.3 (/wrkdirs/usr/ports/security/suricata/work/suricata-6.0.3/rust) - Finished release [optimized + debuginfo] target(s) in 39.10s + Finished release [optimized + debuginfo] target(s) in 36.99s [...] > needs-qa Can you clarify what QA does this change need? This port does not have consumers and I already did QA on Tier 1 platforms and made sure that the package contents did not change.
(In reply to Evgeniy Khramtsov from comment #2) > made sure that the package contents did not change. Ctrl-C/Ctrl-V related typo from bug 258292.
I don't mind adding this but it feels like a high maintenance approach to hardcode RUSTFLAGS from ports directly seeing this is cherry-picked out of Mk/Uses/cargo.mk Cheers, Franco
Ping. This is still an issue even after ports 93924a20b38e.
I approved as maintainer, but I am not a committer. The non-blocking question of maintainability still stands though.
Comment on attachment 227693 [details] v1 (use "git am") I'd prefer an explicit maintainer approval on the specific patchset.
Created attachment 228169 [details] v1.1 (use "git am") Update change after ports 7059b437276. I request maintainer approval on this specific patchset, as requested by leres@.
Comment on attachment 228169 [details] v1.1 (use "git am") Maintainer approval was given for this ticket. The semantics of the patch are clear, but are less interesting as the open question of how this will be maintained when RUSTFLAGS need changes which this doesn't address at all (my original question). Nevertheless, here is an explicit patch approval. Thanks, Franco
Two things about maintainer-approval; first (Evgeniy Khramtsov), you need to specify who the maintainer is when you request it. I think this is a relatively recent change. (I fixed it for your revised patch.) Second (Franco Fichtner), you give maintainer approval by clicking on "details" for the attachment/patch, changing the '?' to a '+', and clicking "submit". Maintainer approval is different from maintainer feedback (the latter seems to be some kind of possible use for tracking maintainer timeout?) Sorry to be pedantic about this but it makes me more comfortable when I commit a PR patch for it to have explicit maintainer approval.
Comment on attachment 228169 [details] v1.1 (use "git am") Well, I did comment on the patch and try to set flag from the same page. Trying again now.
(In reply to Craig Leres from comment #10) Ping.
A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/ports/commit/?id=923567274a0367bc5a67c568c882017eef629b2c commit 923567274a0367bc5a67c568c882017eef629b2c Author: Craig Leres <leres@FreeBSD.org> AuthorDate: 2021-10-08 16:29:37 +0000 Commit: Craig Leres <leres@FreeBSD.org> CommitDate: 2021-10-08 16:29:37 +0000 security/suricata: Respect CC PR: 258294 Reported by: Evgeniy Khramtsov Approved by: Franco Fichtner (maintainer) security/suricata/Makefile | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
Apologies; I totally missed that you approved the patch at the same time you said you were going to. Thanks for the patch!