Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
9 changes: 3 additions & 6 deletions crates/prek/src/cli/cache_gc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -373,18 +373,18 @@ fn hook_env_requirements_from_config(
}

let repo = match HookRepo::remote(
repo_config.repo.clone(),
repo_config.source().to_string(),
repo_config.rev.clone(),
repo_path,
) {
Ok(repo) => repo,
Err(err) => {
warn!(repo = %repo_config.repo, %err, "Failed to load repo manifest, skipping");
warn!(repo = %repo_config.repo(), %err, "Failed to load repo manifest, skipping");
continue;
}
};

let remote_repo = RepoIdentityRef::new(&repo_config.repo, &repo_config.rev);
let remote_repo = RepoIdentityRef::new(repo_config.source(), &repo_config.rev);

for hook_config in &repo_config.hooks {
let Some(manifest_hook) = repo.get_hook(&hook_config.id) else {
Expand Down Expand Up @@ -765,9 +765,6 @@ struct RepoMarker {
}

fn read_repo_marker(root: &Path) -> Option<RepoMarker> {
// NOTE: `Store::clone_repo` serializes `RemoteRepo`, but with some fields skipped during
// serialization (e.g. `hooks`). That means deserializing back into `RemoteRepo` can fail.
// For GC display, we only need `repo` + `rev`.
let content = fs_err::read_to_string(root.join(REPO_MARKER)).ok()?;
serde_json::from_str(&content).ok()
}
Expand Down
32 changes: 20 additions & 12 deletions crates/prek/src/cli/update/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ struct RepoUsage<'a> {

/// One distinct `repo + rev + hook set + update settings` target that should be evaluated.
struct RepoTarget<'a> {
/// The remote repository URL.
/// The repository value exactly as written in the configuration.
repo: &'a str,
/// The currently configured `rev` for this target.
current_rev: &'a str,
Expand All @@ -67,10 +67,10 @@ struct RepoTarget<'a> {
usages: Vec<RepoUsage<'a>>,
}

/// One fetched remote repository URL with all configured revisions that use it.
/// One fetched remote repository source with all configured revisions that use it.
struct RepoSource<'a> {
/// The remote repository URL.
repo: &'a str,
/// The resolved repository source used for fetch and cache identity.
source: &'a str,
/// Distinct configured revisions that should be evaluated against this fetched repo.
targets: Vec<RepoTarget<'a>>,
}
Expand Down Expand Up @@ -280,7 +280,7 @@ fn warn_missing_repos(
) {
let configured_repos = repo_sources
.iter()
.map(|repo_source| repo_source.repo)
.flat_map(|repo_source| repo_source.targets.iter().map(|target| target.repo))
.collect::<FxHashSet<_>>();
let mut missing_repos = filter_repos
.iter()
Expand Down Expand Up @@ -432,16 +432,24 @@ pub(crate) async fn update(
};
let reporter = UpdateReporter::new(printer);

let repo_sources =
let mut repo_sources =
collect_repo_sources(&workspace, freeze, cooldown_days, filesystem.as_ref())?;
warn_missing_repos(&repo_sources, &filter_repos, &tag_filters);
let sources = repo_sources.iter().filter(|repo_source| {
(filter_repos.is_empty() || filter_repos.iter().any(|repo| repo == repo_source.repo))
&& !exclude_repos.iter().any(|repo| repo == repo_source.repo)
});
let outcomes: Vec<RepoUpdate<'_>> = futures_util::stream::iter(sources)
for repo_source in &mut repo_sources {
repo_source.targets.retain(|target| {
(filter_repos.is_empty() || filter_repos.iter().any(|repo| repo == target.repo))
&& !exclude_repos.iter().any(|repo| repo == target.repo)
});
}
repo_sources.retain(|repo_source| !repo_source.targets.is_empty());

let outcomes: Vec<RepoUpdate<'_>> = futures_util::stream::iter(&repo_sources)
.map(async |repo_source| {
let progress = reporter.on_update_start(repo_source.repo);
let display_repo = repo_source
.targets
.first()
.map_or(repo_source.source, |target| target.repo);
let progress = reporter.on_update_start(display_repo);
let result = evaluate_repo_source(repo_source, bleeding_edge, &tag_filters).await;
reporter.on_update_complete(progress);
result
Expand Down
36 changes: 19 additions & 17 deletions crates/prek/src/cli/update/source.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,23 +24,24 @@ use crate::workspace::Workspace;
/// Identifies repo usages that can share one update evaluation.
#[derive(Eq, Hash, PartialEq)]
struct RepoTargetKey<'a> {
repo: &'a str,
current_rev: &'a str,
required_hook_ids: Vec<&'a str>,
cooldown_days: u8,
freeze: bool,
}

type RepoTargetsByKey<'a> = FxHashMap<RepoTargetKey<'a>, RepoTarget<'a>>;
type RepoSourcesByRepo<'a> = FxHashMap<&'a str, RepoTargetsByKey<'a>>;
type RepoSourcesBySource<'a> = FxHashMap<&'a str, RepoTargetsByKey<'a>>;

/// Collects configured remote repos grouped by fetch source, revision, hooks, and settings.
/// Collects configured remote repos grouped by source, configured value, revision, and settings.
pub(super) fn collect_repo_sources<'a>(
workspace: &'a Workspace,
cli_freeze: bool,
cli_cooldown_days: Option<u8>,
filesystem: Option<&FilesystemOptions>,
) -> Result<Vec<RepoSource<'a>>> {
let mut repo_sources: RepoSourcesByRepo<'a> = FxHashMap::default();
let mut repo_sources: RepoSourcesBySource<'a> = FxHashMap::default();

for project in workspace.projects() {
let project_update = project.config().update.as_ref();
Expand Down Expand Up @@ -85,8 +86,9 @@ pub(super) fn collect_repo_sources<'a>(
required_hook_ids.sort_unstable();
required_hook_ids.dedup();

let targets = repo_sources.entry(remote_repo.repo.as_str()).or_default();
let targets = repo_sources.entry(remote_repo.source()).or_default();
let target_key = RepoTargetKey {
repo: remote_repo.repo(),
current_rev: remote_repo.rev.as_str(),
required_hook_ids,
cooldown_days,
Expand All @@ -97,7 +99,7 @@ pub(super) fn collect_repo_sources<'a>(
Entry::Vacant(entry) => {
let required_hook_ids = entry.key().required_hook_ids.clone();
entry.insert(RepoTarget {
repo: remote_repo.repo.as_str(),
repo: remote_repo.repo(),
current_rev: remote_repo.rev.as_str(),
cooldown_days,
freeze,
Expand All @@ -120,16 +122,17 @@ pub(super) fn collect_repo_sources<'a>(

Ok(repo_sources
.into_iter()
.map(|(repo, targets)| {
.map(|(source, targets)| {
let mut targets = targets.into_values().collect::<Vec<_>>();
targets.sort_by(|a, b| {
a.current_rev
.cmp(b.current_rev)
a.repo
.cmp(b.repo)
.then_with(|| a.current_rev.cmp(b.current_rev))
.then_with(|| a.cooldown_days.cmp(&b.cooldown_days))
.then_with(|| a.freeze.cmp(&b.freeze))
.then_with(|| a.required_hook_ids.cmp(&b.required_hook_ids))
});
RepoSource { repo, targets }
RepoSource { source, targets }
})
.collect())
}
Expand Down Expand Up @@ -213,10 +216,10 @@ pub(super) async fn evaluate_repo_source<'a>(
let result = async {
trace!(
"Cloning repository `{}` to `{}`",
repo_source.repo,
repo_source.source,
repo_path.display()
);
setup_and_fetch_repo(repo_source.repo, repo_path).await?;
setup_and_fetch_repo(repo_source.source, repo_path).await?;
let metadata = list_tag_metadata(repo_path).await?;

anyhow::Ok(metadata)
Expand All @@ -238,14 +241,13 @@ pub(super) async fn evaluate_repo_source<'a>(
}
};

let update_tag_timestamps = tag_filters
.filter(repo_source.repo, &tag_timestamps)
.into_iter()
.cloned()
.collect::<Vec<_>>();

let mut updates = Vec::with_capacity(repo_source.targets.len());
for target in &repo_source.targets {
let update_tag_timestamps = tag_filters
.filter(target.repo, &tag_timestamps)
.into_iter()
.cloned()
.collect::<Vec<_>>();
Comment thread
j178 marked this conversation as resolved.
Comment thread
j178 marked this conversation as resolved.
let result = evaluate_repo_target(
repo_path,
target,
Expand Down
86 changes: 77 additions & 9 deletions crates/prek/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -884,9 +884,11 @@ impl TryFrom<RemoteHook> for BuiltinHook {
}
}

#[derive(Debug, Clone, Serialize, Deserialize)]
#[derive(Clone, Serialize, Deserialize)]
pub(crate) struct RemoteRepo {
pub repo: String,
repo: String,
#[serde(skip)]
resolved_source: Option<String>,
pub rev: String,
#[serde(skip_serializing)]
pub hooks: Vec<RemoteHook>,
Expand All @@ -897,13 +899,13 @@ pub(crate) struct RemoteRepo {

#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
pub(crate) struct RemoteRepoKey<'a> {
repo: &'a str,
source: &'a str,
rev: &'a str,
}

impl<'a> RemoteRepoKey<'a> {
pub(crate) fn repo(self) -> &'a str {
self.repo
pub(crate) fn source(self) -> &'a str {
self.source
}

pub(crate) fn rev(self) -> &'a str {
Expand All @@ -915,23 +917,53 @@ impl RemoteRepo {
pub fn new(repo: String, rev: String, hooks: Vec<RemoteHook>) -> Self {
Self {
repo,
resolved_source: None,
rev,
hooks,
_unused_keys: BTreeMap::new(),
}
}

/// The repository value exactly as written in the configuration.
pub(crate) fn repo(&self) -> &str {
&self.repo
}

/// The repository source used for fetch and cache identity.
pub(crate) fn source(&self) -> &str {
self.resolved_source.as_deref().unwrap_or(&self.repo)
}

pub(crate) fn set_resolved_source(&mut self, source: String) {
self.resolved_source = Some(source);
}

pub fn key(&self) -> RemoteRepoKey<'_> {
RemoteRepoKey {
repo: &self.repo,
source: self.source(),
rev: &self.rev,
}
}
}

impl std::fmt::Debug for RemoteRepo {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
let mut debug = f.debug_struct("RemoteRepo");
debug.field("repo", &self.repo);
if let Some(source) = &self.resolved_source {
debug.field("source", source);
}
debug
.field("rev", &self.rev)
.field("hooks", &self.hooks)
.field("_unused_keys", &self._unused_keys)
.finish()
}
}

impl Display for RemoteRepo {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
write!(f, "{}@{}", self.repo, self.rev)
write!(f, "{}@{}", self.repo(), self.rev)
}
}

Expand Down Expand Up @@ -1109,6 +1141,7 @@ impl<'de> Deserialize<'de> for Repo {
};
Ok(Repo::Remote(RemoteRepo {
repo: repo_value,
resolved_source: None,
rev,
hooks,
_unused_keys: unused,
Expand Down Expand Up @@ -1193,6 +1226,40 @@ pub(crate) struct Config {
_unused_keys: BTreeMap<String, serde_json::Value>,
}

impl Config {
/// Resolve local relative repository sources from the config file, not the process cwd.
fn resolve_relative_repo_sources(&mut self, config_path: &Path) -> Result<(), Error> {
let config_dir = config_path
.parent()
.expect("config file must have a parent");
for repo in &mut self.repos {
let Repo::Remote(remote) = repo else {
continue;
};

let configured_repo = remote.repo();
let repo_path = Path::new(configured_repo);
if configured_repo.starts_with("http://")
|| configured_repo.starts_with("https://")
|| !repo_path.is_relative()
{
continue;
}

let resolved = config_dir.join(repo_path);
if resolved.is_dir() {
remote.set_resolved_source(
dunce::canonicalize(resolved)?
.to_string_lossy()
.into_owned(),
);
}
}

Ok(())
}
}

#[derive(Debug, thiserror::Error)]
pub(crate) enum Error {
#[error(transparent)]
Expand Down Expand Up @@ -1319,12 +1386,13 @@ fn warn_unused_paths(path: &Path, entries: &[String]) {
pub(crate) fn load_config(path: &Path) -> Result<Config, Error> {
let content = fs_err::read_to_string(path)?;

let config = match path.extension() {
let mut config: Config = match path.extension() {
Some(ext) if ext.eq_ignore_ascii_case("toml") => toml::from_str(&content)
.map_err(|e| Error::Toml(path.user_display().to_string(), Box::new(e)))?,
_ => serde_saphyr::from_str(&content)
.map_err(|e| Error::Yaml(path.user_display().to_string(), Box::new(e)))?,
};
config.resolve_relative_repo_sources(path)?;

Ok(config)
}
Expand Down Expand Up @@ -1355,7 +1423,7 @@ pub(crate) fn read_config(path: &Path) -> Result<Config, Error> {
if !repos_has_mutable_rev.is_empty() {
let msg = repos_has_mutable_rev
.iter()
.map(|repo| format!("{}: {}", repo.repo.cyan(), repo.rev.yellow()))
.map(|repo| format!("{}: {}", repo.repo().cyan(), repo.rev.yellow()))
.join("\n");

warn_user!(
Expand Down
Loading
Loading