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

RTL8195AM - Respect Toolchains paths in post bulid script. #5042

Merged
merged 1 commit into from Oct 20, 2017

Conversation

Projects
None yet
8 participants
@tung7970
Contributor

tung7970 commented Sep 7, 2017

Description

Respect [GCC_ARM|ARM|IAR]_PATH from mbed_settings.py, for users might set path through that file instead of system path.

Status

READY

Migrations

NO

Related PRs

List related PRs against other branches:

Todos

NONE

Deploy notes

NONE

Steps to test or reproduce

NONE

@tung7970 tung7970 changed the title from rtl8195am - Respect [GCC_ARM|ARM |IAR]_PATH from mbed_settings.py to RTL8195AM - Respect [GCC_ARM|ARM |IAR]_PATH from mbed_settings.py Sep 7, 2017

@bulislaw

This comment has been minimized.

Member

bulislaw commented Sep 7, 2017

@theotherjimmy

This comment has been minimized.

Contributor

theotherjimmy commented Sep 7, 2017

Does this also work when the compilers are found in the PATH?

@theotherjimmy

This comment has been minimized.

Contributor

theotherjimmy commented Sep 7, 2017

FYI, the Cam-CI job was aborted (10 times!) so that's why it's red.

@tung7970

This comment has been minimized.

Contributor

tung7970 commented Sep 7, 2017

If a compiler path is set in mbed_settings.py, the binary hook script will use that. If it is empty, it will fallback to use compiler found in system path.

@tung7970

This comment has been minimized.

Contributor

tung7970 commented Sep 7, 2017

It's a bit confusing on the compiler paths. Some examples in mbed_settings.py point to the installation directory, some points to the binary directory.

The post processing script for RTL8195AM expects binary directory for all compilers. Maybe that's why Cam-CI is failing ?

#ARM_PATH = "C:/Program Files/ARM"
#IAR_PATH = "C:/Program Files (x86)/IAR Systems/Embedded Workbench 7.0/arm"

#GCC_CR_PATH = "C:/code_red/RedSuite_4.2.0_349/redsuite/Tools/bin"
#GOANNA_PATH = "c:/Program Files (x86)/RedLizards/Goanna Central 3.2.3/bin"
@theotherjimmy

This comment has been minimized.

Contributor

theotherjimmy commented Sep 7, 2017

Thanks for the clarification @tung7970. I think this change is a clear improvement, and it sounds like there are no downsides. I'm marking this needs: CI now. See my below comments. I'm actually marking this needs: work until they're implemented.

@theotherjimmy

This comment has been minimized.

Contributor

theotherjimmy commented Sep 7, 2017

@tung7970 You're right. There's another way to get the toolchain paths that will be more compatible... Just a moment. I'll give you a link.

@theotherjimmy

This comment has been minimized.

Contributor

theotherjimmy commented Sep 7, 2017

https://github.com/ARMmbed/mbed-os/blob/master/tools/toolchains/__init__.py#L1538-L1543 is updated by the build scripts to include the paths. The globals are left alone. If you want to support env var specification of toolchain paths, you should use the TOOLCHAIN_PATHS global instead of the settings.

tools/targets/REALTEK_RTL8195AM.py Outdated
@@ -8,6 +8,9 @@
import hashlib
import shutil
from tools.settings import GCC_ARM_PATH
from tools.settings import ARM_PATH
from tools.settings import IAR_PATH

This comment has been minimized.

@theotherjimmy

theotherjimmy Sep 7, 2017

Contributor

Could you use

from tools.toolchains import TOOLCHAIN_PATHS

instead?

@tung7970 tung7970 force-pushed the tung7970:fix-tools branch Sep 7, 2017

@tung7970

This comment has been minimized.

Contributor

tung7970 commented Sep 7, 2017

Done. Let me know if this is OK. Thanks.

tools/targets/REALTEK_RTL8195AM.py Outdated
@@ -153,7 +154,7 @@ def parse_load_segment_armcc(image_elf):
(offset, addr, size) = (0, 0, 0)
segment_list = []
in_segment = False
cmd = 'fromelf --text -v --only=none ' + image_elf
cmd = os.path.join(TOOLCHAIN_PATHS['ARM'], 'bin', 'fromelf --text -v --only=none ') + image_elf

This comment has been minimized.

@theotherjimmy

theotherjimmy Sep 7, 2017

Contributor

What if they compile for uARM? Do you think that you need to handle that case? does your target support uARM?

This comment has been minimized.

@tung7970

tung7970 Sep 7, 2017

Contributor

uARM is not supported at this moment. Will need to check if it can be added easily.

@theotherjimmy

This comment has been minimized.

Contributor

theotherjimmy commented Sep 7, 2017

Thanks for getting back to me so quickly @tung7970. It looks better, and should pass Cam-CI.

FYI, we are focusing CI on the 5.6.0 and 5.5.7 releases right now, so this PR may have to wait until we get through more of those PRs.

@theotherjimmy theotherjimmy changed the title from RTL8195AM - Respect [GCC_ARM|ARM |IAR]_PATH from mbed_settings.py to RTL8195AM - Respect Toolchains paths in post bulid script. Sep 7, 2017

@theotherjimmy

This comment has been minimized.

Contributor

theotherjimmy commented Sep 7, 2017

I also changed the title to match the contents of this PR.

