-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Add downloadCompilers parallel hook for extensible compiler downloads #8009
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
Changes from all commits
83b412e
f5cd4a5
ea42662
ebfcd41
62a09b9
0d075d2
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 | ||
| --- | ||
|
|
||
| Add `SolidityHooks#downloadCompilers` and `SolidityHooks#getCompiler` hooks for extensible custom compiler support ([#8009](https://github.com/NomicFoundation/hardhat/pull/8009)) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -58,6 +58,7 @@ import pMap from "p-map"; | |
|
|
||
| import { FileBuildResultType } from "../../../../types/solidity/build-system.js"; | ||
| import { DEFAULT_BUILD_PROFILE } from "../build-profiles.js"; | ||
| import { getSolcCompilerForConfig } from "../solidity-hooks.js"; | ||
|
|
||
| import { | ||
| getArtifactsDeclarationFile, | ||
|
|
@@ -91,20 +92,6 @@ export function isSolcSolidityCompilerConfig( | |
| 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( | ||
| compilerConfig: SolidityCompilerConfig, | ||
| buildProfilePreferWasm: boolean, | ||
| ): boolean { | ||
| if (isSolcSolidityCompilerConfig(compilerConfig)) { | ||
| return compilerConfig.preferWasm ?? buildProfilePreferWasm; | ||
| } | ||
| return false; | ||
| } | ||
|
|
||
| // Compiler warnings to suppress from build output. | ||
| // Each rule specifies a warning message and the source file it applies to. | ||
| // This allows suppressing known warnings from internal files (e.g., console.sol) | ||
|
|
@@ -506,13 +493,13 @@ export class SolidityBuildSystemImplementation implements SolidityBuildSystem { | |
| let longVersion = longVersionMap.get(compilerConfig.version); | ||
|
|
||
| if (longVersion === undefined) { | ||
| const compiler = await getCompiler(compilerConfig.version, { | ||
| preferWasm: resolvePreferWasm( | ||
| compilerConfig, | ||
| buildProfile.preferWasm, | ||
| ), | ||
| compilerPath: compilerConfig.path, | ||
| }); | ||
| const compiler = await this.#hooks.runHandlerChain( | ||
| "solidity", | ||
| "getCompiler", | ||
| [compilerConfig], | ||
| async (_context, cfg) => | ||
| getSolcCompilerForConfig(cfg, buildProfile.preferWasm), | ||
| ); | ||
| longVersion = compiler.longVersion; | ||
| longVersionMap.set(compilerConfig.version, longVersion); | ||
| isWasmMap.set(compilerConfig.version, compiler.isSolcJs); | ||
|
|
@@ -750,19 +737,16 @@ export class SolidityBuildSystemImplementation implements SolidityBuildSystem { | |
|
|
||
| const { buildProfile } = this.#getBuildProfile(options?.buildProfile); | ||
|
|
||
| const compiler = await getCompiler( | ||
| runnableCompilationJob.solcConfig.version, | ||
| { | ||
| preferWasm: resolvePreferWasm( | ||
| runnableCompilationJob.solcConfig, | ||
| buildProfile.preferWasm, | ||
| ), | ||
| compilerPath: runnableCompilationJob.solcConfig.path, | ||
| }, | ||
| const compiler = await this.#hooks.runHandlerChain( | ||
| "solidity", | ||
| "getCompiler", | ||
| [runnableCompilationJob.solcConfig], | ||
| async (_context, cfg) => | ||
| getSolcCompilerForConfig(cfg, buildProfile.preferWasm), | ||
| ); | ||
|
|
||
| log( | ||
| `Compiling ${numberOfRootFiles} root files and ${numberOfFiles - numberOfRootFiles} dependency files with solc ${runnableCompilationJob.solcConfig.version} using ${compiler.compilerPath}`, | ||
| `Compiling ${numberOfRootFiles} root files and ${numberOfFiles - numberOfRootFiles} dependency files with ${runnableCompilationJob.solcConfig.type ?? "solc"} ${runnableCompilationJob.solcConfig.version} using ${compiler.compilerPath}`, | ||
| ); | ||
|
|
||
| assertHardhatInvariant( | ||
|
|
@@ -1089,8 +1073,10 @@ export class SolidityBuildSystemImplementation implements SolidityBuildSystem { | |
| ): Promise<CompilerOutput> { | ||
| const quiet = options?.quiet ?? false; | ||
|
|
||
| // We download the compiler for the build info as it may not be configured | ||
| // in the HH config, hence not downloaded with the other compilers | ||
| // Build info recompilation is always solc-only: build info files are | ||
| // produced by solc and must be recompiled with the same solc version. | ||
| // We bypass both downloadCompilers and getCompiler hooks — this is a | ||
| // self-contained solc replay path, not plugin-configurable compilation. | ||
| await downloadSolcCompilers(new Set([buildInfo.solcVersion]), quiet); | ||
|
|
||
| const compiler = await getCompiler(buildInfo.solcVersion, { | ||
|
|
@@ -1107,20 +1093,17 @@ export class SolidityBuildSystemImplementation implements SolidityBuildSystem { | |
| return; | ||
| } | ||
|
|
||
| await downloadSolcCompilers(this.#getAllCompilerVersions(), quiet); | ||
| const allSolidityCompilerConfigs = this.#getAllSolidityCompilerConfigs(); | ||
| await this.#hooks.runParallelHandlers("solidity", "downloadCompilers", [ | ||
| allSolidityCompilerConfigs, | ||
| quiet, | ||
| ]); | ||
| this.#configuredCompilersDownloaded = true; | ||
| } | ||
|
|
||
| #getAllCompilerVersions(): Set<string> { | ||
| return new Set( | ||
| Object.values(this.#options.solidityConfig.profiles) | ||
| .map((profile) => [ | ||
| ...profile.compilers.map((compiler) => compiler.version), | ||
| ...Object.values(profile.overrides).map( | ||
| (override) => override.version, | ||
| ), | ||
| ]) | ||
| .flat(1), | ||
| #getAllSolidityCompilerConfigs(): SolidityCompilerConfig[] { | ||
| return Object.values(this.#options.solidityConfig.profiles).flatMap( | ||
| (profile) => [...profile.compilers, ...Object.values(profile.overrides)], | ||
| ); | ||
| } | ||
|
|
||
|
|
@@ -1282,16 +1265,21 @@ export class SolidityBuildSystemImplementation implements SolidityBuildSystem { | |
| } | ||
|
|
||
| for (const job of runnableCompilationJobs) { | ||
| const compilerType = job.solcConfig.type ?? "solc"; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess the changes in this file from here on in fact belong to #8008, but it's not important. Just mentioning it for my future reference and clarity.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Reviewed these changes. LGTM |
||
| const solcVersion = job.solcConfig.version; | ||
| const solcInput = await job.getSolcInput(); | ||
| const evmVersion = | ||
| solcInput.settings.evmVersion ?? | ||
| `Check solc ${solcVersion}'s doc for its default evm version`; | ||
|
|
||
| let jobsPerVersion = jobsPerVersionAndEvmVersion.get(solcVersion); | ||
| // Group by compiler type + Solidity version to produce separate log | ||
| // lines for e.g. "solc 0.8.33" vs "solx 0.1.3 (Solidity 0.8.33)". | ||
| const groupKey = `${compilerType}#${solcVersion}`; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm more of a fan of multi-level maps than this combined keys, as the type system enforces that you give them some thought. I'm not agains this either. |
||
|
|
||
| let jobsPerVersion = jobsPerVersionAndEvmVersion.get(groupKey); | ||
| if (jobsPerVersion === undefined) { | ||
| jobsPerVersion = new Map(); | ||
| jobsPerVersionAndEvmVersion.set(solcVersion, jobsPerVersion); | ||
| jobsPerVersionAndEvmVersion.set(groupKey, jobsPerVersion); | ||
| } | ||
|
|
||
| let jobsPerEvmVersion = jobsPerVersion.get(evmVersion); | ||
|
|
@@ -1303,10 +1291,11 @@ export class SolidityBuildSystemImplementation implements SolidityBuildSystem { | |
| jobsPerEvmVersion.push(job); | ||
| } | ||
|
|
||
| for (const solcVersion of [...jobsPerVersionAndEvmVersion.keys()].sort()) { | ||
| for (const groupKey of [...jobsPerVersionAndEvmVersion.keys()].sort()) { | ||
| /* eslint-disable-next-line @typescript-eslint/no-non-null-assertion -- | ||
| This is a valid key, just sorted */ | ||
| const jobsPerEvmVersion = jobsPerVersionAndEvmVersion.get(solcVersion)!; | ||
| const jobsPerEvmVersion = jobsPerVersionAndEvmVersion.get(groupKey)!; | ||
| const [compilerType, solidityVersion] = groupKey.split("#"); | ||
|
|
||
| for (const evmVersion of [...jobsPerEvmVersion.keys()].sort()) { | ||
| /* eslint-disable-next-line @typescript-eslint/no-non-null-assertion -- | ||
|
|
@@ -1318,12 +1307,25 @@ export class SolidityBuildSystemImplementation implements SolidityBuildSystem { | |
| 0, | ||
| ); | ||
|
|
||
| // For solc, the compiler version is the Solidity version. | ||
| // For other compilers, extract the compiler's own version from the | ||
| // longVersion stored on the compilation job, and show the Solidity | ||
| // version separately. | ||
| let compilerLabel: string; | ||
| if (compilerType === "solc") { | ||
| compilerLabel = `solc ${solidityVersion}`; | ||
| } else { | ||
| const longVersion = jobs[0].solcLongVersion; | ||
| const compilerVersion = longVersion.split("+")[0]; | ||
| compilerLabel = `${compilerType} ${compilerVersion} (Solidity ${solidityVersion})`; | ||
| } | ||
|
|
||
| console.log( | ||
| chalk.bold( | ||
| `Compiled ${rootFiles} Solidity ${pluralize( | ||
| options.scope === "contracts" ? "file" : "test file", | ||
| rootFiles, | ||
| )} with solc ${solcVersion}`, | ||
| )} with ${compilerLabel}`, | ||
| ), | ||
| `(evm target: ${evmVersion})`, | ||
| ); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| import type { SolidityHooks } from "../../../../types/hooks.js"; | ||
|
|
||
| import { downloadSolcCompilersHandler } from "../solidity-hooks.js"; | ||
|
|
||
| export default async (): Promise<Partial<SolidityHooks>> => ({ | ||
| downloadCompilers: async (_context, compilerConfigs, quiet) => { | ||
| await downloadSolcCompilersHandler(compilerConfigs, quiet); | ||
| }, | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -41,6 +41,7 @@ const hardhatPlugin: HardhatPlugin = { | |
| hookHandlers: { | ||
| config: () => import("./hook-handlers/config.js"), | ||
| hre: () => import("./hook-handlers/hre.js"), | ||
| solidity: () => import("./hook-handlers/solidity.js"), | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a surprisingly nice pattern |
||
| }, | ||
| tasks: [ | ||
| { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,53 @@ | ||
| import type { SolidityCompilerConfig } from "../../../types/config.js"; | ||
| import type { Compiler } from "../../../types/solidity.js"; | ||
|
|
||
| import { isSolcSolidityCompilerConfig } from "./build-system/build-system.js"; | ||
| import { | ||
| downloadSolcCompilers, | ||
| getCompiler, | ||
| } from "./build-system/compiler/index.js"; | ||
|
|
||
| /** | ||
| * Downloads solc compilers for the given configs, filtering out non-solc types. | ||
| * This is the default implementation of the `downloadCompilers` hook handler. | ||
| */ | ||
| export async function downloadSolcCompilersHandler( | ||
| compilerConfigs: SolidityCompilerConfig[], | ||
| quiet: boolean, | ||
| ): Promise<void> { | ||
| const solcVersions = new Set( | ||
| compilerConfigs.filter(isSolcSolidityCompilerConfig).map((c) => c.version), | ||
| ); | ||
|
|
||
| if (solcVersions.size > 0) { | ||
| await downloadSolcCompilers(solcVersions, quiet); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Resolves the preferWasm setting for a given compiler config, falling back | ||
| * to the build profile's preferWasm if not set on the compiler. | ||
| */ | ||
| export function resolvePreferWasm( | ||
| compilerConfig: SolidityCompilerConfig, | ||
| buildProfilePreferWasm: boolean, | ||
| ): boolean { | ||
| if (isSolcSolidityCompilerConfig(compilerConfig)) { | ||
| return compilerConfig.preferWasm ?? buildProfilePreferWasm; | ||
| } | ||
| return false; | ||
| } | ||
|
|
||
| /** | ||
| * Creates a solc Compiler for the given config. This is the default | ||
| * implementation used as the fallback in the `getCompiler` hook chain. | ||
| */ | ||
| export async function getSolcCompilerForConfig( | ||
| compilerConfig: SolidityCompilerConfig, | ||
| buildProfilePreferWasm: boolean, | ||
| ): Promise<Compiler> { | ||
| return getCompiler(compilerConfig.version, { | ||
| preferWasm: resolvePreferWasm(compilerConfig, buildProfilePreferWasm), | ||
| compilerPath: compilerConfig.path, | ||
| }); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -24,6 +24,7 @@ import { | |
| } from "@nomicfoundation/hardhat-utils/fs"; | ||
|
|
||
| import { SolidityBuildSystemImplementation } from "../../../../../src/internal/builtin-plugins/solidity/build-system/build-system.js"; | ||
| import createSolidityHookHandlers from "../../../../../src/internal/builtin-plugins/solidity/hook-handlers/solidity.js"; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Small tip: Moving the implementation of the handler into a separate file makes the test nicer. The way we tend to think about it is this:
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For example, this test file has |
||
| import { HookManagerImplementation } from "../../../../../src/internal/core/hook-manager.js"; | ||
|
|
||
| async function emitArtifacts(solidity: SolidityBuildSystem): Promise<void> { | ||
|
|
@@ -107,6 +108,7 @@ describe( | |
| const hooks = new HookManagerImplementation(process.cwd(), []); | ||
| // eslint-disable-next-line @typescript-eslint/consistent-type-assertions -- We don't care about hooks in this context | ||
| hooks.setContext({} as HookContext); | ||
| hooks.registerHandlers("solidity", await createSolidityHookHandlers()); | ||
| solidity = new SolidityBuildSystemImplementation(hooks, { | ||
| solidityConfig, | ||
| projectRoot: process.cwd(), | ||
|
|
@@ -130,6 +132,7 @@ describe( | |
| const hooks = new HookManagerImplementation(process.cwd(), []); | ||
| // eslint-disable-next-line @typescript-eslint/consistent-type-assertions -- We don't care about hooks in this context | ||
| hooks.setContext({} as HookContext); | ||
| hooks.registerHandlers("solidity", await createSolidityHookHandlers()); | ||
| solidity = new SolidityBuildSystemImplementation(hooks, { | ||
| solidityConfig, | ||
| projectRoot: process.cwd(), | ||
|
|
||
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 would say that this is an outdated assumption. BuildInfos should include the compiler type, so we should run the same download logic here.
See this change where I added
compilerTypetoSolidityBuildInfo: 6ed3056There 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 could create a partial SolidityCompilerConfig like this:
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.
There's no way to know if the buildinfo was built with wasm solc, but that's ok. We were already doing
preferWasm: falsebelow