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

directly use object files when linking ELF #22025

Merged
merged 1 commit into from
Sep 17, 2023
Merged

Conversation

t-8ch
Copy link
Contributor

@t-8ch t-8ch commented Sep 14, 2023

Instead of taking all prerequisites and filtering out everything that is not an object just use the objects directly.

It is both easier to understand and also more robust in case other non-object files are added to the prerequisites in the future.

@github-actions github-actions bot added the core label Sep 14, 2023
@tzarc
Copy link
Member

tzarc commented Sep 15, 2023

@t-8ch please use the PR template for future PRs -- not doing so in the future will result in them being closed without review.

@tzarc
Copy link
Member

tzarc commented Sep 15, 2023

@t-8ch please use the PR template for future PRs -- not doing so in the future will result in them being closed without review.

Scratch that, ignore me. PR template says otherwise.

@tzarc
Copy link
Member

tzarc commented Sep 15, 2023

CI flagged failures, looks like there's some unintended side-effects:

image

@tzarc
Copy link
Member

tzarc commented Sep 15, 2023

diff --git a/builddefs/common_rules.mk b/builddefs/common_rules.mk
index 16d6af5438..52dccbe475 100644
--- a/builddefs/common_rules.mk
+++ b/builddefs/common_rules.mk
@@ -12,6 +12,9 @@ vpath %.hpp $(VPATH_SRC)
 vpath %.S $(VPATH_SRC)
 VPATH :=
 
+# Helper to return the distinct elements of a list
+uniq = $(if $1,$(firstword $1) $(call uniq,$(filter-out $(firstword $1),$1)))
+
 # Convert all SRC to OBJ
 define OBJ_FROM_SRC
 $(patsubst %.c,$1/%.o,$(patsubst %.cpp,$1/%.o,$(patsubst %.cc,$1/%.o,$(patsubst %.S,$1/%.o,$(patsubst %.clib,$1/%.a,$($1_SRC))))))
@@ -264,7 +267,7 @@ BEGIN = gccversion sizebefore
 # Note the obj.txt depeendency is there to force linking if a source file is deleted
 %.elf: $(OBJ) $(MASTER_OUTPUT)/cflags.txt $(MASTER_OUTPUT)/ldflags.txt $(MASTER_OUTPUT)/obj.txt | $(BEGIN)
 	@$(SILENT) || printf "$(MSG_LINKING) $@" | $(AWK_CMD)
-	$(eval CMD=MAKE=$(MAKE) $(CC) $(ALL_CFLAGS) $(OBJ) --output $@ $(LDFLAGS))
+	$(eval CMD=MAKE=$(MAKE) $(CC) $(ALL_CFLAGS) $(call uniq,$(OBJ)) --output $@ $(LDFLAGS))
 	@$(BUILD_CMD)

Instead of taking all prerequisites and filtering out everything that is
not an object just use the objects directly.

It is both easier to understand and also more robust in case other
non-object files are added to the prerequisites in the future.
Copy link
Member

@KarlK90 KarlK90 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks.

@tzarc tzarc merged commit 615ca78 into qmk:develop Sep 17, 2023
3 checks passed
mechlovin pushed a commit to mechlovin/qmk_firmware that referenced this pull request Oct 25, 2023
christrotter pushed a commit to christrotter/qmk_firmware that referenced this pull request Nov 28, 2023
zgagnon pushed a commit to zgagnon/qmk_firmware_waterfowl that referenced this pull request Dec 15, 2023
future-figs pushed a commit to future-figs/qmk_firmware that referenced this pull request Dec 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants