fix: pass partial artifact cache to project compiler output #623
Conversation
If the cache contains some artifacts but not all, the project compiler output now contains the cached artifacts in addition to the newly compiled artifacts.
That sounds right to me, that means, in that case, we need to merge the compiler output with the cached artifacts. |
| std::fs::remove_file(&project.paths.sources.join("Dapp.sol")).unwrap(); | ||
| let compiled = project.compile().unwrap(); | ||
| assert!(compiled.find("Dapp").is_none()); |
There was a problem hiding this comment.
this verifies that we clear deleted files from the 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()); |
There was a problem hiding this comment.
This hits from_compiler_output_and_cache since we have a cached artifact but need to compile the NewContract.
lgtm
ethers-solc/src/lib.rs
Outdated
| let cached_artifacts = cache | ||
| .read_artifacts::<Artifacts>(&self.paths.artifacts) | ||
| .unwrap_or_else(|_| BTreeMap::default()); |
There was a problem hiding this comment.
This is unlikely to fail since we checked that those files exists, but could still fail due to some race conditions, like the files got wiped after we checked that they exist...
I feel we should propagate the error here because if that would fail, we end up with a successful output that lacks some artifacts.
There was a problem hiding this comment.
The main failure case (outside of a race) for the fallback was when the artifacts folder is deleted, but the cache metadata still exists.
I'll change it to propagate the error, and fall back to empty cache if there is no artifacts folder. This way if some of the files have been wiped, it'll return the error. There's still a chance that the folder gets deleted at the same time still and then there'd be missing artifacts.
ghost
deleted the
ghost
mentioned this pull request
1. Introduces ethers-solc tracing
2. Fixes various ethers-solc caching bugs
gakonst/ethers-rs#623
gakonst/ethers-rs#629
gakonst/ethers-rs#630
1. Introduces ethers-solc tracing
2. Fixes various ethers-solc caching bugs
gakonst/ethers-rs#623
gakonst/ethers-rs#629
gakonst/ethers-rs#630
* Use relative path for git commands in install cmd * Simplify function signatures in `forge install`
1. Introduces ethers-solc tracing
2. Fixes various ethers-solc caching bugs
gakonst/ethers-rs#623
gakonst/ethers-rs#629
gakonst/ethers-rs#630
1. Introduces ethers-solc tracing
2. Fixes various ethers-solc caching bugs
gakonst/ethers-rs#623
gakonst/ethers-rs#629
gakonst/ethers-rs#630
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
findorinto_artifacts, the cached artifacts would not be present inartifacts.For example, if you run a compile, then create a new contract and run compile again, the
ProjectCompileOutputwill 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