feat: better artifact filenames for different profiles (#241)

klkvr · web-flow · commit b851a6c9ab52 · 2025-01-22T02:03:20.000+04:00
Closes foundry-rs/foundry#9722 This PR introduces a concept of "primary" profile. Primary profile is the one which we've chosen for input source when analyzing it on its own (i.e not as a dependency of other contract). That way `Counter.json` would always correspond to an artifact compiled with the default profile if it was possible.
diff --git a/crates/compilers/src/artifact_output/mod.rs b/crates/compilers/src/artifact_output/mod.rs
@@ -17,7 +17,7 @@ use semver::Version;
 use serde::{de::DeserializeOwned, Deserialize, Serialize};
 use std::{
     borrow::Cow,
-    collections::{btree_map::BTreeMap, HashSet},
+    collections::{btree_map::BTreeMap, HashMap, HashSet},
     ffi::OsString,
     fmt, fs,
     hash::Hash,
@@ -621,8 +621,10 @@ pub trait ArtifactOutput {
         sources: &VersionedSourceFiles,
         layout: &ProjectPathsConfig<L>,
         ctx: OutputContext<'_>,
+        primary_profiles: &HashMap<PathBuf, &str>,
     ) -> Result<Artifacts<Self::Artifact>> {
-        let mut artifacts = self.output_to_artifacts(contracts, sources, ctx, layout);
+        let mut artifacts =
+            self.output_to_artifacts(contracts, sources, ctx, layout, primary_profiles);
         fs::create_dir_all(&layout.artifacts).map_err(|err| {
             error!(dir=?layout.artifacts, "Failed to create artifacts folder");
             SolcIoError::new(err, &layout.artifacts)
@@ -850,6 +852,7 @@ pub trait ArtifactOutput {
         sources: &VersionedSourceFiles,
         ctx: OutputContext<'_>,
         layout: &ProjectPathsConfig<C>,
+        primary_profiles: &HashMap<PathBuf, &str>,
     ) -> Artifacts<Self::Artifact> {
         let mut artifacts = ArtifactsMap::new();
 
@@ -877,6 +880,8 @@ pub trait ArtifactOutput {
                     versioned_contracts.iter().map(|c| &c.version).collect::<HashSet<_>>();
                 let unique_profiles =
                     versioned_contracts.iter().map(|c| &c.profile).collect::<HashSet<_>>();
+                let primary_profile = primary_profiles.get(file);
+
                 for contract in versioned_contracts {
                     non_standalone_sources.insert(file);
 
@@ -892,7 +897,8 @@ pub trait ArtifactOutput {
                         &contract.version,
                         &contract.profile,
                         unique_versions.len() > 1,
-                        unique_profiles.len() > 1,
+                        unique_profiles.len() > 1
+                            && primary_profile.is_none_or(|p| p != &contract.profile),
                     );
 
                     taken_paths_lowercase.insert(artifact_path.to_slash_lossy().to_lowercase());
@@ -1122,8 +1128,15 @@ impl ArtifactOutput for MinimalCombinedArtifactsHardhatFallback {
         sources: &VersionedSourceFiles,
         layout: &ProjectPathsConfig<C>,
         ctx: OutputContext<'_>,
+        primary_profiles: &HashMap<PathBuf, &str>,
     ) -> Result<Artifacts<Self::Artifact>> {
-        MinimalCombinedArtifacts::default().on_output(output, sources, layout, ctx)
+        MinimalCombinedArtifacts::default().on_output(
+            output,
+            sources,
+            layout,
+            ctx,
+            primary_profiles,
+        )
     }
 
     fn read_cached_artifact(path: &Path) -> Result<Self::Artifact> {
diff --git a/crates/compilers/src/compile/project.rs b/crates/compilers/src/compile/project.rs
@@ -108,7 +108,7 @@ use crate::{
     filter::SparseOutputFilter,
     output::{AggregatedCompilerOutput, Builds},
     report,
-    resolver::GraphEdges,
+    resolver::{GraphEdges, ResolvedSources},
     ArtifactOutput, CompilerSettings, Graph, Project, ProjectCompileOutput, Sources,
 };
 use foundry_compilers_core::error::Result;
@@ -128,6 +128,8 @@ pub struct ProjectCompiler<
     /// Contains the relationship of the source files and their imports
     edges: GraphEdges<C::ParsedSource>,
     project: &'a Project<C, T>,
+    /// A mapping from a source file path to the primary profile name selected for it.
+    primary_profiles: HashMap<PathBuf, &'a str>,
     /// how to compile all the sources
     sources: CompilerSources<'a, C::Language, C::Settings>,
 }
@@ -152,7 +154,8 @@ impl<'a, T: ArtifactOutput<CompilerContract = C::CompilerContract>, C: Compiler>
             sources.retain(|f, _| filter.is_match(f))
         }
         let graph = Graph::resolve_sources(&project.paths, sources)?;
-        let (sources, edges) = graph.into_sources_by_version(project)?;
+        let ResolvedSources { sources, primary_profiles, edges } =
+            graph.into_sources_by_version(project)?;
 
         // If there are multiple different versions, and we can use multiple jobs we can compile
         // them in parallel.
@@ -162,7 +165,7 @@ impl<'a, T: ArtifactOutput<CompilerContract = C::CompilerContract>, C: Compiler>
             sources,
         };
 
-        Ok(Self { edges, project, sources })
+        Ok(Self { edges, primary_profiles, project, sources })
     }
 
     /// Compiles all the sources of the `Project` in the appropriate mode
@@ -199,7 +202,7 @@ impl<'a, T: ArtifactOutput<CompilerContract = C::CompilerContract>, C: Compiler>
     ///   - check cache
     fn preprocess(self) -> Result<PreprocessedState<'a, T, C>> {
         trace!("preprocessing");
-        let Self { edges, project, mut sources } = self;
+        let Self { edges, project, mut sources, primary_profiles } = self;
 
         // convert paths on windows to ensure consistency with the `CompilerOutput` `solc` emits,
         // which is unix style `/`
@@ -209,7 +212,7 @@ impl<'a, T: ArtifactOutput<CompilerContract = C::CompilerContract>, C: Compiler>
         // retain and compile only dirty sources and all their imports
         sources.filter(&mut cache);
 
-        Ok(PreprocessedState { sources, cache })
+        Ok(PreprocessedState { sources, cache, primary_profiles })
     }
 }
 
@@ -224,6 +227,9 @@ struct PreprocessedState<'a, T: ArtifactOutput<CompilerContract = C::CompilerCon
 
     /// Cache that holds `CacheEntry` objects if caching is enabled and the project is recompiled
     cache: ArtifactsCache<'a, T, C>,
+
+    /// A mapping from a source file path to the primary profile name selected for it.
+    primary_profiles: HashMap<PathBuf, &'a str>,
 }
 
 impl<'a, T: ArtifactOutput<CompilerContract = C::CompilerContract>, C: Compiler>
@@ -232,7 +238,7 @@ impl<'a, T: ArtifactOutput<CompilerContract = C::CompilerContract>, C: Compiler>
     /// advance to the next state by compiling all sources
     fn compile(self) -> Result<CompiledState<'a, T, C>> {
         trace!("compiling");
-        let PreprocessedState { sources, mut cache } = self;
+        let PreprocessedState { sources, mut cache, primary_profiles } = self;
 
         let mut output = sources.compile(&mut cache)?;
 
@@ -243,7 +249,7 @@ impl<'a, T: ArtifactOutput<CompilerContract = C::CompilerContract>, C: Compiler>
         // contracts again
         output.join_all(cache.project().root());
 
-        Ok(CompiledState { output, cache })
+        Ok(CompiledState { output, cache, primary_profiles })
     }
 }
 
