Skip to content

makefiles: avoid extra "/" when including files from external modules#17598

Merged
maribu merged 1 commit intoRIOT-OS:masterfrom
leandrolanzieri:pr/makefiles/fix_external_module_paths_includes
Feb 1, 2022
Merged

makefiles: avoid extra "/" when including files from external modules#17598
maribu merged 1 commit intoRIOT-OS:masterfrom
leandrolanzieri:pr/makefiles/fix_external_module_paths_includes

Conversation

@leandrolanzieri
Copy link
Contributor

Contribution description

As the dir make function considers the last slash as part of the directory-part of a file name, we don't need to add an extra one. From the function documentation:

The directory-part of the file name is everything up through (and including) the last slash in it.

Alternatively we can go the other way, and remove the slash from the variable. I took this path because of the natural way make seems to consider directory paths.

Testing procedure

tests/external_module_dirs should work as usual

Issues/PRs references

Spotted while reviewing #17596

As the `dir` make function considers the last slash as part of the
directory-part of a file name, we don't need to add an extra one.
@leandrolanzieri leandrolanzieri added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer labels Feb 1, 2022
@github-actions github-actions bot added the Area: build system Area: Build system label Feb 1, 2022
@fjmolinas fjmolinas added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Feb 1, 2022
Copy link
Contributor

@fjmolinas fjmolinas left a comment

Choose a reason for hiding this comment

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

ACK

Copy link
Member

@maribu maribu left a comment

Choose a reason for hiding this comment

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

I'm trusting @leandrolanzieri and Murdock on this one.

@maribu maribu merged commit 8ee3466 into RIOT-OS:master Feb 1, 2022
@leandrolanzieri leandrolanzieri deleted the pr/makefiles/fix_external_module_paths_includes branch February 1, 2022 12:26
@leandrolanzieri
Copy link
Contributor Author

Thanks for the review!

@OlegHahm OlegHahm added this to the Release 2022.04 milestone Apr 25, 2022
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 Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer 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.

4 participants