Skip to content
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

Allow the use of Mbed Studio's version of ARMC6 #10114

Merged
merged 7 commits into from Mar 23, 2019

Conversation

@bridadan
Copy link
Contributor

commented Mar 14, 2019

Description

This seems to be in good working order so far. I need to add a unit test before this is ready to go, hence raising this as a Draft pull request. Raising this early for coordination efforts and to get reviews started.

This allows people to use the ARMC6 compiler bundled with Mbed Studio with the command line tools. It also preserves the ability to use the professional compiler and license server.

FYI @ChiefBureaucraticOfficer @SenRamakri @arekzaluski @thegecko

Pull request type

[ ] Fix
[ ] Refactor
[ ] Target update
[x] Functionality change
[ ] Docs update
[ ] Test update
[ ] Breaking change

Reviewers

Release Notes

You can use the ARMC6 compiler that is bundled with Mbed Studio with the command line tools. Enable this behavior by adding the ARMC6 executable to your system path (the default location of this on Windows is C:/MbedStudio/tools/ac6/bin). Alternatively, you can set this same location with Mbed CLI with the command mbed config ARMC6_PATH C:/MbedStudio/tools/ac6/bin (be sure to use the correct path when you run the command). You will also need to set the ARMLMD_LICENSE_FILE environment variable to the license file that is bundled with Mbed Studio. Check the Mbed OS documentation for further details.

@bridadan bridadan requested a review from theotherjimmy Mar 14, 2019

@theotherjimmy

This comment has been minimized.

Copy link
Contributor

commented Mar 14, 2019

@arekzaluski This should allow you to remove one of your patches.

@ciarmcom

This comment has been minimized.

Copy link
Member

commented Mar 15, 2019

@bridadan, thank you for your changes.
@ARMmbed/mbed-os-tools @ARMmbed/mbed-os-maintainers please review.

@ciarmcom ciarmcom requested review from ARMmbed/mbed-os-maintainers Mar 15, 2019

@cmonr

This comment has been minimized.

Copy link
Contributor

commented Mar 15, 2019

raising this as a Draft pull request.

Look at you, using new GitHub features.

fancy

@cmonr

This comment has been minimized.

Copy link
Contributor

commented Mar 15, 2019

CI started to get some information

@mbed-ci

This comment has been minimized.

Copy link

commented Mar 15, 2019

Test run: FAILED

Summary: 1 of 9 test jobs failed
Build number : 1
Build artifacts

Failed test jobs:

  • jenkins-ci/mbed-os-ci_build-ARMC6
@cmonr

This comment has been minimized.

Copy link
Contributor

commented Mar 15, 2019

AttributeError: 'ARMC6' object has no attribute 'version_name'
'ARMC6' object has no attribute 'version_name'

@cmonr cmonr added needs: work and removed needs: review labels Mar 15, 2019

@0xc0170 0xc0170 requested a review from arekzaluski Mar 15, 2019

@bulislaw

This comment has been minimized.

Copy link
Member

commented Mar 15, 2019

Was that tested both on Windows and OSX?

@bridadan

This comment has been minimized.

Copy link
Contributor Author

commented Mar 15, 2019

@bulislaw I don't have a machine running OSX, so I'll happily take any help there! Though I need to fix the error found here first.

@bridadan bridadan marked this pull request as ready for review Mar 15, 2019

@bridadan

This comment has been minimized.

Copy link
Contributor Author

commented Mar 15, 2019

I was able to reproduce the issue shown in CI, the most recent patches should fix those.

@0xc0170

This comment has been minimized.

Copy link
Member

commented Mar 15, 2019

Will restart once travis tools are green, please review

@bridadan

This comment has been minimized.

Copy link
Contributor Author

commented Mar 15, 2019

I'm working on a few local errors as well, thanks for keeping an eye on this.

@bridadan bridadan force-pushed the bridadan:armc6_mbed_ide branch Mar 15, 2019

@bridadan

This comment has been minimized.

Copy link
Contributor Author

commented Mar 15, 2019

Ok I've tested this locally fairly extensively on Windows with the following compilers:

  • ARMC5
    • From Keil uvision
    • Professional compiler
  • ARMC6
    • From Keil uvision
    • Professional compiler
    • From Mbed Studio

And all seem happy!

@bridadan

This comment has been minimized.

Copy link
Contributor Author

commented Mar 15, 2019

Found a Mac and tested it, seemed to work just fine @bulislaw

@0xc0170

This comment has been minimized.

Copy link
Member

commented Mar 15, 2019

CI started

@0xc0170 0xc0170 added needs: CI and removed needs: work labels Mar 15, 2019

@mbed-ci

This comment has been minimized.

Copy link

commented Mar 15, 2019

Test run: FAILED

Summary: 1 of 13 test jobs failed
Build number : 2
Build artifacts

Failed test jobs:

  • jenkins-ci/mbed-os-ci_exporter
