Bug 232331 - dns/c-ares: Fails to configure if "incorrect" values are put in certain VARIABLES (One example -D* in CFLAGS)
Summary: dns/c-ares: Fails to configure if "incorrect" values are put in certain VARIA...
Status: Open
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Some People
Assignee: Ryan Steinmetz
URL:
Keywords: needs-qa
Depends on:
Blocks:
 
Reported: 2018-10-16 21:26 UTC by rozhuk.im
Modified: 2019-08-08 09:16 UTC (History)
4 users (show)

See Also:
bugzilla: maintainer-feedback? (zi)
koobs: merge-quarterly?


Attachments
patch (385 bytes, patch)
2018-10-16 21:26 UTC, rozhuk.im
no flags Details | Diff
up (412 bytes, patch)
2018-10-22 19:26 UTC, rozhuk.im
no flags Details | Diff
fix (424 bytes, patch)
2018-10-22 19:43 UTC, rozhuk.im
no flags Details | Diff
patch (1.40 KB, patch)
2018-11-07 13:56 UTC, Tijl Coosemans
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description rozhuk.im 2018-10-16 21:26:18 UTC
Created attachment 198237 [details]
patch

I have in /etc/make.conf
CFLAGS+=-O2 -DSTRIP_FBSDID -pipe
CFLAGS+=-D_FORTIFY_SOURCE=2

Build fail with:
...
configure: using CFLAGS: -O2 -pipe -O2 -DSTRIP_FBSDID -pipe -D_FORTIFY_SOURCE=2 -mretpoline  -fno-strict-aliasing 
configure: CFLAGS error: CFLAGS may only be used to specify C compiler flags, not macro definitions. Use CPPFLAGS for: -DSTRIP_FBSDID
configure: CFLAGS error: CFLAGS may only be used to specify C compiler flags, not macro definitions. Use CPPFLAGS for: -D_FORTIFY_SOURCE=2
...
Comment 1 Kubilay Kocak freebsd_committer freebsd_triage 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 freebsd_committer freebsd_triage 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 freebsd_committer freebsd_triage 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.
Comment 4 rozhuk.im 2018-10-22 19:23:40 UTC
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.
Comment 5 rozhuk.im 2018-10-22 19:26:33 UTC
Created attachment 198479 [details]
up
Comment 6 rozhuk.im 2018-10-22 19:43:00 UTC
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
...
Comment 7 rozhuk.im 2018-11-07 00:01:12 UTC
ping
Comment 8 Kubilay Kocak freebsd_committer freebsd_triage 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'
Comment 9 rozhuk.im 2018-11-07 09:09:44 UTC
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 freebsd_committer freebsd_triage 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 freebsd_committer 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 freebsd_committer freebsd_triage 2018-11-09 08:01:07 UTC
Thank you Tijl
Comment 13 Walter Schwarzenfeld freebsd_triage 2019-08-08 09:05:42 UTC
Any news here?
Comment 14 Kubilay Kocak freebsd_committer freebsd_triage 2019-08-08 09:14:59 UTC
Fix incorrect changed merge-quarterly flag value