@@ -252,6 +258,7 @@ impl<'a, T: ArtifactOutput<CompilerContract = C::CompilerContract>, C: Compiler>
 struct CompiledState<'a, T: ArtifactOutput<CompilerContract = C::CompilerContract>, C: Compiler> {
     output: AggregatedCompilerOutput<C>,
     cache: ArtifactsCache<'a, T, C>,
+    primary_profiles: HashMap<PathBuf, &'a str>,
 }
 
 impl<'a, T: ArtifactOutput<CompilerContract = C::CompilerContract>, C: Compiler>
@@ -263,7 +270,7 @@ impl<'a, T: ArtifactOutput<CompilerContract = C::CompilerContract>, C: Compiler>
     /// successful
     #[instrument(skip_all, name = "write-artifacts")]
     fn write_artifacts(self) -> Result<ArtifactsState<'a, T, C>> {
-        let CompiledState { output, cache } = self;
+        let CompiledState { output, cache, primary_profiles } = self;
 
         let project = cache.project();
         let ctx = cache.output_ctx();
@@ -275,6 +282,7 @@ impl<'a, T: ArtifactOutput<CompilerContract = C::CompilerContract>, C: Compiler>
                 &output.sources,
                 ctx,
                 &project.paths,
+                &primary_profiles,
             )
         } else if output.has_error(
             &project.ignored_error_codes,
@@ -287,6 +295,7 @@ impl<'a, T: ArtifactOutput<CompilerContract = C::CompilerContract>, C: Compiler>
                 &output.sources,
                 ctx,
                 &project.paths,
+                &primary_profiles,
             )
         } else {
             trace!(
@@ -300,6 +309,7 @@ impl<'a, T: ArtifactOutput<CompilerContract = C::CompilerContract>, C: Compiler>
                 &output.sources,
                 &project.paths,
                 ctx,
+                &primary_profiles,
             )?;
 
             // emits all the build infos, if they exist
diff --git a/crates/compilers/src/lib.rs b/crates/compilers/src/lib.rs
@@ -732,8 +732,9 @@ impl<T: ArtifactOutput<CompilerContract = C::CompilerContract>, C: Compiler> Art
         sources: &VersionedSourceFiles,
         layout: &ProjectPathsConfig<CP>,
         ctx: OutputContext<'_>,
+        primary_profiles: &HashMap<PathBuf, &str>,
     ) -> Result<Artifacts<Self::Artifact>> {
-        self.artifacts_handler().on_output(contracts, sources, layout, ctx)
+        self.artifacts_handler().on_output(contracts, sources, layout, ctx, primary_profiles)
     }
 
     fn handle_artifacts(
@@ -797,8 +798,15 @@ impl<T: ArtifactOutput<CompilerContract = C::CompilerContract>, C: Compiler> Art
         sources: &VersionedSourceFiles,
         ctx: OutputContext<'_>,
         layout: &ProjectPathsConfig<CP>,
+        primary_profiles: &HashMap<PathBuf, &str>,
     ) -> Artifacts<Self::Artifact> {
-        self.artifacts_handler().output_to_artifacts(contracts, sources, ctx, layout)
+        self.artifacts_handler().output_to_artifacts(
+            contracts,
+            sources,
+            ctx,
+            layout,
+            primary_profiles,
+        )
     }
 
     fn standalone_source_file_to_artifact(
diff --git a/crates/compilers/src/resolver/mod.rs b/crates/compilers/src/resolver/mod.rs
@@ -72,6 +72,26 @@ mod tree;
 pub use parse::SolImportAlias;
 pub use tree::{print, Charset, TreeOptions};
 
+/// Container for result of version and profile resolution of sources contained in [`Graph`].
+#[derive(Debug)]
+pub struct ResolvedSources<'a, C: Compiler> {
+    /// Resolved set of sources.
+    ///
+    /// Mapping from language to a [`Vec`] of compiler inputs consisting of version, sources set
+    /// and settings.
+    pub sources: VersionedSources<'a, C::Language, C::Settings>,
+    /// A mapping from a source file path to the primary profile name selected for it.
+    ///
+    /// This is required because the same source file might be compiled with multiple different
+    /// profiles if it's present as a dependency for other sources. We want to keep a single name
+    /// of the profile which was chosen specifically for each source so that we can default to it.
+    /// Right now, this is used when generating artifact names, "primary" artifact will never have
+    /// a profile suffix.
+    pub primary_profiles: HashMap<PathBuf, &'a str>,
+    /// Graph edges.
+    pub edges: GraphEdges<C::ParsedSource>,
+}
+
 /// The underlying edges of the graph which only contains the raw relationship data.
 ///
 /// This is kept separate from the `Graph` as the `Node`s get consumed when the `Solc` to `Sources`
@@ -468,14 +488,13 @@ impl<L: Language, D: ParsedSource<Language = L>> Graph<D> {
     ///
     /// First we determine the compatible version for each input file (from sources and test folder,
     /// see `Self::resolve`) and then we add all resolved library imports.
-    pub fn into_sources_by_version<C, T, S>(
+    pub fn into_sources_by_version<C, T>(
         self,
         project: &Project<C, T>,
-    ) -> Result<(VersionedSources<'_, L, S>, GraphEdges<D>)>
+    ) -> Result<ResolvedSources<'_, C>>
     where
         T: ArtifactOutput<CompilerContract = C::CompilerContract>,
-        S: CompilerSettings,
-        C: Compiler<ParsedSource = D, Language = L, Settings = S>,
+        C: Compiler<ParsedSource = D, Language = L>,
     {
         /// insert the imports of the given node into the sources map
         /// There can be following graph:
@@ -514,6 +533,7 @@ impl<L: Language, D: ParsedSource<Language = L>> Graph<D> {
         let mut all_nodes = nodes.into_iter().enumerate().collect::<HashMap<_, _>>();
 
         let mut resulted_sources = HashMap::new();
+        let mut default_profiles = HashMap::new();
 
         let profiles = project.settings_profiles().collect::<Vec<_>>();
 
@@ -534,6 +554,8 @@ impl<L: Language, D: ParsedSource<Language = L>> Graph<D> {
                         // set
                         let (path, source) =
                             all_nodes.get(&idx).cloned().expect("node is preset. qed");
+
+                        default_profiles.insert(path.clone(), profiles[profile_idx].0);
                         sources.insert(path, source);
                         insert_imports(
                             idx,
@@ -550,7 +572,7 @@ impl<L: Language, D: ParsedSource<Language = L>> Graph<D> {
             resulted_sources.insert(language, versioned_sources);
         }
 
-        Ok((resulted_sources, edges))
+        Ok(ResolvedSources { sources: resulted_sources, primary_profiles: default_profiles, edges })
     }
 
     /// Writes the list of imported files into the given formatter:
