Skip to content

Conversation

@maxsam4
Copy link

@maxsam4 maxsam4 commented Nov 29, 2018

Fixed #12

About the Issue
solc spits out the output to stdout by default.
shell.js uses node's child process that has a limit of 200KB by default on the stdout buffer which is too small.
For example, size of output generated by solc for our contracts with all the flags used here is ~31MB and as a result, doc generation was failing.

Solution
This PR increases max buffer size to 100MB. As this is the max size and it will not be reserved by default, this PR is safe to merge. I have conservatively set it 100MB to detect any future memory leaks. This can be increased in future if need be.

@ealmansi
Copy link
Contributor

ealmansi commented Dec 3, 2018

LGTM

@maxsam4
Copy link
Author

maxsam4 commented Mar 8, 2019

@frangio @spalladino can this be merged, please?

@frangio
Copy link
Contributor

frangio commented Mar 8, 2019

@maxsam4 Apologies for the delay on this. We're working on a new version of solidity-docgen that will not have this problem because it uses solcjs. It should be released in the next month, so I would prefer to focus on that and not merge this.

Is this affecting your setup currently? Or would waiting a couple more weeks work for you?

@maxsam4
Copy link
Author

maxsam4 commented Mar 9, 2019

Is this affecting your setup currently? Or would waiting a couple more weeks work for you?

I am using my fork of solidity-docgen to generate docs as it's unusable for us without this fix. I just want to move back to using official version instead of my own fork. As this fix does not break anything, I see no reason for this to not be part of the official version. I can keep using my own fork if you don't want to merge this simple fix.

Re. solcjs
Please keep it optional. Native solc is much faster than solcjs and has fewer bugs.
solcjs stops working after the source code size increases to ~1.3 MB. argotorg/solc-js#330

Our source code is currently 887KB + a few KB for OZ contracts on top of that. We might soon be unable to use solcjs for anything in our project. We are currently using native solc for everything and it's much better dev experience once you get it set up :). On mac, on linux and windows and rpi, on everything :).

This was referenced Apr 25, 2019
@frangio
Copy link
Contributor

frangio commented Apr 25, 2019

Thank you for bringing this up @maxsam4. As mentioned before we have rewritten the tool and it is using solcjs for now. But keeping in mind what you said, I've opened #25 to add native solc as an option.

@frangio frangio closed this Apr 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Error: Command line operation failed with code 1.

3 participants