Compile contracts using solcjs or solc #174
Conversation
Using solcjs instead of solc
|
i did not work last week. will look at it soon |
There was a problem hiding this comment.
thanks for the contribution!
it would be great if you could try simplifying the deeply nested matches and if's. since most branches end with a panic! this could be written in an early-return (or in this case "early-panic") style which would make it more readable
bridge/build.rs
Outdated
| } else { | ||
| panic!("an error occurred when trying to spawn `solc`: {}", err); | ||
| } | ||
| } | ||
| } | ||
| } | ||
| fn get_file_name(path: DirEntry) -> String { |
There was a problem hiding this comment.
i think it would be cleaner to inline this above
|
@snd early panic cannot be possible. We are panicking only if solc and solcjs both are not available. Or if any process fails unexpectedly, example:
For any confusion regarding any panics, feel free to raise doubts. I'll be happy to update them with comments or try resolving those doubts. |
There was a problem hiding this comment.
This function is still very complex. Can you simplify it further by separating functions compile_using_solc and compile_using_solcjs. Also removing those nested matches and if/else statements would help a lot with code readibility. It's difficult to find the and of the block now :)
bridge/build.rs
Outdated
| path.unwrap().file_name().into_string().expect( | ||
| "error: the first argument is not a file\ | ||
| system path representable in UTF-8.", | ||
| ) |
There was a problem hiding this comment.
please use Path instead. e.g. Path::new(prepend).join(path.unwrap().file_name()) or maybe even path.unwrap().path()
bridge/build.rs
Outdated
| println!("Removed old files"); | ||
| } | ||
| Err(err) => { | ||
| println!("Files not removed: {}", err); |
There was a problem hiding this comment.
using println! for debugging is usually a bad pattern. Especially in build.rs
|
Tests are failing now due to timeout. Is this because we are using party version 1.10? Because previously, similar tests were working on using parity version 1.9.7 |
|
seems like tests are failing because the code is not formatted with rustfmt. could you run |
|
I resolved the formatting of this file, but the build is still failing. |
This reverts commit e49c30f, this commit is supposed to be in another branch.
|
i think it makes sense to move this into a separate crate since the same functionality is also present in https://github.com/paritytech/sol-rs. |
|
i'm going to create a small crate that makes it easy for any project to interact with / compile sol files with solc and/or solcjs so the bridge, sol-rs and other projects don't have to reimplement it over and over |
|
started working on said crate over here https://github.com/snd/rust_solc/. |
|
wasn't able to get the JSON I/O to work just yet because it's broken on solcjs ethereum/solc-js#126. i got https://github.com/snd/rust_solc to work with file I/O though. we should now be able to replace the solc compilation code in this PR by a simple solc::compile_dir("../contracts", "../compiled_contracts").unwrap();@AyushyaChitransh do you want to modify this PR accordingly or open another PR? let me know. otherwise i'll do it in another PR |
|
Let me rephrase what I understood: You are suggesting we use rust_solc. And on using that, we would not require to use If this is the case, lets have another PR from you and close this one. |
rust_solc is just a crate that shells out to solc or solcjs
sounds good |
|
Am still unable to compile. Facing solc errors.
|
This PR closes #168 and closes #169 and it also improves #170
solcsolcis not found, try using solcjs.