Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't override {C,LD}FLAGS #15

Closed
Whissi opened this Issue Mar 12, 2014 · 1 comment

Comments

Projects
None yet
2 participants
@Whissi
Copy link
Contributor

Whissi commented Mar 12, 2014

$CFLAGS and $LDFLAGS are user variables. Therefore we shouldn't override them. Instead of

if test "$GCC" = "yes"; then
   CFLAGS="${CFLAGS} -W -Wall -Wformat-security -Wshadow -Wcast-align -Wpointer-arith -Wmissing-format-attribute -g"
fi

use something like

if test "$GCC" = "yes"; then
    AM_CFLAGS="-W -Wall -Wformat-security -Wshadow -Wcast-align -Wpointer-arith -Wmissing-format-attribute -g"
fi

But the problem is that we would have to add $AM_CFLAGS to every declaration, for example we would have to add something like

liblogging_stdlog_la_CFLAGS = ${AM_CFLAGS}

to stdlog/Makefile.am because the root Makefile won't automatically pass the value to Makefiles in subdirs.

Another approach would be to create a common.am Makefile where we would set AM_CFLAGS and to include common.am in all the Makefiles in subdirs...

rgerhards added a commit that referenced this issue Mar 18, 2014

do not override user varibale CFLAGS
Thanks to Thomas D. for reporting this problem and suggesting a solution.
closes: #15
@rgerhards

This comment has been minimized.

Copy link
Member

rgerhards commented Mar 18, 2014

I have taken the approach you describe. Given the smallness of the code, it doesn't sound like too much overhead. Any further suggestions or corrections are very welcome. I close this issue, please re-open if necessary.

@rgerhards rgerhards closed this Mar 18, 2014

Whissi added a commit to Whissi/gentoo-overlay that referenced this issue Apr 14, 2014

dev-libs/liblogging: Bumped to v1.0.4
v1.0.4 2014-04-03
- fix build problems on some platforms (namely RHEL/CENTOS 5)
- add --disable-man pages ./configure option
  This permits to totally turn off man page handling. This is useful for
  platforms like RHEL/CENTOS 5 where rst2man is hard to get when building
  from git is desired.
- lower build system requirements to autoconf 2.59
  This permits building on RHEL/CENTOs 5 with stock autotools.

v1.0.3 2014-03-18
- fix build problem in Ubuntu 10.04
  Thanks to Assaf Gordon for reporting
  See: rsyslog/liblogging#11
- do not override user varibale CFLAGS
  Thanks to Thomas D. for reporting this problem and suggesting a solution.
  closes: rsyslog/liblogging#15
- make liblogging-rfc3195 not export private symbols
  Thanks to Michael Biebl for his help in getting this right.
- explain that stdlog_log() return code usually most not be checked
  (if same level of reliability like syslog(3) is desired)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.