fix(solc): consider case sensitive conflicting artifact paths (#1625)
* fix(solc): consider case sensitive conflicting artifact paths * chore: fmt Co-authored-by: Georgios Konstantopoulos <me@gakonst.com>
This commit is contained in:
parent
da8c70ab7f
commit
0707270a05
|
@ -0,0 +1,60 @@
|
|||
use crate::contracts::VersionedContract;
|
||||
use std::{
|
||||
collections::HashMap,
|
||||
hash::Hash,
|
||||
ops::{Deref, DerefMut},
|
||||
path::{Path, PathBuf},
|
||||
};
|
||||
|
||||
/// Container type for all contracts mapped to their output file
|
||||
pub struct MappedArtifactFiles<'a> {
|
||||
/// Represents the determined output artifact file and the contract(s) that target it
|
||||
///
|
||||
/// This is guaranteed to be `len >= 1`
|
||||
///
|
||||
/// If there is more than 1 contract in the map, this means we have a naming conflict where
|
||||
/// different contracts target the same output file. This happens if the solidity file and
|
||||
/// contract name match, but they are in different folders.
|
||||
pub files: HashMap<MappedArtifactFile, Vec<MappedContract<'a>>>,
|
||||
}
|
||||
|
||||
impl<'a> MappedArtifactFiles<'a> {
|
||||
pub fn with_capacity(len: usize) -> Self {
|
||||
Self { files: HashMap::with_capacity(len) }
|
||||
}
|
||||
}
|
||||
|
||||
impl<'a> Deref for MappedArtifactFiles<'a> {
|
||||
type Target = HashMap<MappedArtifactFile, Vec<MappedContract<'a>>>;
|
||||
|
||||
fn deref(&self) -> &Self::Target {
|
||||
&self.files
|
||||
}
|
||||
}
|
||||
|
||||
impl<'a> DerefMut for MappedArtifactFiles<'a> {
|
||||
fn deref_mut(&mut self) -> &mut Self::Target {
|
||||
&mut self.files
|
||||
}
|
||||
}
|
||||
|
||||
/// Represents the targeted path of a contract or multiple contracts
|
||||
///
|
||||
/// To account for case-sensitivity we identify it via lowercase path
|
||||
#[derive(Debug, Hash, PartialEq, Eq)]
|
||||
pub struct MappedArtifactFile {
|
||||
lower_case_path: String,
|
||||
}
|
||||
|
||||
impl MappedArtifactFile {
|
||||
pub fn new(path: &Path) -> Self {
|
||||
Self { lower_case_path: path.to_string_lossy().to_lowercase() }
|
||||
}
|
||||
}
|
||||
|
||||
pub struct MappedContract<'a> {
|
||||
pub file: &'a str,
|
||||
pub name: &'a str,
|
||||
pub contract: &'a VersionedContract,
|
||||
pub artifact_path: PathBuf,
|
||||
}
|
|
@ -17,16 +17,20 @@ use semver::Version;
|
|||
use serde::{de::DeserializeOwned, Serialize};
|
||||
use std::{
|
||||
borrow::Cow,
|
||||
collections::{btree_map::BTreeMap, HashMap, HashSet},
|
||||
collections::{btree_map::BTreeMap, HashSet},
|
||||
ffi::OsString,
|
||||
fmt, fs, io,
|
||||
fmt, fs,
|
||||
hash::Hash,
|
||||
io,
|
||||
ops::Deref,
|
||||
path::{Path, PathBuf},
|
||||
};
|
||||
use tracing::{error, trace};
|
||||
|
||||
mod configurable;
|
||||
use crate::contracts::VersionedContract;
|
||||
pub(crate) mod files;
|
||||
|
||||
use crate::files::MappedContract;
|
||||
pub use configurable::*;
|
||||
|
||||
/// Represents unique artifact metadata for identifying artifacts on output
|
||||
|
@ -120,11 +124,6 @@ impl<T> ArtifactFile<T> {
|
|||
}
|
||||
}
|
||||
|
||||
/// Internal helper type alias that maps all files for the contracts
|
||||
/// `output -> [(file, name, contract)]`
|
||||
pub(crate) type ArtifactFiles<'a> =
|
||||
HashMap<PathBuf, Vec<(&'a str, &'a str, &'a VersionedContract)>>;
|
||||
|
||||
/// local helper type alias `file name -> (contract name -> Vec<..>)`
|
||||
pub(crate) type ArtifactsMap<T> = FileToContractsMap<Vec<ArtifactFile<T>>>;
|
||||
|
||||
|
@ -808,8 +807,9 @@ pub trait ArtifactOutput {
|
|||
// converting stand-alone artifacts in the next step
|
||||
let mut final_artifact_paths = HashSet::new();
|
||||
|
||||
for (artifact_path, contracts) in artifact_files {
|
||||
for (idx, (file, name, contract)) in contracts.iter().enumerate() {
|
||||
for contracts in artifact_files.files.into_values() {
|
||||
for (idx, mapped_contract) in contracts.iter().enumerate() {
|
||||
let MappedContract { file, name, contract, artifact_path } = mapped_contract;
|
||||
// track `SourceFile`s that can be mapped to contracts
|
||||
let source_file = sources.find_file_and_version(file, &contract.version);
|
||||
|
||||
|
@ -824,11 +824,11 @@ pub trait ArtifactOutput {
|
|||
// contracts need to adjust the paths properly
|
||||
|
||||
// we keep the top most conflicting file unchanged
|
||||
let is_top_most = contracts.iter().enumerate().filter(|(i, _)| *i != idx).all(
|
||||
|(_, (f, _, _))| {
|
||||
Path::new(file).components().count() < Path::new(f).components().count()
|
||||
},
|
||||
);
|
||||
let is_top_most =
|
||||
contracts.iter().enumerate().filter(|(i, _)| *i != idx).all(|(_, c)| {
|
||||
Path::new(file).components().count() <
|
||||
Path::new(c.file).components().count()
|
||||
});
|
||||
if !is_top_most {
|
||||
// we resolve the conflicting by finding a new unique, alternative path
|
||||
artifact_path = Self::conflict_free_output_file(
|
||||
|
|
|
@ -3,7 +3,8 @@ use crate::{
|
|||
contract::{CompactContractRef, Contract},
|
||||
FileToContractsMap,
|
||||
},
|
||||
ArtifactFiles, ArtifactOutput, OutputContext,
|
||||
files::{MappedArtifactFile, MappedArtifactFiles, MappedContract},
|
||||
ArtifactOutput, OutputContext,
|
||||
};
|
||||
use semver::Version;
|
||||
use serde::{Deserialize, Serialize};
|
||||
|
@ -47,8 +48,8 @@ impl VersionedContracts {
|
|||
pub(crate) fn artifact_files<T: ArtifactOutput + ?Sized>(
|
||||
&self,
|
||||
ctx: &OutputContext,
|
||||
) -> ArtifactFiles {
|
||||
let mut output_files = ArtifactFiles::with_capacity(self.len());
|
||||
) -> MappedArtifactFiles {
|
||||
let mut output_files = MappedArtifactFiles::with_capacity(self.len());
|
||||
for (file, contracts) in self.iter() {
|
||||
for (name, versioned_contracts) in contracts {
|
||||
for contract in versioned_contracts {
|
||||
|
@ -56,7 +57,7 @@ impl VersionedContracts {
|
|||
// 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) =
|
||||
let artifact_path = if let Some(existing_artifact) =
|
||||
ctx.existing_artifact(file, name, &contract.version).cloned()
|
||||
{
|
||||
trace!("use existing artifact file {:?}", existing_artifact,);
|
||||
|
@ -69,12 +70,18 @@ impl VersionedContracts {
|
|||
|
||||
trace!(
|
||||
"use artifact file {:?} for contract file {} {}",
|
||||
output,
|
||||
artifact_path,
|
||||
file,
|
||||
contract.version
|
||||
);
|
||||
let contract = (file.as_str(), name.as_str(), contract);
|
||||
output_files.entry(output).or_default().push(contract);
|
||||
let artifact = MappedArtifactFile::new(&artifact_path);
|
||||
let contract = MappedContract {
|
||||
file: file.as_str(),
|
||||
name: name.as_str(),
|
||||
contract,
|
||||
artifact_path,
|
||||
};
|
||||
output_files.entry(artifact).or_default().push(contract);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
@ -2226,6 +2226,103 @@ fn can_handle_conflicting_files_recompile() {
|
|||
assert!(inner_recompiled.get_abi().unwrap().functions.contains_key("baz"));
|
||||
}
|
||||
|
||||
// <https://github.com/foundry-rs/foundry/issues/2843>
|
||||
#[test]
|
||||
fn can_handle_conflicting_files_case_sensitive_recompile() {
|
||||
let project = TempProject::<ConfigurableArtifacts>::dapptools().unwrap();
|
||||
|
||||
project
|
||||
.add_source(
|
||||
"a",
|
||||
r#"
|
||||
pragma solidity ^0.8.10;
|
||||
|
||||
contract A {
|
||||
function foo() public{}
|
||||
}
|
||||
"#,
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
project
|
||||
.add_source(
|
||||
"inner/A",
|
||||
r#"
|
||||
pragma solidity ^0.8.10;
|
||||
|
||||
contract A {
|
||||
function bar() public{}
|
||||
}
|
||||
"#,
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
let compiled = project.compile().unwrap();
|
||||
assert!(!compiled.has_compiler_errors());
|
||||
|
||||
let artifacts = compiled.artifacts().count();
|
||||
assert_eq!(artifacts, 2);
|
||||
|
||||
// nothing to compile
|
||||
let compiled = project.compile().unwrap();
|
||||
assert!(compiled.is_unchanged());
|
||||
let artifacts = compiled.artifacts().count();
|
||||
assert_eq!(artifacts, 2);
|
||||
|
||||
let cache = SolFilesCache::read(project.cache_path()).unwrap();
|
||||
|
||||
let mut source_files = cache.files.keys().cloned().collect::<Vec<_>>();
|
||||
source_files.sort_unstable();
|
||||
|
||||
assert_eq!(source_files, vec![PathBuf::from("src/a.sol"), PathBuf::from("src/inner/A.sol"),]);
|
||||
|
||||
let mut artifacts =
|
||||
project.artifacts_snapshot().unwrap().artifacts.into_stripped_file_prefixes(project.root());
|
||||
artifacts.strip_prefix_all(&project.paths().artifacts);
|
||||
|
||||
assert_eq!(artifacts.len(), 2);
|
||||
let mut artifact_files = artifacts.artifact_files().map(|f| f.file.clone()).collect::<Vec<_>>();
|
||||
artifact_files.sort_unstable();
|
||||
|
||||
let expected_files = vec![PathBuf::from("a.sol/A.json"), PathBuf::from("inner/A.sol/A.json")];
|
||||
assert_eq!(artifact_files, expected_files);
|
||||
|
||||
// overwrite conflicting nested file, effectively changing it
|
||||
project
|
||||
.add_source(
|
||||
"inner/A",
|
||||
r#"
|
||||
pragma solidity ^0.8.10;
|
||||
contract A {
|
||||
function bar() public{}
|
||||
function baz() public{}
|
||||
}
|
||||
"#,
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
let compiled = project.compile().unwrap();
|
||||
assert!(!compiled.has_compiler_errors());
|
||||
|
||||
let mut recompiled_artifacts =
|
||||
project.artifacts_snapshot().unwrap().artifacts.into_stripped_file_prefixes(project.root());
|
||||
recompiled_artifacts.strip_prefix_all(&project.paths().artifacts);
|
||||
|
||||
assert_eq!(recompiled_artifacts.len(), 2);
|
||||
let mut artifact_files =
|
||||
recompiled_artifacts.artifact_files().map(|f| f.file.clone()).collect::<Vec<_>>();
|
||||
artifact_files.sort_unstable();
|
||||
assert_eq!(artifact_files, expected_files);
|
||||
|
||||
// ensure that `a.sol/A.json` is unchanged
|
||||
let outer = artifacts.find("src/a.sol", "A").unwrap();
|
||||
let outer_recompiled = recompiled_artifacts.find("src/a.sol", "A").unwrap();
|
||||
assert_eq!(outer, outer_recompiled);
|
||||
|
||||
let inner_recompiled = recompiled_artifacts.find("src/inner/A.sol", "A").unwrap();
|
||||
assert!(inner_recompiled.get_abi().unwrap().functions.contains_key("baz"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn can_checkout_repo() {
|
||||
let project = TempProject::checkout("transmissions11/solmate").unwrap();
|
||||
|
|
Loading…
Reference in New Issue