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

Use pyelftools for Realtek post-build script #6621

Merged
merged 1 commit into from Apr 17, 2018

Conversation

Projects
None yet
7 participants
@theotherjimmy
Contributor

theotherjimmy commented Apr 12, 2018

Description

The online compiler can be very picky about what is and is not allowed in a post-build scirpt. For a while now, the Realtek post-build script has been preventing anyone from successfully compiling for the RTL8195AM in the online compiler. This refactor changes the code that was failing to be much simpler, and it works in the website. I have tested this on a staging site.

For reference, the traceback:

  File "/usr/src/mbed-sdk/tools/toolchains/__init__.py", line 1152, in link_program
    self.binary(r, elf, bin)
  File "/usr/src/mbed-sdk/tools/hooks.py", line 54, in wrapper
    post_res = tooldesc["post"](t_self, *args, **kwargs)
  File "/usr/src/mbed-sdk/tools/targets/__init__.py", line 538, in binary_hook
    rtl8195a_elf2bin(t_self, elf, binf)
  File "/usr/src/mbed-sdk/tools/targets/REALTEK_RTL8195AM.py", line 283, in rtl8195a_elf2bin
    segment = parse_load_segment(t_self.name, image_elf)
  File "/usr/src/mbed-sdk/tools/targets/REALTEK_RTL8195AM.py", line 212, in parse_load_segment
    return parse_load_segment_armcc(image_elf)
  File "/usr/src/mbed-sdk/tools/targets/REALTEK_RTL8195AM.py", line 143, in parse_load_segment_armcc
    for line in subprocess.check_output(cmd, shell=True, universal_newlines=True).split("\n"):
  File "/usr/lib/python2.7/subprocess.py", line 574, in check_output
    raise CalledProcessError(retcode, cmd, output=output)
CalledProcessError: Command '"/opt/armcc5_06_u3/bin/fromelf" --text -v --only=none /tmp/chroots/ch-e3a8e63e-cb37-4195-ae50-1fe69442b3e4/build/mbed-os-example-fat-filesystem.REALTEK_RTL8195AM.elf' returned non-zero exit status 127

Error 127 is "command not found" https://stackoverflow.com/questions/1763156/127-return-code-from#1763178. I fixed this by refactoring so that we don't use any commands.

Pull request type

[X] Fix
[ ] Refactor
[ ] New target
[ ] Feature
[ ] Breaking change

@theotherjimmy

This comment has been minimized.

Contributor

theotherjimmy commented Apr 12, 2018

@samchuarm

This comment has been minimized.

samchuarm commented Apr 12, 2018

Hi @ARMmbed/team-realtek , please review the changes and provide your feedback if any.

@tung7970

Thanks for the effort. It looks much nicer.

One small thing, shouldn't the 'attr' variable be better named 'addr' in function _parse_load_segment_inner ?

@0xc0170

+1 simplification, elftools 💯

@theotherjimmy

This comment has been minimized.

Contributor

theotherjimmy commented Apr 16, 2018

@tung7970 Yes, that's a typo. I'll fix that now.

@theotherjimmy theotherjimmy force-pushed the theotherjimmy:elftools-realtek branch from 8a176d7 to a0b305b Apr 16, 2018

@theotherjimmy

This comment has been minimized.

Contributor

theotherjimmy commented Apr 16, 2018

@0xc0170 @tung7970 I revised the commit to correct the typo. Could you both take another look?

@tung7970

LGTM

@cmonr

This comment has been minimized.

Contributor

cmonr commented Apr 17, 2018

Interesting. Why does the code now no longer need to check compilers?

/morph build

@cmonr cmonr added needs: CI and removed needs: review labels Apr 17, 2018

@tung7970

This comment has been minimized.

Contributor

tung7970 commented Apr 17, 2018

The new script checks the elf file directly rather than output from each compiler’s elf dumper.

@mbed-ci

This comment has been minimized.

mbed-ci commented Apr 17, 2018

Build : SUCCESS

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

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 cmonr merged commit 43bf010 into ARMmbed:master Apr 17, 2018

12 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/docs Local docs testing has passed
Details
travis-ci/events Passed, runtime is 10226 cycles (+1376 cycles)
Details
travis-ci/gitattributestest Local gitattributestest testing has passed
Details
travis-ci/littlefs Passed, code size is 10092B (+0.00%)
Details
travis-ci/tools Local tools testing has passed
Details

@cmonr cmonr removed the ready for merge label Apr 17, 2018

@adbridge

This comment has been minimized.

Contributor

adbridge commented Apr 25, 2018

@theotherjimmy Is this fixing #5976 ??

@theotherjimmy

This comment has been minimized.

Contributor

theotherjimmy commented Apr 25, 2018

Yes, but I want to verify that once this goes live on the website, so I omitted a "Resolves" directive.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment