fix(solc): handle absolute paths properly on conflict (#1784)

This commit is contained in:
Matthias Seitz 2022-10-13 23:54:36 +02:00 committed by GitHub
parent c180528951
commit b47567bfd2
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 60 additions and 18 deletions

View File

@ -576,7 +576,7 @@ pub trait ArtifactOutput {
layout: &ProjectPathsConfig, layout: &ProjectPathsConfig,
ctx: OutputContext, ctx: OutputContext,
) -> Result<Artifacts<Self::Artifact>> { ) -> Result<Artifacts<Self::Artifact>> {
let mut artifacts = self.output_to_artifacts(contracts, sources, ctx); let mut artifacts = self.output_to_artifacts(contracts, sources, ctx, layout);
fs::create_dir_all(&layout.artifacts).map_err(|err| { fs::create_dir_all(&layout.artifacts).map_err(|err| {
error!(dir=?layout.artifacts, "Failed to create artifacts folder"); error!(dir=?layout.artifacts, "Failed to create artifacts folder");
SolcIoError::new(err, &layout.artifacts) SolcIoError::new(err, &layout.artifacts)
@ -652,23 +652,31 @@ pub trait ArtifactOutput {
already_taken: &HashSet<PathBuf>, already_taken: &HashSet<PathBuf>,
conflict: PathBuf, conflict: PathBuf,
contract_file: impl AsRef<Path>, contract_file: impl AsRef<Path>,
artifacts_folder: impl AsRef<Path>,
) -> PathBuf { ) -> PathBuf {
let mut candidate = conflict.clone(); let artifacts_folder = artifacts_folder.as_ref();
let mut rel_candidate = conflict;
if let Ok(stripped) = rel_candidate.strip_prefix(artifacts_folder) {
rel_candidate = stripped.to_path_buf();
}
let mut candidate = rel_candidate.clone();
let contract_file = contract_file.as_ref(); let contract_file = contract_file.as_ref();
let mut current_parent = contract_file.parent(); let mut current_parent = contract_file.parent();
while let Some(parent) = current_parent.and_then(|f| f.file_name()) { while let Some(parent_name) = current_parent.and_then(|f| f.file_name()) {
candidate = Path::new(parent).join(&candidate); // this is problematic if both files are absolute
if !already_taken.contains(&candidate) { candidate = Path::new(parent_name).join(&candidate);
trace!("found alternative output file={:?} for {:?}", candidate, contract_file); let out_path = artifacts_folder.join(&candidate);
return candidate if !already_taken.contains(&out_path) {
trace!("found alternative output file={:?} for {:?}", out_path, contract_file);
return out_path
} }
current_parent = current_parent.and_then(|f| f.parent()); current_parent = current_parent.and_then(|f| f.parent());
} }
// this means we haven't found an alternative yet, which shouldn't actually happen since // this means we haven't found an alternative yet, which shouldn't actually happen since
// `contract_file` are unique, but just to be safe, handle this case in which case // `contract_file` are unique, but just to be safe, handle this case in which case
// we simply numerate // we simply numerate the parent folder
trace!("no conflict free output file found after traversing the file"); trace!("no conflict free output file found after traversing the file");
@ -677,7 +685,7 @@ pub trait ArtifactOutput {
loop { loop {
// this will attempt to find an alternate path by numerating the first component in the // this will attempt to find an alternate path by numerating the first component in the
// path: `<root>+_<num>/....sol` // path: `<root>+_<num>/....sol`
let mut components = conflict.components(); let mut components = rel_candidate.components();
let first = components.next().expect("path not empty"); let first = components.next().expect("path not empty");
let name = first.as_os_str(); let name = first.as_os_str();
let mut numerated = OsString::with_capacity(name.len() + 2); let mut numerated = OsString::with_capacity(name.len() + 2);
@ -799,6 +807,7 @@ pub trait ArtifactOutput {
contracts: &VersionedContracts, contracts: &VersionedContracts,
sources: &VersionedSourceFiles, sources: &VersionedSourceFiles,
ctx: OutputContext, ctx: OutputContext,
layout: &ProjectPathsConfig,
) -> Artifacts<Self::Artifact> { ) -> Artifacts<Self::Artifact> {
let mut artifacts = ArtifactsMap::new(); let mut artifacts = ArtifactsMap::new();
@ -840,6 +849,7 @@ pub trait ArtifactOutput {
&final_artifact_paths, &final_artifact_paths,
artifact_path, artifact_path,
file, file,
&layout.artifacts,
); );
} }
} }
@ -898,6 +908,7 @@ pub trait ArtifactOutput {
&final_artifact_paths, &final_artifact_paths,
artifact_path, artifact_path,
file, file,
&layout.artifacts,
); );
final_artifact_paths.insert(artifact_path.clone()); final_artifact_paths.insert(artifact_path.clone());
} }
@ -1087,26 +1098,46 @@ mod tests {
let mut already_taken = HashSet::new(); let mut already_taken = HashSet::new();
let file = "v1/tokens/Greeter.sol"; let file = "v1/tokens/Greeter.sol";
let conflict = PathBuf::from("Greeter.sol/Greeter.json"); let conflict = PathBuf::from("out/Greeter.sol/Greeter.json");
let alternative = ConfigurableArtifacts::conflict_free_output_file( let alternative = ConfigurableArtifacts::conflict_free_output_file(
&already_taken, &already_taken,
conflict.clone(), conflict.clone(),
file, file,
"out",
); );
assert_eq!(alternative, PathBuf::from("tokens/Greeter.sol/Greeter.json")); assert_eq!(alternative, PathBuf::from("out/tokens/Greeter.sol/Greeter.json"));
already_taken.insert("tokens/Greeter.sol/Greeter.json".into()); already_taken.insert("out/tokens/Greeter.sol/Greeter.json".into());
let alternative = ConfigurableArtifacts::conflict_free_output_file( let alternative = ConfigurableArtifacts::conflict_free_output_file(
&already_taken, &already_taken,
conflict.clone(), conflict.clone(),
file, file,
"out",
); );
assert_eq!(alternative, PathBuf::from("v1/tokens/Greeter.sol/Greeter.json")); assert_eq!(alternative, PathBuf::from("out/v1/tokens/Greeter.sol/Greeter.json"));
already_taken.insert("v1/tokens/Greeter.sol/Greeter.json".into()); already_taken.insert("out/v1/tokens/Greeter.sol/Greeter.json".into());
let alternative = let alternative =
ConfigurableArtifacts::conflict_free_output_file(&already_taken, conflict, file); ConfigurableArtifacts::conflict_free_output_file(&already_taken, conflict, file, "out");
assert_eq!(alternative, PathBuf::from("Greeter.sol_1/Greeter.json")); assert_eq!(alternative, PathBuf::from("Greeter.sol_1/Greeter.json"));
} }
#[test]
fn can_find_alternate_path_conflict() {
let mut already_taken = HashSet::new();
let file = "/Users/carter/dev/goldfinch/mono/packages/protocol/test/forge/mainnet/utils/BaseMainnetForkingTest.t.sol";
let conflict = PathBuf::from("/Users/carter/dev/goldfinch/mono/packages/protocol/artifacts/BaseMainnetForkingTest.t.sol/BaseMainnetForkingTest.json");
already_taken.insert("/Users/carter/dev/goldfinch/mono/packages/protocol/artifacts/BaseMainnetForkingTest.t.sol/BaseMainnetForkingTest.json".into());
let alternative = ConfigurableArtifacts::conflict_free_output_file(
&already_taken,
conflict.clone(),
file,
"/Users/carter/dev/goldfinch/mono/packages/protocol/artifacts",
);
assert_eq!(alternative, PathBuf::from("/Users/carter/dev/goldfinch/mono/packages/protocol/artifacts/utils/BaseMainnetForkingTest.t.sol/BaseMainnetForkingTest.json"));
}
} }

View File

@ -305,10 +305,20 @@ impl<'a, T: ArtifactOutput> CompiledState<'a, T> {
// write all artifacts via the handler but only if the build succeeded and project wasn't // write all artifacts via the handler but only if the build succeeded and project wasn't
// configured with `no_artifacts == true` // configured with `no_artifacts == true`
let compiled_artifacts = if project.no_artifacts { let compiled_artifacts = if project.no_artifacts {
project.artifacts_handler().output_to_artifacts(&output.contracts, &output.sources, ctx) project.artifacts_handler().output_to_artifacts(
&output.contracts,
&output.sources,
ctx,
&project.paths,
)
} else if output.has_error() { } else if output.has_error() {
trace!("skip writing cache file due to solc errors: {:?}", output.errors); trace!("skip writing cache file due to solc errors: {:?}", output.errors);
project.artifacts_handler().output_to_artifacts(&output.contracts, &output.sources, ctx) project.artifacts_handler().output_to_artifacts(
&output.contracts,
&output.sources,
ctx,
&project.paths,
)
} else { } else {
trace!( trace!(
"handling artifact output for {} contracts and {} sources", "handling artifact output for {} contracts and {} sources",

View File

@ -933,8 +933,9 @@ impl<T: ArtifactOutput> ArtifactOutput for Project<T> {
contracts: &VersionedContracts, contracts: &VersionedContracts,
sources: &VersionedSourceFiles, sources: &VersionedSourceFiles,
ctx: OutputContext, ctx: OutputContext,
layout: &ProjectPathsConfig,
) -> Artifacts<Self::Artifact> { ) -> Artifacts<Self::Artifact> {
self.artifacts_handler().output_to_artifacts(contracts, sources, ctx) self.artifacts_handler().output_to_artifacts(contracts, sources, ctx, layout)
} }
fn standalone_source_file_to_artifact( fn standalone_source_file_to_artifact(