Bug 232331

Comment 1 Kubilay Kocak 2018-10-17 02:28:16 UTC

For reference, the checks and errors were added upstream in 037656b2d89f7bb8fb697afff5b67c3a9b1c1d87 [1]. 

It also adds checks/errors for "improper" use CPPFLAGS, LDFLAGS, LIBS.

Thoughts:

1) Unless the patch supports (checks *and* fixes) all the above variables and values, its really not sufficient to solving the issue of users arbitrarily putting incorrect values in the wrong variables.

2) Just deleting/removing these flags seems like a POLA violation of 'respecting user *FLAGS'. The alternative is to extend the solution to 'move' the correct arguments to the correct variables. However ...

3) If the patch is extended support (put things in the right place, not just delete them) all the above cases , then it seems, if the mechanism is deemed to be necessary/needed at all, that it would better placed/used/done at the ports framework level, not in individual ports.

Why isn't the sole root cause of this "local use of values in incorrect VARIABLES", and the solution for the user to put the correct values into the correct variables, as the error suggests?

[1] 037656b2d89f7bb8fb697afff5b67c3a9b1c1d87

Comment 2 Kubilay Kocak 2018-10-17 02:28:42 UTC

(In reply to Kubilay Kocak from comment #1)

[1] was supposed to link to the upstream commit: https://github.com/c-ares/c-ares/commit/037656b2d89f7bb8fb697afff5b67c3a9b1c1d87

Comment 3 Kubilay Kocak 2018-10-17 02:41:51 UTC

See Also:

https://patchwork.ozlabs.org/patch/515209/ (patch to use CPPFLAGS, not CFLAGS)

Issue was also apparent in curl:

https://sourceforge.net/p/curl/bugs/1261/ 

Mk/bsd.ldap.mk was using CFLAGS not CPPFLAGS:

https://lists.freebsd.org/pipermail/freebsd-ports/2013-August/085339.html 

Upstream replaced error with warning.

I change it to:
CFLAGS:=	${CFLAGS:N-D*}
CPPFLAGS:=	${CFLAGS:M-D*}

and got:
...
configure: running /bin/sh ./configure --disable-option-checking '--prefix=/usr/local'  '--disable-werror' '--disable-debug' '--enable-symbol-hiding' '--disable-optimize' '--localstatedir=/var' '--mandir=/usr/local/man' '--disable-silent-rules' '--infodir=/usr/local/info/' '--build=amd64-portbld-freebsd11.2' 'build_alias=amd64-portbld-freebsd11.2' 'CC=cc' 'CFLAGS=-O2 -pipe -O2 -pipe -mretpoline  -fno-strict-aliasing ' 'LDFLAGS= ' 'LIBS=' 'CPPFLAGS=' 'CPP=cpp' 'CXX=c++' 'CXXFLAGS=-O2 -pipe -O2 -pipe -mretpoline -fno-strict-aliasing -DSTRIP_FBSDID -D_FORTIFY_SOURCE=2 -mretpoline  ' --cache-file=/dev/null --srcdir=.
...

but does not see my macro
...
/bin/sh ./libtool  --tag=CC   --mode=compile cc -DHAVE_CONFIG_H   -I. -I.   -DCARES_BUILDING_LIBRARY -DCARES_SYMBOL_HIDING   -fvisibility=hidden  -pipe -pipe -mretpoline -fno-strict-aliasing -Qunused-arguments -g0 -O0 -MT libcares_la-windows_port.lo -MD -MP -MF .deps/libcares_la-windows_port.Tpo -c -o libcares_la-windows_port.lo `test -f 'windows_port.c' || echo './'`windows_port.c
libtool: compile:  cc -DHAVE_CONFIG_H -I. -I. -DCARES_BUILDING_LIBRARY -DCARES_SYMBOL_HIDING -fvisibility=hidden -pipe -pipe -mretpoline -fno-strict-aliasing -Qunused-arguments -g0 -O0 -MT libcares_la-windows_port.lo -MD -MP -MF .deps/libcares_la-windows_port.Tpo -c windows_port.c  -fPIC -DPIC -o .libs/libcares_la-windows_port.o
...
cc -DHAVE_CONFIG_H   -I. -I.     -pipe -pipe -mretpoline -fno-strict-aliasing -Qunused-arguments -g0 -O0 -MT adig-ares_getopt.o -MD -MP -MF .deps/adig-ares_getopt.Tpo -c -o adig-ares_getopt.o `test -f 'ares_getopt.c' || echo './'`ares_getopt.c
...

c-ares ignore CPPFLAGS and -O2.
Also, at least _FORTIFY_SOURCE=2 should be in CFLAGS.
This upstream error.

Created attachment 198479 [details]
up

Created attachment 198480 [details]
fix

Ohh, wrong order.

Now:
...
configure: running /bin/sh ./configure --disable-option-checking '--prefix=/usr/local'  '--disable-werror' '--disable-debug' '--enable-symbol-hiding' '--disable-optimize' '--localstatedir=/var' '--mandir=/usr/local/man' '--disable-silent-rules' '--infodir=/usr/local/info/' '--build=amd64-portbld-freebsd11.2' 'build_alias=amd64-portbld-freebsd11.2' 'CC=cc' 'CFLAGS=-O2 -pipe -O2 -pipe -mretpoline  -fno-strict-aliasing ' 'LDFLAGS= ' 'LIBS=' 'CPPFLAGS= -DSTRIP_FBSDID -D_FORTIFY_SOURCE=2' 'CPP=cpp' 'CXX=c++' 'CXXFLAGS=-O2 -pipe -O2 -pipe -mretpoline -fno-strict-aliasing -DSTRIP_FBSDID -D_FORTIFY_SOURCE=2 -mretpoline  ' --cache-file=/dev/null --srcdir=.
...
libtool: compile:  cc -DHAVE_CONFIG_H -I. -I. -DCARES_BUILDING_LIBRARY -DCARES_SYMBOL_HIDING -DSTRIP_FBSDID -D_FORTIFY_SOURCE=2 -fvisibility=hidden -pipe -pipe -mretpoline -fno-strict-aliasing -Qunused-arguments -g0 -O0 -MT libcares_la-ares__close_sockets.lo -MD -MP -MF .deps/libcares_la-ares__close_sockets.Tpo -c ares__close_sockets.c  -fPIC -DPIC -o libcares_la-ares__close_sockets.o >/dev/null 2>&1
...
libtool: link: cc -pipe -pipe -mretpoline -fno-strict-aliasing -Qunused-arguments -g0 -O0 -o .libs/ahost ahost-ahost.o ahost-ares_getopt.o ahost-ares_nowarn.o ahost-ares_strcasecmp.o  ./.libs/libcares.so -Wl,-rpath -Wl,/usr/local/lib
cc -DHAVE_CONFIG_H   -I. -I.   -DSTRIP_FBSDID -D_FORTIFY_SOURCE=2  -pipe -pipe -mretpoline -fno-strict-aliasing -Qunused-arguments -g0 -O0 -MT adig-adig.o -MD -MP -MF .deps/adig-adig.Tpo -c -o adig-adig.o `test -f 'adig.c' || echo './'`adig.c
...

ping

Comment 8 Kubilay Kocak 2018-11-07 04:15:38 UTC

Considerations / questions in comment 1 still need to be addressed. 

I still don't believe, at this point at least, that the proposed change is a appropriate, viable, correct, complete or a permanent approach/solution to the root of the described issue, namely:

'some piece of software (c-ares) does additional checks for incorrect usage of variables/values and stops the build because of them'

Patch move all definitions from CFLAGS to CPPFLAGS.

This is work:
 - configure pass all checks
 - definitions pass to compiler

I have no idea where it can be placed in port framework.

Comment 10 Kubilay Kocak 2018-11-07 09:39:05 UTC

Would like to get Tijl's thoughts on this issue at a broad level, given his expertise in the area of build system mechanics (autotools maintainer, et al) and previous improvements to *FLAGS,LIBS,etc standardization and correctness in the framework

Comment 11 Tijl Coosemans 2018-11-07 13:56:16 UTC

Created attachment 199053 [details]
patch

The message is technically correct but it's a bit over the top to enforce it like this so this patch removes the error from the configure script.  What the configure script could do is scan CFLAGS for -D and only error if that flag isn't also in CPPFLAGS, because there's no harm if the flag is in both.  It's certainly a good idea to add "CPPFLAGS+=-D_FORTIFY_SOURCE=2 -DSTRIP_FBSDID" to your make.conf.

The configure script also checks that LIBS only contains -l flags which is incorrect.  It can also contain -L flags, e.g. output of pkgconf --libs.

Comment 12 Kubilay Kocak 2018-11-09 08:01:07 UTC

Thank you Tijl

Comment 13 Walter Schwarzenfeld 2019-08-08 09:05:42 UTC

Any news here?

Comment 14 Kubilay Kocak 2019-08-08 09:14:59 UTC

Fix incorrect changed merge-quarterly flag value

Comment 15 Ryan Steinmetz 2022-04-10 21:36:57 UTC

Port has been updated, so any accepted upstream changes are now present.