Skip to content

cmake/jsoncpp.cmake: update to jsoncpp v1.8.4 #3467

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 3 commits into from
Apr 23, 2018
Merged

cmake/jsoncpp.cmake: update to jsoncpp v1.8.4 #3467

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0b49fd3
CMake: Update jsoncpp to v1.8.4
aarlt Feb 15, 2018
a49dac8
Install & use cmake provided by scripts/install_cmake.sh for emscripten
aarlt Feb 27, 2018
fa2a28a
CMake: enable C++11 for jsoncpp
axic Apr 17, 2018
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
1 change: 1 addition & 0 deletions Changelog.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
### 0.4.24 (unreleased)

Features:
* Build System: Update internal dependency of jsoncpp to 1.8.4, which introduces more strictness and reduces memory usage.
* Optimizer: Remove unnecessary masking of the result of known short instructions (``ADDRESS``, ``CALLER``, ``ORIGIN`` and ``COINBASE``).
* Type Checker: Make literals (without explicit type casting) an error for tight packing as experimental 0.5.0 feature.

Expand Down
22 changes: 10 additions & 12 deletions cmake/jsoncpp.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,15 @@ else()
set(JSONCPP_CMAKE_COMMAND ${CMAKE_COMMAND})
endif()

# Disable implicit fallthrough warning in jsoncpp for gcc >= 7 until the upstream handles it properly
if (("${CMAKE_CXX_COMPILER_ID}" MATCHES "GNU") AND CMAKE_CXX_COMPILER_VERSION VERSION_GREATER 7.0)
set(JSONCCP_EXTRA_FLAGS -Wno-implicit-fallthrough)
else()
set(JSONCCP_EXTRA_FLAGS "")
endif()

include(GNUInstallDirs)
set(prefix "${CMAKE_BINARY_DIR}/deps")
set(JSONCPP_LIBRARY "${prefix}/lib/${CMAKE_STATIC_LIBRARY_PREFIX}jsoncpp${CMAKE_STATIC_LIBRARY_SUFFIX}")
set(JSONCPP_LIBRARY "${prefix}/${CMAKE_INSTALL_LIBDIR}/${CMAKE_STATIC_LIBRARY_PREFIX}jsoncpp${CMAKE_STATIC_LIBRARY_SUFFIX}")
Copy link
Contributor

@axic axic Apr 17, 2018

Choose a reason for hiding this comment

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

@chfast isn't this pointing to some system wide directory?

Copy link
Contributor

@chfast chfast Apr 17, 2018

Choose a reason for hiding this comment

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

${CMAKE_INSTALL_LIBDIR} should be just "lib".

Copy link
Contributor

@axic axic Apr 17, 2018

Choose a reason for hiding this comment

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

I mean together with prefix?

Copy link
Collaborator Author

@aarlt aarlt Apr 18, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

@chfast if ${CMAKE_INSTALL_LIBDIR} would be just lib, we would have problems with lib64 again, or am i wrong?

set(JSONCPP_INCLUDE_DIR "${prefix}/include")

if(NOT MSVC)
set(JSONCPP_EXTRA_FLAGS "-std=c++11")
endif()

set(byproducts "")
if(CMAKE_VERSION VERSION_GREATER 3.1)
set(byproducts BUILD_BYPRODUCTS "${JSONCPP_LIBRARY}")
Expand All @@ -25,9 +23,9 @@ endif()
ExternalProject_Add(jsoncpp-project
PREFIX "${prefix}"
DOWNLOAD_DIR "${CMAKE_SOURCE_DIR}/deps/downloads"
DOWNLOAD_NAME jsoncpp-1.7.7.tar.gz
URL https://github.com/open-source-parsers/jsoncpp/archive/1.7.7.tar.gz
URL_HASH SHA256=087640ebcf7fbcfe8e2717a0b9528fff89c52fcf69fa2a18cc2b538008098f97
DOWNLOAD_NAME jsoncpp-1.8.4.tar.gz
URL https://github.com/open-source-parsers/jsoncpp/archive/1.8.4.tar.gz
URL_HASH SHA256=c49deac9e0933bcb7044f08516861a2d560988540b23de2ac1ad443b219afdb6
Copy link
Contributor

@chfast chfast Apr 17, 2018

Choose a reason for hiding this comment

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

Can you try adding another CMAKE_ARGS: -DCMAKE_CXX_STANDARD=11?

Copy link
Contributor

@axic axic Apr 17, 2018

Choose a reason for hiding this comment

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

Added.

CMAKE_COMMAND ${JSONCPP_CMAKE_COMMAND}
CMAKE_ARGS -DCMAKE_INSTALL_PREFIX=<INSTALL_DIR>
-DCMAKE_C_COMPILER=${CMAKE_C_COMPILER}
Expand All @@ -36,7 +34,7 @@ ExternalProject_Add(jsoncpp-project
-DCMAKE_POSITION_INDEPENDENT_CODE=${BUILD_SHARED_LIBS}
-DJSONCPP_WITH_TESTS=OFF
-DJSONCPP_WITH_PKGCONFIG_SUPPORT=OFF
-DCMAKE_CXX_FLAGS=${JSONCCP_EXTRA_FLAGS}
-DCMAKE_CXX_FLAGS=${JSONCPP_EXTRA_FLAGS}
# Overwrite build and install commands to force Release build on MSVC.
BUILD_COMMAND cmake --build <BINARY_DIR> --config Release
INSTALL_COMMAND cmake --build <BINARY_DIR> --config Release --target install
Expand Down
4 changes: 2 additions & 2 deletions libdevcore/JSON.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@
using namespace std;

static_assert(
(JSONCPP_VERSION_MAJOR == 1) && (JSONCPP_VERSION_MINOR == 7) && (JSONCPP_VERSION_PATCH == 7),
"Unexpected jsoncpp version: " JSONCPP_VERSION_STRING ". Expecting 1.7.7."
(JSONCPP_VERSION_MAJOR == 1) && (JSONCPP_VERSION_MINOR == 8) && (JSONCPP_VERSION_PATCH == 4),
"Unexpected jsoncpp version: " JSONCPP_VERSION_STRING ". Expecting 1.8.4."
);

namespace dev
Expand Down
2 changes: 1 addition & 1 deletion scripts/create_source_tarball.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ REPO_ROOT="$(dirname "$0")"/..
fi
# Add dependencies
mkdir -p "$SOLDIR/deps/downloads/" 2>/dev/null || true
wget -O "$SOLDIR/deps/downloads/jsoncpp-1.7.7.tar.gz" https://github.com/open-source-parsers/jsoncpp/archive/1.7.7.tar.gz
wget -O "$SOLDIR/deps/downloads/jsoncpp-1.8.4.tar.gz" https://github.com/open-source-parsers/jsoncpp/archive/1.8.4.tar.gz
Copy link
Contributor

@axic axic Feb 20, 2018

Choose a reason for hiding this comment

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

This is slightly unrelated to this PR, but I think this script should also check for the downloaded hash.

Copy link
Contributor

@axic axic Feb 20, 2018

Choose a reason for hiding this comment

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

@chfast is it possible to run cmake to download deps only? :)

Copy link
Contributor

@chfast chfast Feb 20, 2018

Choose a reason for hiding this comment

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

It should be possible by make jsoncpp-project-download. If not, you have to "export" this ExternalProject step.

CMake will check the hash anyway, so no need to do it twice.

mkdir -p "$REPO_ROOT/upload"
tar czf "$REPO_ROOT/upload/solidity_$versionstring.tar.gz" -C "$TEMPDIR" "solidity_$versionstring"
rm -r "$TEMPDIR"
Expand Down
2 changes: 2 additions & 0 deletions scripts/install_cmake.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ esac

BIN=$PREFIX/bin

PATH=$PREFIX/bin:$PATH
Copy link
Contributor

@chriseth chriseth Apr 23, 2018

Choose a reason for hiding this comment

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

Does this have any effect after the script has run?


if test -f $BIN/cmake && ($BIN/cmake --version | grep -q "$VERSION"); then
echo "CMake $VERSION already installed in $BIN"
else
Expand Down
2 changes: 1 addition & 1 deletion scripts/release_ppa.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ mv solidity solc

# Fetch jsoncpp dependency
mkdir -p ./solc/deps/downloads/ 2>/dev/null || true
wget -O ./solc/deps/downloads/jsoncpp-1.7.7.tar.gz https://github.com/open-source-parsers/jsoncpp/archive/1.7.7.tar.gz
wget -O ./solc/deps/downloads/jsoncpp-1.8.4.tar.gz https://github.com/open-source-parsers/jsoncpp/archive/1.8.4.tar.gz
Copy link
Contributor

@axic axic Feb 20, 2018

Choose a reason for hiding this comment

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

@chriseth why does the ppa script needs to download this separately? I thought cmake will do that on its own?

Copy link
Contributor

@chfast chfast Feb 20, 2018

Choose a reason for hiding this comment

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

Because you cannot download anything in PPA builds. So you have to prepare the sources yourself. If you put the archive in the place where CMake would download it, it will only check the hash and unzip it from there.

Copy link
Contributor

@axic axic Feb 20, 2018

Choose a reason for hiding this comment

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

Not even with cmake? :)

Copy link
Contributor

@chfast chfast Feb 27, 2018

Choose a reason for hiding this comment

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

What I heard is the internet access is restricted only to official packages repo.

Copy link
Contributor

@chriseth chriseth Apr 23, 2018

Choose a reason for hiding this comment

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

You have no internet access at all. You can declare dependencies on other packages, that's it, and I think this is a good thing: Building a package should be a deterministic, offline process.


# Determine version
cd solc
Expand Down
10 changes: 10 additions & 0 deletions scripts/travis-emscripten/build_emscripten.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,12 @@ if ! type git &>/dev/null; then
apt-get -y install git-core
fi

if ! type wget &>/dev/null; then
# We need wget to install cmake
apt-get update
apt-get -y install wget
fi

WORKSPACE=/root/project

# Increase nodejs stack size
Expand Down Expand Up @@ -67,6 +73,10 @@ rm -rf b2 libs doc tools more bin.v2 status
)
echo -en 'travis_fold:end:compiling_boost\\r'

echo -en 'travis_fold:start:install_cmake.sh\\r'
source $WORKSPACE/scripts/install_cmake.sh
echo -en 'travis_fold:end:install_cmake.sh\\r'

# Build dependent components and solidity itself
echo -en 'travis_fold:start:compiling_solidity\\r'
cd $WORKSPACE
Expand Down