Skip to content

Fix Solidity compilation in subprocesses#6535

Merged
alcuadrado merged 4 commits intomainfrom
fix-compilation-in-subprocess
May 19, 2025
Merged

Fix Solidity compilation in subprocesses#6535
alcuadrado merged 4 commits intomainfrom
fix-compilation-in-subprocess

Conversation

@alcuadrado
Copy link
Member

This PR modifies the two solidity ICompiler implementations, making them more robust.

@changeset-bot
Copy link

changeset-bot bot commented Apr 3, 2025

🦋 Changeset detected

Latest commit: ae7fbc0

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
hardhat Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@vercel
Copy link

vercel bot commented Apr 3, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
hardhat ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 3, 2025 11:40pm

@alcuadrado
Copy link
Member Author

Inspired (AKA stolen from) NomicFoundation/hardhat-vscode#642

@alcuadrado alcuadrado added no changeset needed This PR doesn't require a changeset and removed no changeset needed This PR doesn't require a changeset labels Apr 3, 2025
@alcuadrado alcuadrado force-pushed the fix-compilation-in-subprocess branch from 1c6baaf to 67786e4 Compare April 3, 2025 23:31
@alcuadrado alcuadrado force-pushed the fix-compilation-in-subprocess branch from 67786e4 to 15cc50c Compare April 3, 2025 23:37
@alcuadrado alcuadrado requested a review from antico5 April 3, 2025 23:45

output = stdout;
} catch (e: any) {
throw new HardhatError(ERRORS.SOLC.CANT_RUN_NATIVE_COMPILER, {}, e);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be ERRORS.SOLC.SOLCJS_ERROR?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alcuadrado I think the error types are swapped between socljs and native implementations 😅

@antico5
Copy link
Contributor

antico5 commented Apr 21, 2025

Do we have integration tests for this, at least the happy path ? both for running solcjs and native compiler

@alcuadrado
Copy link
Member Author

Do we have integration tests for this, at least the happy path ? both for running solcjs and native compiler

Yes, actually, I think we use the compilers too much in integration tests.

@alcuadrado alcuadrado merged commit 46da544 into main May 19, 2025
127 checks passed
@alcuadrado alcuadrado deleted the fix-compilation-in-subprocess branch May 19, 2025 15:33
@github-project-automation github-project-automation bot moved this from Backlog to Done in Hardhat May 19, 2025
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 18, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

2 participants