@0xc0170

This comment has been minimized.

Copy link
Member

commented Mar 16, 2019

cc @mbed-os-test ^^

We will restart exporters later today

@0xc0170

This comment has been minimized.

Copy link
Member

commented Mar 16, 2019

Reviews are opened!

@0xc0170

This comment has been minimized.

Copy link
Member

commented Mar 16, 2019

exporters restarted

Version check the compiler in all build functions
This enables the use of Mbed Studio's version of ARMC6.

@bridadan bridadan force-pushed the bridadan:armc6_mbed_ide branch to 6e629de Mar 19, 2019

@bridadan

This comment has been minimized.

Copy link
Contributor Author

commented Mar 19, 2019

Ok @arekzaluski @thegecko @cmonr, I've removed the auto-setting of the license file. I still need to detect that it is the Mbed Studio version of ARMC6 so I can conditionally supply the --ide=mbed flag to each of the compiler, assembler, linker, and fromelf binaries.

@cmonr

This comment has been minimized.

Copy link
Contributor

commented Mar 20, 2019

CI started whist discussions move forward.

@cmonr

This comment has been minimized.

Copy link
Contributor

commented Mar 20, 2019

@cmonr

This comment has been minimized.

Copy link
Contributor

commented Mar 20, 2019

@bulislaw ^^^

@mbed-ci

This comment has been minimized.

Copy link

commented Mar 20, 2019

Test run: SUCCESS

Summary: 13 of 13 test jobs passed
Build number : 5
Build artifacts

@0xc0170 0xc0170 added needs: review and removed needs: CI labels Mar 20, 2019

@0xc0170

This comment has been minimized.

Copy link
Member

commented Mar 20, 2019

Ok @arekzaluski @thegecko @cmonr, I've removed the auto-setting of the license file. I still need to detect that it is the Mbed Studio version of ARMC6 so I can conditionally supply the --ide=mbed flag to each of the compiler, assembler, linker, and fromelf binaries.

Waiting for Mbed Studio team review

@thegecko
Copy link
Member

left a comment

LGTM

@0xc0170
Copy link
Member

left a comment

Needs our final approval based on some recent concerns that are being taken care of = request changes.

@0xc0170

This comment has been minimized.

Copy link
Member

commented Mar 22, 2019

Started CI

@0xc0170 0xc0170 added needs: CI and removed needs: review labels Mar 22, 2019

@mbed-ci

This comment has been minimized.

Copy link

commented Mar 22, 2019

Test run: SUCCESS

Summary: 13 of 13 test jobs passed
Build number : 6
Build artifacts

@0xc0170 0xc0170 added ready for merge and removed needs: CI labels Mar 22, 2019

We will merge this whether we hear from DSG or not by COB today

@0xc0170

This comment has been minimized.

Copy link
Member

commented Mar 23, 2019

We will merge this whether we hear from DSG or not by COB today

+1. This should be ready, merging now

@0xc0170 0xc0170 merged commit aca0f2f into ARMmbed:master Mar 23, 2019

28 checks passed

continuous-integration/jenkins/pr-head This commit looks good
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
jenkins-ci/build-ARMC5 Success
Details
jenkins-ci/build-ARMC6 Success
Details
jenkins-ci/build-GCC_ARM Success
Details
jenkins-ci/build-IAR8 Success
Details
jenkins-ci/cloud-client-test Success
Details
jenkins-ci/dynamic-memory-usage RTOS ROM(+0 bytes) RAM(+0 bytes)
Details
jenkins-ci/exporter Success
Details
jenkins-ci/greentea-test Success
Details
jenkins-ci/mbed2-build-ARMC5 Success
Details
jenkins-ci/mbed2-build-ARMC6 Success
Details
jenkins-ci/mbed2-build-GCC_ARM Success
Details
jenkins-ci/mbed2-build-IAR8 Success
Details
jenkins-ci/unittests Success
Details
travis-ci/astyle Local astyle testing has passed
Details
travis-ci/docs Local docs testing has passed
Details
travis-ci/doxy-spellcheck Local doxy-spellcheck testing has passed
Details
travis-ci/events Passed, runtime is 10327 cycles (-45 cycles)
Details
travis-ci/gitattributestest Local gitattributestest testing has passed
Details
travis-ci/include_check Local include_check testing has passed
Details
travis-ci/licence_check Local licence_check testing has passed
Details
travis-ci/littlefs Passed, code size is 8408B (+0.00%)
Details
travis-ci/psa-autogen Local psa-autogen testing has passed
Details
travis-ci/tools-py2.7 Local tools-py2.7 testing has passed
Details
travis-ci/tools-py3.5 Local tools-py3.5 testing has passed
Details
travis-ci/tools-py3.6 Local tools-py3.6 testing has passed
Details
travis-ci/tools-py3.7 Local tools-py3.7 testing has passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.