Opened 9 years ago

Closed 7 years ago

#488 closed enhancement (fixed)

Behavior of './configure CXXFLAGS=foo'

Reported by: Peter Johansson Owned by: Peter Johansson
Priority: minor Milestone: svndigest 0.10
Component: build Version: trunk
Keywords: Cc:

Description

The typical behavior of configure in a GNU package is that

./configure

sets CXXFLAGS to "-g -O2" and

./configure CXXFLAGS=someflags

sets CXXFLAGS to "someflags". In other words CXXFLAGS's default value is "-g -O2" and if a value is provided by user configure does not change that value. Let's compare with the behavior of svndigest'c configure

./configure

sets CXXFLAGS to "-pedantic -Wno-long-long -O3"

./configure --enable-debug

sets CXXFLAGS to "-pedantic -Wno-long-long -g -O"

./configure CXXFLAGS=someflags

sets CXXFLAGS to "-pedantic -Wno-long-long -O3 someflags"

./configure --enable-debug CXXFLAGS=someflags

sets CXXFLAGS to "-pedantic -Wno-long-long -g -O someflags"

I think this behavior has some room for improvements because it is a bit unexpected and lacks some flexibility.

  1. The switch --enable-debug is a bit misleading because the switch

turns on generation of debug info (-g) and tunes down the optimization to -O1 as well as enables assertions (not really CXXFLAGS but still)

  1. As a user you cannot set CXXFLAGS to whatever you want. If you, for

example, would prefer compiling without --pedantic, I think you should have the freedom to do so. The fundamental assumption must be that the user knows better what he wants than we do. We should of course provide a sensible default for the ignorant user, but if users wanna use some options configure should be as flexible as possible.

  1. Our default value "-pedantic -Wno-long-long -O3" might be

unexpected for a user used to configure in GNU packages (where default is -g -O2). I don't think this as serious and one could discuss what the default value should be (see e.g. http://lists.gnu.org/archive/html/automake/2010-11/msg00126.html). A note in this context is that as we use Automake we have a target install-strip which strips away debug symbols of the installed binary. The problem with that target is of course that only advanced user will use it and they can choose with or without debug during configure, can't they?

So far only my thoughts about the current behavior. What to change? I can live with 1). I can live with 3) too although I think it should be clarified in the README. 2) is my major fly-in-the-ear. I don't like that we append CXXFLAGS (and CPPFLAGS and LDFLAGS). I think we should provide default values but these should not be added to the flags if the user chooses to set the *FLAGS himself. The exception is of course when we append values that are needed to find headers and libraries. So if go back to the cases above I think we should have this behavior intead:

./configure

sets CXXFLAGS to "-pedantic -Wno-long-long -O3"

./configure --enable-debug

sets CXXFLAGS to "-pedantic -Wno-long-long -g -O"

./configure CXXFLAGS=someflags

sets CXXFLAGS to "someflags"

./configure --enable-debug CXXFLAGS=someflags

sets CXXFLAGS to "someflags"

The latter is debatable, but I think it is logical that a user provided CXXFLAGS has precedence over --enable-debug, i.e., --enable-debug only changes the default values.

Likewise, we should only set CPPFLAGS=-DNDEBUG when CPPFLAGS is not set.

Opinions?

Change History (15)

comment:1 Changed 9 years ago by Jari Häkkinen

I have opinions but I'll post them later. I am not in a position to test it now but is the behaviour of

CXXFLAGS=someflags ./configure

The same as of

./configure CXXFLAGS=someflags

with respect to CXXFLAGS? I know that config.log will show different invoked configure command.

comment:2 Changed 9 years ago by Peter Johansson

Yes, they should be the same. Only difference is that configure will remember its commandline, so when config.status --recheck is invoked it will run ./configure with approx. the same commandline as before. That's why the second version is preferred as it preserves the value of CXXFLAGS.

comment:3 Changed 8 years ago by Peter Johansson

Kind of realted to this ticket, I noticed that within macport svndigest is configured with CXXFLAGS="-pipe -O2 -arch x86_64"

/usr/bin/g++-4.2 -DHAVE_CONFIG_H -I. -I.. -I../yat  -I.. -DNDEBUG -I/opt/local/include/apr-1
-DDARWIN -DSIGPROCMASK_SETS_THREAD_MASK -no-cpp-precomp -DDARWIN_10 -I/opt/local/include 
-pedantic -Wno-long-long -O3 -pipe -O2 -arch x86_64 -MT AddStats.o -MD -MP -MF 
.deps/AddStats.Tpo -c -o AddStats.o AddStats.cc

that is with both -O3 and -O2. I suppose that g++ listens to the latter one here and ignores -O2.

comment:4 Changed 8 years ago by Peter Johansson

comment:5 Changed 8 years ago by Peter Johansson

Given the problem reported here it would be nice being able to turn off the -pedantic flag because then I could build svndigest against svn 1.7. With current version of svndigest's configure I cannot do that; -pedantic is mandated. I don't like that for two reasons: 1) It prevents me from building svndigest 2) we break the principle that the user is always right - if she likes to build without -pedantic she should be able to set that via configure.

