fix(solc): use cache context when determining artifact files

CHANGELOG.md

@@ -117,6 +117,8 @@
 
 ### Unreleased
 
+- Add `OutputContext` to `ArtifactOutput` trait
+  [#1621](https://github.com/gakonst/ethers-rs/pull/1621)
 - On windows all paths in the `ProjectCompilerOutput` are now slashed by default
   [#1540](https://github.com/gakonst/ethers-rs/pull/1540)
 - `ArtifactOutput::write_extras` now takes the `Artifacts` directly

ethers-solc/src/artifact_output/mod.rs

@@ -10,7 +10,7 @@ use crate::{
     error::Result,
     sourcemap::{SourceMap, SyntaxError},
     sources::VersionedSourceFile,
-    utils, HardhatArtifact, ProjectPathsConfig, SolcError,
+    utils, HardhatArtifact, ProjectPathsConfig, SolFilesCache, SolcError,
 };
 use ethers_core::{abi::Abi, types::Bytes};
 use semver::Version;
@@ -23,7 +23,7 @@ use std::{
     ops::Deref,
     path::{Path, PathBuf},
 };
-use tracing::trace;
+use tracing::{error, trace};
 
 mod configurable;
 use crate::contracts::VersionedContract;
@@ -98,6 +98,7 @@ pub struct ArtifactFile<T> {
 impl<T: Serialize> ArtifactFile<T> {
     /// Writes the given contract to the `out` path creating all parent directories
     pub fn write(&self) -> Result<()> {
+        trace!("writing artifact file {:?} {}", self.file, self.version);
         utils::create_parent_dir_all(&self.file)?;
         fs::write(&self.file, serde_json::to_vec_pretty(&self.artifact)?)
             .map_err(|err| SolcError::io(err, &self.file))?;
@@ -574,8 +575,9 @@ pub trait ArtifactOutput {
         contracts: &VersionedContracts,
         sources: &VersionedSourceFiles,
         layout: &ProjectPathsConfig,
+        ctx: OutputContext,
     ) -> Result<Artifacts<Self::Artifact>> {
-        let mut artifacts = self.output_to_artifacts(contracts, sources);
+        let mut artifacts = self.output_to_artifacts(contracts, sources, ctx);
         artifacts.join_all(&layout.artifacts);
         artifacts.write_all()?;
 
@@ -792,14 +794,15 @@ pub trait ArtifactOutput {
         &self,
         contracts: &VersionedContracts,
         sources: &VersionedSourceFiles,
+        ctx: OutputContext,
     ) -> Artifacts<Self::Artifact> {
         let mut artifacts = ArtifactsMap::new();
 
         // this tracks all the `SourceFile`s that we successfully mapped to a contract
         let mut non_standalone_sources = HashSet::new();
 
         // this holds all output files and the contract(s) it belongs to
-        let artifact_files = contracts.artifact_files::<Self>();
+        let artifact_files = contracts.artifact_files::<Self>(&ctx);
 
         // this tracks the final artifacts, which we use as lookup for checking conflicts when
         // converting stand-alone artifacts in the next step
@@ -932,6 +935,46 @@ pub trait ArtifactOutput {
     ) -> Option<Self::Artifact>;
 }
 
+/// Additional context to use during [`ArtifactOutput::on_output()`]
+#[derive(Debug, Clone, Default)]
+#[non_exhaustive]
+pub struct OutputContext<'a> {
+    /// Cache file of the project or empty if no caching is enabled
+    ///
+    /// This context is required for partially cached recompile with conflicting files, so that we
+    /// can use the same adjusted output path for conflicting files like:
+    ///
+    /// ```text
+    /// src
+    /// ├── a.sol
+    /// └── inner
+    ///     └── a.sol
+    /// ```
+    pub cache: Cow<'a, SolFilesCache>,
+}
+
+// === impl OutputContext
+
+impl<'a> OutputContext<'a> {
+    /// Create a new context with the given cache file
+    pub fn new(cache: &'a SolFilesCache) -> Self {
+        Self { cache: Cow::Borrowed(cache) }
+    }
+
+    /// Returns the path of the already existing artifact for the `contract` of the `file` compiled
+    /// with the `version`.
+    ///
+    /// Returns `None` if no file exists
+    pub fn existing_artifact(
+        &self,
+        file: impl AsRef<Path>,
+        contract: &str,
+        version: &Version,
+    ) -> Option<&PathBuf> {
+        self.cache.entry(file)?.find_artifact(contract, version)
+    }
+}
+
 /// An `Artifact` implementation that uses a compact representation
 ///
 /// Creates a single json artifact with
@@ -984,8 +1027,9 @@ impl ArtifactOutput for MinimalCombinedArtifactsHardhatFallback {
         output: &VersionedContracts,
         sources: &VersionedSourceFiles,
         layout: &ProjectPathsConfig,
+        ctx: OutputContext,
     ) -> Result<Artifacts<Self::Artifact>> {
-        MinimalCombinedArtifacts::default().on_output(output, sources, layout)
+        MinimalCombinedArtifacts::default().on_output(output, sources, layout, ctx)
     }
 
     fn read_cached_artifact(path: impl AsRef<Path>) -> Result<Self::Artifact> {
@@ -994,10 +1038,10 @@ impl ArtifactOutput for MinimalCombinedArtifactsHardhatFallback {
         if let Ok(a) = serde_json::from_str(&content) {
             Ok(a)
         } else {
-            tracing::error!("Failed to deserialize compact artifact");
-            tracing::trace!("Fallback to hardhat artifact deserialization");
+            error!("Failed to deserialize compact artifact");
+            trace!("Fallback to hardhat artifact deserialization");
             let artifact = serde_json::from_str::<HardhatArtifact>(&content)?;
-            tracing::trace!("successfully deserialized hardhat artifact");
+            trace!("successfully deserialized hardhat artifact");
             Ok(artifact.into_contract_bytecode())
         }
     }

ethers-solc/src/cache.rs

@@ -5,8 +5,8 @@ use crate::{
     error::{Result, SolcError},
     filter::{FilteredSource, FilteredSourceInfo, FilteredSources},
     resolver::GraphEdges,
-    utils, ArtifactFile, ArtifactOutput, Artifacts, ArtifactsMap, Project, ProjectPathsConfig,
-    Source,
+    utils, ArtifactFile, ArtifactOutput, Artifacts, ArtifactsMap, OutputContext, Project,
+    ProjectPathsConfig, Source,
 };
 use semver::Version;
 use serde::{de::DeserializeOwned, Deserialize, Serialize};
@@ -523,6 +523,11 @@ impl CacheEntry {
         self.artifacts.values().flat_map(|artifacts| artifacts.iter())
     }
 
+    /// Returns the artifact file for the contract and version pair
+    pub fn find_artifact(&self, contract: &str, version: &Version) -> Option<&PathBuf> {
+        self.artifacts.get(contract).and_then(|files| files.get(version))
+    }
+
     /// Iterator that yields all artifact files and their version
     pub fn artifacts_for_version<'a>(
         &'a self,
@@ -864,6 +869,13 @@ impl<'a, T: ArtifactOutput> ArtifactsCache<'a, T> {
         }
     }
 
+    pub fn output_ctx(&self) -> OutputContext {
+        match self {
+            ArtifactsCache::Ephemeral(_, _) => Default::default(),
+            ArtifactsCache::Cached(inner) => OutputContext::new(&inner.cache),
+        }
+    }
+
     pub fn project(&self) -> &'a Project<T> {
         match self {
             ArtifactsCache::Ephemeral(_, project) => project,
@@ -948,7 +960,8 @@ impl<'a, T: ArtifactOutput> ArtifactsCache<'a, T> {
                                         }).unwrap_or_default();
                                     if !retain {
                                         tracing::trace!(
-                                            "purging obsolete cached artifact for contract {} and version {}",
+                                            "purging obsolete cached artifact {:?} for contract {} and version {}",
+                                            f.file,
                                             name,
                                             f.version
                                         );

ethers-solc/src/compile/output/contracts.rs

@@ -3,11 +3,12 @@ use crate::{
         contract::{CompactContractRef, Contract},
         FileToContractsMap,
     },
-    ArtifactFiles, ArtifactOutput,
+    ArtifactFiles, ArtifactOutput, OutputContext,
 };
 use semver::Version;
 use serde::{Deserialize, Serialize};
 use std::{collections::BTreeMap, ops::Deref, path::Path};
+use tracing::trace;
 
 /// file -> [(contract name  -> Contract + solc version)]
 #[derive(Debug, Clone, PartialEq, Default, Serialize, Deserialize)]
@@ -40,16 +41,38 @@ impl VersionedContracts {
     }
 
     /// Returns all the artifact files mapped with their contracts
-    pub(crate) fn artifact_files<T: ArtifactOutput + ?Sized>(&self) -> ArtifactFiles {
+    ///
+    /// This will compute the appropriate output file paths but will _not_ write them.
+    /// The `ctx` is used to avoid possible conflicts
+    pub(crate) fn artifact_files<T: ArtifactOutput + ?Sized>(
+        &self,
+        ctx: &OutputContext,
+    ) -> ArtifactFiles {
         let mut output_files = ArtifactFiles::with_capacity(self.len());
         for (file, contracts) in self.iter() {
             for (name, versioned_contracts) in contracts {
                 for contract in versioned_contracts {
-                    let output = if versioned_contracts.len() > 1 {
+                    // if an artifact for the contract already exists (from a previous compile job)
+                    // we reuse the path, this will make sure that even if there are conflicting
+                    // files (files for witch `T::output_file()` would return the same path) we use
+                    // consistent output paths
+                    let output = if let Some(existing_artifact) =
+                        ctx.existing_artifact(file, name, &contract.version).cloned()
+                    {
+                        trace!("use existing artifact file {:?}", existing_artifact,);
+                        existing_artifact
+                    } else if versioned_contracts.len() > 1 {
                         T::output_file_versioned(file, name, &contract.version)
                     } else {
                         T::output_file(file, name)
                     };
+
+                    trace!(
+                        "use artifact file {:?} for contract file {} {}",
+                        output,
+                        file,
+                        contract.version
+                    );
                     let contract = (file.as_str(), name.as_str(), contract);
                     output_files.entry(output).or_default().push(contract);
                 }