Fix include paths for the assembler for makefiles #9946
Conversation
| new_sub_flags = [] | ||
| for sub_flag in sub_flags: | ||
| new_sub_flags.append(_fix_include_asm_flag(sub_flag, ctx)) | ||
| new_asm_flags.append(','.join(new_sub_flags)) |
There was a problem hiding this comment.
one question - how does this fixes the issue - why paths for asm preprocessed files were not correct? I can see magic above based on -I or preinclude - shall we add some details to the commit msg?
The Makefile is run from the build directory. The source files were properly prefixed with "../", however the paths provided to the assmebler were not. This ensures the assembler include paths are prefixed properly.
|
CI started |
|
@bridadan Has this been tested against the export script? https://mbed-os.mbedcloudtesting.com/blue/rest/organizations/jenkins/pipelines/mbed-os-ci/branches/PR-9946/runs/1/nodes/6/log/?start=0 |
bridadan
force-pushed
the
fix_make_armc6
branch
2 times, most recently
from
17e68b3 to
c129336
Compare
|
I fixed my dumb syntax error and updated the commit message. I will run further tests locally in the meantime. |
|
CI restarted |
|
Ug, think there might be more issues, still debugging. |
Kk. Will let CI still run in the meanwhile. Out of curiosity, do we have a way to test against the online compiler, or would we need to locally test somehow? |
|
@cmonr pretty sure the issue I was seeing locally was a problem with my path config, not the implementation. Trying now. Regarding the online compiler, we would need to test this when we make a tools release as this change wouldn't go live until then. |
ghost
added
the
Test run: SUCCESSSummary: 13 of 13 test jobs passed |
|
Merging to see if this unblocks the other 5.12 PRs. |
Description
Should fix #9938.
Seems to work fine with ARMC5, ARMC6, and GCC_ARM.
Pull request type
Reviewers
@0xc0170 - I tried my darndest to get git to ignore the whitespace changes, but I couldn't figure it out. If you feel really strongly about it let me know.
Release Notes