-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Improve shell_commands #8564
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
Improve shell_commands #8564
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -86,3 +86,12 @@ endif | |
| ifneq (native,$(BOARD)) | ||
| INCLUDES += -I$(RIOTBASE)/sys/libc/include | ||
| endif | ||
|
|
||
| ifneq (,$(filter shell_commands,$(USEMODULE))) | ||
| ifneq (,$(filter cortexm_common,$(USEMODULE))) | ||
| LINKFLAGS += $(RIOTBASE)/sys/shell/commands/shell_commands_cortexm.ld | ||
| else | ||
| LINKFLAGS += $(RIOTBASE)/sys/shell/commands/shell_commands.ld | ||
| endif | ||
| UNDEF += -Wl,--whole-archive $(BINDIR)/shell_commands.a -Wl,--no-whole-archive | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Mmm, interesting. I was wondering how you avoided the linker discarding the symbols, which is the reason why #9105 is still unmerged: it needs either #8711 (ld -r) or "--whole-archive" (which I think is the preferred option in conjuction with #10195 ) I see that you avoided much of the problems that have been holding #8711 by limiting the "--whole-archive" to just the shell module. @cladmi , @gebart Wouldn't this be a good approach "unlock" #8711, or its equivalent with "--whole-archive"? As far as I understand, most of the problems come from a minority of files which define things like interrupt handlers. The "--whole-archive" could be enabled only for select modules (or, if most things work, enabled by default and disabled for select modules.)
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For #8711 I was thinking about rebasing/cleaning it, changing it to Not to say it should be supported only on some boards, but simplify the review. We now have a server to build the two versions and compare the elf files, just need to take the time to analyze and solve all the issues.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Agree. I insist, if there is some problematic module (if I recall correcyly we were having too few symbols discarded sometimes?) we can, as a quirk, disable it just for that module. |
||
| endif | ||
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.
What does this do? Can this renaming of the section affect something else?
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.
Sorry, I don't remember why this was done.