cmake/jsoncpp.cmake: update to jsoncpp v1.8.4 by aarlt · Pull Request #3467 · argotorg/solidity

aarlt · 2018-02-07T01:18:42Z

To get version 1.8.4 to work, we need to get rid of the usage of deprecated API. PR #3532 (libdevcore/JSON.h - new API) will remove the deprecated API calls.

Remember, that scripts/travis-emscripten/build_emscripten.sh is currently using the systems cmake, that is shipped with trzeci/emscripten:sdk, it is not using the one that is installed with scripts/install_cmake.sh.

The new jsoncpp (at least version 1.8.4) needs a minimum cmake version of 3.1, that is currently not provided by trzeci/emscripten:sdk, at least not with version 1.37.33-64bit. So minor changes on the build scripts are needed.

chriseth · 2018-02-07T14:11:07Z

The jsoncpp version we currently use also does not support move semantics, so this would also be a big benefit!

chriseth · 2018-02-07T16:24:24Z

Please rebase and remove the merge commit, it is not possible to review it properly.

chriseth

Please remove code repetitions.

chriseth · 2018-02-07T16:24:57Z

libdevcore/JSON.h

-	Json::FastWriter writer;
-	writer.omitEndingLineFeed();
-	return writer.write(_input);
+	std::stringstream stream;


Please combine this code with the one above.

chriseth · 2018-02-07T16:25:30Z

libdevcore/JSON.h

-	return writer.write(_input);
+	std::stringstream stream;
+	Json::StreamWriterBuilder builder;
+	builder.settings_["indentation"] = "";


The compact layout should not even have newlines. Does it behave like that?

Yes, the behaviour is as expected. To be very sure, I will add a JSON Testsuite that will cover that - test/libdevcore/JSON.cpp.

chriseth · 2018-02-07T16:34:50Z

libsolc/libsolc.cpp

 	Json::Value input;
-	if (!reader.parse(_input, input, false))
+	std::stringstream inputStream(_input);
+	Json::CharReaderBuilder readerBuilder;


Can you write a helper function that does all this for us? It could even re-use a static variable of type Json::CharReaderBuilder.

aarlt · 2018-02-07T22:43:37Z

I wasn't able to run the ipc specific tests with eth. I used the most recent version of develop. After running

eth/eth --test -d /tmp/ipc
test/soltest -- --ipcpath /tmp/ipc/geth.ipc --no-smt

➜  cmake-build-debug git:(strict-mode-jsoncpp-1.8.4) ✗ test/soltest -- --ipcpath /tmp/ipc/geth.ipc --no-smt
Running 1733 test cases...
/Users/alex/git/aarlt/solidity/test/RPCSession.cpp:332: fatal error: in "SolidityAuctionRegistrar/creation": critical check dev::jsonParseStrict(reply, &result, &errs) has failed
/Users/alex/git/aarlt/solidity/test/RPCSession.cpp:332: fatal error: in "SolidityAuctionRegistrar/reserve": critical check dev::jsonParseStrict(reply, &result, &errs) has failed
/Users/alex/git/aarlt/solidity/test/RPCSession.cpp:332: fatal error: in "SolidityAuctionRegistrar/double_reserve_long": critical check dev::jsonParseStrict(reply, &result, &errs) has failed
...

^[[1;37mcpp-ethereum, a C++ Ethereum client^[[0m
cpp-ethereum 1.3.0
  By cpp-ethereum contributors, (c) 2013-2018.
  See the README for contributors and credits.
Networking disabled. To start, use netstart or pass --bootstrap or a remote host.
JSONRPC Admin Session Key: GZeXowHW3aQ=
^[[91m ⚡    ^[[35m23:34:13.065^[[0m^[[30m|^[[34m^[[0m  Stop worker^[[0m 590 ms
^[[94m  ℹ  ^[[35m23:34:13.066^[[0m^[[30m|^[[34m^[[0m  Killing blockchain & extras database (WithExisting::Kill).
^[[91m ⚡    ^[[35m23:34:14.078^[[0m^[[30m|^[[34meth^[[0m  Worker stopping^[[0m 1013 ms
^[[101m^[[1;30m  ✘  ^[[35m23:34:14.328^[[0m^[[30m|^[[34meth^[[0m  Sender: dde147e35eedb9384833d66a487f1f246fb3406b^[[0m  Invalid Nonce: Require ^[[34m0^[[0m  Got ^[[34m3^[[0m
^[[101m^[[1;30m  ✘  ^[[35m23:34:14.328^[[0m^[[30m|^[[34meth^[[0m  Sender: dde147e35eedb9384833d66a487f1f246fb3406b^[[0m  Invalid Nonce: Require ^[[34m0^[[0m  Got ^[[34m3^[[0m
^[[101m^[[1;30m  ✘  ^[[35m23:34:14.328^[[0m^[[30m|^[[34meth^[[0m  Sender: dde147e35eedb9384833d66a487f1f246fb3406b^[[0m  Invalid Nonce: Require ^[[34m0^[[0m  Got ^[[34m3^[[0m
...

A lot of ipc related tests seem not to work. Sometimes eth just crashes. Am I doing something wrong? Should I file an issue?

However, because of that I couldn't run the ipc related tests. I was also not able to test my changes in solfuzzer - I didn't find out how it is used.

Additionally to this, it looks like that jsoncpp strict-mode still allow comments, even if the specific setting allowComments is set to false.

void CharReaderBuilder::strictMode(Json::Value* settings)
{
//! [CharReaderBuilderStrictMode]
  (*settings)["allowComments"] = false;
  (*settings)["strictRoot"] = true;
  (*settings)["allowDroppedNullPlaceholders"] = false;
  (*settings)["allowNumericKeys"] = false;
  (*settings)["allowSingleQuotes"] = false;
  (*settings)["stackLimit"] = 1000;
  (*settings)["failIfExtra"] = true;
  (*settings)["rejectDupKeys"] = true;
  (*settings)["allowSpecialFloats"] = false;
//! [CharReaderBuilderStrictMode]
}

It looks like a jsoncpp bug to me.

chriseth · 2018-02-08T10:58:58Z

There are compilation errors on travis.

aarlt · 2018-02-08T19:58:35Z

Oops.. its always good to use different compilers :)

aarlt · 2018-02-08T22:01:57Z

On Node.js:6 Compiler: gcc C++

CMake Error at CMakeLists.txt:3 (CMAKE_MINIMUM_REQUIRED):
  CMake 3.1 or higher is required.  You are running version 3.0.2

In jsoncpp 1.8.4 they are using CMAKE_MINIMUM_REQUIRED(VERSION 3.1), but here cmake seem to be version 3.0.2, that is contradicting to the output of scripts/install_cmake.sh some lines above.

$ test "$TRAVIS_OS_NAME" != "linux" || (scripts/install_cmake.sh)
CMake 3.7.1 already installed in /home/travis/.local/bin

Is it possible that version 3.0.2 was used all the time and the new jsoncpp is now the first thing that really needs version 3.1 as minimum?

aarlt · 2018-02-08T22:10:00Z

The C++ travis build is not finding the make target for deps/lib/libjsoncpp.a. Somehow the lib seem to be installed in /solidity/deps/lib64/libjsoncpp.a - is lib vs. lib64 the problem here?

chriseth · 2018-02-09T11:10:26Z

Somehow it sometimes uses lib and somtimes lib64. I guess we need @chfast to take a look at this...

chriseth · 2018-02-09T11:12:38Z

Please also re-enable the fallthrough warning in cmake/jsoncpp.cmake

aarlt · 2018-02-09T16:50:57Z

jsoncpp is compiled with -Wall -Wconversion -Wshadow -Werror=conversion -Werror=sign-compare, where -Wimplicit-fallthrough is only enabled, if its explicitly set, or if -Wextra is set - for gnu compilers. This is not the case for jsoncpp.

We can manually do -Wimplicit-fallthrough, but this seem to be only ok for gnu compilers - I tested it with GCC 7.3.1. If enabled by default, clang will still warn about the implicit fallthroughs, they are still there in jsoncpp 1.8.4. I tested clang versions Apple LLVM version 9.0.0 (clang-900.0.39.2) and fedora's 5.0.1.

We shouldn't re-enable the implicit-fallthrough, because jsoncpp will still generate fallthrough warnings for clang. jsoncpp is not compiled with -Wextra, so we can get rid of the whole implicit-fallthrough-block in jsoncpp.cmake.

However, do you mean "re-enable" in the sense of explicitly enabling -Wimplicit-fallthrough?

I think that its safe to just remove the whole implicit-fallthrough-block in jsoncpp.cmake, it looks like that -Wimplicit-fallthrough and/or -Wextra is never set by the build system.

axic · 2018-02-09T16:54:05Z

libsolidity/interface/StandardCompiler.cpp


 	Json::Value const& sources = _input["sources"];
-	if (!sources)
+	if (sources.empty())


Is this a bug fix?

Nope, they just removed operator!() in version 1.8.4

bool Value::operator!() const { return isNull(); }

Lol, now I was able to fully understand why I had problems using that operator. On my system where was somehow an old jsoncpp header used - will check the build system, how that was possible - and because they don't provided an implementation of operator!() I had linking problems. I just realised that they remove the operator!(), thats why I used sources.empty() here. Now I see that they also providing

Value::operator bool() const { return ! isNull(); }

So everything is still fine. Wow, sorry for that! The original code still works very nice with the new version 1.8.4. I will remove that change.

... turned out it was a bug fix

bool Value::empty() const { if (isNull() || isArray() || isObject()) return size() == 0u; else return false; }

In contrast to Value::operator bool() , Value::empty() is also checking whether an empty array, or an empty object was provided. I just added two new test-cases in test/libsolidity/StandardCompiler.cpp

Can you create a new PR which adds the test and removes the use of the ! operator (without upgrading jsoncpp)?

Sure! See pull-request #3502.

chriseth · 2018-02-09T18:07:27Z

We recently (6 months?) added some code to cmake/jsoncpp.cmake about fallthrough warnings. I though the new version might be fixed in that regard, so we could try removing that code.

aarlt · 2018-02-11T23:44:50Z

I tried to fix the issue with the emscripten build. I saw that somehow a very old cmake was used, that prevented the build of jsoncpp, it required a minimum version 3.1. After fixing that, during the build a missing function _ZN4Json5Value6appendEOS0_ was shown, I did some debugging sessions. After finding out that built-in demangle support had some problems, I activated DEMANGLE_SUPPORT=1 which didn't solved the problem - I thought that the buggy demangling caused somehow the optimiser to remove the function. However, I rewrote the scripts/emscripten.sh build, so it can be also used in OSX and updated to trzeci/emscripten:sdk-tag-1.37.33-64bit. The problem with the built-in demangling support seem to be fixed here. However, I saw there

RangeError: Maximum call stack size exceeded
    at JSON.stringify (<anonymous>)
    at Object.<anonymous> (/emsdk_portable/sdk/tools/js-optimizer.js:8389:16)
    at Module._compile (module.js:635:30)
    at Object.Module._extensions..js (module.js:646:10)
    at Module.load (module.js:554:32)
    at tryModuleLoad (module.js:497:12)
    at Function.Module._load (module.js:489:3)
    at Function.Module.runMain (module.js:676:10)
    at startup (bootstrap_node.js:187:16)
    at bootstrap_node.js:608:3
fs.js:646
  return binding.open(pathModule._makeLong(path), stringToFlags(flags), mode);

I tried to increment the node_js stack even more to 16MB, but I had no success. I disabled the JavaScript optimizer (--js-opts 0), everything worked, but the resulting soljson.js had a size of 75MB. After setting EMCC_DEBUG=2 I was able to see the different optimizer steps:

DEBUG:root:emcc step "part of js opts" took 3.37 seconds
DEBUG:root:applying js optimization passes: asm aggressiveVariableElimination
chunkification: num funcs: 7053 actual num chunks: 9 chunk size range: 35726801 - 2026058
splitting up js optimization into 9 chunks, using 6 cores  (total: 58.66 MB)
.
.
.
.
.
/emsdk_portable/sdk/tools/eliminator/node_modules/uglify-js/lib/process.js:228
                                var ret = gen.apply(ast, ast.slice(1));
                                              ^

RangeError: Maximum call stack size exceeded

So somehow the aggressiveVariableElimination made problems, I disabled them so I was finally able to build a soljson.js (7.6MB).

However, I tried now my findings on travis, but it looks like that there are still other problems.

warning: unresolved symbol: _ZNKSt3__112basic_stringIcNS_11char_traitsIcEENS_9allocatorIcEEE7compareEPKc
...
warning: unresolved symbol: _ZTINSt3__115basic_streambufIcNS_11char_traitsIcEEEE
warning: unresolved symbol: _ZTVNSt3__113basic_istreamIcNS_11char_traitsIcEEEE

Now it looks like that functions from the standard library are missing. I have no idea why I don't see that locally. I ran the stuff also within a trzeci/emscripten:sdk-tag-1.37.33-64bit container.

Any ideas?

One question - because I want to push my version of scripts/build_emscripten.sh- does travis run everything in a container too? so if i create a named docker container inside travis, there will be no conflicts with other potential parallel builds using the same container-name?

axic · 2018-02-13T20:40:37Z

We recently (6 months?) added some code to cmake/jsoncpp.cmake about fallthrough warnings. I though the new version might be fixed in that regard, so we could try removing that code.

We added code to ignore fallthrough warnings, but later the fallthroughs were fixed and the ignore directive removed.

aarlt · 2018-02-13T20:54:15Z

@chriseth do you know why the build was successful for ci/circleci: build_emscripten?

aarlt · 2018-02-13T21:22:07Z

We recently (6 months?) added some code to cmake/jsoncpp.cmake about fallthrough warnings. I though the new version might be fixed in that regard, so we could try removing that code.

We added code to ignore fallthrough warnings, but later the fallthroughs were fixed and the ignore directive removed.

# 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()

I removed the whole block, because from the compiler flags point of view it should never check for falltroughs. With gnu compilers they are only enabled, if explicitly set, or with -Wextra - that is not set in JSONCCP_EXTRA_FLAGS. And for clang it looks like that it need to be set explicitly with -Wimplicit-fallthrough. If fallthroughs warnings are enabled, clang will still produce a lot of fallthrough warnings.

Probably the way how you built jsoncpp changed somehow - and you probably used CMAKE_CXX_FLAGS from EthCompilerSettings.cmake that sets -Wextra and -Wimplicit-fallthrough.

chriseth · 2018-02-13T21:42:12Z

circle uses trzeci/emscripten:sdk-tag-1.37.21-64bit - try to see if you can use that for travis, too.

aarlt · 2018-02-13T21:45:33Z

@chriseth interesting.. i tried 1.37.33-64 bit.. and there i had this optimizer bug with the aggressiveVariableElimination.. i will check that version now

axic · 2018-02-13T21:55:21Z

~~#3491 does that but it failed~~ not sure where I got .19 from, probably an old version of the circleci PR.

axic · 2018-04-17T11:02:59Z

cmake/jsoncpp.cmake

+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}")


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

${CMAKE_INSTALL_LIBDIR} should be just "lib".

I mean together with prefix?

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

axic · 2018-04-20T20:14:14Z

@aarlt this is now compiling :)

axic · 2018-04-20T20:15:14Z

Without the std=c++11 changes there were certain warnings:

-- Build files have been written to: /root/project/build/deps/src/jsoncpp-project-build
[  5%] Performing build step for 'jsoncpp-project'
make[3]: warning: jobserver unavailable: using -j1.  Add '+' to parent make rule.
Scanning dependencies of target jsoncpp_lib_static
[ 25%] Building CXX object src/lib_json/CMakeFiles/jsoncpp_lib_static.dir/json_reader.cpp.o
In file included from /root/project/build/deps/src/jsoncpp-project/src/lib_json/json_reader.cpp:9:
In file included from /root/project/build/deps/src/jsoncpp-project/include/json/reader.h:11:
/root/project/build/deps/src/jsoncpp-project/include/json/value.h:404:3: warning: explicit conversion functions are a C++11 extension [-Wc++11-extensions]
  explicit operator bool() const;
  ^~~~~~~~
1 warning generated.
[ 50%] Building CXX object src/lib_json/CMakeFiles/jsoncpp_lib_static.dir/json_value.cpp.o
In file included from /root/project/build/deps/src/jsoncpp-project/src/lib_json/json_value.cpp:8:
/root/project/build/deps/src/jsoncpp-project/include/json/value.h:404:3: warning: explicit conversion functions are a C++11 extension [-Wc++11-extensions]
  explicit operator bool() const;
  ^~~~~~~~
1 warning generated.
[ 75%] Building CXX object src/lib_json/CMakeFiles/jsoncpp_lib_static.dir/json_writer.cpp.o
In file included from /root/project/build/deps/src/jsoncpp-project/src/lib_json/json_writer.cpp:7:
In file included from /root/project/build/deps/src/jsoncpp-project/include/json/writer.h:10:
/root/project/build/deps/src/jsoncpp-project/include/json/value.h:404:3: warning: explicit conversion functions are a C++11 extension [-Wc++11-extensions]
  explicit operator bool() const;
  ^~~~~~~~
1 warning generated.
[100%] Linking CXX static library libjsoncpp.a

chfast · 2018-04-20T20:04:48Z

cmake/jsoncpp.cmake

               -DJSONCPP_WITH_PKGCONFIG_SUPPORT=OFF
-               -DCMAKE_CXX_FLAGS=${JSONCCP_EXTRA_FLAGS}
+	       -DCMAKE_CXX_FLAGS=${JSONCPP_EXTRA_FLAGS}
+               -DCMAKE_CXX_STANDARD=11


Try keeping CMAKE_CXX_STANDARD=11 only, remove "extra flags".

You mean remove EXTRA_FLAGS above or remove the line adding c++11 to EXTRA_FLAGS?

Actually it seems the extra flags are needed as the warnings are back now. Also there's an excessive amount of warnings from cmake.

Ok, so bring it back as it was but remove -DCMAKE_CXX_STANDARD=11.

Updated.

chfast · 2018-04-20T20:22:18Z

cmake/jsoncpp.cmake

               -DJSONCPP_WITH_TESTS=OFF
               -DJSONCPP_WITH_PKGCONFIG_SUPPORT=OFF
-               -DCMAKE_CXX_FLAGS=${JSONCCP_EXTRA_FLAGS}
+	       -DCMAKE_CXX_FLAGS=${JSONCPP_EXTRA_FLAGS}


Remove this line.

Changed.

chfast · 2018-04-20T20:29:00Z

scripts/travis-emscripten/build_emscripten.sh

  -DTESTS=0 \
  ..
-make -j 4
+make VERBOSE=1 -j 4


After successful build, this should be reverted.

chfast · 2018-04-20T20:29:23Z

.travis.yml

    # Disable tests unless run on the release branch, on tags or with daily cron
    #- if [ "$TRAVIS_BRANCH" != release -a -z "$TRAVIS_TAG" -a "$TRAVIS_EVENT_TYPE" != cron ]; then SOLC_TESTS=Off; fi
-    - SOLC_TESTS=Off
+    #- SOLC_TESTS=Off


After successful build, this should be probably reverted.

aarlt · 2018-04-20T21:51:16Z

@axic yep its compiling :) i wrote that on gitter - but looks like u missed it :)

chfast · 2018-04-20T21:58:31Z

cmake/jsoncpp.cmake

-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}")
 set(JSONCPP_INCLUDE_DIR "${prefix}/include")
+set(JSONCPP_EXTRA_FLAGS "-std=c++11")


In the end it must be only for GCC / clang. You can wrap it with if(NOT MSVC).

Are you sure? Appveyor seems to be building fine.

So Appveyor works with this, do you think it is still needed?

Maybe it works because of this flag: https://msdn.microsoft.com/en-us/library/mt490614.aspx.
Let's leave it if AppVeyor is ok.

axic · 2018-04-23T10:09:45Z

For some reason now Travis is emotional again:

Testing overwriting files
Testing soljson via the fuzzer...
Commandline tests successful.
No output has been received in the last 10m0s, this potentially indicates a stalled build or something wrong with the build itself.
Check the details on how to adjust your build configuration on: https://docs.travis-ci.com/user/common-build-problems/#Build-times-out-because-no-output-was-received
The build has been terminated

axic · 2018-04-23T11:39:46Z

@chriseth can you have a look at this?

axic · 2018-04-23T12:09:07Z

@chriseth are you ok with this? Travis emscripten works. I'll remove the last commit if you're happy with this version before merging.

chriseth · 2018-04-23T12:43:41Z

scripts/install_cmake.sh


 BIN=$PREFIX/bin

+PATH=$PREFIX/bin:$PATH


Does this have any effect after the script has run?

Updated.

LianaHus added in progress and removed in progress labels Feb 7, 2018

chriseth previously requested changes Feb 7, 2018

View reviewed changes

aarlt force-pushed the strict-mode-jsoncpp-1.8.4 branch from a92eb55 to bd09335 Compare February 7, 2018 21:49

axic reviewed Feb 9, 2018

View reviewed changes

aarlt force-pushed the strict-mode-jsoncpp-1.8.4 branch 5 times, most recently from 6f8d731 to 70bb519 Compare February 11, 2018 22:24

aarlt force-pushed the strict-mode-jsoncpp-1.8.4 branch from f8398dd to b65c3e3 Compare February 13, 2018 20:14

axic mentioned this pull request Apr 17, 2018

Add static_assert for the correct jsoncpp version #3913

Merged

axic force-pushed the strict-mode-jsoncpp-1.8.4 branch from eb21045 to 1be2339 Compare April 17, 2018 10:05

axic reviewed Apr 17, 2018

View reviewed changes

aarlt force-pushed the strict-mode-jsoncpp-1.8.4 branch from 75f6fb3 to 81388f7 Compare April 18, 2018 21:51

axic force-pushed the strict-mode-jsoncpp-1.8.4 branch from 9ca84f1 to 689ae8d Compare April 20, 2018 19:48

axic mentioned this pull request Apr 20, 2018

Fixes jsoncpp compilation when it is also installed system-wide #3933

Merged

axic force-pushed the strict-mode-jsoncpp-1.8.4 branch from 689ae8d to 53a09d0 Compare April 20, 2018 20:14

chfast reviewed Apr 20, 2018

View reviewed changes

chfast previously requested changes Apr 20, 2018

View reviewed changes

axic force-pushed the strict-mode-jsoncpp-1.8.4 branch from 53a09d0 to 3baffc1 Compare April 20, 2018 20:25

chfast previously requested changes Apr 20, 2018

View reviewed changes

chfast reviewed Apr 20, 2018

View reviewed changes

aarlt added 2 commits April 23, 2018 11:10

CMake: Update jsoncpp to v1.8.4

0b49fd3

Install & use cmake provided by scripts/install_cmake.sh for emscripten

a49dac8

axic force-pushed the strict-mode-jsoncpp-1.8.4 branch from 191fdbc to 56b75c3 Compare April 23, 2018 11:33

CMake: enable C++11 for jsoncpp

fa2a28a

axic force-pushed the strict-mode-jsoncpp-1.8.4 branch from 56b75c3 to fa2a28a Compare April 23, 2018 12:41

chriseth reviewed Apr 23, 2018

View reviewed changes

scripts/install_cmake.sh

BIN=$PREFIX/bin

PATH=$PREFIX/bin:$PATH

Copy link

Contributor

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?

chriseth approved these changes Apr 23, 2018

View reviewed changes

axic merged commit c7ee2ca into argotorg:develop Apr 23, 2018

sifmelcara mentioned this pull request Jun 6, 2018

Fix cmake when custom CMAKE_INSTALL_LIBDIR is given #4235

Merged

Conversation

aarlt commented Feb 7, 2018 • edited by axic Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chriseth commented Feb 7, 2018

Uh oh!

chriseth commented Feb 7, 2018

Uh oh!

chriseth left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aarlt commented Feb 7, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chriseth commented Feb 8, 2018

Uh oh!

aarlt commented Feb 8, 2018

Uh oh!

aarlt commented Feb 8, 2018

Uh oh!

aarlt commented Feb 8, 2018

Uh oh!

chriseth commented Feb 9, 2018

Uh oh!

chriseth commented Feb 9, 2018

Uh oh!

aarlt commented Feb 9, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aarlt Feb 10, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aarlt Feb 13, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chriseth commented Feb 9, 2018

Uh oh!

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

Uh oh!

Uh oh!

axic commented Feb 13, 2018

Uh oh!

aarlt commented Feb 13, 2018

Uh oh!

aarlt commented Feb 13, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chriseth commented Feb 13, 2018

Uh oh!

aarlt commented Feb 13, 2018

Uh oh!

axic commented Feb 13, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aarlt Apr 18, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

aarlt commented Feb 7, 2018 •

edited by axic

Loading

aarlt commented Feb 7, 2018 •

edited

Loading

aarlt Feb 10, 2018 •

edited

Loading

aarlt Feb 13, 2018 •

edited

Loading

aarlt commented Feb 11, 2018 •

edited

Loading

aarlt commented Feb 13, 2018 •

edited

Loading

aarlt Apr 18, 2018 •

edited

Loading