-
Notifications
You must be signed in to change notification settings - Fork 17
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
CA-407107: makefile fixes to honour LDFLAGS #23
Draft
edwintorok
wants to merge
5
commits into
xenserver:master
Choose a base branch
from
edwintorok:private/edvint/build
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
This file contains 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
xha.spec does a 'make install' which installs the debug build by default. The only difference between debug and release is NDEBUG (no -O flags are used), but considering that we've always shipped the debug version in production builds, keep that one. There are some advantages to shipping the debug version: * stacktraces may work better than at O2 * if the assertions finds a bug in the code it might be better to stop, rather than continue and do something incorrect (corrupt memory, etc.). Given that in the past years that we've been shipping XHA with asserts on we haven't really found failed assertions, keep them on. Signed-off-by: Edwin Török <[email protected]>
The GCC manual recommends this over O0 to improve the debugging experience: > It is a better choice than -O0 for producing debuggable code because some compiler passes > that collect debug information are disabled at -O0.:w It is also better than -O2 that rpmbuild would use because according to the GCC manual: > To get more accurate stack traces, it is possible to use options such as -O0, -O1, or -Og (which, for instance, prevent most function inlining), -fno-optimize-sibling-calls (which prevents optimizing sibling and tail recursive calls; this option is implicit for -O0, -O1, or -Og), or -fno-ipa-icf (which disables Identical Code Folding for functions) `-ipa-icf` is only enabled at -O2 and above, so using -Og here should be fine. (stacktraces are printed by log.c) Signed-off-by: Edwin Török <[email protected]>
Add it to LIBS at the end. Signed-off-by: Edwin Török <[email protected]>
Stacktraces are printed by log.c, to make them better use -Og which prevents some optimizations that would alter stacktraces significantly. Signed-off-by: Edwin Török <[email protected]>
Parallel builds can be enabled by using `$(MAKE)` instead of `make`. However the current rules have race conditions and do not fully declare their dependencies: * the .a rule was producing an archive out of whatever .o files it could fine at the time it ran, and it also deleted the files it produced * the .a rule always rebuilt all the files * the .a rule produced some file that were also produced by the %.o: %.c rule, leading to a race condition Move the .a rule to the lib subdirectory, and explicitly specify dependencies and only use files during a build that have been declared as a dependency of a rule. Also enable incremental builds by removing the PHONY from buildid.h. During an RPM build we'll get a clean build and redefined buildid anyway. Signed-off-by: Edwin Török <[email protected]>
This will depend on some of my other warning fixes, because some of these warnings are now fatal. |
lindig
approved these changes
Feb 24, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unrelated but would suggest to add a format
target to avoid the problems of inconsistent indentation.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
No description provided.