ApeWorX / py-solc-x

Python wrapper and version management tool for the solc Solidity compiler.
https://solcx.readthedocs.io/
MIT License
138 stars 48 forks source link

Support for solc path mappings #85

Closed MatthiasLohr closed 4 years ago

MatthiasLohr commented 4 years ago

According to the solc documentation (https://solidity.readthedocs.io/en/v0.4.21/using-the-compiler.html#using-the-commandline-compiler), solc supports providing path mappings for includes, e.g.

solc github.com/ethereum/dapp-bin/=/usr/local/lib/dapp-bin/ =/usr/local/lib/fallback file.sol

I'm not sure if it's already support, however, if supported, I couldn't find the option.

If it is not supported: I would request/suggest to add an option to the compile methods which allows for specifying path mappings as described by the solc documentation.

iamdefinitelyahuman commented 4 years ago

I believe it's possible via the import_remappings kwarg:

https://github.com/iamdefinitelyahuman/py-solc-x/blob/38f767f88f83046358d096f0ec27b60c612cd329/solcx/wrapper.py#L102-L103

You should be able to give this kwarg to compile_source or compile_files. If you have any issues, please let me know.

MatthiasLohr commented 4 years ago

Thanks for your quick reply! I'm using compile_sources.

There seems to be a problem using import_remappings:

solcx.compile_source(contract_code)

returns Error: Source "mediator.sol" not found: File outside of allowed directories - expected so far. Now, I add the remapping:

solcx.compile_source(contract_code, import_remappings=['=bdtsim/protocol/smartjudge/'])

Now I get the following error (and yeah, that's all):

solcx.exceptions.SolcError: An error occurred during execution
> command: `/home/mlohr/.solcx/solc-v0.4.26 --combined-json abi,asm,ast,bin,bin-runtime,clone-bin,devdoc,opcodes,userdoc =bdtsim/protocol/smartjudge/`
> return code: `1`
> stderr:

> stdout:

Just for fun, I tried the following:

solcx.compile_source(contract_code, import_remappings=['=bdtsim/protocol/smartjudge/', '-'])

... and it worked. Could't see that in the code, but it looks like that when defining the remappings, the wrapper does not add the - for reading from stdin. But feels like a really ugly workaround...

iamdefinitelyahuman commented 4 years ago

Yeah, that's not exactly a useful and meaningful error message :rofl:

Glad you found a workaround, but I agree it's very hacky and shouldn't be required. Seems this endpoint needs some unique attention, as opposed to being just another kwarg fed into the wrapper.

MatthiasLohr commented 4 years ago

Ouh. v1.0.0 sounds like "far in the future"... Any chance of an ETA? Right now this is blocking my work, and even the workaround does not seem to work reliable.

iamdefinitelyahuman commented 4 years ago

Ahh, sorry! Didn't mean to seem dismissive applying that tag. I'm pretty sure we can address it right away in a non-breaking manner. I know the - is needed in solc versions >=0.5.0, and only in certain cases:

https://github.com/iamdefinitelyahuman/py-solc-x/blob/38f767f88f83046358d096f0ec27b60c612cd329/solcx/wrapper.py#L171-L172

So, probably just need to amend / expand that check, and add the - when import_remappings is being used and it's a version that requires it. Also I see a test case in test_compile.py related to this, but clearly it's not doing a very good job.

I can take a look at it this evening, but if you have time earlier I'm also happy to receive a PR.

MatthiasLohr commented 4 years ago

I think I'm missing the background and details about the versions and its requirements... would be just guessing, to if you're fine with that, I would leave it for you...

iamdefinitelyahuman commented 4 years ago

Which version of solc are you using? Starting with v0.5.0 it seems that =/usr/local/lib/fallback is not allowed, you have to open with a dot or a slash. And I'm always at getting meaningful output on a failure, even as far back as v0.4.11.

iamdefinitelyahuman commented 4 years ago

OK found the issue. I was looking at compile_files not compile_sources :fearful:

iamdefinitelyahuman commented 4 years ago

Should be good now :) I can cut a new patch release right away, or wait for #85.

MatthiasLohr commented 4 years ago

Thanks!

A release in near time would be nice. Do you mean "waiting for #86"?

MatthiasLohr commented 4 years ago

I'm sorry to bother you again, but...

$HOME/.solcx/solc-v0.4.26 --combined-json abi,asm,ast,bin,bin-runtime,devdoc,opcodes,userdoc =/my/path/

works vs.

$HOME/.solcx/solc-v0.6.1 --combined-json abi,asm,ast,bin,bin-runtime,devdoc,opcodes,userdoc =/my/path/

gives:

Invalid remapping: "=/my/path/".

Any idea? Should that be solved in py-solc-x (since it already has some kind of version dependend configuration of the compiler call) or by the calling application, which would require version dependend code...

iamdefinitelyahuman commented 4 years ago

Yeah, I ran into this issue when testing. From 0.5.0 onward, the import remappings (prefix=path) have to include a prefix value.

From the v0.4.26 documentation:

solc will not read files from the filesystem that lie outside of the remapping targets and outside of the directories where explicitly specified source files reside, so things like import "/etc/passwd"; only work if you add =/ as a remapping.

And from v0.5.0 documentation:

solc will not read files from the filesystem that lie outside of the remapping targets and outside of the directories where explicitly specified source files reside, so things like import "/etc/passwd"; only work if you add /=/ as a remapping.

An empty remapping prefix is not allowed.