From 3ed2286e1643dd01e2b3b589add2357d63225b51 Mon Sep 17 00:00:00 2001 From: Arsenii Kulikov Date: Mon, 11 Mar 2024 16:47:41 +0400 Subject: [PATCH] refactor: extra files logic (#88) Resolves https://github.com/foundry-rs/foundry/issues/6241 Removes `write_extra` logic from `ArtifactOutput` trait Adds new methods: - `handle_artifacts` - by default empty implementation which can be overloaded to implement handling of compiled artifacts. `ConfigurableArtifacts` implements it to write extra files - `is_dirty` - method allowing `ArtifactOutput` implementations to reject cached artifacts and enforce recompilation. Used by `ConfigurableArtifacts` to reject artifacts which data is not enough to write required extra output files. - `handle_cached_artifacts` - same as `handle_artifacts` but for those which were not recompiled. Used by `ConfigurableArtifacts` to write extra files from cached artifacts. --- src/artifact_output/configurable.rs | 239 +++++++++++++++++++++++----- src/artifact_output/mod.rs | 44 ++--- src/cache.rs | 12 ++ src/compile/project.rs | 33 +++- src/lib.rs | 16 +- 5 files changed, 267 insertions(+), 77 deletions(-) diff --git a/src/artifact_output/configurable.rs b/src/artifact_output/configurable.rs index 6aab634f..9cc83444 100644 --- a/src/artifact_output/configurable.rs +++ b/src/artifact_output/configurable.rs @@ -16,11 +16,12 @@ use crate::{ BytecodeOutputSelection, ContractOutputSelection, DeployedBytecodeOutputSelection, EvmOutputSelection, EwasmOutputSelection, }, - Ast, CompactContractBytecodeCow, DevDoc, Evm, Ewasm, FunctionDebugData, GasEstimates, - GeneratedSource, LosslessMetadata, Metadata, Offsets, Settings, StorageLayout, UserDoc, + Ast, BytecodeObject, CompactContractBytecodeCow, DevDoc, Evm, Ewasm, FunctionDebugData, + GasEstimates, GeneratedSource, LosslessMetadata, Metadata, Offsets, Settings, + StorageLayout, UserDoc, }, sources::VersionedSourceFile, - Artifact, ArtifactOutput, SolcConfig, SolcError, SourceFile, + utils, Artifact, ArtifactFile, ArtifactOutput, SolcConfig, SolcError, SourceFile, }; use alloy_json_abi::JsonAbi; use alloy_primitives::hex; @@ -233,8 +234,24 @@ impl ConfigurableArtifacts { impl ArtifactOutput for ConfigurableArtifacts { type Artifact = ConfigurableContractArtifact; - fn write_contract_extras(&self, contract: &Contract, file: &Path) -> Result<(), SolcError> { - self.additional_files.write_extras(contract, file) + /// Writes extra files for compiled artifact based on [Self::additional_files] + fn handle_artifacts( + &self, + contracts: &crate::contracts::VersionedContracts, + artifacts: &crate::Artifacts, + ) -> Result<(), SolcError> { + for (file, contracts) in contracts.as_ref().iter() { + for (name, versioned_contracts) in contracts { + for contract in versioned_contracts { + if let Some(artifact) = artifacts.find_artifact(file, name, &contract.version) { + let file = &artifact.file; + utils::create_parent_dir_all(file)?; + self.additional_files.write_extras(&contract.contract, file)?; + } + } + } + } + Ok(()) } fn contract_to_artifact( @@ -370,6 +387,87 @@ impl ArtifactOutput for ConfigurableArtifacts { ..Default::default() }) } + + /// We want to enforce recompilation if artifact is missing data we need for writing extra + /// files. + fn is_dirty(&self, artifact_file: &ArtifactFile) -> Result { + let artifact = &artifact_file.artifact; + let ExtraOutputFiles { + abi: _, + metadata, + ir, + ir_optimized, + ewasm, + assembly, + source_map, + generated_sources, + bytecode: _, + deployed_bytecode: _, + __non_exhaustive: _, + } = self.additional_files; + + if metadata && artifact.metadata.is_none() { + return Ok(true); + } + if ir && artifact.ir.is_none() { + return Ok(true); + } + if ir_optimized && artifact.ir_optimized.is_none() { + return Ok(true); + } + if ewasm && artifact.ewasm.is_none() { + return Ok(true); + } + if assembly && artifact.assembly.is_none() { + return Ok(true); + } + if source_map && artifact.get_source_map_str().is_none() { + return Ok(true); + } + if generated_sources { + // We can't check if generated sources are missing or just empty. + return Ok(true); + } + Ok(false) + } + + /// Writes extra files for cached artifacts based on [Self::additional_files]. + fn handle_cached_artifacts( + &self, + artifacts: &crate::Artifacts, + ) -> Result<(), SolcError> { + for artifacts in artifacts.values() { + for artifacts in artifacts.values() { + for artifact_file in artifacts { + let file = &artifact_file.file; + let artifact = &artifact_file.artifact; + self.additional_files.process_abi(artifact.abi.as_ref(), file)?; + self.additional_files.process_assembly(artifact.assembly.as_deref(), file)?; + self.additional_files + .process_bytecode(artifact.bytecode.as_ref().map(|b| &b.object), file)?; + self.additional_files.process_deployed_bytecode( + artifact + .deployed_bytecode + .as_ref() + .and_then(|d| d.bytecode.as_ref()) + .map(|b| &b.object), + file, + )?; + self.additional_files + .process_generated_sources(Some(&artifact.generated_sources), file)?; + self.additional_files.process_ir(artifact.ir.as_deref(), file)?; + self.additional_files + .process_ir_optimized(artifact.ir_optimized.as_deref(), file)?; + self.additional_files.process_ewasm(artifact.ewasm.as_ref(), file)?; + self.additional_files.process_metadata(artifact.metadata.as_ref(), file)?; + self.additional_files + .process_source_map(artifact.get_source_map_str().as_deref(), file)?; + } + } + } + + Ok(()) + } } /// Determines the additional values to include in the contract's artifact file @@ -596,90 +694,149 @@ impl ExtraOutputFiles { config } - /// Write the set values as separate files - pub fn write_extras(&self, contract: &Contract, file: &Path) -> Result<(), SolcError> { + fn process_abi(&self, abi: Option<&JsonAbi>, file: &Path) -> Result<(), SolcError> { if self.abi { - if let Some(ref abi) = contract.abi { + if let Some(abi) = abi { let file = file.with_extension("abi.json"); fs::write(&file, serde_json::to_string_pretty(abi)?) .map_err(|err| SolcError::io(err, file))? } } + Ok(()) + } + fn process_metadata(&self, metadata: Option<&Metadata>, file: &Path) -> Result<(), SolcError> { if self.metadata { - if let Some(ref metadata) = contract.metadata { + if let Some(metadata) = metadata { let file = file.with_extension("metadata.json"); - fs::write(&file, serde_json::to_string_pretty(&metadata.raw_json()?)?) + fs::write(&file, serde_json::to_string_pretty(metadata)?) .map_err(|err| SolcError::io(err, file))? } } + Ok(()) + } - if self.ir_optimized { - if let Some(ref iropt) = contract.ir_optimized { - let file = file.with_extension("iropt"); - fs::write(&file, iropt).map_err(|err| SolcError::io(err, file))? - } - } - + fn process_ir(&self, ir: Option<&str>, file: &Path) -> Result<(), SolcError> { if self.ir { - if let Some(ref ir) = contract.ir { + if let Some(ir) = ir { let file = file.with_extension("ir"); fs::write(&file, ir).map_err(|err| SolcError::io(err, file))? } } + Ok(()) + } + + fn process_ir_optimized( + &self, + ir_optimized: Option<&str>, + file: &Path, + ) -> Result<(), SolcError> { + if self.ir_optimized { + if let Some(ir_optimized) = ir_optimized { + let file = file.with_extension("iropt"); + fs::write(&file, ir_optimized).map_err(|err| SolcError::io(err, file))? + } + } + Ok(()) + } + fn process_ewasm(&self, ewasm: Option<&Ewasm>, file: &Path) -> Result<(), SolcError> { if self.ewasm { - if let Some(ref ewasm) = contract.ewasm { + if let Some(ewasm) = ewasm { let file = file.with_extension("ewasm"); fs::write(&file, serde_json::to_vec_pretty(ewasm)?) .map_err(|err| SolcError::io(err, file))?; } } + Ok(()) + } + fn process_assembly(&self, asm: Option<&str>, file: &Path) -> Result<(), SolcError> { if self.assembly { - if let Some(ref evm) = contract.evm { - if let Some(ref asm) = evm.assembly { - let file = file.with_extension("asm"); - fs::write(&file, asm).map_err(|err| SolcError::io(err, file))? - } + if let Some(asm) = asm { + let file = file.with_extension("asm"); + fs::write(&file, asm).map_err(|err| SolcError::io(err, file))? } } + Ok(()) + } + fn process_generated_sources( + &self, + generated_sources: Option<&Vec>, + file: &Path, + ) -> Result<(), SolcError> { if self.generated_sources { - if let Some(ref evm) = contract.evm { - if let Some(ref bytecode) = evm.bytecode { - let file = file.with_extension("gensources"); - fs::write(&file, serde_json::to_vec_pretty(&bytecode.generated_sources)?) - .map_err(|err| SolcError::io(err, file))?; - } + if let Some(generated_sources) = generated_sources { + let file = file.with_extension("gensources"); + fs::write(&file, serde_json::to_vec_pretty(generated_sources)?) + .map_err(|err| SolcError::io(err, file))?; } } + Ok(()) + } + fn process_source_map(&self, source_map: Option<&str>, file: &Path) -> Result<(), SolcError> { if self.source_map { - if let Some(ref evm) = contract.evm { - if let Some(ref bytecode) = evm.bytecode { - if let Some(ref sourcemap) = bytecode.source_map { - let file = file.with_extension("sourcemap"); - fs::write(&file, sourcemap).map_err(|err| SolcError::io(err, file))? - } - } + if let Some(source_map) = source_map { + let file = file.with_extension("sourcemap"); + fs::write(&file, source_map).map_err(|err| SolcError::io(err, file))? } } + Ok(()) + } + fn process_bytecode( + &self, + bytecode: Option<&BytecodeObject>, + file: &Path, + ) -> Result<(), SolcError> { if self.bytecode { - if let Some(ref code) = contract.get_bytecode_bytes() { - let code = hex::encode(code.as_ref()); + if let Some(bytecode) = bytecode { + let code = hex::encode(bytecode.as_ref()); let file = file.with_extension("bin"); fs::write(&file, code).map_err(|err| SolcError::io(err, file))? } } + Ok(()) + } + + fn process_deployed_bytecode( + &self, + deployed: Option<&BytecodeObject>, + file: &Path, + ) -> Result<(), SolcError> { if self.deployed_bytecode { - if let Some(ref code) = contract.get_deployed_bytecode_bytes() { - let code = hex::encode(code.as_ref()); + if let Some(deployed) = deployed { + let code = hex::encode(deployed.as_ref()); let file = file.with_extension("deployed-bin"); fs::write(&file, code).map_err(|err| SolcError::io(err, file))? } } + Ok(()) + } + + /// Write the set values as separate files + pub fn write_extras(&self, contract: &Contract, file: &Path) -> Result<(), SolcError> { + self.process_abi(contract.abi.as_ref(), file)?; + self.process_metadata(contract.metadata.as_ref().map(|m| &m.metadata), file)?; + self.process_ir(contract.ir.as_deref(), file)?; + self.process_ir_optimized(contract.ir_optimized.as_deref(), file)?; + self.process_ewasm(contract.ewasm.as_ref(), file)?; + + let evm = contract.evm.as_ref(); + self.process_assembly(evm.and_then(|evm| evm.assembly.as_deref()), file)?; + + let bytecode = evm.and_then(|evm| evm.bytecode.as_ref()); + self.process_generated_sources(bytecode.map(|b| &b.generated_sources), file)?; + + let deployed_bytecode = evm.and_then(|evm| evm.deployed_bytecode.as_ref()); + self.process_source_map(bytecode.and_then(|b| b.source_map.as_deref()), file)?; + self.process_bytecode(bytecode.map(|b| &b.object), file)?; + self.process_deployed_bytecode( + deployed_bytecode.and_then(|d| d.bytecode.as_ref()).map(|b| &b.object), + file, + )?; Ok(()) } diff --git a/src/artifact_output/mod.rs b/src/artifact_output/mod.rs index 6611ae50..3f54210c 100644 --- a/src/artifact_output/mod.rs +++ b/src/artifact_output/mod.rs @@ -623,41 +623,17 @@ pub trait ArtifactOutput { artifacts.join_all(&layout.artifacts); artifacts.write_all()?; - self.write_extras(contracts, &artifacts)?; + self.handle_artifacts(contracts, &artifacts)?; Ok(artifacts) } - /// Write additional files for the contract - fn write_contract_extras(&self, contract: &Contract, file: &Path) -> Result<()> { - ExtraOutputFiles::all().write_extras(contract, file) - } - - /// Writes additional files for the contracts if the included in the `Contract`, such as `ir`, - /// `ewasm`, `iropt`. - /// - /// By default, these fields are _not_ enabled in the [`crate::artifacts::Settings`], see - /// [`crate::artifacts::output_selection::OutputSelection::default_output_selection()`], and the - /// respective fields of the [`Contract`] will `None`. If they'll be manually added to the - /// `output_selection`, then we're also creating individual files for this output, such as - /// `Greeter.iropt`, `Gretter.ewasm` - fn write_extras( + /// Invoked after artifacts has been written to disk for additional processing. + fn handle_artifacts( &self, - contracts: &VersionedContracts, - artifacts: &Artifacts, + _contracts: &VersionedContracts, + _artifacts: &Artifacts, ) -> Result<()> { - for (file, contracts) in contracts.as_ref().iter() { - for (name, versioned_contracts) in contracts { - for c in versioned_contracts { - if let Some(artifact) = artifacts.find_artifact(file, name, &c.version) { - let file = &artifact.file; - utils::create_parent_dir_all(file)?; - self.write_contract_extras(&c.contract, file)?; - } - } - } - } - Ok(()) } @@ -985,6 +961,16 @@ pub trait ArtifactOutput { _path: &str, _file: &VersionedSourceFile, ) -> Option; + + /// Handler allowing artifacts handler to enforce artifact recompilation. + fn is_dirty(&self, _artifact_file: &ArtifactFile) -> Result { + Ok(false) + } + + /// Invoked with all artifacts that were not recompiled. + fn handle_cached_artifacts(&self, _artifacts: &Artifacts) -> Result<()> { + Ok(()) + } } /// Additional context to use during [`ArtifactOutput::on_output()`] diff --git a/src/cache.rs b/src/cache.rs index 66d9099f..0cbe48ef 100644 --- a/src/cache.rs +++ b/src/cache.rs @@ -804,6 +804,18 @@ impl<'a, T: ArtifactOutput> ArtifactsCacheInner<'a, T> { return true; } + // If any requested extra files are missing for any artifact, mark source as dirty to + // generate them + for artifacts in self.cached_artifacts.values() { + for artifacts in artifacts.values() { + for artifact_file in artifacts { + if self.project.artifacts_handler().is_dirty(artifact_file).unwrap_or(true) { + return true; + } + } + } + } + // all things match, can be reused false } diff --git a/src/compile/project.rs b/src/compile/project.rs index e3fb7c60..34a4f4d6 100644 --- a/src/compile/project.rs +++ b/src/compile/project.rs @@ -329,6 +329,13 @@ impl<'a, T: ArtifactOutput> CompiledState<'a, T> { ctx, )?; + match cache { + ArtifactsCache::Cached(ref cache) => { + project.artifacts_handler().handle_cached_artifacts(&cache.cached_artifacts)?; + } + ArtifactsCache::Ephemeral(..) => {} + } + // emits all the build infos, if they exist output.write_build_infos(project.build_info_path())?; @@ -679,7 +686,10 @@ fn compile_parallel( #[cfg(all(feature = "project-util", feature = "svm-solc"))] mod tests { use super::*; - use crate::{project_util::TempProject, MinimalCombinedArtifacts}; + use crate::{ + artifacts::output_selection::ContractOutputSelection, project_util::TempProject, + ConfigurableArtifacts, MinimalCombinedArtifacts, + }; fn init_tracing() { let _ = tracing_subscriber::fmt() @@ -832,4 +842,25 @@ mod tests { let compiler = ProjectCompiler::new(&project).unwrap(); let _out = compiler.compile().unwrap(); } + + #[test] + fn extra_output_cached() { + let root = PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("test-data/dapp-sample"); + let paths = ProjectPathsConfig::builder().sources(root.join("src")).lib(root.join("lib")); + let mut project = TempProject::::new(paths.clone()).unwrap(); + + // Compile once without enabled extra output + project.compile().unwrap(); + + // Enable extra output of abi + project.project_mut().artifacts = + ConfigurableArtifacts::new([], [ContractOutputSelection::Abi]); + + // Ensure that abi appears after compilation and that we didn't recompile anything + let abi_path = project.project().paths.artifacts.join("Dapp.sol/Dapp.abi.json"); + assert!(!abi_path.exists()); + let output = project.compile().unwrap(); + assert!(output.compiler_output.is_empty()); + assert!(abi_path.exists()); + } } diff --git a/src/lib.rs b/src/lib.rs index d6259f0a..af485256 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -904,16 +904,12 @@ impl ArtifactOutput for Project { self.artifacts_handler().on_output(contracts, sources, layout, ctx) } - fn write_contract_extras(&self, contract: &Contract, file: &Path) -> Result<()> { - self.artifacts_handler().write_contract_extras(contract, file) - } - - fn write_extras( + fn handle_artifacts( &self, contracts: &VersionedContracts, artifacts: &Artifacts, ) -> Result<()> { - self.artifacts_handler().write_extras(contracts, artifacts) + self.artifacts_handler().handle_artifacts(contracts, artifacts) } fn output_file_name(name: impl AsRef) -> PathBuf { @@ -987,6 +983,14 @@ impl ArtifactOutput for Project { ) -> Option { self.artifacts_handler().standalone_source_file_to_artifact(path, file) } + + fn is_dirty(&self, artifact_file: &ArtifactFile) -> Result { + self.artifacts_handler().is_dirty(artifact_file) + } + + fn handle_cached_artifacts(&self, artifacts: &Artifacts) -> Result<()> { + self.artifacts_handler().handle_cached_artifacts(artifacts) + } } // Rebases the given path to the base directory lexically.