@theotherjimmy theotherjimmy added needs: CI and removed needs: work labels Sep 7, 2017

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Sep 27, 2017

/morph test

1 similar comment
@studavekar

This comment has been minimized.

Collaborator

studavekar commented Sep 28, 2017

/morph test

@mbed-bot

This comment has been minimized.

mbed-bot commented Sep 28, 2017

Result: FAILURE

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

/morph test

Output

mbed Build Number: 1414

Build failed!

@theotherjimmy

This comment has been minimized.

Contributor

theotherjimmy commented Oct 2, 2017

00:41:52 Traceback (most recent call last):
00:41:52   File "C:\mj\workspace\bm_wrap\1516\mbed-os\tools\test_api.py", line 2100, in build_test_worker
00:41:52     bin_file = build_project(*args, **kwargs)
00:41:52   File "C:\mj\workspace\bm_wrap\1516\mbed-os\tools\build_api.py", line 552, in build_project
00:41:52     res, _ = toolchain.link_program(resources, build_path, name)
00:41:52   File "C:\mj\workspace\bm_wrap\1516\mbed-os\tools\toolchains\__init__.py", line 1145, in link_program
00:41:52     self.binary(r, elf, bin)
00:41:52   File "C:\mj\workspace\bm_wrap\1516\mbed-os\tools\hooks.py", line 54, in wrapper
00:41:52     post_res = tooldesc["post"](t_self, *args, **kwargs)
00:41:52   File "C:\mj\workspace\bm_wrap\1516\mbed-os\tools\targets\__init__.py", line 536, in binary_hook
00:41:52     rtl8195a_elf2bin(t_self, elf, binf)
00:41:52   File "C:\mj\workspace\bm_wrap\1516\mbed-os\tools\targets\REALTEK_RTL8195AM.py", line 261, in rtl8195a_elf2bin
00:41:52     segment = parse_load_segment(t_self.name, image_elf)
00:41:52   File "C:\mj\workspace\bm_wrap\1516\mbed-os\tools\targets\REALTEK_RTL8195AM.py", line 226, in parse_load_segment
00:41:52     return parse_load_segment_gcc(image_elf)
00:41:52   File "C:\mj\workspace\bm_wrap\1516\mbed-os\tools\targets\REALTEK_RTL8195AM.py", line 127, in parse_load_segment_gcc
00:41:52     for line in subprocess.check_output(cmd, shell=True, universal_newlines=True).split("\n"):
00:41:52   File "c:\python27\Lib\subprocess.py", line 574, in check_output
00:41:52     raise CalledProcessError(retcode, cmd, output=output)
00:41:52 CalledProcessError: Command 'C:/Program Files (x86)/GNU Tools ARM Embedded/gcc-arm-none-eabi-6-2017-q1-update-win32/bin/arm-none-eabi-readelf -l C:\mj\workspace\bm_wrap\1516\mbed-os\BUILD\tests\REALTEK_RTL8195AM\GCC_ARM\.\TESTS\mbedmicro-rtos-mbed\mutex\mutex.elf' returned non-zero exit status 1

This looks related to your PR.

@tung7970

This comment has been minimized.

Contributor

tung7970 commented Oct 3, 2017

Not a big fan of windows, but this is weird. A simple readelf command occasionally fails. Let me check if I can reproduce it locally.

@0xc0170 0xc0170 added needs: work and removed needs: CI labels Oct 4, 2017

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Oct 9, 2017

@tung7970 Any update?

@tung7970 tung7970 force-pushed the tung7970:fix-tools branch Oct 9, 2017

@tung7970

This comment has been minimized.

Contributor

tung7970 commented Oct 9, 2017

@0xc0170 Can't reproduce this issue using local greentea, but I guess the space in windows path might have something to do with this.

Anyway, I have slightly tweaked the way command is concatenated. Please check if the modified version does any difference.

Also rebased to the latest master.

@mbed-ci

This comment has been minimized.

mbed-ci commented Oct 9, 2017

rtl8195am - use TOOLCHAIN_PATHS to locate toolchain
Use TOOLCHAIN_PATHS to locate toolchain binaries for users might set
compiler paths, through mbed_settings.py, env vars, or system path.

Signed-off-by: Tony Wu <tonywu@realtek.com>

@tung7970 tung7970 force-pushed the tung7970:fix-tools branch to 7d6e66d Oct 9, 2017

@mbed-ci

This comment has been minimized.

mbed-ci commented Oct 9, 2017

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Oct 10, 2017

/morph build

1 similar comment
@0xc0170

This comment has been minimized.

Member

0xc0170 commented Oct 19, 2017

/morph build

@0xc0170 0xc0170 added needs: CI and removed needs: review labels Oct 19, 2017

@mbed-ci

This comment has been minimized.

mbed-ci commented Oct 19, 2017

Build : SUCCESS

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

Skipping test trigger, missing label 'NEED CI'

@studavekar

This comment has been minimized.

Collaborator

studavekar commented Oct 19, 2017

/morph test

@mbed-ci

This comment has been minimized.

@adbridge adbridge merged commit e9e0595 into ARMmbed:master Oct 20, 2017

7 checks passed

AWS-CI uVisor Build & Test Success
Details
AWS-CI uvisor Test DIDNT RUN UVISOR TESTS
Details
Cam-CI uvisor Build & Test DIDNT RUN UVISOR TESTS
Details
ci-morph-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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment