-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Passing toolchain path info to subprocesses #3274
Passing toolchain path info to subprocesses #3274
Conversation
/morph test |
Result: SUCCESSYour command has finished executing! Here's what you wrote!
OutputAll builds and test passed! |
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.
Style edit not a blocker. LGTM.
@@ -2122,7 +2128,6 @@ def build_tests(tests, base_source_paths, build_path, target, toolchain_name, | |||
|
|||
Returns a tuple of the build result (True or False) followed by the test | |||
build data structure""" | |||
|
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.
Why the style edit?
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.
I think I just missed that change when I had some debug prints. If you feel REALLLY strongly about it I'll remove it, otherwise I might just let it simmer :)
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.
¯_(ツ)_/¯
Since the toolchain path info lives as a global variable, its not propigating to the subprocesses that are created to compile the tests in parallel. This change manually passes these global variables and then reassigns them.
05140dc
to
57e693d
Compare
Fixed the style :) Shouldn't be necessary to re run tests, just added a blank line again. |
@0xc0170 Can you take a look please? Should be ready to go if you're happy. |
@0xc0170 Bumpity bump :) |
Description
Since the toolchain path info lives as a global variable, its not
propagating to the subprocesses that are created to compile the tests in
parallel. This change manually passes these global variables and then
reassigns them.
This seems to only be an issue with the IAR compiler, however I haven't thoroughly tested every environemnt and every compiler.
Status
READY
Migrations
If this PR changes any APIs or behaviors, give a short description of what API users should do when this PR is merged.
NO
Todos
Steps to test or reproduce
On a machine with the IAR compiler in the system PATH (
iccarm.exe
), runmbed test --compile -m K64F -t IAR
. It will complete the library build but fail to build any tests.Switch to these changes and the same command should complete successfully.
@maclobdell This may explain some of the issues you were seeing