Skip to content

Add Travis CI jobs for build testing several targets #2102

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

Merged
merged 1 commit into from
Nov 16, 2017

Conversation

akosthekiss
Copy link
Member

Hitherto, code under the targets directory was not tested and so
its maintenance was sometimes speculative. This commit adds build
testing for several targets to prevent them from bit rotting.
Targets covered by this commit are: ESP8266, Mbed, Mbed OS 5,
NuttX, RIOT, Tizen RT, and Zephyr.

Some issues were revealed and fixed:

  • ESP8266: added missing include for uint32_t typedef.
  • Tizen RT: replaced missing str_to_uint with strtol.

JerryScript-DCO-1.0-Signed-off-by: Akos Kiss akiss@inf.u-szeged.hu

@akosthekiss akosthekiss added the infrastructure Related to GH Actions or the tested targets label Nov 16, 2017
Hitherto, code under the `targets` directory was not tested and so
its maintenance was sometimes speculative. This commit adds build
testing for several targets to prevent them from bit rotting.
Targets covered by this commit are: ESP8266, Mbed, Mbed OS 5,
NuttX, RIOT, Tizen RT, and Zephyr.

Some issues were revealed and fixed:
- ESP8266: added missing include for `uint32_t` typedef.
- Tizen RT: replaced missing `str_to_uint` with `strtol`.

JerryScript-DCO-1.0-Signed-off-by: Akos Kiss akiss@inf.u-szeged.hu
## Targets for building ESP8266 with JerryScript.

# Build the firmware (ESP8266 with JerryScript).
script:
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd add install target as a dependency of this target to prevent the parallel "build" of script and install

Copy link
Contributor

Choose a reason for hiding this comment

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

If you agree in my opinion, then please do it in the other new makefiles too.

Copy link
Member Author

Choose a reason for hiding this comment

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

That never happens (I mean the parallel execution of script and install). If all is called as the default target of make, then it explicitly lists the order of execution by invoking sub-makes in a specific order (install, script). While .travis.yml calls make twice, first with install target in the install stage and then with script target in script stage. (The make targets intentionally reflect the names of the Travis CI job stage names to highlight which target is called in which stage.) If install was a dependency of script, install would run again in script stage.

See:

    - env: JOBNAME="ESP8266 Build Test"
      cache: ccache
      install: make -f ./targets/esp8266/Makefile.travis install
      script: make -f ./targets/esp8266/Makefile.travis script

Copy link
Contributor

Choose a reason for hiding this comment

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

I see now, why you did it in this way. It is good, leave as is.

Copy link
Contributor

@LaszloLango LaszloLango left a comment

Choose a reason for hiding this comment

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

I like this PR, LGTM

@akosthekiss
Copy link
Member Author

Issues found during job scripting:

  • ESP8266:
    • The apt dependencies list was overly extensive (at least compared to what Travis CI had already installed on the worker image), sometimes even erroneous (e.g., didn't have to install any libc6-dev* packages and actually couldn't install libtool-bin because such a package was not available).
    • The creation of the /opt/Espressif directory is described together with the dependencies step although it belongs to the crosstool-NG building step. Moreover, it should be noted that neither the building of ct-ng nor the execution of ct-ng to build a cross toolchain requires elevated privileges (it can be done somewhere in the user's home directory, not under /opt only).
    • Building crosstool-NG didn't work for me exactly as described, make detected recursion and bailed out.
    • When building the firmware with JerryScript, why must BIN_PATH be specified? Couldn't it have at least a default to be the same as BUILD_DIR?
  • Mbed OS 5:
    • The target needs heavy maintenance. As is, building an example app or a library from sources present under targets/mbedos5 with the Makefile is not possible. make all errors because there is no main in the code base, make library errors because launcher sources require jerry-targetjs.h that are explicitly not generated for libraries. Then the readme suggests to do a gulp-based build, but that build system uses a pinned JerryScript version (https://github.com/ARMmbed/mbed-js-gulp/blob/master/main.js) not master. So that seems never building with latest jerry. The target should always be able to test latest developments.
  • NuttX:
    • The readme doesn't mention which version of NuttX was last seen working. (7.22 worked for me, see makefile.)
    • The readme doesn't describe how configuration can be automated (without menuconfig that requires user interaction). Moreover, the readme suggests fetching the kconfig sources from a "3rd party" while the nuttx/tools repo contains its own kconfig sources.
  • Particle: I couldn't get it build at all.
  • RIOT:
    • The readme doesn't pin the last known working version of RIOT. (2017.10 worked for me.)
  • Tizen RT:
    • The readme doesn't pin the last known working version of Tizen RT. (1.1_Public_Release worked for me but only if I adapted the romfs patch.)
    • PPA-based GCC6 version worked, too (readme mentions GCC4.9, but that just seems to be a lower bound, not a must).
  • Zephyr:
    • The readme doesn't pin the last known working version of Zephyr, which is quite an issue. Zephyr has changed build system on master to something cmake-based, which is completely incompatible with the code in targets/zephyr right now. For me, Zephyr 1.9.1 and Zephyr SDK 0.9.1 worked. (But even in this release, Zephyr doesn't seem to have the readme-suggested qemu make target anymore.)
    • Non-interactive installation required quite some recursive digging into self-extracting archives within self-extracting archives, because documentation suggests interactive installation of the SDK by "ansering all questions" (out of question on Travis).
    • Directory structure suggested by readme has zephyr-project, although when I clone it out from github, it will be named zephyr by default.
    • Calling make with ./targets/zephyr/Makefile.zephyr fails to build the JerryScript library with quite weird errors... unless you set CC to point deep within the Zephyr SDK: zephyr-sdk-0.9.1/sysroots/x86_64-pokysdk-linux/usr/bin/i586-zephyr-elfiamcu/i586-zephyr-elfiamcu-gcc. Which is not documented anywhere and was a hell to debug. Note: zephyr sets its the build environment for its own build system up by sourcing zephyr-env.sh, but JerryScript's build system will know nothing about that. And Makefile.zepyhr will use $CC which is usually still set to the native compiler.

This PR does NOT want to deal with the above issue but they are left for target maintainers for consideration.

@LaszloLango
Copy link
Contributor

This PR does NOT want to deal with the above issues but they are left for target maintainers for consideration.

@akosthekiss Please open an issue for them.

Copy link
Member

@dbatyai dbatyai left a comment

Choose a reason for hiding this comment

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

LGTM

@dbatyai dbatyai merged commit da24727 into jerryscript-project:master Nov 16, 2017
@akosthekiss akosthekiss deleted the travis-targets branch November 16, 2017 12:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
infrastructure Related to GH Actions or the tested targets
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants