-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Improve multi-compiler support in the build system by adding a SolidityCompilerConfig base interface #8008
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
Improve multi-compiler support in the build system by adding a SolidityCompilerConfig base interface #8008
Changes from all commits
6fb3c27
a015127
ddd6566
db7c590
1aab861
57d1075
ecb715b
1163e75
95d0f30
4238ea7
f344087
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| "hardhat": patch | ||
| --- | ||
|
|
||
| Introduce multi-compiler abstraction that allows plugins to define new Solidity compiler types ([#8008](https://github.com/NomicFoundation/hardhat/pull/8008)). | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| --- | ||
| "@nomicfoundation/hardhat-errors": patch | ||
| "hardhat": patch | ||
| --- | ||
|
|
||
| Introduce the `ConfigHooks#validateResolvedConfig` hook and the `HardhatConfigValidationError` type to be able to run global validations on the resolved config ([#8008](https://github.com/NomicFoundation/hardhat/pull/8008)). |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -104,6 +104,8 @@ schaable | |
| sokol | ||
| solcjs | ||
| SOLCJS | ||
| solx | ||
| Solx | ||
| sourcify | ||
| Sourcify | ||
| SOURCIFY | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -39,6 +39,7 @@ | |||||||||||||
| "./console.sol": "./console.sol", | ||||||||||||||
| "./internal/coverage": "./dist/src/internal/builtin-plugins/coverage/exports.js", | ||||||||||||||
| "./internal/gas-analytics": "./dist/src/internal/builtin-plugins/gas-analytics/exports.js", | ||||||||||||||
| "./internal/solidity": "./dist/src/internal/builtin-plugins/solidity/exports.js", | ||||||||||||||
|
Comment on lines
+40
to
+42
|
||||||||||||||
| "./internal/coverage": "./dist/src/internal/builtin-plugins/coverage/exports.js", | |
| "./internal/gas-analytics": "./dist/src/internal/builtin-plugins/gas-analytics/exports.js", | |
| "./internal/solidity": "./dist/src/internal/builtin-plugins/solidity/exports.js", | |
| "./internal/coverage": "./dist/src/internal/builtin-plugins/coverage/index.js", | |
| "./internal/gas-analytics": "./dist/src/internal/builtin-plugins/gas-analytics/index.js", | |
| "./internal/solidity": "./dist/src/internal/builtin-plugins/solidity/index.js", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wouldn't pay attention to this comment, LGTM
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,11 @@ | ||
| import type { CompileCache } from "./cache.js"; | ||
| import type { DependencyGraphImplementation } from "./dependency-graph.js"; | ||
| import type { Artifact } from "../../../../types/artifacts.js"; | ||
| import type { SolcConfig, SolidityConfig } from "../../../../types/config.js"; | ||
| import type { | ||
| SolidityCompilerConfig, | ||
| SolcSolidityCompilerConfig, | ||
| SolidityConfig, | ||
| } from "../../../../types/config.js"; | ||
| import type { HookManager } from "../../../../types/hooks.js"; | ||
| import type { | ||
| SolidityBuildSystem, | ||
|
|
@@ -79,14 +83,26 @@ import { shouldSuppressWarning } from "./warning-suppression.js"; | |
| const log = debug("hardhat:core:solidity:build-system"); | ||
|
|
||
| /** | ||
| * Resolves the preferWasm setting for a given solc config, falling back | ||
| * Returns true if the given compiler config is a SolcSolidityCompilerConfig. | ||
| */ | ||
| export function isSolcSolidityCompilerConfig( | ||
| config: SolidityCompilerConfig, | ||
| ): config is SolcSolidityCompilerConfig { | ||
| return config.type === undefined || config.type === "solc"; | ||
| } | ||
|
|
||
| /** | ||
| * Resolves the preferWasm setting for a given compiler config, falling back | ||
| * to the build profile's preferWasm if not set on the compiler. | ||
| */ | ||
| function resolvePreferWasm( | ||
| solcConfig: SolcConfig, | ||
| compilerConfig: SolidityCompilerConfig, | ||
| buildProfilePreferWasm: boolean, | ||
| ): boolean { | ||
| return solcConfig.preferWasm ?? buildProfilePreferWasm; | ||
| if (isSolcSolidityCompilerConfig(compilerConfig)) { | ||
| return compilerConfig.preferWasm ?? buildProfilePreferWasm; | ||
| } | ||
| return false; | ||
| } | ||
|
|
||
| // Compiler warnings to suppress from build output. | ||
|
|
@@ -439,7 +455,7 @@ export class SolidityBuildSystemImplementation implements SolidityBuildSystem { | |
| ); | ||
|
|
||
| let subgraphsWithConfig: Array< | ||
| [SolcConfig, DependencyGraphImplementation] | ||
| [SolidityCompilerConfig, DependencyGraphImplementation] | ||
| > = []; | ||
|
Comment on lines
457
to
459
|
||
| for (const [rootFile, resolvedFile] of dependencyGraph.getRoots()) { | ||
| log( | ||
|
|
@@ -459,19 +475,47 @@ export class SolidityBuildSystemImplementation implements SolidityBuildSystem { | |
| } | ||
|
|
||
| // get longVersion and isWasm from the compiler for each version | ||
| const solcVersionToLongVersion = new Map<string, string>(); | ||
| const versionIsWasm = new Map<string, boolean>(); | ||
| for (const [solcConfig] of subgraphsWithConfig) { | ||
| let solcLongVersion = solcVersionToLongVersion.get(solcConfig.version); | ||
|
|
||
| if (solcLongVersion === undefined) { | ||
| const compiler = await getCompiler(solcConfig.version, { | ||
| preferWasm: resolvePreferWasm(solcConfig, buildProfile.preferWasm), | ||
| compilerPath: solcConfig.path, | ||
| // These maps are keyed by compiler type first, then version, to avoid | ||
| // collisions between different compiler types using the same version string. | ||
| const solidityVersionToLongVersionPerCompilerType = new Map< | ||
| string, | ||
| Map<string, string> | ||
| >(); | ||
| const versionIsWasmPerCompilerType = new Map< | ||
| string, | ||
| Map<string, boolean> | ||
| >(); | ||
| for (const [compilerConfig] of subgraphsWithConfig) { | ||
| const compilerType = compilerConfig.type ?? "solc"; | ||
| let longVersionMap = | ||
| solidityVersionToLongVersionPerCompilerType.get(compilerType); | ||
| if (longVersionMap === undefined) { | ||
| longVersionMap = new Map(); | ||
| solidityVersionToLongVersionPerCompilerType.set( | ||
| compilerType, | ||
| longVersionMap, | ||
| ); | ||
| } | ||
|
|
||
| let isWasmMap = versionIsWasmPerCompilerType.get(compilerType); | ||
| if (isWasmMap === undefined) { | ||
| isWasmMap = new Map(); | ||
| versionIsWasmPerCompilerType.set(compilerType, isWasmMap); | ||
| } | ||
|
|
||
| let longVersion = longVersionMap.get(compilerConfig.version); | ||
|
|
||
| if (longVersion === undefined) { | ||
| const compiler = await getCompiler(compilerConfig.version, { | ||
| preferWasm: resolvePreferWasm( | ||
| compilerConfig, | ||
| buildProfile.preferWasm, | ||
| ), | ||
| compilerPath: compilerConfig.path, | ||
| }); | ||
| solcLongVersion = compiler.longVersion; | ||
| solcVersionToLongVersion.set(solcConfig.version, solcLongVersion); | ||
| versionIsWasm.set(solcConfig.version, compiler.isSolcJs); | ||
| longVersion = compiler.longVersion; | ||
| longVersionMap.set(compilerConfig.version, longVersion); | ||
| isWasmMap.set(compilerConfig.version, compiler.isSolcJs); | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -480,17 +524,26 @@ export class SolidityBuildSystemImplementation implements SolidityBuildSystem { | |
| const sharedContentHashes = new Map<string, string>(); | ||
| await Promise.all( | ||
| subgraphsWithConfig.map(async ([config, subgraph]) => { | ||
| const solcLongVersion = solcVersionToLongVersion.get(config.version); | ||
| const compilerType = config.type ?? "solc"; | ||
| const longVersionMap = | ||
| solidityVersionToLongVersionPerCompilerType.get(compilerType); | ||
|
|
||
| assertHardhatInvariant( | ||
| solcLongVersion !== undefined, | ||
| "solcLongVersion should not be undefined", | ||
| longVersionMap !== undefined, | ||
| `No long version map for compiler type ${compilerType}`, | ||
| ); | ||
|
|
||
| const longVersion = longVersionMap.get(config.version); | ||
|
|
||
| assertHardhatInvariant( | ||
| longVersion !== undefined, | ||
| "longVersion should not be undefined", | ||
| ); | ||
|
|
||
| const individualJob = new CompilationJobImplementation( | ||
| subgraph, | ||
| config, | ||
| solcLongVersion, | ||
| longVersion, | ||
| this.#hooks, | ||
| sharedContentHashes, | ||
| ); | ||
|
|
@@ -516,7 +569,15 @@ export class SolidityBuildSystemImplementation implements SolidityBuildSystem { | |
| for (const [rootFile, compilationJob] of indexedIndividualJobs.entries()) { | ||
| const jobHash = await compilationJob.getBuildId(); | ||
| const cacheResult = this.#compileCache[rootFile]; | ||
| const isWasm = versionIsWasm.get(compilationJob.solcConfig.version); | ||
| const compilerType = compilationJob.solcConfig.type ?? "solc"; | ||
| const isWasmMap = versionIsWasmPerCompilerType.get(compilerType); | ||
|
|
||
| assertHardhatInvariant( | ||
| isWasmMap !== undefined, | ||
| `No isWasm map for compiler type ${compilerType}`, | ||
| ); | ||
|
|
||
| const isWasm = isWasmMap.get(compilationJob.solcConfig.version); | ||
|
|
||
| assertHardhatInvariant( | ||
| isWasm !== undefined, | ||
|
|
@@ -529,6 +590,7 @@ export class SolidityBuildSystemImplementation implements SolidityBuildSystem { | |
| cacheResult === undefined || | ||
| cacheResult.jobHash !== jobHash || | ||
| cacheResult.isolated !== isolated || | ||
| cacheResult.compilerType !== compilerType || | ||
| cacheResult.wasm !== isWasm | ||
| ) { | ||
| rootFilesToCompile.add(rootFile); | ||
|
|
@@ -576,13 +638,15 @@ export class SolidityBuildSystemImplementation implements SolidityBuildSystem { | |
| log(`Merging compilation jobs`); | ||
|
|
||
| const mergedSubgraphsByConfig: Map< | ||
| SolcConfig, | ||
| SolidityCompilerConfig, | ||
| DependencyGraphImplementation | ||
| > = new Map(); | ||
|
|
||
| // Note: This groups the subgraphs by solc config. It compares the configs | ||
| // based on reference, and not by deep equality. It misses some merging | ||
| // opportunities, but this is Hardhat v2's behavior and works well enough. | ||
| // Note: This groups the subgraphs by compiler config. It compares the | ||
| // configs based on reference, and not by deep equality. This is | ||
| // inherently type-aware: two configs with different types will always be | ||
| // different references. It misses some merging opportunities, but this is | ||
| // Hardhat v2's behavior and works well enough. | ||
| for (const [config, subgraph] of subgraphsWithConfig) { | ||
| const rootFile = getSingleRootFilePath(subgraph); | ||
|
|
||
|
|
@@ -613,18 +677,27 @@ export class SolidityBuildSystemImplementation implements SolidityBuildSystem { | |
| } | ||
|
|
||
| const compilationJobsPerFile = new Map<string, CompilationJob>(); | ||
| for (const [solcConfig, subgraph] of subgraphsWithConfig) { | ||
| const solcLongVersion = solcVersionToLongVersion.get(solcConfig.version); | ||
| for (const [compilerConfig, subgraph] of subgraphsWithConfig) { | ||
| const compilerType = compilerConfig.type ?? "solc"; | ||
| const longVersionMap = | ||
| solidityVersionToLongVersionPerCompilerType.get(compilerType); | ||
|
|
||
| assertHardhatInvariant( | ||
| longVersionMap !== undefined, | ||
| `No long version map for compiler type ${compilerType}`, | ||
| ); | ||
|
|
||
| const longVersion = longVersionMap.get(compilerConfig.version); | ||
|
|
||
| assertHardhatInvariant( | ||
| solcLongVersion !== undefined, | ||
| "solcLongVersion should not be undefined", | ||
| longVersion !== undefined, | ||
| "longVersion should not be undefined", | ||
| ); | ||
|
|
||
| const runnableCompilationJob = new CompilationJobImplementation( | ||
| subgraph, | ||
| solcConfig, | ||
| solcLongVersion, | ||
| compilerConfig, | ||
| longVersion, | ||
| this.#hooks, | ||
| sharedContentHashes, | ||
| ); | ||
|
|
@@ -1124,6 +1197,7 @@ export class SolidityBuildSystemImplementation implements SolidityBuildSystem { | |
| this.#compileCache[rootFilePath] = { | ||
| jobHash, | ||
| isolated, | ||
| compilerType: individualJob.solcConfig.type ?? "solc", | ||
| artifactPaths, | ||
| buildInfoPath: emitArtifactsResult.buildInfoPath, | ||
| buildInfoOutputPath: emitArtifactsResult.buildInfoOutputPath, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need a separate changeset for the new config hook.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
9bab76a