comment:6 in reply to:  5 ; Changed 8 years ago by Jari Häkkinen

Replying to peter:

Given the problem reported here it would be nice being able to turn off the -pedantic flag because then I could build svndigest against svn 1.7. With current version of svndigest's configure I cannot do that; -pedantic is mandated. I don't like that for two reasons: 1) It prevents me from building svndigest 2) we break the principle that the user is always right - if she likes to build without -pedantic she should be able to set that via configure.

Couldn't this be fixed with a straightforward ./configure --without-pedantic option?

comment:7 in reply to:  6 ; Changed 8 years ago by Peter Johansson

Replying to jari:

Couldn't this be fixed with a straightforward ./configure --without-pedantic option?

Sure, it would solve it.

Generally speaking I prefer using already existing interface: ./configure CXXFLAGS=-O3 because if that is fixed to work as I suggest it automatically fixes it for every option. In yat, e.g., CXXFLAGS is set to "-pedantic -Wall" which implies your approach would require two options: --without-pedantic and --without-wall, and that is only for CXXFLAGS. Then we have CPPFLAGS too.

Another thing is why is default "-pedantic -Wall" in yat and only "-pedantic" here in svndigest. Is that on purpose?

comment:8 in reply to:  7 ; Changed 8 years ago by Jari Häkkinen

Replying to peter:

Replying to jari:

Couldn't this be fixed with a straightforward ./configure --without-pedantic option?

Sure, it would solve it.

Generally speaking I prefer using already existing interface: ./configure CXXFLAGS=-O3 because if that is fixed to work as I suggest it automatically fixes it for every option. In yat, e.g., CXXFLAGS is set to "-pedantic -Wall" which implies your approach would require two options: --without-pedantic and --without-wall, and that is only for CXXFLAGS. Then we have CPPFLAGS too.

What I want to retain is the more strict compiler options, i.e., the developer must turn off strictness not on. My experience with devs are that they are lazy and happily "forget" to turn off strictness.

Another thing is why is default "-pedantic -Wall" in yat and only "-pedantic" here in svndigest. Is that on purpose?

Not that I can recall. It should be a -Wall also in svndigest. Again trying to enforce strictness.

comment:9 Changed 8 years ago by Peter Johansson

Ticket #506 was marked as related.

comment:10 in reply to:  8 ; Changed 8 years ago by Peter Johansson

Replying to jari:

Replying to peter:

Replying to jari:

Couldn't this be fixed with a straightforward ./configure --without-pedantic option?

Sure, it would solve it.

Generally speaking I prefer using already existing interface: ./configure CXXFLAGS=-O3 because if that is fixed to work as I suggest it automatically fixes it for every option. In yat, e.g., CXXFLAGS is set to "-pedantic -Wall" which implies your approach would require two options: --without-pedantic and --without-wall, and that is only for CXXFLAGS. Then we have CPPFLAGS too.

What I want to retain is the more strict compiler options, i.e., the developer must turn off strictness not on. My experience with devs are that they are lazy and happily "forget" to turn off strictness.

I don't think there is any conflict between your wish and my suggestion. I typically run ./configure or ./configure --enable-debug and in those cases the result would be exactly the same. In other words I typically ignore the CXXFLAGS variable and in that case I will happily "forget" to turn off strictness.

Another thing is why is default "-pedantic -Wall" in yat and only "-pedantic" here in svndigest. Is that on purpose?

Not that I can recall. It should be a -Wall also in svndigest. Again trying to enforce strictness.

OK, so let's reintroduce -Wall in trunk.

comment:11 in reply to:  10 ; Changed 8 years ago by Peter Johansson

Replying to peter:

Replying to jari:

Not that I can recall. It should be a -Wall also in svndigest. Again trying to enforce strictness.

OK, so let's reintroduce -Wall in trunk.

I plan to add -Wall in trunk when 0.9.4 has been merged into trunk. Should -Wall follow how -pedantic now works (see #506) and only be turned on within --enable-debug? (Needless to say -Wno-long-long is only needed in conjunction with -Wall).

comment:12 in reply to:  11 Changed 8 years ago by Jari Häkkinen

Replying to peter:

I plan to add -Wall in trunk when 0.9.4 has been merged into trunk. Should -Wall follow how -pedantic now works (see #506) and only be turned on within --enable-debug? (Needless to say -Wno-long-long is only needed in conjunction with -Wall).

I am somewhat undecided. Maybe it is most clean to move it into --enable-debug and only introduce verbose output for devs only?

comment:13 Changed 8 years ago by Peter Johansson

(In [1407]) reintroduce -Wall (in enable-debug mode) refs #488

comment:14 Changed 7 years ago by Peter Johansson

Status: newassigned

comment:15 Changed 7 years ago by Peter Johansson

Resolution: fixed
Status: assignedclosed

(In [1514]) Closes #488. Change how configure sets CXXFLAGS and CPPFLAGS. There are three cases: 1) As before, if --enable-debug is given, we append '-pedantic -Wall -Wno-long-long -g -O'. 2) Otherwise, and CXXFLAGS was not set by user, we set CXXFLAGS to '-O3' just as before.

Note: See TracTickets for help on using tickets.