Skip to content

Makefile.base: cleanup non selected source object files [backport 2021.10]#16953

Merged
benpicco merged 2 commits intoRIOT-OS:2021.10-branchfrom
leandrolanzieri:backport/2021.10/pr/build_system/delete_non_selected_objects
Oct 19, 2021
Merged

Makefile.base: cleanup non selected source object files [backport 2021.10]#16953
benpicco merged 2 commits intoRIOT-OS:2021.10-branchfrom
leandrolanzieri:backport/2021.10/pr/build_system/delete_non_selected_objects

Conversation

@leandrolanzieri
Copy link
Contributor

@leandrolanzieri leandrolanzieri commented Oct 5, 2021

Backport of #16945 and #16981

Contribution description

As explained in #16942, since #14754 we select all object files when linking, which is a problem when the selection of module sources change between two builds. This adds a cleanup for non selected source object files on modules.

Testing procedure

A minimal example that shows the current issue:

diff --git a/tests/external_module_dirs/Makefile b/tests/external_module_dirs/Makefile
index f44d1e3e8e..f4a82e6c16 100644
--- a/tests/external_module_dirs/Makefile
+++ b/tests/external_module_dirs/Makefile
@@ -4,4 +4,6 @@ USEMODULE += random
 USEMODULE += external_module
 EXTERNAL_MODULE_DIRS += external_modules
 
+USEMODULE += external_module_implementation_a
+
 include $(RIOTBASE)/Makefile.include
diff --git a/tests/external_module_dirs/external_modules/external_module/Makefile b/tests/external_module_dirs/external_modules/external_module/Makefile
index 48422e909a..a0fd02f4b4 100644
--- a/tests/external_module_dirs/external_modules/external_module/Makefile
+++ b/tests/external_module_dirs/external_modules/external_module/Makefile
@@ -1 +1,9 @@
+# enable submodules
+SUBMODULES := 1
+
+# base module name
+BASE_MODULE := external_module
+
+SRC += external_module.c
+
 include $(RIOTBASE)/Makefile.base
diff --git a/tests/external_module_dirs/external_modules/external_module/Makefile.include b/tests/external_module_dirs/external_modules/external_module/Makefile.include
index b6d95d22b5..d7225f8d78 100644
--- a/tests/external_module_dirs/external_modules/external_module/Makefile.include
+++ b/tests/external_module_dirs/external_modules/external_module/Makefile.include
@@ -1,3 +1,6 @@
 # Use an immediate variable to evaluate `MAKEFILE_LIST` now
 USEMODULE_INCLUDES_external_module := $(LAST_MAKEFILEDIR)/include
 USEMODULE_INCLUDES += $(USEMODULE_INCLUDES_external_module)
+
+PSEUDOMODULES += external_module_implementation_a
+PSEUDOMODULES += external_module_implementation_b
diff --git a/tests/external_module_dirs/external_modules/external_module/implementation_a.c b/tests/external_module_dirs/external_modules/external_module/implementation_a.c
new file mode 100644
index 0000000000..19c48d0020
--- /dev/null
+++ b/tests/external_module_dirs/external_modules/external_module/implementation_a.c
@@ -0,0 +1,2 @@
+
+char *message_implementation = "Linked implementation A";
diff --git a/tests/external_module_dirs/external_modules/external_module/implementation_b.c b/tests/external_module_dirs/external_modules/external_module/implementation_b.c
new file mode 100644
index 0000000000..8b126371e8
--- /dev/null
+++ b/tests/external_module_dirs/external_modules/external_module/implementation_b.c
@@ -0,0 +1,2 @@
+
+char *message_implementation = "Linked implementation B";
diff --git a/tests/external_module_dirs/external_modules/external_module/include/external_module.h b/tests/external_module_dirs/external_modules/external_module/include/external_module.h
index f358ef4786..aefd12679e 100644
--- a/tests/external_module_dirs/external_modules/external_module/include/external_module.h
+++ b/tests/external_module_dirs/external_modules/external_module/include/external_module.h
@@ -29,6 +29,8 @@ extern "C" {
  */
 extern char *external_module_message;
 
+extern char *message_implementation;
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/tests/external_module_dirs/main.c b/tests/external_module_dirs/main.c
index 9695fb6826..421438a0c3 100644
--- a/tests/external_module_dirs/main.c
+++ b/tests/external_module_dirs/main.c
@@ -34,5 +34,6 @@ int main(void)
 {
     puts("If it compiles, it works!");
     printf("Message: %s\n", external_module_message);
+    printf("Submodule message: %s\n", message_implementation);
     return 0;
 }

Compile once with USEMODULE += external_module_implementation_a and once with USEMODULE += external_module_implementation_b, message_implementation will have two implementations at link time on master. With this PR this should be fixed.

Issues/PRs references

#16942
Issue introduced by #14754


Contribution description

Since #16945 we cleanup unneeded object files. The list of "needed object files" is defined based on present source files, but bindist is a special case, as bindist objects should always be preserved. As described in #16977, the bindist example fails to link due to the cleanup. This PR avoids cleaning bindist modules object files when building.

Testing procedure

Issues/PRs references

Fixes #16977
Issue introduced in #16945

@leandrolanzieri leandrolanzieri added Area: build system Area: Build system CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Process: release backport Integration Process: The PR is a release backport of a change previously provided to master Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) labels Oct 5, 2021
@benpicco
Copy link
Contributor

This needs a rebase

@leandrolanzieri leandrolanzieri force-pushed the backport/2021.10/pr/build_system/delete_non_selected_objects branch from 19d798c to 51612fe Compare October 11, 2021 15:17
@leandrolanzieri
Copy link
Contributor Author

Rebased

@benpicco
Copy link
Contributor

Oh sorry, the branch has become out of date again.
Please rebase yet again - feel free to hit merge as soon as you see the green button.

@leandrolanzieri
Copy link
Contributor Author

Let's wait until #16977 is sorted.

@leandrolanzieri leandrolanzieri force-pushed the backport/2021.10/pr/build_system/delete_non_selected_objects branch from 51612fe to 0bd1c9e Compare October 19, 2021 11:27
@leandrolanzieri
Copy link
Contributor Author

@benpicco I rebased and cherry-picked the commit from #16981

@benpicco benpicco merged commit 8908aad into RIOT-OS:2021.10-branch Oct 19, 2021
@leandrolanzieri leandrolanzieri deleted the backport/2021.10/pr/build_system/delete_non_selected_objects branch October 19, 2021 13:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: build system Area: Build system CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Process: release backport Integration Process: The PR is a release backport of a change previously provided to master Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants