Repository: gakonst/ethers-rs
Pull request #623 (2021-11-26 01:23:57)

fix: pass partial artifact cache to project compiler output
## Motivation

If the cache contains some artifacts, but not all, and there are changed files, then the project compiler output does not contain the cached artifacts.

This means when calling `find` or `into_artifacts`, the cached artifacts would not be present in `artifacts`. 

For example, if you run a compile, then create a new contract and run compile again, the `ProjectCompileOutput` 
will not contain the cached artifacts and you have read the cache manually in order to get them. The provided
test covers this scenario. 

This also results in some strange behavior when you run compile multiple times, as the previous cache is overwritten
with a new partial cache covering only the new files.

This originated from using foundry and seeing the above issue when running a newly created test and having it flip flop
between the old and new tests.
If I misunderstood how this works, or this is the intended behavior of the library and if cache reading should be done manually, then feel free to close this PR.

## Solution

If any cached artifacts exists, they are now returned as part of the project compiler output. Any missing files are removed from the cache.

## PR Checklist

- [x] Added Tests
- [x] Updated the changelog


mattsse: > If the cache contains some artifacts, but not all, and there are changed files, then the project compiler output does not contain the cached artifacts.

That sounds right to me, that means, in that case, we need to merge the compiler output with the cached artifacts.



Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 6385b314..29c82cf7 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -9,6 +9,9 @@
   [597](https://github.com/gakonst/ethers-rs/pull/597)
 - Implement hex display format for `ethers::core::Bytes` [#624](https://github.com/gakonst/ethers-rs/pull/624).
 
+## ethers-solc
+- Return cached artifacts from project `compile` when the cache only contains some files
+
 ### Unreleased
 
 ### 0.6.0
diff --git a/ethers-solc/src/lib.rs b/ethers-solc/src/lib.rs
index d678521b..38644689 100644
--- a/ethers-solc/src/lib.rs
+++ b/ethers-solc/src/lib.rs
@@ -1,4 +1,4 @@
-#![doc = include_str!("../README.md")]
+#![doc = include_str ! ("../README.md")]
 
 pub mod artifacts;
 
@@ -8,9 +8,11 @@ use std::collections::btree_map::Entry;
 pub mod cache;
 
 mod compile;
+
 pub use compile::*;
 
 mod config;
+
 pub use config::{
     AllowedLibPaths, Artifact, ArtifactOutput, MinimalCombinedArtifacts, ProjectPathsConfig,
     SolcConfig,
@@ -22,6 +24,7 @@ use crate::{artifacts::Source, cache::SolFilesCache};
 
 pub mod error;
 pub mod utils;
+
 use crate::artifacts::Sources;
 use error::Result;
 use std::{
@@ -248,22 +251,28 @@ impl<Artifacts: ArtifactOutput> Project<Artifacts> {
         }
 
         // If there's a cache set, filter to only re-compile the files which were changed
-        let sources = if self.cached && self.paths.cache.exists() {
-            let cache = SolFilesCache::read(&self.paths.cache)?;
+        let (sources, cached_artifacts) = if self.cached && self.paths.cache.exists() {
+            let mut cache = SolFilesCache::read(&self.paths.cache)?;
+            cache.remove_missing_files();
             let changed_files = cache.get_changed_or_missing_artifacts_files::<Artifacts>(
                 sources,
                 Some(&self.solc_config),
                 &self.paths.artifacts,
             );
 
+            let cached_artifacts = if self.paths.artifacts.exists() {
+                cache.read_artifacts::<Artifacts>(&self.paths.artifacts)?
+            } else {
+                BTreeMap::default()
+            };
             // if nothing changed and all artifacts still exist
             if changed_files.is_empty() {
-                let artifacts = cache.read_artifacts::<Artifacts>(&self.paths.artifacts)?;
-                return Ok(ProjectCompileOutput::from_unchanged(artifacts))
+                return Ok(ProjectCompileOutput::from_unchanged(cached_artifacts))
             }
-            changed_files
+            // There are changed files and maybe some cached files
+            (changed_files, cached_artifacts)
         } else {
-            sources
+            (sources, BTreeMap::default())
         };
 
         // replace absolute path with source name to make solc happy
@@ -303,7 +312,11 @@ impl<Artifacts: ArtifactOutput> Project<Artifacts> {
         if !self.no_artifacts {
             Artifacts::on_output(&output, &self.paths)?;
         }
-        Ok(ProjectCompileOutput::from_compiler_output(output, self.ignored_error_codes.clone()))
+        Ok(ProjectCompileOutput::from_compiler_output_and_cache(
+            output,
+            cached_artifacts,
+            self.ignored_error_codes.clone(),
+        ))
     }
 }
 
@@ -512,6 +525,14 @@ impl<T: ArtifactOutput> ProjectCompileOutput<T> {
         }
     }
 
+    pub fn from_compiler_output_and_cache(
+        compiler_output: CompilerOutput,
+        cache: BTreeMap<PathBuf, T::Artifact>,
+        ignored_error_codes: Vec<u64>,
+    ) -> Self {
+        Self { compiler_output: Some(compiler_output), artifacts: cache, ignored_error_codes }
+    }
+
     /// Get the (merged) solc compiler output
     /// ```no_run
     /// use std::collections::BTreeMap;
@@ -648,7 +669,6 @@ impl<T: ArtifactOutput> fmt::Display for ProjectCompileOutput<T> {
 
 #[cfg(test)]
 mod tests {
-
     #[test]
     #[cfg(all(feature = "svm", feature = "async"))]
     fn test_build_all_versions() {


Test patch:
diff --git a/ethers-solc/test-data/cache-sample/NewContract.sol b/ethers-solc/test-data/cache-sample/NewContract.sol
new file mode 100644
index 00000000..8c5031e8
--- /dev/null
+++ b/ethers-solc/test-data/cache-sample/NewContract.sol
@@ -0,0 +1,5 @@
+// SPDX-License-Identifier: GPL-3.0-or-later
+pragma solidity >=0.6.6;
+
+contract NewContract {
+}
diff --git a/ethers-solc/tests/project.rs b/ethers-solc/tests/project.rs
index b5d90b7b..6caebe3e 100644
--- a/ethers-solc/tests/project.rs
+++ b/ethers-solc/tests/project.rs
@@ -1,7 +1,10 @@
 //! project tests
 
 use ethers_solc::{cache::SOLIDITY_FILES_CACHE_FILENAME, Project, ProjectPathsConfig};
-use std::path::PathBuf;
+use std::{
+    io,
+    path::{Path, PathBuf},
+};
 use tempdir::TempDir;
 
 #[test]
@@ -75,3 +78,67 @@ fn can_compile_dapp_sample() {
     assert!(compiled.find("Dapp").is_some());
     assert!(!compiled.is_unchanged());
 }
+
+#[test]
+fn can_compile_dapp_sample_with_cache() {
+    let tmp_dir = TempDir::new("root").unwrap();
+    let root = tmp_dir.path();
+    let cache = root.join("cache").join(SOLIDITY_FILES_CACHE_FILENAME);
+    let artifacts = root.join("out");
+
+    let manifest_dir = PathBuf::from(env!("CARGO_MANIFEST_DIR"));
+    let orig_root = manifest_dir.join("test-data/dapp-sample");
+    let new_file = manifest_dir.join("test-data/cache-sample/NewContract.sol");
+    copy_dir_all(orig_root, &tmp_dir).unwrap();
+    let paths = ProjectPathsConfig::builder()
+        .cache(cache)
+        .sources(root.join("src"))
+        .artifacts(artifacts)
+        .lib(root.join("lib"))
+        .root(root)
+        .build()
+        .unwrap();
+
+    // first compile
+    let project = Project::builder().paths(paths).build().unwrap();
+    let compiled = project.compile().unwrap();
+    assert!(compiled.find("Dapp").is_some());
+    assert!(!compiled.has_compiler_errors());
+
+    // cache is used when nothing to compile
+    let compiled = project.compile().unwrap();
+    assert!(compiled.find("Dapp").is_some());
+    assert!(compiled.is_unchanged());
+
+    // deleted artifacts cause recompile even with cache
+    std::fs::remove_dir_all(&project.paths.artifacts).unwrap();
+    let compiled = project.compile().unwrap();
+    assert!(compiled.find("Dapp").is_some());
+    assert!(!compiled.is_unchanged());
+
+    // new file is compiled even with partial cache
+    std::fs::copy(new_file, root.join("src/NewContract.sol")).unwrap();
+    let compiled = project.compile().unwrap();
+    assert!(compiled.find("Dapp").is_some());
+    assert!(compiled.find("NewContract").is_some());
+    assert!(!compiled.is_unchanged());
+
+    // deleted artifact is not taken from the cache
+    std::fs::remove_file(&project.paths.sources.join("Dapp.sol")).unwrap();
+    let compiled = project.compile().unwrap();
+    assert!(compiled.find("Dapp").is_none());
+}
+
+fn copy_dir_all(src: impl AsRef<Path>, dst: impl AsRef<Path>) -> io::Result<()> {
+    std::fs::create_dir_all(&dst)?;
+    for entry in std::fs::read_dir(src)? {
+        let entry = entry?;
+        let ty = entry.file_type()?;
+        if ty.is_dir() {
+            copy_dir_all(entry.path(), dst.as_ref().join(entry.file_name()))?;
+        } else {
+            std::fs::copy(entry.path(), dst.as_ref().join(entry.file_name()))?;
+        }
+    }
+    Ok(())
+}