Skip to content

Readable error when toolchain paths not set. #2416

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

Readable error when toolchain paths not set. #2416

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions tools/toolchains/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -849,6 +849,21 @@ def compile_command(self, source, object, includes):

return None

def check_toolchain_path(self, name):
"""Check if the path to toolchain is valid. Exit if not.

Positional arguments:
name -- the name of the toolchain, as used when creating compile
commands to execute. Valid key for TOOLCHAIN_PATHS.

No return value but causes a system exit of execution if the path
does not exist.
"""
Copy link
Contributor

Choose a reason for hiding this comment

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

❤️ the docstring!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have seen the light! Going to try to add them to everything I modify in the future.

Copy link
Contributor

Choose a reason for hiding this comment

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

Do we really want to exit inside of this function? It seems like we'd just want to "check_toolchain_path" here, return True or False, and delegate the appropriate action farther up the call stack. For instance, the toolchains could call this, check the return value, and throw an exception. That could be caught by the CLI scripts (build.py, make.py, test.py, etc) and they could exit gracefully.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It should exit gracefully with the error? Unless you need some exit value. Check the new PR.

Copy link

Choose a reason for hiding this comment

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

@bridadan do you think it is breaking change. I see that the build has failed while trying to get GCC_ARM toolchain path. I see

17:13:53 + mbed-cli test --compile --test-spec test_spec.json -vv -t GCC_ARM -m K64F
17:14:19 [ERROR] Toolchain path does not exist for GCC_ARM.

in http://e108747.cambridge.arm.com:8080/job/mbed-os/job/mbed-os-pr-build-parameterized/612/console

Copy link
Contributor

Choose a reason for hiding this comment

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

@sarahmarshy It does exit gracefully with an error, but I think we typically try to stay away from calling exit within API functions (like toolchains or build_api). Usually I think we try to return error codes or throw exceptions in API functions and then let the top level CLI script that's calling them handle the exception and exit. Thoughts on this @theotherjimmy ?

@mazimkhan You're right I think this might cause issues with GCC_ARM because the toolchains can find it in the system PATH. I think this is only for GCC_ARM though unfortunately :/ @sarahmarshy sounds like another corner case.

if not exists(TOOLCHAIN_PATHS[name]):
print('[ERROR] Toolchain path does not exist for %s.\n'
'Current value: %s' % (name, TOOLCHAIN_PATHS[name]))
sys.exit()

@abstractmethod
def parse_dependencies(self, dep_path):
"""Parse the dependency information generated by the compiler.
Expand Down
2 changes: 2 additions & 0 deletions tools/toolchains/arm.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ class ARM(mbedToolchain):
def __init__(self, target, options=None, notify=None, macros=None, silent=False, extra_verbose=False):
mbedToolchain.__init__(self, target, options, notify, macros, silent, extra_verbose=extra_verbose)

self.check_toolchain_path("ARM")

if target.core == "Cortex-M0+":
cpu = "Cortex-M0"
elif target.core == "Cortex-M4F":
Expand Down
4 changes: 4 additions & 0 deletions tools/toolchains/gcc.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,8 @@ class GCC_ARM(GCC):
def __init__(self, target, options=None, notify=None, macros=None, silent=False, extra_verbose=False):
GCC.__init__(self, target, options, notify, macros, silent, TOOLCHAIN_PATHS['GCC_ARM'], extra_verbose=extra_verbose)

self.check_toolchain_path("GCC_ARM")

# Use latest gcc nanolib
if "big-build" in self.options:
use_nano = False
Expand All @@ -297,6 +299,8 @@ class GCC_CR(GCC):
def __init__(self, target, options=None, notify=None, macros=None, silent=False, extra_verbose=False):
GCC.__init__(self, target, options, notify, macros, silent, TOOLCHAIN_PATHS['GCC_CR'], extra_verbose=extra_verbose)

self.check_toolchain_path("GCC_CR")

additional_compiler_flags = [
"-D__NEWLIB__", "-D__CODE_RED", "-D__USE_CMSIS", "-DCPP_USE_HEAP",
]
Expand Down
3 changes: 3 additions & 0 deletions tools/toolchains/iar.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,9 @@ class IAR(mbedToolchain):

def __init__(self, target, options=None, notify=None, macros=None, silent=False, extra_verbose=False):
mbedToolchain.__init__(self, target, options, notify, macros, silent, extra_verbose=extra_verbose)

self.check_toolchain_path("IAR")

if target.core == "Cortex-M7F" or target.core == "Cortex-M7FD":
cpuchoice = "Cortex-M7"
else:
Expand Down