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

Removing default toolchain paths #3476

Merged
merged 1 commit into from
Dec 23, 2016

Conversation

bridadan
Copy link
Contributor

@bridadan bridadan commented Dec 20, 2016

Description

This change was spurred by a confusing error. I attempted to compile for the RZ_A1H (a Cortex-A device), and I had the standalone ARM compiler in my system path, which supports Cortex-A. However, the default path for the ARM compiler in settings.py uses a Keil installation, which only supports Cortex-M. It found my Keil installation and used that instead, causing a compiler error. This change proposes to remove this default behavior and instead requires the user to explicitly set the intended compiler path, either by a settings file, mbed CLI, environment variables, or by placing the compiler in your PATH.

Status

READY

Migrations

YES

This will now require that a user sets the compiler paths via mbed CLI, environment variables, settings files, or by placing them in the system PATH.

I realize this could be a breaking change for some developers, so perhaps we should wait a release before doing this? We could detect that the default path in settings.py is being used and warn the users to set the compiler path explicitly. Open to suggestions here!

Todos

  • Tests
  • Documentation

Notes to Maintainers

I requested a lot of reviewers on this PR since this is changing some legacy behavior that we've had for quite some time now. It'd be good to get input from everyone since this may affect many different parts of the mbed ecosystem.

This change was spurred by a confusing error. I attempted to compile for
the RZ_A1H (a Cortex-A device), and I had the standalone ARM compiler in
my system path, which supports Cortex-A. However, the default path for
the ARM compiler in settings.py uses a Keil installation, which only
supports Cortex-M. It found my Keil installation and used that instead.
This change proposes to remove this default behavior and instead
requires the user to explicitly set the intended compiler, either by a
settings file, mbed CLI, environment variables, or by placing the
compiler in your PATH.
Copy link
Contributor

@sg- sg- left a comment

Choose a reason for hiding this comment

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

Is there a helpful message printed when the tools aren't found?

@bridadan
Copy link
Contributor Author

@sg- Yes, after removing the compilers from my PATH and renaming my Keil installation folder, I was able to get an error from the tools:

C:\Users\bridan01\Documents\dev\m_mbed\mbed>mbed test -m NUCLEO_F411RE -t ARM -v
[mbed] Working path "C:\Users\bridan01\Documents\dev\m_mbed\mbed" (program)
[mbed] Exec "python -u C:\Users\bridan01\Documents\dev\m_mbed\mbed\tools\test.py -t ARM -m NUCLEO_F411RE --source C:\Users\bridan01\Documents\dev\m_mbed\mbed --build C:\Users\bridan01\Documents\dev\m_
mbed\mbed\BUILD\tests\NUCLEO_F411RE\ARM --test-spec C:\Users\bridan01\Documents\dev\m_mbed\mbed\BUILD\tests\NUCLEO_F411RE\ARM\test_spec.json -v" in C:\Users\bridan01\Documents\dev\m_mbed\mbed
WARNING: MBED_ARM_PATH set as environment variable but doesn't exist
usage: test.py [-h] [-m MCU] [-t TOOLCHAIN] [--color] [--cflags CFLAGS]
               [--asmflags ASMFLAGS] [--ldflags LDFLAGS] [-c]
               [--profile PROFILE] [--app-config APP_CONFIG] [-D MACROS]
               [-j JOBS] [--source SOURCE_DIR] [--build BUILD_DIR] [-l]
               [-p PATHS] [-f FORMAT] [--continue-on-build-fail] [-n NAMES]
               [--test-spec TEST_SPEC]
               [--build-report-junit BUILD_REPORT_JUNIT] [-v]
test.py: error: Could not find executable for ARM.
Currently set search path: C:/Keil_v5/ARM/ARMCC
[mbed] ERROR: "python" returned error code 2.
[mbed] ERROR: Command "python -u C:\Users\bridan01\Documents\dev\m_mbed\mbed\tools\test.py -t ARM -m NUCLEO_F411RE --source C:\Users\bridan01\Documents\dev\m_mbed\mbed --build C:\Users\bridan01\Docume
nts\dev\m_mbed\mbed\BUILD\tests\NUCLEO_F411RE\ARM --test-spec C:\Users\bridan01\Documents\dev\m_mbed\mbed\BUILD\tests\NUCLEO_F411RE\ARM\test_spec.json -v" in "C:\Users\bridan01\Documents\dev\m_mbed\mb
ed"

@jupe
Copy link
Contributor

jupe commented Dec 20, 2016

@miklis is this affecting to our CI infra?

@bridadan
Copy link
Contributor Author

@jupe, good catch, thanks for checking this

@bridadan bridadan requested a review from miklis December 20, 2016 20:02
@miklis
Copy link
Contributor

miklis commented Dec 21, 2016

@jupe @bridadan we have always had all compilers in PATH. If there are no other requirements this does not affect.

@0xc0170
Copy link
Contributor

0xc0170 commented Dec 21, 2016

retest uvisor

Copy link
Contributor

@0xc0170 0xc0170 left a comment

Choose a reason for hiding this comment

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

These paths were old anyway

@0xc0170
Copy link
Contributor

0xc0170 commented Dec 21, 2016

/morph test

@0xc0170
Copy link
Contributor

0xc0170 commented Dec 21, 2016

@mbed-bot: TEST
HOST_OSES=ALL
BUILD_TOOLCHAINS=ALL
TARGETS=ALL

@mbed-bot
Copy link

[Build 1189]
SUCCESS: Building succeeded and tests were run! Be sure to check the test results

@mbed-bot
Copy link

Result: SUCCESS

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

/morph test

Output

mbed Build Number: 1307

All builds and test passed!

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.

8 participants