Skip to content

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

Closed
cladmi wants to merge 16 commits intoRIOT-OS:masterfrom
cladmi:pr/big/jerryscript_cleanup
Closed

pkg/jerryscript: many cleanups (split PRs)#9820
cladmi wants to merge 16 commits intoRIOT-OS:masterfrom
cladmi:pr/big/jerryscript_cleanup

Conversation

@cladmi
Copy link
Copy Markdown
Contributor

@cladmi cladmi commented Aug 22, 2018

Contribution description

When debugging jerryscript issues with llvm I noticed it was not finding stdio.h on my arch linux computer:

/home/cladmi/git/work/RIOT/examples/javascript/bin/pkg/iotlab-m3/jerryscript/jerry-core/jrt/jrt.h:25:10: fatal error: 'stdio.h' file not found
#include <stdio.h>

I found out that jerryscript was not using INCLUDES and so not getting newlib-nano and llvm includes configuration.

This PR also includes many other fixes and I can split it in separate PRs

It may even fix the HAVE_TIME_H issue from #9808 should investigate more on how to test this one. still does not work without it 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 ...

This fails on master with stdio.h not found on my computer.

rm -rf bin/; make -C examples/javascript all BOARD=iotlab-m3  TOOLCHAIN=llvm

Issues/PRs references

#9809
#9808

cladmi added 6 commits August 22, 2018 16:25
Remove unknown rules from .PHONY.
Make 'all' be the default target for jerryscript. This should trigger
the normal 'DIRS' handling of Makefile.base.
Jerryscript was never using INCLUDES so never having the 'newlib-nano' or llvm
includes.
@cladmi cladmi added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) Area: build system Area: Build system Area: pkg Area: External package ports State: waiting for other PR State: The PR requires another PR to be merged first CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Aug 22, 2018
@cladmi
Copy link
Copy Markdown
Contributor Author

cladmi commented Aug 22, 2018

Clean fails if the directory does not exist, will fix.

@cladmi
Copy link
Copy Markdown
Contributor Author

cladmi commented Aug 22, 2018

Found out I can replace the 'clean' things by just building in $(BINDIR) instead of in the package source directory. I will do this instead.

cladmi added 4 commits August 22, 2018 17:20
Build into the BINDIR directory instead of the source repository.
This makes 'clean' work as expected without other intervention.

Also it goes in the direction of having the package source repository
board independant.
@cladmi cladmi 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 Aug 23, 2018
@cladmi cladmi 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 Aug 23, 2018
@cladmi cladmi added this to the Release 2018.10 milestone Aug 23, 2018
@cladmi cladmi 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 Aug 24, 2018
cladmi added 3 commits August 24, 2018 17:14
…le.base

Do not include Makefile.base as 'DIRS' are not used here.
Also if 'all' target was to be used, there would be two rules creating
`$(BINDIR)/jerryscript.a`.
It does not trigger the DIRS anymore.
@miri64
Copy link
Copy Markdown
Member

miri64 commented Sep 11, 2018

@cladmi can you clarify: is this PR already split? Can this PR be closed then?

@cladmi
Copy link
Copy Markdown
Contributor Author

cladmi commented Sep 11, 2018

Yes it is split, the goal was to have all PRs merged in the same place to verify that separate PRs still work together. But it is not needed anymore as issues have been resolved (except the esp support but that is something else).

@cladmi cladmi closed this Sep 11, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: build system Area: Build system Area: pkg Area: External package ports CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR State: waiting for other PR State: The PR requires another PR to be merged first Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants