Rename jsonCompiler to libsolc by axic · Pull Request #3010 · argotorg/solidity

axic · 2017-10-02T20:26:03Z

Part of #2864.

chriseth · 2017-10-04T09:32:00Z

I think this might break the emscripten deploy step (also solc-js has to be adapted).

axic · 2017-10-04T09:33:12Z

I'd just keep the name as soljson for solc-js/solc-bin in this PR. We could consider renaming that part separately to this to minimize dependencies.

axic · 2017-10-18T11:25:48Z

@chfast the executable solc and the library libsolc are conflicting at link time (i.e. solfuzzer cannot decide which one to link to). Can this be fixed in cmake?

chfast · 2017-10-18T11:32:13Z

If you use the same target names you will have the problem.
There are 2 solutions:

Use common pattern of starting library names with capital letter, libSolc.a.
Use target name "libsolc" and set the output name to "solc" for it.
```
set_target_properties(libsolc PROPERTIES OUTPUT_NAME solc)
```

axic · 2017-10-18T11:39:17Z

I am actually using libsolc as target name. Can you have a look at the changes?

chfast · 2017-10-18T11:42:32Z

libsolc/CMakeLists.txt

@@ -0,0 +1,11 @@
+set(


Not needed?

I was wondering about this. The same applies to solc/CMakeLists.txt too I guess.

The set() just sets the variable value. It is sources = "". If sources are not used anywhere remove it.

chfast · 2017-10-18T11:43:18Z

libsolc/CMakeLists.txt

+	set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -s EXPORTED_FUNCTIONS='[\"_compileJSON\",\"_license\",\"_version\",\"_compileJSONMulti\",\"_compileJSONCallback\",\"_compileStandard\"]' -s RESERVED_FUNCTION_POINTERS=20")
+	add_executable(libsolc libsolc.cpp)
+else()
+	add_library(libsolc libsolc.cpp)


Without changing the OUTPUT_NAME you will get a file of name liblibsolc.a.

axic · 2017-10-18T11:44:34Z

scripts/bytecodecompare/storebytecode.sh

    if [[ "$SOLC_EMSCRIPTEN" = "On" ]]
    then
-        cp "$REPO_ROOT/build/solc/soljson.js" .
+        cp "$REPO_ROOT/build/libsolc/libsolc.js" .


. needs to be replaced with soljson.js

chfast · 2017-10-18T11:50:31Z

libsolc/CMakeLists.txt

@@ -0,0 +1,11 @@
+set(


You forgot to include this script in main CMakeLists.txt script. CMake does not know what libsolc is and it assumed its some system library.

axic · 2017-10-18T12:09:59Z

The properties thing doesn't work here:

  Cannot find source file:

    PROPERTIES

  Tried extensions .c .C .c++ .cc .cpp .cxx .m .M .mm .h .hh .h++ .hm .hpp
  .hxx .in .txx

chfast · 2017-10-18T12:12:23Z

This is a separated command:

add_library(libsolc ...)
set_target_properties(libsolc PROPERTIES OUTPUT_NAME solc)

axic · 2017-10-18T12:17:17Z

My bad. It seems to work perfectly now, thanks!

axic · 2017-10-18T12:35:22Z

libsolc/CMakeLists.txt

+	add_executable(soljson libsolc.cpp)
+	target_link_libraries(soljson PRIVATE solidity)
+else()
+	add_library(libsolc SHARED libsolc.cpp)


The SHARED part could be skipped for this PR.

Why do you need it to be a shared library?

Ultimate goal of #2864.

So let's keep it here now.

axic · 2017-11-17T15:09:57Z

@chriseth happy to merge?

chriseth · 2017-12-11T11:05:46Z

needs rebase

axic force-pushed the libsolc branch 2 times, most recently from 7c1317d to cac25a2 Compare October 2, 2017 21:39

axic force-pushed the libsolc branch from cac25a2 to 132d41b Compare October 4, 2017 10:06

axic force-pushed the libsolc branch from 132d41b to 77089d2 Compare October 18, 2017 11:25

chfast reviewed Oct 18, 2017

View reviewed changes

axic commented Oct 18, 2017

View reviewed changes

chfast suggested changes Oct 18, 2017

View reviewed changes

axic force-pushed the libsolc branch 2 times, most recently from 93e36a4 to 19b8e4f Compare October 18, 2017 12:33

axic commented Oct 18, 2017

View reviewed changes

axic force-pushed the libsolc branch from 19b8e4f to 142621b Compare October 18, 2017 12:46

chfast approved these changes Oct 18, 2017

View reviewed changes

axic force-pushed the libsolc branch from 142621b to 4e3f542 Compare November 16, 2017 14:05

chriseth approved these changes Dec 11, 2017

View reviewed changes

Rename jsonCompiler to libsolc

1734d44

axic force-pushed the libsolc branch from 4e3f542 to 1734d44 Compare December 11, 2017 12:49

axic merged commit 1ddd4e2 into develop Dec 12, 2017

axic deleted the libsolc branch December 12, 2017 03:03

Conversation

axic commented Oct 2, 2017

Uh oh!

chriseth commented Oct 4, 2017

Uh oh!

axic commented Oct 4, 2017

Uh oh!

axic commented Oct 18, 2017

Uh oh!

chfast commented Oct 18, 2017

Uh oh!

axic commented Oct 18, 2017

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!

axic commented Oct 18, 2017

Uh oh!

chfast commented Oct 18, 2017

Uh oh!

axic commented Oct 18, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chfast Oct 18, 2017 • 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!

Choose a reason for hiding this comment

Uh oh!

axic commented Nov 17, 2017

Uh oh!

chriseth commented Dec 11, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

chfast Oct 18, 2017 •

edited

Loading