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

Tools: Move exporter alias handling to CLI #7410

Merged
merged 6 commits into from Jul 11, 2018

Conversation

Projects
None yet
6 participants
@theotherjimmy
Contributor

theotherjimmy commented Jul 3, 2018

Description

Both the online compiler and the offline tools use the same code for
exporting and compilation. This is great, as it significantly reduces
code duplication and maintenance efforts. A knock-on effect of this
is that the exporters available on the command line are all available
online. Including the aliases.

This PR removes the aliases from the online compiler.

@thegecko as promised.

Pull request type

[x] Fix
[ ] Refactor
[ ] New target
[ ] Feature
[ ] Breaking change
@thegecko

This comment has been minimized.

Member

thegecko commented Jul 3, 2018

@0xc0170 0xc0170 requested a review from ARMmbed/mbed-os-tools Jul 4, 2018

@cmonr

This comment has been minimized.

Contributor

cmonr commented Jul 5, 2018

/morph build

@mbed-ci

This comment has been minimized.

mbed-ci commented Jul 5, 2018

@cmonr

This comment has been minimized.

Contributor

cmonr commented Jul 5, 2018

/morph build

@mbed-ci

This comment has been minimized.

mbed-ci commented Jul 6, 2018

Build : SUCCESS

Build number : 2546
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/7410/

Triggering tests

/morph test
/morph uvisor-test
/morph export-build
/morph mbed2-build

@mbed-ci

This comment has been minimized.

@mbed-ci

This comment has been minimized.

@cmonr

This comment has been minimized.

Contributor

cmonr commented Jul 6, 2018

@theotherjimmy It looks like there were CI node issues:

02:59:48 Traceback (most recent call last):
02:59:48   File "/builds/ws/exporter-build-matrix/2190/ARCH_PRO_gnuarmeclipse/sources/examples/mbed-os-example-blinky/mbed-os/tools/project.py", line 311, in <module>
02:59:48     main()
02:59:48   File "/builds/ws/exporter-build-matrix/2190/ARCH_PRO_gnuarmeclipse/sources/examples/mbed-os-example-blinky/mbed-os/tools/project.py", line 270, in main
02:59:48     profile = extract_profile(parser, options, toolchain_name, fallback="debug")
02:59:48 UnboundLocalError: local variable 'toolchain_name' referenced before assignment
@theotherjimmy

This comment has been minimized.

Contributor

theotherjimmy commented Jul 6, 2018

@cmonr That's a bug.

@theotherjimmy theotherjimmy force-pushed the theotherjimmy:explicit-exporer-aliases branch from a3e6b89 to 71323af Jul 6, 2018

theotherjimmy added some commits Jul 6, 2018

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Jul 9, 2018

/morph build

@mbed-ci

This comment has been minimized.

mbed-ci commented Jul 9, 2018

Build : SUCCESS

Build number : 2550
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/7410/

Triggering tests

/morph test
/morph uvisor-test
/morph export-build
/morph mbed2-build

@mbed-ci

This comment has been minimized.

@mbed-ci

This comment has been minimized.

@0xc0170 0xc0170 added needs: work and removed needs: CI labels Jul 9, 2018

@cmonr

This comment has been minimized.

Contributor

cmonr commented Jul 9, 2018

11:47:18 usage: examples.py export [-h] [-m MCU] [ide [ide ...]]
11:47:18 examples.py export: error: argument ide: uvision is not a supported ide. Supported ides are:
11:47:18 sw4stm32,        netbeans,        eclipse_iar,     make_iar, 
11:47:18 eclipse_armc5,   mcuxpresso,      make_armc6,      make_armc5, 
11:47:18 vscode_iar,      iar,             gnuarmeclipse,   ds5_5, 
11:47:18 cces,            vscode_gcc_arm,  embitz,          vscode_armc5, 
11:47:18 uvision5,        e2studio,        codeblocks,      make_gcc_arm, 
11:47:18 qtcreator,       eclipse_gcc_arm, cmake_gcc_arm    

@theotherjimmy You'll need to look at how the export job is configured to reproduce locally.

@theotherjimmy

This comment has been minimized.

Contributor

theotherjimmy commented Jul 9, 2018

Thanks @cmonr I have enough info to take it from here.

@cmonr cmonr added needs: CI and removed needs: review labels Jul 9, 2018

@mbed-ci

This comment has been minimized.

mbed-ci commented Jul 9, 2018

Build : SUCCESS

Build number : 2554
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/7410/

Triggering tests

/morph test
/morph uvisor-test
/morph export-build
/morph mbed2-build

@mbed-ci

This comment has been minimized.

@theotherjimmy

This comment has been minimized.

Contributor

theotherjimmy commented Jul 9, 2018

Looks like I have to do this the hard way: make the CI not use the aliases.

@cmonr

This comment has been minimized.

Contributor

cmonr commented Jul 9, 2018

@studavekar You'll help will be needed here.

@mbed-ci

This comment has been minimized.

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Jul 10, 2018

Looks like I have to do this the hard way: make the CI not use the aliases.

Trying to understand it here. uvision is being removed completely from exporters ? That is what is failing in the CI. This is a breaking change, isn't it?

@0xc0170 0xc0170 added needs: work and removed needs: CI labels Jul 10, 2018

@theotherjimmy

This comment has been minimized.

Contributor

theotherjimmy commented Jul 10, 2018

@0xc0170 No. uvision is still supported.

@theotherjimmy

This comment has been minimized.

Contributor

theotherjimmy commented Jul 10, 2018

@0xc0170 What's failing CI is that the CI is attempting to inspect the uvision exporter's supported list, which can't be done on account of the fact that the actual exporter is uvision5. uvision is now treated like another name for the uvision5 exporter instead of an identical exporter.

@cmonr

This comment has been minimized.

Contributor

cmonr commented Jul 10, 2018

@studavekar Mind taking a look at this? This needs a tweak to the CI config.

@cmonr

This comment has been minimized.

Contributor

cmonr commented Jul 10, 2018

@0xc0170 Changing this back to Needs: CI since no PR work is needed and is still ready to run.

@cmonr cmonr added needs: CI and removed needs: work labels Jul 10, 2018

@studavekar

This comment has been minimized.

Collaborator

studavekar commented Jul 10, 2018

@0xc0170 What's failing CI is that the CI is attempting to inspect the uvision exporter's supported list, which can't be done on account of the fact that the actual exporter is uvision5. uvision is now treated like another name for the uvision5 exporter instead of an identical exporter.

@cmonr @theotherjimmy

If I go ahead and make the changes in CI right now, Other PR would fail which are in the queue.
I can make the changes lets halt the pipeline till this PR gets merged, does that sound ok?

@theotherjimmy

This comment has been minimized.

Contributor

theotherjimmy commented Jul 10, 2018

@studavekar Sure, if that's needed. We're changing from one valid exporter to another, so I did not think that would change much.

@cmonr

This comment has been minimized.

Contributor

cmonr commented Jul 10, 2018

The exporter queue was empty so it fine to do.

@mbed-ci

This comment has been minimized.

@studavekar

This comment has been minimized.

Collaborator

studavekar commented Jul 10, 2018

@cmonr can you merge this PR. other PR exporter build would fail if this change is not in..

@theotherjimmy

This comment has been minimized.

Contributor

theotherjimmy commented Jul 10, 2018

@studavekar As I have said multiple times in this thread, that's not true. This does not change anything about what mbed export accepts to it's -i option.

@studavekar

This comment has been minimized.

Collaborator

studavekar commented Jul 10, 2018

@studavekar As I have said multiple times in this thread, that's not true. This does not change anything about what mbed export accepts to it's -i option.

@theotherjimmyI felt that this would break exporter because the last exporter result #7410 (comment)

00:00:25.441   File "../mbed-os/tools/test/examples/examples.py", line 133, in <module>
00:00:25.441     sys.exit(main())
00:00:25.441   File "../mbed-os/tools/test/examples/examples.py", line 84, in main
00:00:25.441     return args.fn(args, config, examples)
00:00:25.441   File "../mbed-os/tools/test/examples/examples.py", line 90, in do_export
00:00:25.441     results = lib.export_repos(config, args.ide, args.mcu, examples)
00:00:25.441   File "C:\builds\ws\exporter-build-matrix\2199\ARCH_PRO_uvision\sources\mbed-os\tools\test\examples\examples_lib.py", line 279, in export_repos
00:00:25.441     example['features'], example['toolchains']):
00:00:25.441   File "C:\builds\ws\exporter-build-matrix\2199\ARCH_PRO_uvision\sources\mbed-os\tools\test\examples\examples_lib.py", line 120, in target_cross_ide
00:00:25.441     if (EXPORTERS[ide].is_target_supported(target) and
00:00:25.441 KeyError: u'uvision

We have updated the CI to use uvision5 as argument instead of uvision and its passing

@theotherjimmy

This comment has been minimized.

Contributor

theotherjimmy commented Jul 10, 2018

@studavekar That's part of the changes in this PR. I did not add uvision5 in this PR, it's been here for 2 years.

@cmonr cmonr merged commit 45b8ec9 into ARMmbed:master Jul 11, 2018

14 checks passed

AWS-CI uVisor Build & Test Success
Details
ci-morph-build build completed
Details
ci-morph-exporter build completed
Details
ci-morph-mbed2-build build completed
Details
ci-morph-test test completed
Details
continuous-integration/jenkins/pr-head This commit looks good
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
travis-ci/astyle Passed, 791 files
Details
travis-ci/docs Local docs testing has passed
Details
travis-ci/events Passed, runtime is 9939 cycles (+318 cycles)
Details
travis-ci/gitattributestest Local gitattributestest testing has passed
Details
travis-ci/licence_check Local licence_check testing has passed
Details
travis-ci/littlefs Passed, code size is 9960B (+0.00%)
Details
travis-ci/tools-py2.7 Local tools-py2.7 testing has passed
Details

@0xc0170 0xc0170 removed the needs: CI label Jul 11, 2018

pan- pushed a commit to pan-/mbed that referenced this pull request Aug 22, 2018

Merge pull request ARMmbed#7410 from theotherjimmy/explicit-exporer-a…
…liases

Tools: Move exporter alias handling to CLI
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment