Skip to content

feat: better artifact filenames for different profiles #241

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jan 21, 2025
Merged

feat: better artifact filenames for different profiles #241

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 17 additions & 4 deletions crates/compilers/src/artifact_output/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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();

Expand Down Expand Up @@ -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);

Expand All @@ -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());
Expand Down Expand Up @@ -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> {
Expand Down
26 changes: 18 additions & 8 deletions crates/compilers/src/compile/project.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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>,
}
Expand All @@ -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.
Expand All @@ -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
Expand Down Expand Up @@ -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 `/`
Expand All @@ -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 })
}
}

Expand All @@ -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>
Expand All @@ -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)?;

Expand All @@ -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 })
}
}

Expand All @@ -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>
Expand All @@ -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();
Expand All @@ -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,
Expand All @@ -287,6 +295,7 @@ impl<'a, T: ArtifactOutput<CompilerContract = C::CompilerContract>, C: Compiler>
&output.sources,
ctx,
&project.paths,
&primary_profiles,
)
} else {
trace!(
Expand All @@ -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
Expand Down
12 changes: 10 additions & 2 deletions crates/compilers/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand Down
32 changes: 27 additions & 5 deletions crates/compilers/src/resolver/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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<_>>();

Expand All @@ -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,
Expand All @@ -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:
Expand Down
Loading