Skip to content
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.

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

Add Docker Support #2

wants to merge 14 commits into
base: master


None yet
2 participants
Copy link

commented Mar 21, 2019

PR for #1

  • The plugin should override the internalTask TASK_COMPILE_RUN_COMPILER. You can read its definition here. This is part of PR #190 which will be merged soon.
  • The overriden task receives the standard JSON input of solc, and returns the standard output JSON. You can read more about them here.
  • The plugin should throw a BuidlerPluginError if docker is not running.
  • The plugin should pull the docker with the right version of solc if necessary. The version is specified in the config.
  • The plugin should throw a BuidlerPluginError if the version is invalid.
  • The plugin should throw a BuidlerPluginError if it can't pull the docker image.
  • The plugin should have unit and/or integration tests. We can provide guidance on how to test each part.
  • The plugin must be based in the Buidler plugin boilerplate.
  • Update README with more documentation
  • Update Travis

jacobcreech added some commits Mar 17, 2019

Initial Commit
Initial commit for the buidler-docker-solc plugin. Added boilerplate code found at [buidler-ts-plugin-boilerplate]( This is related to issue #1
Override the task TASK_COMPILE_RUN_COMPILER. Added test to run the task. Added docker implementation for everything docker related. Now just to make TASK_COMPILE_RUN_COMPILER run through the docker functions, and add the proper testing. Oh, and of course, add lazy loading to docker containers.
Updated docker validation
Added validation for versioning
Refactored docker initialization
Previously, it was possible to get an error with the docker image loading incorrectly displayed because all of the functions were async. Now, loading the docker image does all the validations in order that they have to occur, correctly displaying the correct BuidlerPluginError as necessary
Use node-docker-api for interacting with docker
Changed out using child_process to using node-docker-api to avoid exposing the rpc. Changed task to be overridden with passing the json for contracts
Docker run supported
Got docker run with attached volume supported. Adding tests and coverage, plus lazy loading next
Updated error handling
Docker is not running error was in the incorrect spot, as we'll run into a
 docker error earlier in the flow on requiring docker
Updated docker configuration
Running solc on your contracts in docker now works.
Updated test coverage + child_process
Updated docker to run with child_process and return the correct json formatted result from the compiler. Updated test coverage. Updated information under package.json
Added additional documentation on how to install and use buidler-docker-solc
Changing test permisions
Updated permissions of the run-tests file so that travis can run the unit tests

This comment has been minimized.

Copy link

commented Mar 22, 2019

@alcuadrado Please review and let me know any feedback. I will submit on Gitcoin, but this is still a living PR if further updates are required.

Show resolved Hide resolved src/docker.ts Outdated
Show resolved Hide resolved src/docker.ts Outdated
Show resolved Hide resolved src/docker.ts Outdated
Show resolved Hide resolved src/docker.ts
Show resolved Hide resolved package.json Outdated
Show resolved Hide resolved package.json Outdated

This comment has been minimized.

Copy link

commented Apr 8, 2019

@itirabasso Thank you for the feedback, I will update asap

jacobcreech added some commits Apr 20, 2019

Refactoring for efficiency
Removed unnecessary packages from package. Removed import of entire environment. Passed the solcConfig on Docker object creation, instead of unused config.
Package bug fix
Package was missing comma
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.