Open
Conversation
added 5 commits
August 1, 2017 15:32
If CONFIG_CFLAGS is specified in tup.config, then its value is used as our CFLAGS. Otherwise, we provide sensible defaults. Flags which are needed in any case, such as include paths, are added unconditionally.
This template should be a good starting point for those wanting to develop tup. Many diagnostics are enabled, warnings are considered errors, debug information is included and an optimization level which doesn't interfere with debugging is specified.
This template should be a good starting point for those wanting to get a release build of tup. Basic diagnostics are enabled to help catch errors, but the compiler shouldn't otherwise be very strict with these settings. An intermediate optimization level is set.
Like gcc-dev.config, this template should be useful to those wanting to develop tup. The difference in this one is the use of LTO, which can help uncover errors in the code. At least here, tup did not build with these settings; presumably, there's something to fix, and this template is a step in that direction.
gittup
requested changes
Aug 4, 2017
| AR = ar | ||
| endif | ||
|
|
||
| CFLAGS += -W |
Owner
There was a problem hiding this comment.
I think we should put the CFLAGS -W through -fno-common into the group with -I$(TUP_CWD)/src, so that they are always included. IMO these are important warnings that should be included in all profiles.
Since these flags would be before the 'ifdef CFLAGS' line, it is still possible for a user to override certain flags with their tup.config. For example, they could set CONFIG_CFLAGS to -Wno-switch-enum if they wanted to override the default -Wswitch-enum flag.
The 'else' block of the 'ifdef CFLAGS' can just be -O2 for the default optimization, since this is the most common thing that you are changing in the configs/*.config files.
This would also make gcc-dev.config and gcc-dev-lto.config much smaller.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The goal of this pull request is to make it easier to change the flags with which tup itself is built. Previously, doing that required tweaking the Tupfiles themselves; with these commits, a level of configurability is supported via tup.config, which the user can provide. This way is better not only because it separates the what to build from the how to do it, but also because Kconfig syntax poses a lower entry level for those wanting to build tup but unfamiliar with it (i.e. packagers).
Being able to change build flags is desirable for several reasons. It's also required when targetting various platforms, and it frees us from the burden of providing the right flags for every situation.
In case no configuration is specified, the previous flags are used. I have tested building with no configuration on Windows, and it works.
To help users and contributors, a couple config templates are supplied.