CA-407107: makefile fixes to honour LDFLAGS#23
Draft
edwintorok wants to merge 5 commits intoxenserver:masterfrom
Draft
CA-407107: makefile fixes to honour LDFLAGS#23edwintorok wants to merge 5 commits intoxenserver:masterfrom
edwintorok wants to merge 5 commits intoxenserver:masterfrom
Conversation
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 <edwin.torok@cloud.com>
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 <edwin.torok@cloud.com>
Add it to LIBS at the end. Signed-off-by: Edwin Török <edwin.torok@cloud.com>
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 <edwin.torok@cloud.com>
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 <edwin.torok@cloud.com>
Contributor
Author
|
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
Contributor
lindig
left a comment
lindig
left a comment
There was a problem hiding this comment.
Unrelated but would suggest to add a format target to avoid the problems of inconsistent indentation.
GeraldEV
approved these changes
Mar 5, 2025
Comment on lines
+22
to
+26
| @cd include; $(MAKE) clean DEFMAKE=default-debug.mk | ||
| @cd lib; $(MAKE) clean DEFMAKE=default-debug.mk | ||
| @cd daemon; $(MAKE) clean DEFMAKE=default-debug.mk | ||
| @cd commands; $(MAKE) clean DEFMAKE=default-debug.mk | ||
| @cd scripts; $(MAKE) clean DEFMAKE=default-debug.mk |
Collaborator
There was a problem hiding this comment.
Suggested change
| @cd include; $(MAKE) clean DEFMAKE=default-debug.mk | |
| @cd lib; $(MAKE) clean DEFMAKE=default-debug.mk | |
| @cd daemon; $(MAKE) clean DEFMAKE=default-debug.mk | |
| @cd commands; $(MAKE) clean DEFMAKE=default-debug.mk | |
| @cd scripts; $(MAKE) clean DEFMAKE=default-debug.mk | |
| $(MAKE) -C include clean DEFMAKE=default-debug.mk | |
| $(MAKE) -C lib clean DEFMAKE=default-debug.mk | |
| $(MAKE) -C daemon clean DEFMAKE=default-debug.mk | |
| $(MAKE) -C command clean DEFMAKE=default-debug.mk | |
| $(MAKE) -C scripts clean DEFMAKE=default-debug.mk |
Comment on lines
+34
to
+38
| @cd include; $(MAKE) install DESTDIR=$(DESTDIR) DEFMAKE=default-debug.mk | ||
| @cd lib; $(MAKE) install DESTDIR=$(DESTDIR) DEFMAKE=default-debug.mk | ||
| @cd daemon; $(MAKE) install DESTDIR=$(DESTDIR) DEFMAKE=default-debug.mk | ||
| @cd commands; $(MAKE) install DESTDIR=$(DESTDIR) DEFMAKE=default-debug.mk | ||
| @cd scripts; $(MAKE) install DESTDIR=$(DESTDIR) DEFMAKE=default-debug.mk |
Collaborator
There was a problem hiding this comment.
Suggested change
| @cd include; $(MAKE) install DESTDIR=$(DESTDIR) DEFMAKE=default-debug.mk | |
| @cd lib; $(MAKE) install DESTDIR=$(DESTDIR) DEFMAKE=default-debug.mk | |
| @cd daemon; $(MAKE) install DESTDIR=$(DESTDIR) DEFMAKE=default-debug.mk | |
| @cd commands; $(MAKE) install DESTDIR=$(DESTDIR) DEFMAKE=default-debug.mk | |
| @cd scripts; $(MAKE) install DESTDIR=$(DESTDIR) DEFMAKE=default-debug.mk | |
| $(MAKE) -C include install DESTDIR=$(DESTDIR) DEFMAKE=default-debug.mk | |
| $(MAKE) -C lib install DESTDIR=$(DESTDIR) DEFMAKE=default-debug.mk | |
| $(MAKE) -C daemon install DESTDIR=$(DESTDIR) DEFMAKE=default-debug.mk | |
| $(MAKE) -C commands install DESTDIR=$(DESTDIR) DEFMAKE=default-debug.mk | |
| $(MAKE) -C scripts install DESTDIR=$(DESTDIR) DEFMAKE=default-debug.mk |
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
3 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.
No description provided.