Skip to content

Update emscripten version to 1.38.8. #4441

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.

Sign up for GitHub

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
axic merged 1 commit into from
Jan 17, 2019
Merged

Update emscripten version to 1.38.8. #4441

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .circleci/config.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ version: 2
jobs:
build_emscripten:
docker:
- image: trzeci/emscripten:sdk-tag-1.37.21-64bit
- image: trzeci/emscripten:sdk-tag-1.38.8-64bit
environment:
TERM: xterm
steps:
Expand Down
2 changes: 1 addition & 1 deletion .travis.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ matrix:
before_install:
- nvm install 8
- nvm use 8
- docker pull trzeci/emscripten:sdk-tag-1.37.21-64bit
- docker pull trzeci/emscripten:sdk-tag-1.38.8-64bit
env:
- SOLC_EMSCRIPTEN=On
- SOLC_INSTALL_DEPS_TRAVIS=Off
Expand Down
1 change: 1 addition & 0 deletions Changelog.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ Bugfixes:


Build System:
* Emscripten: Upgrade to emscripten 1.38.8 on travis and circleci.


### 0.5.2 (2018-12-19)
Expand Down
5 changes: 5 additions & 0 deletions cmake/EthCompilerSettings.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,11 @@ if (("${CMAKE_CXX_COMPILER_ID}" MATCHES "GNU") OR ("${CMAKE_CXX_COMPILER_ID}" MA
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -s ERROR_ON_UNDEFINED_SYMBOLS=1")
# Disallow deprecated emscripten build options.
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -s STRICT=1")
# Export the Emscripten-generated auxiliary methods which are needed by solc-js.
# Which methods of libsolc itself are exported is specified in libsolc/CMakeLists.txt.
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -s EXTRA_EXPORTED_RUNTIME_METHODS=['cwrap','addFunction','removeFunction','Pointer_stringify','lengthBytesUTF8','_malloc','stringToUTF8','setValue']")
Copy link
Contributor

@axic axic Jan 17, 2019

Choose a reason for hiding this comment

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

I'm not sure we should have them here or in the libsolc makefile.

The libsolc makefile has the functions exported by libsolc. These are the functions also needed by solc-js.

Copy link
Collaborator Author

@ekpyron ekpyron Jan 17, 2019

Choose a reason for hiding this comment

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

Actually, I considered moving them, but then kept them here in the end. To me it feels more like a setting for Emscripten to make it export these functions - these are not really defined in libsolc after all... but I'd be fine with moving them over as well, if you prefer that.

Copy link
Contributor

@axic axic Jan 17, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

I'm not sure which is cleaner:

  • having all in libsolc
  • having all here
  • having them split because they are for different purposes?
  • (An additional possibility is to actually use emscripten's annotations in the code to mark exported fnuctions to be kept.)

In any case probably we should keep a note in both places.

Copy link
Collaborator Author

@ekpyron ekpyron Jan 17, 2019

Choose a reason for hiding this comment

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

I added a notice on both sides. I think it makes sense to split it this way - if e.g. libyul should export some functions, then we should specify it there and not have to duplicate the interface functions exported here.

# Do not build as a WebAssembly target - we need an asm.js output.
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -s WASM=0")
endif()
endif()

Expand Down
3 changes: 3 additions & 0 deletions libsolc/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
if (EMSCRIPTEN)
# Specify which functions to export in soljson.js.
# Note that additional Emscripten-generated methods needed by solc-js are
# defined to be exported in cmake/EthCompilerSettings.cmake.
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -s EXPORTED_FUNCTIONS='[\"_solidity_license\",\"_solidity_version\",\"_solidity_compile\"]' -s RESERVED_FUNCTION_POINTERS=20")
add_executable(soljson libsolc.cpp libsolc.h)
target_link_libraries(soljson PRIVATE solidity)
Expand Down
2 changes: 1 addition & 1 deletion scripts/build_emscripten.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,5 +30,5 @@ set -e

if [[ "$OSTYPE" != "darwin"* ]]; then
./scripts/travis-emscripten/install_deps.sh
docker run -v $(pwd):/root/project -w /root/project trzeci/emscripten:sdk-tag-1.37.21-64bit ./scripts/travis-emscripten/build_emscripten.sh
docker run -v $(pwd):/root/project -w /root/project trzeci/emscripten:sdk-tag-1.38.8-64bit ./scripts/travis-emscripten/build_emscripten.sh
fi
8 changes: 4 additions & 4 deletions scripts/travis-emscripten/build_emscripten.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,11 +49,11 @@ fi
WORKSPACE=/root/project

# Increase nodejs stack size
if [ -e ~/.emscripten ]
if ! [ -e /emsdk_portable/node/bin/node_orig ]
then
sed -i -e 's/NODE_JS="nodejs"/NODE_JS=["nodejs", "--stack_size=8192"]/' ~/.emscripten
else
echo 'NODE_JS=["nodejs", "--stack_size=8192"]' > ~/.emscripten
mv /emsdk_portable/node/bin/node /emsdk_portable/node/bin/node_orig
echo -e '#!/bin/sh\nexec /emsdk_portable/node/bin/node_orig --stack-size=8192 $@' > /emsdk_portable/node/bin/node
Copy link
Contributor

@chriseth chriseth Jan 17, 2019

Choose a reason for hiding this comment

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

Do we really need bash? ;)

Copy link
Collaborator Author

@ekpyron ekpyron Jan 17, 2019

Choose a reason for hiding this comment

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

As a matter of fact I think we do for having well specified behavior with the arguments here :-).

chmod 755 /emsdk_portable/node/bin/node
fi

# Boost
Expand Down