pkg/jerryscript: fix jerryscript not using system includes by cladmi · Pull Request #9821 · RIOT-OS/RIOT

cladmi · 2018-08-22T14:56:01Z

Contribution description

Jerryscript was never using INCLUDES so never having the 'newlib-nano' or llvm
includes.

On my computer it cannot even find stdio.h.

~~It may even fix the HAVE_TIME_H issue from #9808 should investigate more on how to test this one.~~ still required for ek-lm4f120xl

Testing procedure

With gcc, we should get a 'nano' output that we don't have in master.

cd examples/javascript
make clean ${PWD}/bin/iotlab-m3/jerryscript.a  BOARD=iotlab-m3 VERBOSE=1 2>&1  | grep 'nano'

...  -isystem /usr/arm-none-eabi/include/newlib-nano ...

Issues/PRs references

#9820

cladmi · 2018-08-23T10:12:35Z

It also now compiles with TOOLCHAIN=llvm:

cd examples/javascript
rm -rf bin
make BOARD=samr21-xpro TOOLCHAIN=llvm

It fails on master and we have:

arm-none-eabi-nm bin/samr21-xpro/jerryscript.a | grep setjmp
         U _setjmp

With this PR it compiles and we have:

arm-none-eabi-nm bin/samr21-xpro/jerryscript.a | grep setjmp
         U setjmp

cladmi · 2018-08-23T11:43:46Z

@leandrolanzieri maybe you could take a look too

miri64 · 2018-09-09T14:49:32Z

I think this causes problems with our newly ported esp boards now :-/

cladmi · 2018-09-10T09:51:21Z

/opt/esp/newlib-xtensa/xtensa-lx106-elf/include/sys/features.h:267:35: error: "_POSIX_C_SOURCE" is not defined [-Werror=undef]
 #elif defined(_ISOC99_SOURCE) || (_POSIX_C_SOURCE - 0) >= 200112L || \

I will investigate.

cladmi · 2018-09-11T13:52:54Z

Maybe @gschorcht also has an idea

gschorcht · 2018-09-11T15:43:18Z

How can I reproduce the problem? I used the current master and changed EXTERNAL_COMPILE_FLAGS to

	 -DEXTERNAL_COMPILE_FLAGS="$(INCLUDES) $(EXT_CFLAGS)" \

in pkg/javascript/Makfile.jerryscript which seems to be the only change of this PR. Compilation of examples/javascript works without any problems on my host. I will try it with riotdocker.

gschorcht · 2018-09-11T16:06:44Z

Ok, in riotdocker I can reproduce the problem. The only difference I see between riotdocker and my local host is the cmake version.

aabadie · 2018-12-19T11:13:58Z

@cladmi, what is the status of this PR ? It would at least require a rebase

cladmi · 2018-12-19T12:15:50Z

I rebased.

I retested and I can compile with llvm without errors on my machine and with docker.
I get Hello from JerryScript for the examples application for iotlab-m3, samr21-xpro and native. (native not build in docker).

cladmi · 2018-12-19T12:20:23Z

It correctly compiles in docker with esp8266-olimex-mod and not without the patch for the cpu. I did not re-check line by line.

aabadie

Checked the code and tested locally this PR on Arduino Zero. All builds are fine and examples/javascript works as expected. I trust your tests with the esp8266.

All build were fine and this PR fixes the incompatibility with LLVM, which is super nice.

ACK!

aabadie · 2018-12-19T14:26:32Z

And please squash!

cladmi · 2018-12-19T14:45:38Z

I rebased and updated the commit messages.
I mentioned that I only tested the compilation on esp8266 and not the execution.

gschorcht · 2018-12-19T15:18:13Z

@cladmi I can test.

gschorcht · 2018-12-19T15:27:10Z

@cladmi

I get Hello from JerryScript for the examples application for esp8266-esp-12x but it is not compilable for esp32-wroom-32.

aabadie · 2018-12-19T15:27:59Z

Indeed, Murdock is reporting issues with ESP32.

cladmi · 2018-12-19T15:28:23Z

@gschorcht thank for testing.

I will look into the esp32 issues.

gschorcht · 2018-12-19T15:30:06Z

@cladmi In current master it works for ESP32 boards.

cladmi · 2018-12-19T15:32:33Z

I rebased now that the other jerryscript PR was merged. (should not have any impact).

@gschorcht it may be the same kind of issues as with the esp8266.

cladmi · 2018-12-19T15:44:12Z

I added -Wundef for esp32 to make it compile. I have another commit that explains that it is only _GNU_VISIBLE and __BSD_VISIBLE that are not defined but I will not push it until rebasing to no re-trigger CI.

@gschorcht I noticed I still have comments saying issue with newlib version XXXX and for esp32 they are both in stdio.h so do not know what version I should point to. I am building with our docker image.
I could remove this part about the version if there is no specific version to put there, the date could also be enough.

gschorcht · 2018-12-19T15:45:16Z

@cladmi Setting EXT_CFLAGS += -Wno-undef -Wno-error=undef solves the compilation problem and the example works as expected.

gschorcht · 2018-12-19T16:05:15Z

@cladmi
newlib version for ESP8266 is 3.0.0.
newlib version for ESP32 is 2.2.0.

cladmi · 2018-12-19T16:09:26Z

Thanks I will update it.
I will squash directly as I need to rewrite the first commit but not change PR reference so should be easy to compare the two force-push.

Jerryscript was never using INCLUDES so never having the 'newlib-nano' or llvm includes. It prevented working with `llvm`. Disable unsupported warnings for ESP32 and ESP8266 newlib that show when using system includes. * tested newlib version for ESP32 is 2.2.0 * tested newlib version for ESP8266 is 3.0.0

Re-enable LLVM/clang now that system includes are used. It compiles and correctly executes examples/javascript.

cladmi · 2018-12-19T16:38:36Z

And murdock is also happy with the rebase.

aabadie · 2018-12-19T16:42:37Z

@gschorcht, can re-test on ESP32 and merge after ? My ACK still holds, even with the last changes.

gschorcht · 2018-12-19T16:59:09Z

@aabadie Works on ESP32 boards. Not sure about the right merge option "Create a merge commit" or "Squash and merge"?

aabadie · 2018-12-19T18:54:21Z

I just did "Create a merge commit". This is what we usually do in the upstream repo.

gschorcht · 2018-12-19T21:44:31Z

@aabadie Ok, now it's clear. Thanks.

cladmi · 2018-12-20T12:35:45Z

Thank you for the review and the tests :)

This was referenced Aug 22, 2018

pkg/jerryscript: many cleanups (split PRs) #9820

Closed

murdock: add selected cortex-m boards to LLVM build #9809

Merged

cladmi mentioned this pull request Aug 23, 2018

pkg: blacklist selected pkgs for LLVM/clang #9734

Merged

cladmi requested a review from emmanuelsearch August 23, 2018 11:42

cladmi added this to the Release 2018.10 milestone Aug 23, 2018

cladmi requested a review from kaspar030 August 24, 2018 11:51

miri64 added a commit to miri64/RIOT that referenced this pull request Sep 9, 2018

Merge 'pr/make/jerryscript/includes' (RIOT-OS#9821)

2a591c5

miri64 added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Sep 9, 2018

miri64 added a commit to miri64/RIOT that referenced this pull request Sep 11, 2018

Merge 'pr/make/jerryscript/includes' (RIOT-OS#9821)

40314ba

miri64 added a commit to miri64/RIOT that referenced this pull request Sep 11, 2018

Merge 'pr/make/jerryscript/includes' (RIOT-OS#9821)

7ea18ce

cladmi force-pushed the pr/make/jerryscript/includes branch from b2af4fb to 611b193 Compare December 19, 2018 12:01

aabadie approved these changes Dec 19, 2018

View reviewed changes

cladmi force-pushed the pr/make/jerryscript/includes branch from 611b193 to c1cc830 Compare December 19, 2018 14:44

cladmi force-pushed the pr/make/jerryscript/includes branch from c1cc830 to c1db04d Compare December 19, 2018 15:31

cladmi added 2 commits December 19, 2018 17:12

jerryscript: re-enable LLVM/clang

04a91e9

Re-enable LLVM/clang now that system includes are used. It compiles and correctly executes examples/javascript.

cladmi force-pushed the pr/make/jerryscript/includes branch from 2a2b9c0 to 04a91e9 Compare December 19, 2018 16:13

aabadie merged commit d2478af into RIOT-OS:master Dec 19, 2018

cladmi deleted the pr/make/jerryscript/includes branch December 20, 2018 12:36

Conversation

cladmi commented Aug 22, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Contribution description

Testing procedure

Issues/PRs references

Uh oh!

cladmi commented Aug 23, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cladmi commented Aug 23, 2018

Uh oh!

miri64 commented Sep 9, 2018

Uh oh!

cladmi commented Sep 10, 2018

Uh oh!

cladmi commented Sep 11, 2018

Uh oh!

gschorcht commented Sep 11, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gschorcht commented Sep 11, 2018

Uh oh!

aabadie commented Dec 19, 2018

Uh oh!

cladmi commented Dec 19, 2018

Uh oh!

cladmi commented Dec 19, 2018

Uh oh!

aabadie left a comment

Choose a reason for hiding this comment

Uh oh!

aabadie commented Dec 19, 2018

Uh oh!

cladmi commented Dec 19, 2018

Uh oh!

gschorcht commented Dec 19, 2018

Uh oh!

gschorcht commented Dec 19, 2018

Uh oh!

aabadie commented Dec 19, 2018

Uh oh!

cladmi commented Dec 19, 2018

Uh oh!

gschorcht commented Dec 19, 2018

Uh oh!

cladmi commented Dec 19, 2018

Uh oh!

cladmi commented Dec 19, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gschorcht commented Dec 19, 2018

Uh oh!

gschorcht commented Dec 19, 2018

Uh oh!

cladmi commented Dec 19, 2018

Uh oh!

cladmi commented Dec 19, 2018

Uh oh!

aabadie commented Dec 19, 2018

Uh oh!

gschorcht commented Dec 19, 2018

Uh oh!

aabadie commented Dec 19, 2018

Uh oh!

gschorcht commented Dec 19, 2018

Uh oh!

cladmi commented Dec 20, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

cladmi commented Aug 22, 2018 •

edited

Loading

cladmi commented Aug 23, 2018 •

edited

Loading

gschorcht commented Sep 11, 2018 •

edited

Loading

cladmi commented Dec 19, 2018 •

edited

Loading