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

Change default uvision exporter to uvision5 #2614

Merged
merged 3 commits into from
Sep 22, 2016
Merged

Change default uvision exporter to uvision5 #2614

merged 3 commits into from
Sep 22, 2016

Conversation

MarceloSalazar
Copy link

uVision4 is no longer maintained by the Keil team (support was stopped
long time ago).

This is why we need to make the uvision exporter able to generate uVision5
projects by default (which is based on software packs).

Also, adding note that the uvision4 exporter is now deprecated and will be
removed in the future.

@0xc0170 @sg-

@sg-
Copy link
Contributor

sg- commented Sep 9, 2016

@MarceloSalazar needs a rebase

@sarahmarshy review?

Marcelo Salazar added 2 commits September 12, 2016 19:46
uVision4 is no longer maintained by the Keil team (support was stopped
long time ago).

This is why we need to make the uvision exporter able to generate uVision5
projects by default (which is based on software packs).

Also, adding note that the uvision4 exporter is now deprecated and will be
removed in the future.
@theotherjimmy
Copy link
Contributor

LGTM 👍

@sarahmarshy
Copy link
Contributor

LGTM

if tool == 'uvision' or tool == 'uvision5':
log = path.join(project_dir, "build", "build_log.txt")
if tool == 'uvision' or tool == 'uvision4':
log = path.join(project_dir,"build","build_log.txt")
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you keep the spaces that were there before?

Copy link
Author

Choose a reason for hiding this comment

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

my bad, should be fixed now

@sg-
Copy link
Contributor

sg- commented Sep 16, 2016

/morph export-build

@mbed-bot
Copy link

Result: FAILURE

Your command has finished executing! Here's what you wrote!

/morph export-build

@sg-
Copy link
Contributor

sg- commented Sep 17, 2016

Seems like there was a change to the entry point for export that needs to be reflected in the tests and possibly documentation.

16:54:57 + python tools/test/export/build_test.py -i uvision5
16:55:08 usage: build_test.py [-h] [-i IDES] [-p PROGRAMS] [-n PROGRAMS]
16:55:08                      [-m MCU [MCU ...]] [-c]
16:55:08 build_test.py: error: argument -i: uvision5 is not a supported toolchain. Supported toolchains are:
16:55:08 iar,      uvision,  uvision4  
16:55:08 Build step 'Execute shell' marked build as failure
16:55:08 Finished: FAILURE
...
16:56:34 + python tools/test/export/build_test.py -i iar
16:56:51 
16:56:51 + deactivate

@sg- sg- added needs: work and removed needs: CI labels Sep 17, 2016
@sarahmarshy
Copy link
Contributor

sarahmarshy commented Sep 18, 2016

@sg @MarceloSalazar - That is actually just an entry point for the export build tests, as it limits the options for that switch to the three listed.

This is the line responsible.

@MarceloSalazar
Copy link
Author

Guys, I'm not entirely sure how this is expected to be fixed. Would you be able to help here?

@bridadan
Copy link
Contributor

@sarahmarshy Would you mind helping @MarceloSalazar on what changes need to be made to build_test.py? If you need me to change a command in CI just let me know.

@theotherjimmy
Copy link
Contributor

@bridadan: the arguments to the build_test.py need to change in the CI to uvision from uvision5.

@bridadan
Copy link
Contributor

Thanks @theotherjimmy, done.

/morph export-build

@sarahmarshy
Copy link
Contributor

@bridadan @theotherjimmy No, that will still fail. I'll need to also change that line in the script. Marcelo, the line that I linked to above should become ['iar','uvision5'].

@sarahmarshy
Copy link
Contributor

Let me test locally. Just a moment.

@theotherjimmy
Copy link
Contributor

The line in that script was changed in this PR to include uvision5

@mbed-bot
Copy link

Result: SUCCESS

Your command has finished executing! Here's what you wrote!

/morph export-build

@sarahmarshy
Copy link
Contributor

@theotherjimmy cool, am an idiot. Good work Marcelo. LGTM

@theotherjimmy
Copy link
Contributor

@bridadan Why the do not merge label?

@bridadan
Copy link
Contributor

Hang on, the pass was a false success because I was changing the CI config. Need to rerun.

/morph test

@mbed-bot
Copy link

Result: ABORTED

Your command has finished executing! Here's what you wrote!

/morph test

@bridadan
Copy link
Contributor

Just realized I used the wrong test command 😠

/morph export-build

@mbed-bot
Copy link

Result: FAILURE

Your command has finished executing! Here's what you wrote!

/morph export-build

@bridadan
Copy link
Contributor

IAR has existing failures, uvision is all passing

@theotherjimmy
Copy link
Contributor

Those IAR failures are clearly not related to this patch, and I don't think they are regressions.

@bridadan
Copy link
Contributor

@theotherjimmy I agree! LGTM. @sarahmarshy ?

@sg- sg- merged commit 6fce892 into ARMmbed:master Sep 22, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants