diff --git a/src/changelog.rs b/src/changelog.rs index 87f7bf9..e11283e 100644 --- a/src/changelog.rs +++ b/src/changelog.rs @@ -82,10 +82,6 @@ impl Fragments { self.fragments.iter().filter(move |f| f.package == package) } - pub fn count_for(&self, package: &str) -> usize { - self.for_package(package).count() - } - /// Problem messages alone, for the callers that only render prose. pub fn problem_messages(&self) -> Vec { self.problems @@ -114,9 +110,8 @@ fn versions_dir(workspace: &Workspace, package: &str) -> PathBuf { pub fn load_fragments(workspace: &Workspace) -> Result { let mut result = Fragments::default(); let dir = unreleased_dir(workspace); - let entries = match std::fs::read_dir(&dir) { - Ok(entries) => entries, - Err(_) => return Ok(result), // nothing unreleased yet + let Ok(entries) = std::fs::read_dir(&dir) else { + return Ok(result); // nothing unreleased yet }; let mut paths: Vec = entries .filter_map(|entry| entry.ok().map(|e| e.path())) @@ -190,7 +185,7 @@ pub fn load_fragments(workspace: &Workspace) -> Result { } else { format!( "fragment `{display}`: category `{category}` is not one of {}", - category_labels(categories) + categories.join(", ") ) })); continue; @@ -220,10 +215,6 @@ pub fn kind_labels(kinds: &[KindConfig]) -> String { .join(", ") } -pub fn category_labels(categories: &[String]) -> String { - categories.join(", ") -} - /// The entry recording that a workspace dependency bumped in this release. /// /// A ripple is modelled as an ordinary fragment so the rest of the engine @@ -239,6 +230,7 @@ pub fn dependency_fragment( dependency_version: &str, ) -> Result { let body = render( + &minijinja::Environment::new(), &config.dependency_body, "dependency_body", // `project` is the pre-1.0 spelling of `package`, kept so existing @@ -351,14 +343,20 @@ pub fn apply_bump(current: &semver::Version, bump: Bump) -> semver::Version { // ---- rendering --------------------------------------------------------------- -fn render(template: &str, what: &str, context: minijinja::Value) -> Result { - let mut env = minijinja::Environment::new(); - env.add_template(what, template) - .with_context(|| format!("invalid {what} template"))?; - env.get_template(what) - .expect("just added") - .render(context) - .with_context(|| format!("failed to render {what} template")) +fn render( + env: &minijinja::Environment<'_>, + template: &str, + what: &str, + context: minijinja::Value, +) -> Result { + env.render_str(template, context).map_err(|err| { + let stage = if err.kind() == minijinja::ErrorKind::SyntaxError { + "invalid" + } else { + "failed to render" + }; + anyhow::Error::new(err).context(format!("{stage} {what} template")) + }) } /// The `{{ series }}` variable available to `version_format`: the semver @@ -392,7 +390,9 @@ pub fn render_section( ) -> Result { // Empty for a prerelease, which belongs to no series. let series = compatibility_series(version).unwrap_or_default(); + let env = minijinja::Environment::new(); let mut out = render( + &env, &config.version_format, "version_format", minijinja::context! { name, version, date, tag, series }, @@ -401,7 +401,7 @@ pub fn render_section( if !config.categories_enabled() { // Axis off: kind headings sit directly under the version heading. - render_kinds(config, &mut out, name, version, fragments, None)?; + render_kinds(config, &env, &mut out, name, version, fragments, None)?; return Ok(out); } @@ -414,8 +414,16 @@ pub fn render_section( if entries.is_empty() { continue; // same skip-empty rule kinds follow } - render_category_heading(config, &mut out, category, name, version)?; - render_kinds(config, &mut out, name, version, &entries, Some(category))?; + render_category_heading(config, &env, &mut out, category, name, version)?; + render_kinds( + config, + &env, + &mut out, + name, + version, + &entries, + Some(category), + )?; } // Everything that named no category, including the generated ripple @@ -427,8 +435,16 @@ pub fn render_section( .collect(); if !uncategorized.is_empty() { let label = &config.uncategorized_label; - render_category_heading(config, &mut out, label, name, version)?; - render_kinds(config, &mut out, name, version, &uncategorized, Some(label))?; + render_category_heading(config, &env, &mut out, label, name, version)?; + render_kinds( + config, + &env, + &mut out, + name, + version, + &uncategorized, + Some(label), + )?; } Ok(out) } @@ -437,6 +453,7 @@ pub fn render_section( /// custom `category_format` shapes every heading in the section alike. fn render_category_heading( config: &ChangelogConfig, + env: &minijinja::Environment<'_>, out: &mut String, category: &str, name: &str, @@ -444,6 +461,7 @@ fn render_category_heading( ) -> Result<()> { out.push('\n'); out.push_str(&render( + env, &config.category_format, "category_format", minijinja::context! { category, name, version }, @@ -456,6 +474,7 @@ fn render_category_heading( /// with a blank line, which also separates it from a category heading above. fn render_kinds( config: &ChangelogConfig, + env: &minijinja::Environment<'_>, out: &mut String, name: &str, version: &str, @@ -463,12 +482,17 @@ fn render_kinds( category: Option<&str>, ) -> Result<()> { for kind in &config.kinds { - let entries: Vec<&&Fragment> = fragments.iter().filter(|f| f.kind == kind.label).collect(); + let entries: Vec<&Fragment> = fragments + .iter() + .copied() + .filter(|f| f.kind == kind.label) + .collect(); if entries.is_empty() { continue; } out.push('\n'); out.push_str(&render( + env, config.kind_format(), "kind_format", minijinja::context! { kind => kind.label, category, name, version }, @@ -477,6 +501,7 @@ fn render_kinds( out.push('\n'); for fragment in entries { out.push_str(&render( + env, &config.change_format, "change_format", minijinja::context! { @@ -514,23 +539,21 @@ pub struct Adoption { /// content is drift rather than history. pub fn plan_adoption( workspace: &Workspace, - package: &str, + member: &Member, current: &str, ) -> Result> { - let idx = workspace - .member_index(package) - .with_context(|| format!("unknown package `{package}`"))?; + let package = &member.name; let dir = versions_dir(workspace, package); let already_batched = std::fs::read_dir(&dir).is_ok_and(|entries| { entries - .filter_map(|entry| entry.ok()) + .filter_map(Result::ok) .any(|entry| entry.path().extension().is_some_and(|ext| ext == "md")) }); if already_batched { return Ok(None); } - let path = workspace.members[idx].path.join("CHANGELOG.md"); + let path = member.path.join("CHANGELOG.md"); let Ok(text) = std::fs::read_to_string(&path) else { return Ok(None); }; @@ -555,7 +578,7 @@ pub fn plan_adoption( /// Everything after a leading `# ` header line and the blank lines under it. fn strip_header(text: &str) -> &str { let rest = match text.strip_prefix("# ") { - Some(after) => after.split_once('\n').map(|(_, rest)| rest).unwrap_or(""), + Some(after) => after.split_once('\n').map_or("", |(_, rest)| rest), None => text, }; rest.trim() @@ -567,35 +590,35 @@ fn strip_header(text: &str) -> &str { pub fn latest_changelog_version(text: &str) -> Option { text.lines() .filter_map(|line| line.strip_prefix("## ")) - .filter_map(|heading| { - let token = heading.split_whitespace().next()?; - let token = token.trim_matches(['[', ']']); - let token = token.rsplit_once("-v").map(|(_, v)| v).unwrap_or(token); - let token = token.strip_prefix('v').unwrap_or(token); - semver::Version::parse(token).ok() - }) + .filter_map(heading_version) .max() } +/// The version a `## ...` heading names, in any of the tolerated shapes. +pub(crate) fn heading_version(heading: &str) -> Option { + let token = heading.split_whitespace().next()?; + let token = token.trim_matches(['[', ']']); + let token = token.rsplit_once("-v").map_or(token, |(_, v)| v); + let token = token.strip_prefix('v').unwrap_or(token); + semver::Version::parse(token).ok() +} + // ---- batch + merge ----------------------------------------------------------- /// Render a package's complete CHANGELOG.md with an optional pending section /// and an optional block of adopted pre-trellis history. pub fn render_merged_changelog( workspace: &Workspace, - package: &str, + member: &Member, pending: Option<(&semver::Version, &str)>, adopted: Option<&Adoption>, ) -> Result { - workspace - .member_index(package) - .with_context(|| format!("unknown package `{package}`"))?; let config = &workspace.config.changelog; - let dir = versions_dir(workspace, package); + let dir = versions_dir(workspace, &member.name); let mut sections: Vec<(semver::Version, String)> = Vec::new(); if let Ok(entries) = std::fs::read_dir(&dir) { - for entry in entries.filter_map(|e| e.ok()) { + for entry in entries.filter_map(Result::ok) { let path = entry.path(); let Some(stem) = path.file_stem().and_then(|s| s.to_str()) else { continue; @@ -623,7 +646,7 @@ pub fn render_merged_changelog( } sections.sort_by(|a, b| b.0.cmp(&a.0)); - let header = render_header(config, package)?; + let header = render_header(config, &member.name)?; let mut out = header.trim_end().to_string(); out.push('\n'); for (_, section) in §ions { @@ -639,6 +662,7 @@ pub fn render_merged_changelog( /// stub, so seeded changelogs match regenerated ones). pub fn render_header(config: &ChangelogConfig, name: &str) -> Result { render( + &minijinja::Environment::new(), &config.header_format, "header_format", minijinja::context! { name }, @@ -648,20 +672,17 @@ pub fn render_header(config: &ChangelogConfig, name: &str) -> Result { /// Write a pre-rendered version section and complete package changelog. pub fn write_batch( workspace: &Workspace, - package: &str, + member: &Member, version: &semver::Version, section: &str, changelog: &str, ) -> Result<()> { - let idx = workspace - .member_index(package) - .with_context(|| format!("unknown package `{package}`"))?; - let dir = versions_dir(workspace, package); + let dir = versions_dir(workspace, &member.name); std::fs::create_dir_all(&dir).with_context(|| format!("failed to create {}", dir.display()))?; let section_path = dir.join(format!("v{version}.md")); std::fs::write(§ion_path, section) .with_context(|| format!("failed to write {}", section_path.display()))?; - let path = workspace.members[idx].path.join("CHANGELOG.md"); + let path = member.path.join("CHANGELOG.md"); std::fs::write(&path, changelog).with_context(|| format!("failed to write {}", path.display())) } diff --git a/src/commands/changelog.rs b/src/commands/changelog.rs index 5c257bf..26884aa 100644 --- a/src/commands/changelog.rs +++ b/src/commands/changelog.rs @@ -70,7 +70,7 @@ pub fn new_fragment( } bail!( "unknown category `{category}`; configured categories: {}", - changelog::category_labels(categories) + categories.join(", ") ); } if body.trim().is_empty() { @@ -115,6 +115,8 @@ pub struct CheckOptions { struct PackageStatus { name: String, fragments: usize, + /// What the check concluded for this package, shown by every format. + state: EntryState, /// Whether the diff touched this package's own files. False for a package /// the branch documented without editing — a break that propagates to a /// dependent, say. Such a package is only here because it has a fragment, @@ -123,6 +125,19 @@ struct PackageStatus { changed: bool, } +/// What `check` has to say about one package. +#[derive(Debug, Clone, Copy)] +enum EntryState { + /// It has this many fragments. + Present(usize), + /// Nothing is asked of it: `off`, or it never changed. + NotAsked, + /// It needs an entry, and strictness is `warn`. + Warn, + /// It needs an entry, and strictness is `error`. + Error, +} + /// Map the base...head diff to releasable packages and decide which still /// need a changelog fragment. Returns false (non-zero exit) when one does and /// strictness is `error`, or when any fragment is invalid. @@ -144,33 +159,37 @@ pub fn check(workspace: &Workspace, options: &CheckOptions) -> Result { // a break propagating to a dependent is documented where it lands, not // where it originated — and reporting only the diff left that package out // of the table while the release preview happily listed the bump. + // Under `off` the diff is still mapped and reported — the rows are useful + // on their own — but nothing is *asked* of the contributor, so there is no + // verdict to carry into the payload, the preview, or the exit code. let statuses: Vec = workspace .members .iter() .enumerate() .filter(|(_, member)| member.releasable()) .filter_map(|(idx, member)| { - let count = fragments.count_for(&member.name); + let count = fragments.for_package(&member.name).count(); let touched = changed.contains(&idx); + let state = match strictness { + _ if count > 0 => EntryState::Present(count), + Strictness::Off => EntryState::NotAsked, + _ if !touched => EntryState::NotAsked, + Strictness::Warn => EntryState::Warn, + Strictness::Error => EntryState::Error, + }; (touched || count > 0).then(|| PackageStatus { name: member.name.clone(), fragments: count, + state, changed: touched, }) }) .collect(); - - // Under `off` the diff is still mapped and reported — the rows are useful - // on their own — but nothing is *asked* of the contributor, so there is no - // verdict to carry into the payload, the preview, or the exit code. - let needs_entry: Vec<&str> = match strictness { - Strictness::Off => Vec::new(), - Strictness::Warn | Strictness::Error => statuses - .iter() - .filter(|status| status.changed && status.fragments == 0) - .map(|status| status.name.as_str()) - .collect(), - }; + let needs_entry: Vec<&str> = statuses + .iter() + .filter(|status| matches!(status.state, EntryState::Warn | EntryState::Error)) + .map(|status| status.name.as_str()) + .collect(); // A fragment that does not parse is malformed input, not a judgment call, // so it fails at every strictness — and unlike the counts it is *not* // scoped to the branch. A broken fragment on the base branch blocks the @@ -188,7 +207,7 @@ pub fn check(workspace: &Workspace, options: &CheckOptions) -> Result { } else { Vec::new() }; - let preview = preview(&statuses, &needs_entry, &invalid, strictness, &releases); + let preview = preview(&statuses, &needs_entry, &invalid, &releases); match options.format { CheckFormat::Json => { @@ -229,17 +248,14 @@ pub fn check(workspace: &Workspace, options: &CheckOptions) -> Result { ); } for status in &statuses { - let state = if status.fragments > 0 { - format!("{} fragment(s)", status.fragments) - } else if !needs_entry.contains(&status.name.as_str()) { + let state = match status.state { + EntryState::Present(count) => format!("{count} fragment(s)"), // `off`: report the fact, ask for nothing. - crate::term::dim("no entries") - } else if strictness == Strictness::Warn { + EntryState::NotAsked => crate::term::dim("no entries"), // Named as advisory, so a green exit code doesn't read as // the line having been ignored. - crate::term::warn("needs a changelog entry (warning)") - } else { - crate::term::err("needs a changelog entry") + EntryState::Warn => crate::term::warn("needs a changelog entry (warning)"), + EntryState::Error => crate::term::err("needs a changelog entry"), }; crate::status!("{}: {state}", crate::term::package(&status.name)); } @@ -262,14 +278,7 @@ fn print_github_outputs( preview: &str, ) -> Result<()> { println!("ok={ok}"); - println!( - "strictness={}", - match strictness { - Strictness::Warn => "warn", - Strictness::Error => "error", - Strictness::Off => "off", - } - ); + println!("strictness={}", strictness.key()); println!("has_entries={has_entries}"); println!("needs_entry={}", !needs_entry.is_empty()); println!( @@ -334,9 +343,11 @@ fn fragments_changed_by( fragments: fragments .fragments .iter() - .filter(|fragment| match &fragment.path { - Some(path) => !untouched.contains(path), - None => true, + .filter(|fragment| { + fragment + .path + .as_ref() + .is_none_or(|path| !untouched.contains(path)) }) .cloned() .collect(), @@ -373,19 +384,20 @@ fn plan_releases( .for_package(&entry.name) .chain(entry.generated.iter()) .collect(); - let tag = workspace.config.exact_tag(&entry.name, &entry.next); + let next = entry.next.to_string(); + let tag = workspace.config.exact_tag(&entry.name, &next); let section = changelog::render_section( &workspace.config.changelog, &entry.name, - &entry.next, + &next, &tag, &date, &rendered, )?; Ok(ReleasePreview { name: entry.name.clone(), - current: entry.current.clone(), - next: entry.next.clone(), + current: entry.current.to_string(), + next, section, }) }) @@ -399,7 +411,6 @@ fn preview( statuses: &[PackageStatus], needs_entry: &[&str], invalid: &[String], - strictness: Strictness, releases: &[ReleasePreview], ) -> String { let mut out = String::from("### Changelog check\n\n"); @@ -408,20 +419,19 @@ fn preview( } else { out.push_str("| package | fragments | version |\n| --- | --- | --- |\n"); for status in statuses { - let cell = if status.fragments > 0 { - format!("✅ {}", status.fragments) - } else if !needs_entry.contains(&status.name.as_str()) { - "— none".to_string() - } else if strictness == Strictness::Warn { - "⚠️ no entry".to_string() - } else { - "❌ needs an entry".to_string() + let cell = match status.state { + EntryState::Present(count) => format!("✅ {count}"), + EntryState::NotAsked => "— none".to_string(), + EntryState::Warn => "⚠️ no entry".to_string(), + EntryState::Error => "❌ needs an entry".to_string(), }; let version = releases .iter() .find(|release| release.name == status.name) - .map(|release| format!("{} → {}", release.current, release.next)) - .unwrap_or_else(|| "—".to_string()); + .map_or_else( + || "—".to_string(), + |release| format!("{} → {}", release.current, release.next), + ); out.push_str(&format!("| {} | {cell} | {version} |\n", status.name)); } if !needs_entry.is_empty() { diff --git a/src/commands/ci.rs b/src/commands/ci.rs index cc1760b..5ee8a1d 100644 --- a/src/commands/ci.rs +++ b/src/commands/ci.rs @@ -61,7 +61,7 @@ pub fn outputs(workspace: &Workspace) -> Result<()> { .members .iter() .filter(|m| m.releasable()) - .map(|m| format!("{}/gleam.toml", m.rel_path)) + .map(|m| m.rel_file("gleam.toml")) .collect(); let tags: Vec = workspace .members @@ -72,18 +72,15 @@ pub fn outputs(workspace: &Workspace) -> Result<()> { // Deduplicated: a repository-wide series tag is one tag, however many // members move it. let mut series_tags: Vec = Vec::new(); - for member in workspace + for tag in workspace .members .iter() - .filter(|m| m.releasable() && m.tags.iter().any(|t| t.is_series())) + .enumerate() + .filter(|(_, m)| m.releasable()) + .flat_map(|(idx, _)| workspace.series_tags_of(idx)) { - for tag in workspace - .config - .series_tags(&member.name, member.version(), &member.tags) - { - if !series_tags.contains(&tag) { - series_tags.push(tag); - } + if !series_tags.contains(&tag) { + series_tags.push(tag); } } diff --git a/src/commands/doctor.rs b/src/commands/doctor.rs index dffea7b..46c4d27 100644 --- a/src/commands/doctor.rs +++ b/src/commands/doctor.rs @@ -46,95 +46,66 @@ pub struct DoctorOptions { /// apply it. The fix content is computed at check time (it's exactly what the /// canonical command would write), so applying is a single write. /// -/// Every variant carries a package and a workspace-relative path, because +/// Every fix carries a package and a workspace-relative path, because /// `--format json` reports a fix the same way whichever check produced it. -enum Fix { +struct Fix { + kind: FixKind, + package: String, + rel_path: String, + path: PathBuf, + contents: String, +} + +#[derive(Clone, Copy)] +enum FixKind { /// Seed a releasable member's missing CHANGELOG.md with the rendered /// header, so it matches regenerated output byte-for-byte. - SeedChangelog { - package: String, - rel_path: String, - path: PathBuf, - contents: String, - }, + SeedChangelog, /// Rewrite a manifest.toml's locked workspace-internal versions — the same /// operation `version apply` performs. - PatchLockfile { - package: String, - rel_path: String, - path: PathBuf, - contents: String, - }, + PatchLockfile, /// Capture a package's pre-trellis CHANGELOG.md body as a version section, /// so regenerating the changelog preserves it. `version apply` does this on /// a first release anyway; doing it here makes it visible beforehand. - AdoptChangelog { - package: String, - rel_path: String, - path: PathBuf, - contents: String, - }, + AdoptChangelog, } impl Fix { /// Stable identifier for the wire format; `describe` is the prose beside it. fn kind(&self) -> &'static str { - match self { - Fix::SeedChangelog { .. } => "seed_changelog", - Fix::PatchLockfile { .. } => "patch_lockfile", - Fix::AdoptChangelog { .. } => "adopt_changelog", + match self.kind { + FixKind::SeedChangelog => "seed_changelog", + FixKind::PatchLockfile => "patch_lockfile", + FixKind::AdoptChangelog => "adopt_changelog", } } fn describe(&self) -> String { - match self { - Fix::SeedChangelog { package, .. } => format!("seed CHANGELOG.md for `{package}`"), - Fix::PatchLockfile { rel_path, .. } => format!("patch locked versions in {rel_path}"), - Fix::AdoptChangelog { package, .. } => { - format!("adopt existing changelog history for `{package}`") + match self.kind { + FixKind::SeedChangelog => format!("seed CHANGELOG.md for `{}`", self.package), + FixKind::PatchLockfile => format!("patch locked versions in {}", self.rel_path), + FixKind::AdoptChangelog => { + format!("adopt existing changelog history for `{}`", self.package) } } } - fn package(&self) -> &str { - match self { - Fix::SeedChangelog { package, .. } - | Fix::PatchLockfile { package, .. } - | Fix::AdoptChangelog { package, .. } => package, - } - } - - fn rel_path(&self) -> &str { - match self { - Fix::SeedChangelog { rel_path, .. } - | Fix::PatchLockfile { rel_path, .. } - | Fix::AdoptChangelog { rel_path, .. } => rel_path, - } - } - fn record(&self) -> FixRecord<'_> { FixRecord { kind: self.kind(), description: self.describe(), - file: self.rel_path(), - package: Some(self.package()), + file: &self.rel_path, + package: Some(&self.package), } } fn apply(&self) -> Result<()> { - let (path, contents) = match self { - Fix::SeedChangelog { path, contents, .. } => (path, contents), - Fix::PatchLockfile { path, contents, .. } => (path, contents), - Fix::AdoptChangelog { path, contents, .. } => { - if let Some(parent) = path.parent() { - std::fs::create_dir_all(parent) - .with_context(|| format!("failed to create {}", parent.display()))?; - } - (path, contents) - } - }; - std::fs::write(path, contents) - .with_context(|| format!("failed to write {}", path.display())) + if let Some(parent) = self.path.parent() { + std::fs::create_dir_all(parent) + .with_context(|| format!("failed to create {}", parent.display()))?; + } + std::fs::write(&self.path, &self.contents) + .with_context(|| format!("failed to write {}", self.path.display())) } } @@ -158,12 +129,6 @@ impl Report { fn push(&mut self, finding: Finding) { self.findings.push(finding); } - fn error(&mut self, check: Check, message: impl Into) { - self.push(Finding::error(check, message)); - } - fn fix(&mut self, fix: Fix) { - self.fixes.push(fix); - } fn of_severity(&self, severity: Severity) -> impl Iterator { self.findings.iter().filter(move |f| f.severity == severity) } @@ -412,7 +377,7 @@ fn check_fragments(workspace: &Workspace, report: &mut Report) { report.push(finding); } } - Err(err) => report.error(Check::ChangelogFragment, format!("{err:#}")), + Err(err) => report.push(Finding::error(Check::ChangelogFragment, format!("{err:#}"))), } } @@ -573,7 +538,7 @@ fn check_exclusions(workspace: &Workspace, report: &mut Report) { ), ) .at(format!("{}/gleam.toml", member.rel_path)) - .in_package(member.name.clone()), + .in_package(&member.name), ); } } @@ -581,34 +546,27 @@ fn check_exclusions(workspace: &Workspace, report: &mut Report) { } fn check_member_glob(workspace: &Workspace, label: &str, pattern: &str, report: &mut Report) { - let matches = globset::Glob::new(pattern) - .ok() - .map(|glob| glob.compile_matcher()) - .map(|matcher| { - workspace + let problem = match globset::Glob::new(pattern).map(|glob| glob.compile_matcher()) { + Err(_) => "is invalid", + Ok(matcher) + if !workspace .members .iter() - .any(|member| matcher.is_match(&member.rel_path)) - }); - // Both cases are a claim about the root manifest's [tools.trellis] table, - // which is where the glob was written. - match matches { - Some(true) => {} - Some(false) => report.push( - Finding::error( - Check::ExclusionGlob, - format!("{label} `{pattern}` matches no member (typo?)"), - ) - .at(crate::workspace::GLEAM_TOML), - ), - None => report.push( - Finding::error( - Check::ExclusionGlob, - format!("{label} `{pattern}` is invalid"), - ) - .at(crate::workspace::GLEAM_TOML), - ), - } + .any(|m| matcher.is_match(&m.rel_path)) => + { + "matches no member (typo?)" + } + Ok(_) => return, + }; + // Either way the claim is about the root manifest's [tools.trellis] + // table, which is where the glob was written. + report.push( + Finding::error( + Check::ExclusionGlob, + format!("{label} `{pattern}` {problem}"), + ) + .at(crate::workspace::GLEAM_TOML), + ); } /// Check 7: no two releasable members produce the same tag, for series tags as @@ -663,7 +621,7 @@ fn check_tag_collisions(workspace: &Workspace, report: &mut Report) { let series_members: Vec<&str> = workspace .members .iter() - .filter(|m| m.releasable() && m.tags.iter().any(|t| t.is_series())) + .filter(|m| m.releasable() && m.has_series_tag()) .map(|m| m.name.as_str()) .collect(); let names = |members: &[&str]| { @@ -707,15 +665,11 @@ fn check_tag_collisions(workspace: &Workspace, report: &mut Report) { // Members sharing a legacy `{name}`-less series tag is intentional — the // ambiguity warning above covers it — so claim each such tag only once. let mut legacy_claimed: std::collections::HashSet = std::collections::HashSet::new(); - for member in workspace - .members - .iter() - .filter(|m| m.releasable() && m.tags.iter().any(|t| t.is_series())) - { - for tag in workspace - .config - .series_tags(&member.name, member.version(), &member.tags) - { + for (idx, member) in workspace.members.iter().enumerate() { + if !member.releasable() { + continue; + } + for tag in workspace.series_tags_of(idx) { if repo_wide && !legacy_claimed.insert(tag.clone()) { continue; } @@ -770,42 +724,21 @@ fn check_lockfiles(workspace: &Workspace, report: &mut Report) { .collect(); for member in &workspace.members { - let path = member.path.join("manifest.toml"); - if !path.is_file() { - continue; // not generated yet; nothing to drift - } - let rel_path = format!("{}/manifest.toml", member.rel_path); - let text = match std::fs::read_to_string(&path) { - Ok(text) => text, - Err(err) => { - report.push( - Finding::error( - Check::LockfileDrift, - format!("failed to read {}: {err}", path.display()), - ) - .at(&rel_path) - .in_package(member.name.clone()), - ); - continue; - } - }; - let (new_text, patched) = match lockfile::patch_locked_versions(&text, &versions) { - Ok(result) => result, + let lockfile = match lockfile::patch_member(member, &versions) { + Ok(Some(lockfile)) => lockfile, + Ok(None) => continue, // no lockfile yet, or nothing drifted Err(err) => { report.push( Finding::error(Check::LockfileDrift, format!("{err:#}")) - .at(&rel_path) - .in_package(member.name.clone()), + .at(format!("{}/manifest.toml", member.rel_path)) + .in_package(&member.name), ); continue; } }; - if patched.is_empty() { - continue; - } // One rewrite clears every drifted entry in this manifest, so each of // these findings is fixable by the single fix pushed below. - for entry in &patched { + for entry in &lockfile.patched { report.push( Finding::error( Check::LockfileDrift, @@ -815,16 +748,17 @@ fn check_lockfiles(workspace: &Workspace, report: &mut Report) { member.rel_path, entry.name, entry.old, entry.new ), ) - .at(&rel_path) - .in_package(member.name.clone()) + .at(&lockfile.rel_path) + .in_package(&member.name) .fixable(), ); } - report.fix(Fix::PatchLockfile { + report.fixes.push(Fix { + kind: FixKind::PatchLockfile, package: member.name.clone(), - rel_path, - path, - contents: new_text, + rel_path: lockfile.rel_path, + path: lockfile.path, + contents: lockfile.text, }); } } @@ -870,11 +804,12 @@ fn check_changelogs(workspace: &Workspace, report: &mut Report) { format!("releasable package `{}` has no CHANGELOG.md", member.name), ) .at(&rel_changelog) - .in_package(member.name.clone()) + .in_package(&member.name) .fixable_if(header.is_ok()), ); match header { - Ok(header) => report.fix(Fix::SeedChangelog { + Ok(header) => report.fixes.push(Fix { + kind: FixKind::SeedChangelog, package: member.name.clone(), rel_path: rel_changelog, path: changelog, @@ -888,7 +823,7 @@ fn check_changelogs(workspace: &Workspace, report: &mut Report) { member.name ), ) - .in_package(member.name.clone()), + .in_package(&member.name), ), } continue; @@ -900,7 +835,7 @@ fn check_changelogs(workspace: &Workspace, report: &mut Report) { format!("could not read {}/CHANGELOG.md", member.rel_path), ) .at(&rel_changelog) - .in_package(member.name.clone()), + .in_package(&member.name), ); continue; }; @@ -915,7 +850,7 @@ fn check_changelogs(workspace: &Workspace, report: &mut Report) { ), ) .at(format!("{}/gleam.toml", member.rel_path)) - .in_package(member.name.clone()), + .in_package(&member.name), ); continue; }; @@ -933,7 +868,7 @@ fn check_changelogs(workspace: &Workspace, report: &mut Report) { ), ) .at(format!("{}/gleam.toml", member.rel_path)) - .in_package(member.name.clone()), + .in_package(&member.name), ); } @@ -941,7 +876,7 @@ fn check_changelogs(workspace: &Workspace, report: &mut Report) { // was never batched would vanish on the next release. `version apply` // adopts it automatically; surfacing it here means nobody meets it for // the first time mid-release. - match crate::changelog::plan_adoption(workspace, &member.name, member.version()) { + match crate::changelog::plan_adoption(workspace, member, member.version()) { Ok(Some(adoption)) => { report.push( Finding::warning( @@ -953,10 +888,11 @@ fn check_changelogs(workspace: &Workspace, report: &mut Report) { ), ) .at(&rel_changelog) - .in_package(member.name.clone()) + .in_package(&member.name) .fixable(), ); - report.fix(Fix::AdoptChangelog { + report.fixes.push(Fix { + kind: FixKind::AdoptChangelog, package: member.name.clone(), rel_path: rel_changelog, path: adoption.path, @@ -970,7 +906,7 @@ fn check_changelogs(workspace: &Workspace, report: &mut Report) { format!("cannot read `{}`'s changelog history: {err:#}", member.name), ) .at(&rel_changelog) - .in_package(member.name.clone()), + .in_package(&member.name), ), } } diff --git a/src/commands/exec.rs b/src/commands/exec.rs index 0c8f14b..2c54d7e 100644 --- a/src/commands/exec.rs +++ b/src/commands/exec.rs @@ -27,7 +27,7 @@ pub fn run(workspace: &Workspace, options: &ExecOptions) -> Result { releasable_only: false, })?; - let jobs = selected + let jobs: Vec = selected .into_iter() .map(|idx| Job { member: idx, @@ -39,36 +39,17 @@ pub fn run(workspace: &Workspace, options: &ExecOptions) -> Result { }) .collect(); - let parallelism = if options.serial { - 1 - } else { - options.jobs.unwrap_or_else(|| { - std::thread::available_parallelism() - .map(|n| n.get()) - .unwrap_or(4) - }) + let run_options = RunOptions { + parallelism: RunOptions::parallelism(options.serial, options.jobs), + keep_going: options.keep_going, + json: options.json, }; - let results = runner::run_jobs( - workspace, - jobs, - &RunOptions { - parallelism, - keep_going: options.keep_going, - json: options.json, - }, - )?; - let ok = runner::all_succeeded(&results); - if options.json { - let document = crate::json::ExecDocument { + runner::run_and_report(workspace, &jobs, &run_options, |ok, results| { + serde_json::to_string_pretty(&crate::json::ExecDocument { schema: crate::json::ExecDocument::SCHEMA, ok, command: &options.command, - results: results - .iter() - .map(|result| crate::json::TaskResult::new(workspace, result)) - .collect(), - }; - println!("{}", serde_json::to_string_pretty(&document)?); - } - Ok(ok) + results, + }) + }) } diff --git a/src/commands/generate.rs b/src/commands/generate.rs index 6175484..9953fb3 100644 --- a/src/commands/generate.rs +++ b/src/commands/generate.rs @@ -14,7 +14,6 @@ use anyhow::{Context, Result}; use clap::CommandFactory; use clap_complete::aot::Shell; use std::fs; -use std::io::Write; use std::path::Path; /// The environment variable the registration scripts set to ask `trellis` for @@ -58,12 +57,15 @@ pub fn completions(shell: Shell) -> Result<()> { let completer = shells .completer(&name) .with_context(|| format!("no completion support for shell `{name}`"))?; - let mut buf = Vec::new(); completer - .write_registration(COMPLETE_VAR, "trellis", "trellis", "trellis", &mut buf) - .with_context(|| format!("generating the {name} completion script"))?; - std::io::stdout().write_all(&buf)?; - Ok(()) + .write_registration( + COMPLETE_VAR, + "trellis", + "trellis", + "trellis", + &mut std::io::stdout().lock(), + ) + .with_context(|| format!("generating the {name} completion script")) } /// The preamble `roff::Roff::to_writer` emits on every call, defining the `\*(Aq` diff --git a/src/commands/info.rs b/src/commands/info.rs index c271ae5..90a9894 100644 --- a/src/commands/info.rs +++ b/src/commands/info.rs @@ -38,50 +38,42 @@ pub fn run(workspace: &Workspace, name: &str, json: bool) -> Result<()> { workspace.config.exact_tag(&member.name, member.version()) ); } - if member.tags.iter().any(|level| level.is_series()) { - for tag in workspace - .config - .series_tags(&member.name, member.version(), &member.tags) - { - crate::status!("{} {tag}", label("series tag:")); - } + for tag in workspace.series_tags_of(idx) { + crate::status!("{} {tag}", label("series tag:")); } - let format_names = |indices: &[usize]| -> String { - if indices.is_empty() { - "(none)".to_string() - } else { - indices - .iter() - .map(|&i| workspace.members[i].name.clone()) - .collect::>() - .join(", ") - } - }; + let names = + |indices: &[usize]| or_none(indices.iter().map(|&i| workspace.members[i].name.as_str())); crate::status!( "{} {}", label("workspace deps:"), - format_names(workspace.deps_of(idx)) + names(workspace.deps_of(idx)) ); crate::status!( "{} {}", label("workspace dependents:"), - format_names(workspace.dependents_of(idx)) + names(workspace.dependents_of(idx)) ); - let hex_deps: Vec = member - .manifest - .dependencies - .iter() - .filter(|dep| matches!(dep.requirement, Requirement::Hex(_))) - .map(|dep| dep.name.clone()) - .collect(); crate::status!( "{} {}", label("hex deps:"), - if hex_deps.is_empty() { - "(none)".to_string() - } else { - hex_deps.join(", ") - } + or_none( + member + .manifest + .dependencies + .iter() + .filter(|dep| matches!(dep.requirement, Requirement::Hex(_))) + .map(|dep| dep.name.as_str()) + ) ); Ok(()) } + +/// Comma-separated, or `(none)` when empty. +fn or_none<'a>(names: impl Iterator) -> String { + let joined = names.collect::>().join(", "); + if joined.is_empty() { + "(none)".to_string() + } else { + joined + } +} diff --git a/src/commands/lockfile.rs b/src/commands/lockfile.rs index e78504c..d3c7f32 100644 --- a/src/commands/lockfile.rs +++ b/src/commands/lockfile.rs @@ -4,20 +4,15 @@ //! retry policy. use crate::tools; -use crate::workspace::Workspace; +use crate::workspace::{SelectionFilter, Workspace}; use anyhow::{Context, Result, bail}; use std::process::Command; pub fn refresh(workspace: &Workspace, package: Option<&str>) -> Result { - let targets: Vec = match package { - Some(name) => { - let idx = workspace - .member_index(name) - .with_context(|| format!("unknown package `{name}`"))?; - vec![idx] - } - None => (0..workspace.members.len()).collect(), - }; + let targets = workspace.select(&SelectionFilter { + names: package.map(str::to_string).into_iter().collect(), + ..SelectionFilter::default() + })?; let retry = &workspace.config.publish.retry; for idx in targets { diff --git a/src/commands/mod.rs b/src/commands/mod.rs index 345d67d..7babed9 100644 --- a/src/commands/mod.rs +++ b/src/commands/mod.rs @@ -30,8 +30,7 @@ pub fn markdown_help() -> String { // supplies the page title, and a second H1 would double the heading. let body = body .find("**Command Overview:**") - .map(|idx| &body[idx..]) - .unwrap_or(&body); + .map_or(body.as_str(), |idx| &body[idx..]); format!( "---\n\ title: CLI reference\n\ diff --git a/src/commands/pin.rs b/src/commands/pin.rs index cbc1914..f27e12d 100644 --- a/src/commands/pin.rs +++ b/src/commands/pin.rs @@ -8,9 +8,10 @@ use crate::workspace::{SelectionFilter, Workspace}; use anyhow::{Context, Result}; +use std::collections::hash_map::Entry; use std::collections::{BTreeMap, HashMap, HashSet}; use std::path::Path; -use toml_edit::{DocumentMut, Value}; +use toml_edit::DocumentMut; /// The comment marker recording a pinned dependency's tracked ref. The /// trailing space is part of the marker: `# trellis:pin` with no ref after it @@ -144,9 +145,7 @@ fn apply_changes( .and_then(|dep| dep.get_mut("ref")) && reference.as_str() != Some(change.new_ref.as_str()) { - let mut replacement = Value::from(change.new_ref.clone()); - *replacement.decor_mut() = reference.decor().clone(); - *reference = replacement; + crate::lockfile::set_str_keep_decor(reference, &change.new_ref); changed = true; } let decor = value.decor_mut(); @@ -155,10 +154,10 @@ fn apply_changes( .and_then(|raw| raw.as_str()) .unwrap_or_default() .to_string(); - let base = match old_suffix.find(MARKER) { - Some(idx) => old_suffix[..idx].trim_end(), - None => old_suffix.trim_end(), - }; + let base = old_suffix + .find(MARKER) + .map_or(old_suffix.as_str(), |idx| &old_suffix[..idx]) + .trim_end(); let new_suffix = match &change.comment { Some(tracked) => format!("{base} {MARKER}{tracked}"), None => base.to_string(), @@ -178,13 +177,12 @@ fn resolve( url: &str, refname: &str, ) -> Result> { - let key = (url.to_string(), refname.to_string()); - if let Some(sha) = cache.get(&key) { - return Ok(sha.clone()); + match cache.entry((url.to_string(), refname.to_string())) { + Entry::Occupied(hit) => Ok(hit.get().clone()), + Entry::Vacant(miss) => Ok(miss + .insert(crate::git::ls_remote_commit(root, url, refname)?) + .clone()), } - let sha = crate::git::ls_remote_commit(root, url, refname)?; - cache.insert(key, sha.clone()); - Ok(sha) } pub fn run(workspace: &Workspace, options: &PinOptions) -> Result { @@ -216,7 +214,9 @@ pub fn run(workspace: &Workspace, options: &PinOptions) -> Result { for dep in scan_git_deps(&text).with_context(|| format!("in {}/gleam.toml", member.rel_path))? { - match options.mode { + // Each mode decides the verb, the new `ref`, the tracked ref to + // record (`None` strips the pin comment), and the dimmed detail. + let (verb, new_ref, comment, detail) = match options.mode { Mode::Pin => { if is_full_sha(&dep.git_ref) { continue; // already pinned, or a hand-written SHA @@ -225,22 +225,8 @@ pub fn run(workspace: &Workspace, options: &PinOptions) -> Result { .with_context(|| { format!("ref `{}` not found on {}", dep.git_ref, dep.url) })?; - crate::status!( - "[{}] {} {} {} {}", - crate::term::package(&member.name), - crate::term::ok("pinned"), - dep.name, - short(&sha), - crate::term::dim(&format!("tracking {}", dep.git_ref)) - ); - commits.insert(dep.name.clone(), sha.clone()); - changes.insert( - (dep.section.to_string(), dep.name), - RefChange { - new_ref: sha, - comment: Some(dep.git_ref), - }, - ); + let detail = format!("tracking {}", dep.git_ref); + ("pinned", sha, Some(dep.git_ref), detail) } Mode::Update => { let Some(tracked) = dep.pinned else { @@ -253,47 +239,41 @@ pub fn run(workspace: &Workspace, options: &PinOptions) -> Result { if sha == dep.git_ref { continue; } - crate::status!( - "[{}] {} {} {} {}", - crate::term::package(&member.name), - crate::term::ok("updated"), - dep.name, - short(&sha), - crate::term::dim(&format!( - "was {}, tracking {tracked}", - short(&dep.git_ref) - )) - ); - commits.insert(dep.name.clone(), sha.clone()); - changes.insert( - (dep.section.to_string(), dep.name), - RefChange { - new_ref: sha, - comment: Some(tracked), - }, - ); + let detail = format!("was {}, tracking {tracked}", short(&dep.git_ref)); + ("updated", sha, Some(tracked), detail) } Mode::Unpin => { let Some(tracked) = dep.pinned else { continue; }; - crate::status!( - "[{}] {} {} {}", - crate::term::package(&member.name), - crate::term::ok("unpinned"), - dep.name, - crate::term::dim(&format!("restored {tracked}")) - ); - changes.insert( - (dep.section.to_string(), dep.name), - RefChange { - new_ref: tracked, - comment: None, - }, - ); + let detail = format!("restored {tracked}"); + ("unpinned", tracked, None, detail) } + // ponytail: Check is handled before this match; splitting Mode into + // Check | Rewrite(..) in main.rs would remove this arm. Mode::Check => unreachable!("handled above"), + }; + // Pinning and updating name the SHA they landed on; unpinning + // has none to show. + let sha = if comment.is_some() { + format!("{} ", short(&new_ref)) + } else { + String::new() + }; + crate::status!( + "[{}] {} {} {sha}{}", + crate::term::package(&member.name), + crate::term::ok(verb), + dep.name, + crate::term::dim(&detail) + ); + if comment.is_some() { + commits.insert(dep.name.clone(), new_ref.clone()); } + changes.insert( + (dep.section.to_string(), dep.name), + RefChange { new_ref, comment }, + ); } if changes.is_empty() { continue; diff --git a/src/commands/release.rs b/src/commands/release.rs index b7f7ea4..9f51367 100644 --- a/src/commands/release.rs +++ b/src/commands/release.rs @@ -6,15 +6,14 @@ //! //! `trellis release bootstrap` — `tag create` for adopting trellis on a //! repository that already has the versions and changelogs it wants; see -//! [`bootstrap`]. +//! [`crate::commands::tag::create`]. use crate::commands::version_override::Overrides; use crate::commands::{tag, version}; +use crate::git::{git_output, git_stdout, git_with_identity}; use crate::github::GitHubClient; use crate::workspace::Workspace; use anyhow::{Context, Result, bail}; -use std::path::Path; -use std::process::Command; pub struct PrOptions { /// Base branch the PR targets. @@ -52,25 +51,10 @@ pub fn pr(workspace: &Workspace, options: &PrOptions) -> Result { } build_release_commit_and_pr(&workspace, options, &plan) })(); - crate::term::trace_command("git", &["checkout", &original_branch], root); - let _ = Command::new("git") - .args(["checkout", &original_branch]) - .current_dir(root) - .output(); + let _ = git_output(root, &["checkout", &original_branch]); result } -/// `trellis release bootstrap` — an alias for `tag create` under the release -/// umbrella, for the repository *adopting* trellis: versions and changelogs -/// are already right, only the tags (and GitHub Releases) are missing. -/// Unlike `release pr`, it never runs `version apply` and requires no -/// unreleased changelog fragments — `tag::plan_tags` reads versions straight -/// off `gleam.toml`. -pub fn bootstrap(workspace: &Workspace, options: &tag::CreateOptions) -> Result { - tag::create(workspace, options)?; - Ok(true) -} - fn build_release_commit_and_pr( workspace: &Workspace, options: &PrOptions, @@ -89,10 +73,7 @@ fn build_release_commit_and_pr( let title = format!("release: {summary}"); git_stdout(root, &["add", "-A"])?; - let mut commit_args = crate::git::identity_fallback_args(root); - commit_args.extend(["commit".into(), "-m".into(), format!("release: {summary}")]); - let commit_args: Vec<&str> = commit_args.iter().map(String::as_str).collect(); - git_stdout(root, &commit_args)?; + git_with_identity(root, &["commit", "-m", &title])?; // Prepare on detached HEAD so failures never move an existing local // release branch; only the remote branch is replaced after the commit is @@ -132,14 +113,8 @@ fn pr_body(workspace: &Workspace, plan: &[version::PlanEntry]) -> String { )); } for entry in plan { - let Some(idx) = workspace.member_index(&entry.name) else { - continue; - }; - let changelog = workspace.members[idx].path.join("CHANGELOG.md"); - if let Some(section) = std::fs::read_to_string(changelog) - .ok() - .and_then(|text| tag::changelog_section(&text, &entry.next)) - { + let member = &workspace.members[entry.member]; + if let Some(section) = tag::release_notes(member, &entry.next.to_string()) { body.push_str(&format!( "\n## {} v{}\n\n{section}\n", entry.name, entry.next @@ -148,20 +123,3 @@ fn pr_body(workspace: &Workspace, plan: &[version::PlanEntry]) -> String { } body } - -fn git_stdout(cwd: &Path, args: &[&str]) -> Result { - crate::term::trace_command("git", args, cwd); - let output = Command::new("git") - .args(args) - .current_dir(cwd) - .output() - .context("failed to run `git`")?; - if !output.status.success() { - bail!( - "`git {}` failed: {}", - args.join(" "), - String::from_utf8_lossy(&output.stderr).trim() - ); - } - Ok(String::from_utf8_lossy(&output.stdout).into_owned()) -} diff --git a/src/commands/run.rs b/src/commands/run.rs index d4a914c..26ff8e5 100644 --- a/src/commands/run.rs +++ b/src/commands/run.rs @@ -29,6 +29,8 @@ pub struct TaskOptions { impl Target { /// The flag value as the user gave it, for the JSON payload. + // ponytail: mirrors the clap ValueEnum names by hand; deriving Serialize + // and typing RunDocument.target as Target would delete it. fn as_str(self) -> &'static str { match self { Target::Erlang => "erlang", @@ -49,11 +51,7 @@ pub fn run(workspace: &Workspace, options: &TaskOptions) -> Result { releasable_only: false, })?; if let Some(patterns) = workspace.config.exclude.get(&options.task) { - let mut builder = globset::GlobSetBuilder::new(); - for pattern in patterns { - builder.add(globset::Glob::new(pattern)?); - } - let excluded = builder.build()?; + let excluded = crate::workspace::build_globset(patterns)?; selected.retain(|&idx| !excluded.is_match(&workspace.members[idx].rel_path)); } @@ -67,40 +65,19 @@ pub fn run(workspace: &Workspace, options: &TaskOptions) -> Result { }); } - let results = runner::run_jobs( - workspace, - jobs, - &RunOptions { - parallelism: effective_jobs(options), - keep_going: options.keep_going, - json: options.json, - }, - )?; - let ok = runner::all_succeeded(&results); - if options.json { - let document = crate::json::RunDocument { + let run_options = RunOptions { + parallelism: RunOptions::parallelism(options.serial, options.jobs), + keep_going: options.keep_going, + json: options.json, + }; + runner::run_and_report(workspace, &jobs, &run_options, |ok, results| { + serde_json::to_string_pretty(&crate::json::RunDocument { schema: crate::json::RunDocument::SCHEMA, ok, task: &options.task, target: options.target.map(Target::as_str), - results: results - .iter() - .map(|result| crate::json::TaskResult::new(workspace, result)) - .collect(), - }; - println!("{}", serde_json::to_string_pretty(&document)?); - } - Ok(ok) -} - -fn effective_jobs(options: &TaskOptions) -> usize { - if options.serial { - return 1; - } - options.jobs.unwrap_or_else(|| { - std::thread::available_parallelism() - .map(|n| n.get()) - .unwrap_or(4) + results, + }) }) } @@ -195,7 +172,7 @@ fn targeted( fn gleam(args: &[&str], package_dir: &Path) -> CommandSpec { CommandSpec { program: crate::tools::gleam_bin(), - args: args.iter().map(|s| s.to_string()).collect(), + args: args.iter().map(|&s| s.to_string()).collect(), cwd: package_dir.to_path_buf(), } } diff --git a/src/commands/tag.rs b/src/commands/tag.rs index 4365d8c..ea82a70 100644 --- a/src/commands/tag.rs +++ b/src/commands/tag.rs @@ -10,16 +10,15 @@ //! package's list. Only immutable tags can carry a GitHub Release. use crate::config::TagLevel; -use crate::git::git_stdout; +use crate::git::{git_output, git_stdout, git_with_identity}; use crate::github::GitHubClient; use crate::gleam::GleamManifest; use crate::json::TagPlanDocument; -use crate::workspace::Workspace; +use crate::workspace::{Member, Workspace}; use anyhow::{Context, Result, bail}; use serde::Serialize; use std::collections::{HashMap, HashSet}; use std::path::Path; -use std::process::Command; /// Which tag lifecycle a planned tag belongs to. The serialized names are wire /// format — see `crate::json`. @@ -80,11 +79,16 @@ pub(crate) fn plan_tags(workspace: &Workspace) -> Result> { .collect(); let mut planned: Vec = Vec::new(); let mut claimed: HashSet = HashSet::new(); - for (member, index) in workspace + let mut claim = |planned: &mut Vec, entry: PlannedTag| { + if claimed.insert(entry.tag.clone()) { + planned.push(entry); + } + }; + for (index, member) in workspace .members .iter() - .zip(0..) - .filter(|(member, _)| member.releasable()) + .enumerate() + .filter(|(_, member)| member.releasable()) { if member.tags.contains(&TagLevel::Exact) { let tag = workspace.config.exact_tag(&member.name, member.version()); @@ -93,15 +97,16 @@ pub(crate) fn plan_tags(workspace: &Workspace) -> Result> { } else { TagAction::Create }; - if claimed.insert(tag.clone()) { - planned.push(PlannedTag { + claim( + &mut planned, + PlannedTag { member: index, version: member.version().to_string(), tag, kind: TagKind::Exact, action, - }); - } + }, + ); } // A prerelease belongs to no series, and so moves no tag. if member.tags.iter().any(|level| level.is_series()) { @@ -116,15 +121,16 @@ pub(crate) fn plan_tags(workspace: &Workspace) -> Result> { } else { TagAction::Move }; - if claimed.insert(tag.clone()) { - planned.push(PlannedTag { + claim( + &mut planned, + PlannedTag { member: index, version: member.version().to_string(), tag, kind: TagKind::Series, action, - }); - } + }, + ); } } } @@ -290,6 +296,7 @@ pub fn plan(workspace: &Workspace, json: bool) -> Result<()> { Ok(()) } +#[derive(Debug, Clone, Copy)] pub struct CreateOptions { pub push: bool, pub github_release: bool, @@ -299,7 +306,12 @@ pub struct CreateOptions { } pub fn create(workspace: &Workspace, options: &CreateOptions) -> Result<()> { - let push = options.push || options.github_release; + // A GitHub Release needs the tag on origin, so it implies a push. + let options = CreateOptions { + push: options.push || options.github_release, + ..*options + }; + let push = options.push; if push { reconcile_remote_repository_series_tag(workspace)?; } @@ -361,16 +373,11 @@ pub fn create(workspace: &Workspace, options: &CreateOptions) -> Result<()> { for planned in targets { let remote_oid = remote_oids.get(&planned.tag).map(String::as_str); match planned.kind { - TagKind::Exact => create_exact_tag( - workspace, - planned, - options, - github.as_ref(), - push, - remote_oid, - )?, + TagKind::Exact => { + create_exact_tag(workspace, planned, options, github.as_ref(), remote_oid)? + } TagKind::Series | TagKind::RepositorySeries => { - move_series_tag(workspace, planned, options, push, remote_oid)? + move_series_tag(workspace, planned, options, remote_oid)? } } } @@ -422,9 +429,8 @@ fn reconcile_remote_repository_series_tag(workspace: &Workspace) -> Result<()> { fn create_exact_tag( workspace: &Workspace, planned: &PlannedTag, - options: &CreateOptions, + options: CreateOptions, github: Option<&GitHubClient>, - push: bool, remote_oid: Option<&str>, ) -> Result<()> { let member = &workspace.members[planned.member]; @@ -443,20 +449,12 @@ fn create_exact_tag( } else if options.dry_run { crate::status!("{}", crate::term::dim(&format!("would tag {tag}"))); } else { - let mut args = crate::git::identity_fallback_args(&workspace.root); - args.extend([ - "tag".into(), - "-a".into(), - tag.clone(), - "-m".into(), - format!("{} {}", member.name, planned.version), - ]); - let args: Vec<&str> = args.iter().map(String::as_str).collect(); - git_stdout(&workspace.root, &args)?; + let message = format!("{} {}", member.name, planned.version); + git_with_identity(&workspace.root, &["tag", "-a", tag, "-m", &message])?; crate::status!("{} {tag}", crate::term::ok("tagged")); } } - if push && remote_oid.is_none() { + if options.push && remote_oid.is_none() { if options.dry_run { crate::status!("{}", crate::term::dim(&format!("would push {tag}"))); } else { @@ -477,7 +475,8 @@ fn create_exact_tag( crate::term::dim(&format!("would create GitHub release {tag}")) ); } else { - let notes = release_notes(workspace, planned.member); + let notes = release_notes(member, member.version()) + .unwrap_or_else(|| format!("{} {}", member.name, member.version())); github.create_release(tag, tag, ¬es)?; crate::status!("{} GitHub release {tag}", crate::term::ok("created")); } @@ -492,8 +491,7 @@ fn create_exact_tag( fn move_series_tag( workspace: &Workspace, planned: &PlannedTag, - options: &CreateOptions, - push: bool, + options: CreateOptions, remote_oid: Option<&str>, ) -> Result<()> { let member = &workspace.members[planned.member]; @@ -508,21 +506,12 @@ fn move_series_tag( if options.dry_run { crate::status!("{}", crate::term::dim(&format!("would {verb} {tag}"))); } else { - let mut args = crate::git::identity_fallback_args(&workspace.root); - args.extend([ - "tag".into(), - "-f".into(), - "-a".into(), - tag.clone(), - "-m".into(), - format!("{} {}", member.name, member.version()), - ]); - let args: Vec<&str> = args.iter().map(String::as_str).collect(); - git_stdout(&workspace.root, &args)?; + let message = format!("{} {}", member.name, member.version()); + git_with_identity(&workspace.root, &["tag", "-f", "-a", tag, "-m", &message])?; crate::status!("{} {tag}", crate::term::ok(done)); } } - if push { + if options.push { // Re-read after any move: a re-tag writes a fresh annotated object, so // it never matches origin (in a dry run `moved` stands in for that); // the comparison also catches "already where it belongs, but origin @@ -556,27 +545,18 @@ fn move_series_tag( } fn local_tag_oid(root: &Path, tag: &str) -> Result> { - rev_parse(root, &format!("refs/tags/{tag}"), tag) -} - -fn rev_parse(root: &Path, reference: &str, subject: &str) -> Result> { - let args = ["rev-parse", "--verify", "--quiet", reference]; - crate::term::trace_command("git", &args, root); - let output = Command::new("git") - .args(args) - .current_dir(root) - .output() - .context("failed to run git")?; + let reference = format!("refs/tags/{tag}"); + let output = git_output(root, &["rev-parse", "--verify", "--quiet", &reference])?; match output.status.code() { Some(0) => output .stdout - .split(|byte| byte.is_ascii_whitespace()) + .split(u8::is_ascii_whitespace) .find(|part| !part.is_empty()) .map(|oid| String::from_utf8_lossy(oid).into_owned()) .map(Some) .context("git rev-parse returned no object ID"), Some(1) => Ok(None), - _ => bail!("git rev-parse failed while checking `{subject}`"), + _ => bail!("git rev-parse failed while checking `{tag}`"), } } @@ -605,38 +585,14 @@ fn remote_tag_oids(root: &Path, tags: &[&str]) -> Result /// it. Used by the repository-series reconciliation, which fetches and /// compares one specific tag rather than a planned batch. fn remote_tag_oid(root: &Path, tag: &str) -> Result> { - let reference = format!("refs/tags/{tag}"); - let args = ["ls-remote", "--exit-code", "--tags", "origin", &reference]; - crate::term::trace_command("git", &args, root); - let output = Command::new("git") - .args(args) - .current_dir(root) - .output() - .context("failed to run git")?; - match output.status.code() { - Some(0) => output - .stdout - .split(|byte| byte.is_ascii_whitespace()) - .find(|part| !part.is_empty()) - .map(|oid| String::from_utf8_lossy(oid).into_owned()) - .map(Some) - .context("git ls-remote returned no object ID"), - Some(2) => Ok(None), - _ => bail!( - "git ls-remote failed while checking tag `{tag}`: {}", - String::from_utf8_lossy(&output.stderr).trim() - ), - } + Ok(remote_tag_oids(root, &[tag])?.remove(tag)) } -/// The member's CHANGELOG section for its current version, or a minimal -/// fallback body. -fn release_notes(workspace: &Workspace, idx: usize) -> String { - let member = &workspace.members[idx]; +/// The member's CHANGELOG section for `version`, if the file has one. +pub(crate) fn release_notes(member: &Member, version: &str) -> Option { std::fs::read_to_string(member.path.join("CHANGELOG.md")) .ok() - .and_then(|text| changelog_section(&text, member.version())) - .unwrap_or_else(|| format!("{} {}", member.name, member.version())) + .and_then(|text| changelog_section(&text, version)) } /// Extract the `## …` section whose heading names `version`, using the same @@ -652,7 +608,7 @@ pub fn changelog_section(text: &str, version: &str) -> Option { if section.is_some() { break; // next section starts; we're done } - if heading_version(heading) == Some(wanted.clone()) { + if crate::changelog::heading_version(heading).as_ref() == Some(&wanted) { section = Some(String::new()); } } else if let Some(section) = section.as_mut() { @@ -665,14 +621,6 @@ pub fn changelog_section(text: &str, version: &str) -> Option { .filter(|s| !s.is_empty()) } -fn heading_version(heading: &str) -> Option { - let token = heading.split_whitespace().next()?; - let token = token.trim_matches(['[', ']']); - let token = token.rsplit_once("-v").map(|(_, v)| v).unwrap_or(token); - let token = token.strip_prefix('v').unwrap_or(token); - semver::Version::parse(token).ok() -} - /// A pushed tag resolved back to the package it names. #[derive(Debug, Clone, PartialEq, Eq)] pub enum ResolvedTag { @@ -750,9 +698,9 @@ fn candidates(workspace: &Workspace, wanted: impl Fn(&[TagLevel]) -> bool) -> Ve workspace .members .iter() - .zip(0..) - .filter(|(member, _)| member.releasable() && wanted(&member.tags)) - .map(|(member, index)| (index, member.name.as_str())) + .enumerate() + .filter(|(_, member)| member.releasable() && wanted(&member.tags)) + .map(|(index, member)| (index, member.name.as_str())) .collect() } diff --git a/src/commands/version.rs b/src/commands/version.rs index 0ec2c56..5b624e1 100644 --- a/src/commands/version.rs +++ b/src/commands/version.rs @@ -15,9 +15,11 @@ use std::path::PathBuf; #[derive(Debug)] pub struct PlanEntry { + /// Index into `workspace.members`. + pub member: usize, pub name: String, - pub current: String, - pub next: String, + pub current: semver::Version, + pub next: semver::Version, /// How many fragments the package owns on disk. pub fragments: usize, /// Workspace dependencies that bumped in this same plan, sorted by name. @@ -142,13 +144,14 @@ pub fn compute_plan_from( }, ); plan.push(PlanEntry { + member: idx, name: member.name.clone(), - current: member.version().to_string(), - next: next.to_string(), fragments: owned.len(), updated_deps, generated, was_prerelease: !current.pre.is_empty(), + current, + next, }); } if overrides.promoting() && !plan.iter().any(|entry| entry.was_prerelease) { @@ -234,8 +237,8 @@ fn bumps(plan: &[PlanEntry]) -> Vec> { plan.iter() .map(|entry| Bump { name: &entry.name, - current: &entry.current, - next: &entry.next, + current: entry.current.to_string(), + next: entry.next.to_string(), fragments: entry.fragments, updated_dependencies: entry .updated_deps @@ -273,10 +276,7 @@ pub fn apply(workspace: &Workspace, overrides: &Overrides, json: bool) -> Result let date = changelog::today(); let mut prepared_versions = Vec::new(); for entry in &plan { - let idx = workspace - .member_index(&entry.name) - .expect("plan entries come from members"); - let member = &workspace.members[idx]; + let member = &workspace.members[entry.member]; let member_fragments: Vec<&changelog::Fragment> = fragments.for_package(&entry.name).collect(); // Generated ripple entries render alongside the real ones, but are @@ -286,23 +286,24 @@ pub fn apply(workspace: &Workspace, overrides: &Overrides, json: bool) -> Result .copied() .chain(entry.generated.iter()) .collect(); - let next = semver::Version::parse(&entry.next).expect("plan versions are valid"); - let tag = workspace.config.exact_tag(&entry.name, &entry.next); + let next = entry.next.clone(); + let next_text = next.to_string(); + let tag = workspace.config.exact_tag(&entry.name, &next_text); let section = changelog::render_section( &workspace.config.changelog, &entry.name, - &entry.next, + &next_text, &tag, &date, &rendered, )?; // A package releasing for the first time may already have a // hand-written CHANGELOG.md; adopt it so regenerating preserves it. - let adoption = changelog::plan_adoption(workspace, &entry.name, &entry.current) + let adoption = changelog::plan_adoption(workspace, member, &entry.current.to_string()) .with_context(|| format!("failed to read `{}`'s changelog history", entry.name))?; let changelog = changelog::render_merged_changelog( workspace, - &entry.name, + member, Some((&next, §ion)), adoption.as_ref(), ) @@ -313,6 +314,7 @@ pub fn apply(workspace: &Workspace, overrides: &Overrides, json: bool) -> Result let manifest = changelog::render_manifest_version(&manifest, &next) .with_context(|| format!("failed to bump `{}`", entry.name))?; prepared_versions.push(PreparedVersion { + member: entry.member, name: entry.name.clone(), next, section, @@ -323,33 +325,22 @@ pub fn apply(workspace: &Workspace, overrides: &Overrides, json: bool) -> Result }); } - let mut versions: BTreeMap = workspace + // Plan entries come last, so a bumped package's next version wins. + let versions: BTreeMap = workspace .members .iter() .map(|member| (member.name.clone(), member.version().to_string())) + .chain( + plan.iter() + .map(|entry| (entry.name.clone(), entry.next.to_string())), + ) .collect(); - for entry in &plan { - versions.insert(entry.name.clone(), entry.next.clone()); - } - let mut prepared_lockfiles = Vec::new(); - for member in &workspace.members { - let path = member.path.join("manifest.toml"); - if !path.is_file() { - continue; - } - let text = std::fs::read_to_string(&path) - .with_context(|| format!("failed to read {}", path.display()))?; - let (new_text, patched) = lockfile::patch_locked_versions(&text, &versions) - .with_context(|| format!("failed to patch {}", path.display()))?; - if !patched.is_empty() { - prepared_lockfiles.push(PreparedLockfile { - display: format!("{}/manifest.toml", member.rel_path), - path, - text: new_text, - }); - } - } + let prepared_lockfiles: Vec<_> = workspace + .members + .iter() + .filter_map(|member| lockfile::patch_member(member, &versions).transpose()) + .collect::>()?; for prepared in &prepared_versions { std::fs::write(&prepared.manifest_path, &prepared.manifest) @@ -367,7 +358,7 @@ pub fn apply(workspace: &Workspace, overrides: &Overrides, json: bool) -> Result .member_index(&entry.name) .with_context(|| format!("package `{}` disappeared during apply", entry.name))?; let actual = workspace.members[idx].version(); - if actual != entry.next { + if actual != entry.next.to_string() { bail!( "version bump did not land for `{}`: gleam.toml says {actual}, expected {}", entry.name, @@ -389,7 +380,7 @@ pub fn apply(workspace: &Workspace, overrides: &Overrides, json: bool) -> Result } changelog::write_batch( &workspace, - &prepared.name, + &workspace.members[prepared.member], &prepared.next, &prepared.section, &prepared.changelog, @@ -410,7 +401,7 @@ pub fn apply(workspace: &Workspace, overrides: &Overrides, json: bool) -> Result let patched_files: Vec<&str> = prepared_lockfiles .iter() - .map(|prepared| prepared.display.as_str()) + .map(|prepared| prepared.rel_path.as_str()) .collect(); if json { @@ -459,6 +450,7 @@ fn display_path(workspace: &Workspace, path: &std::path::Path) -> String { } struct PreparedVersion { + member: usize, name: String, next: semver::Version, section: String, @@ -468,9 +460,3 @@ struct PreparedVersion { /// Pre-trellis changelog history to preserve, on a first release. adoption: Option, } - -struct PreparedLockfile { - display: String, - path: PathBuf, - text: String, -} diff --git a/src/commands/version_override.rs b/src/commands/version_override.rs index 8459a1d..35ad30f 100644 --- a/src/commands/version_override.rs +++ b/src/commands/version_override.rs @@ -174,6 +174,11 @@ impl Overrides { changelog::apply_bump(current, bump) } + /// Whether `--bump pkg=` or `--set pkg=` named this package. + fn names(&self, package: &str) -> bool { + self.pinned.contains_key(package) || self.per_package_bump.contains_key(package) + } + /// `--pre none`: a package already in a cycle drops its label and releases; /// one that gained fragments after the RC was cut bumps normally, so a late /// arrival does not block the promotion. @@ -186,7 +191,7 @@ impl Overrides { if current.pre.is_empty() { return Ok(self.base(package, current, derived)); } - if self.pinned.contains_key(package) || self.per_package_bump.contains_key(package) { + if self.names(package) { bail!( "`{package}` is named by --bump or --set as well as --pre none; promoting takes \ the version the prerelease was already working toward" @@ -204,9 +209,7 @@ impl Overrides { derived: Bump, label: &str, ) -> Result { - let explicit = - self.pinned.contains_key(package) || self.per_package_bump.contains_key(package); - let base = if explicit { + let base = if self.names(package) { // An explicit --bump/--set retargets the cycle even mid-flight, // measured from the base rather than from `rc.1` itself. self.base(package, &release_of(current), derived) @@ -277,8 +280,8 @@ mod tests { } fn overrides(bump: &[&str], set: &[&str], pre: Option<&str>) -> Overrides { - let bump: Vec = bump.iter().map(|s| s.to_string()).collect(); - let set: Vec = set.iter().map(|s| s.to_string()).collect(); + let bump: Vec = bump.iter().map(|&s| s.to_string()).collect(); + let set: Vec = set.iter().map(|&s| s.to_string()).collect(); Overrides::parse(&bump, &set, pre).unwrap() } diff --git a/src/completion.rs b/src/completion.rs index 04300bd..ae77c87 100644 --- a/src/completion.rs +++ b/src/completion.rs @@ -48,12 +48,12 @@ pub fn packages() -> ArgValueCandidates { /// Only members that participate in releases — the ones `changelog new` /// accepts (release lifecycle `git_only` or `hex`). pub fn releasable_packages() -> ArgValueCandidates { - ArgValueCandidates::new(|| member_candidates(|m| m.releasable())) + ArgValueCandidates::new(|| member_candidates(crate::workspace::Member::releasable)) } /// Only members `publish` will actually accept: release lifecycle `hex`. pub fn hex_packages() -> ArgValueCandidates { - ArgValueCandidates::new(|| member_candidates(|m| m.publishes_to_hex())) + ArgValueCandidates::new(|| member_candidates(crate::workspace::Member::publishes_to_hex)) } /// Built-in verbs plus every `[tools.trellis.tasks]` entry. The built-ins are diff --git a/src/config.rs b/src/config.rs index a98a11c..f70c11f 100644 --- a/src/config.rs +++ b/src/config.rs @@ -8,7 +8,6 @@ use anyhow::{Context, Result, bail}; use serde::Deserialize; use std::collections::BTreeMap; -use std::path::Path; /// Prefix reserved for special `exclude` keys ([`RELEASE_EXCLUDE_KEY`] and /// [`MEMBERS_EXCLUDE_KEY`]) so they can never collide with a task name — @@ -49,7 +48,7 @@ pub struct ConfigFile { #[serde(default)] pub doctor: DoctorConfig, /// Keys under `[tools.trellis]` that no field claimed. Collected rather - /// than deserialized — see [`ConfigFile::from_gleam_toml`]. + /// than deserialized — see [`ConfigFile::from_document`]. #[serde(skip)] pub unknown_keys: Vec, /// Keys spelled in the pre-0.8 kebab-case style. Accepted, then reported — @@ -87,6 +86,17 @@ pub enum Strictness { Off, } +impl Strictness { + /// The configuration spelling, for output that is not serde. + pub fn key(self) -> &'static str { + match self { + Strictness::Warn => "warn", + Strictness::Error => "error", + Strictness::Off => "off", + } + } +} + #[derive(Debug, Clone, Default, Deserialize)] #[serde(rename_all = "snake_case")] pub struct DoctorConfig { @@ -97,23 +107,6 @@ pub struct DoctorConfig { pub shared_dependencies: Strictness, } -/// Deserialize the `[tools.trellis]` table, recording the keys no field -/// claimed rather than dropping them on the floor. -/// -/// Parsing stays lenient — an unrecognized key does not stop the workspace -/// loading. Straight `deny_unknown_fields` would mean a workspace using a key -/// from a newer trellis becomes unloadable under a pinned older one, which is a -/// bad failure for a tool CI pins. The keys are reported by `doctor` instead. -/// -/// The free-form tables ([`FREE_FORM_TABLES`]) accept any key by construction, -/// so serde consumes them and they are never reported. -fn deserialize_collecting_unknown(trellis: &toml::Value) -> Result<(ConfigFile, Vec)> { - let mut ignored = Vec::new(); - let config = serde_ignored::deserialize(trellis.clone(), |path| ignored.push(path.to_string())) - .context("invalid [tools.trellis] configuration")?; - Ok((config, ignored)) -} - /// Tables under `[tools.trellis]` whose *keys* are chosen by the user rather /// than by trellis: task names, `exclude` selectors, and the member-path globs /// of `publish.package_tags_overrides` and `publish.lifecycle.packages`. A @@ -695,20 +688,24 @@ fn default_dependency_body() -> String { } impl ConfigFile { - /// Load from the workspace root's `gleam.toml`, reading the - /// `[tools.trellis]` table. - pub fn load(path: &Path) -> Result { - let text = std::fs::read_to_string(path) - .with_context(|| format!("failed to read {}", path.display()))?; - Self::from_gleam_toml(&text).with_context(|| format!("in {}", path.display())) - } - + #[cfg(test)] pub fn from_gleam_toml(text: &str) -> Result { let document: toml::Value = toml::from_str(text).context("failed to parse gleam.toml")?; + Self::from_document(&document) + } + + /// Load from an already-parsed `gleam.toml`, reading the `[tools.trellis]` + /// table. Unknown keys are recorded (see [`ConfigFile::unknown_keys`]) + /// rather than rejected, so a workspace using a key from a newer trellis + /// still loads under a pinned older one; `doctor` reports them. + pub fn from_document(document: &toml::Value) -> Result { let Some(trellis) = document.get("tools").and_then(|tools| tools.get("trellis")) else { bail!("gleam.toml has no [tools.trellis] table"); }; - let (mut config, ignored) = deserialize_collecting_unknown(trellis)?; + let mut ignored = Vec::new(); + let mut config: Self = + serde_ignored::deserialize(trellis.clone(), |path| ignored.push(path.to_string())) + .context("invalid [tools.trellis] configuration")?; config.deprecated_keys = collect_deprecated_keys(trellis, &ignored); config.unknown_keys = ignored; config.validate()?; @@ -789,30 +786,23 @@ impl ConfigFile { // what the tag is called, and which series it tracks. Defaulting any // of them would let a half-written config publish a tag nobody asked // for, or configure one that silently produces nothing. - let declared: Vec<&str> = [ - self.publish - .repository_tag_package - .is_some() - .then_some("repository_tag_package"), - self.publish - .repository_tag_format - .is_some() - .then_some("repository_tag_format"), - (!self.publish.repository_tags.is_empty()).then_some("repository_tags"), - ] - .into_iter() - .flatten() - .collect(); - if !declared.is_empty() && declared.len() < 3 { - let missing: Vec = [ + let keys = [ + ( "repository_tag_package", + self.publish.repository_tag_package.is_some(), + ), + ( "repository_tag_format", - "repository_tags", - ] - .into_iter() - .filter(|key| !declared.contains(key)) - .map(|key| format!("`{key}`")) + self.publish.repository_tag_format.is_some(), + ), + ("repository_tags", !self.publish.repository_tags.is_empty()), + ]; + let missing: Vec = keys + .iter() + .filter(|(_, present)| !present) + .map(|(key, _)| format!("`{key}`")) .collect(); + if !missing.is_empty() && missing.len() < keys.len() { bail!( "the repository tag needs `repository_tag_package`, `repository_tag_format`, \ and a non-empty `repository_tags`; missing {}", @@ -1229,14 +1219,14 @@ mod tests { #[test] fn hyphens_in_free_form_table_keys_are_not_deprecations() { let config = ConfigFile::from_gleam_toml( - r###" + r#" [tools.trellis] members = ["packages/*"] exclude = { "check-all" = ["examples/*"] } [tools.trellis.tasks.check-all] command = "gleam check" - "###, + "#, ) .unwrap(); assert!(config.tasks.contains_key("check-all")); @@ -1270,13 +1260,13 @@ mod tests { #[test] fn keys_added_after_the_last_kebab_release_have_no_alias() { let config = ConfigFile::from_gleam_toml( - r###" + r#" [tools.trellis.changelog] dependency-kind = "Docs" [tools.trellis.doctor] shared-dependencies = "error" - "###, + "#, ) .unwrap(); // The defaults stand, and the keys are reported as unrecognized. @@ -1347,11 +1337,11 @@ mod tests { #[test] fn lifecycle_parses_nested_table_and_inline_map() { let config = ConfigFile::from_gleam_toml( - r###" + r#" [tools.trellis.publish.lifecycle] default = "hex" packages = { "member/path/**" = "git_only", "examples/**" = "workspace" } - "###, + "#, ) .unwrap(); assert_eq!(config.publish.lifecycle.default, ReleaseLifecycle::Hex); @@ -1401,10 +1391,10 @@ mod tests { #[test] fn dependency_kind_must_name_a_configured_kind() { let err = ConfigFile::from_gleam_toml( - r###" + r#" [tools.trellis.changelog] kinds = [{ label = "Docs", bump = "patch" }] - "###, + "#, ) .unwrap_err(); let message = format!("{err:#}"); @@ -1423,12 +1413,12 @@ mod tests { #[test] fn dependency_kind_may_point_at_an_existing_kind() { let config = ConfigFile::from_gleam_toml( - r###" + r#" [tools.trellis.changelog] dependency_kind = "Docs" dependency_body = "{{ dependency }} is now {{ dependency_version }}" kinds = [{ label = "Docs", bump = "patch" }] - "###, + "#, ) .unwrap(); assert_eq!(config.changelog.dependency_kind, "Docs"); @@ -1543,14 +1533,14 @@ mod tests { #[test] fn parses_package_tags_and_overrides() { let config = ConfigFile::from_gleam_toml( - r###" + r#" [tools.trellis] members = ["packages/*"] [tools.trellis.publish] package_tags = ["minor"] package_tags_overrides = { "packages/lat_*" = ["exact", "minor"], "packages/old" = ["exact"] } - "###, + "#, ) .unwrap(); assert_eq!(config.publish.package_tags, [TagLevel::Minor]); diff --git a/src/git.rs b/src/git.rs index 9fc23d7..b76f668 100644 --- a/src/git.rs +++ b/src/git.rs @@ -11,9 +11,8 @@ use std::process::Command; /// committed changes (`since...HEAD`), uncommitted changes, and untracked /// files, so the answer is the same locally and in CI. pub fn changed_members(workspace: &Workspace, since: &str) -> Result> { - let repo_root = git_stdout(&workspace.root, &["rev-parse", "--show-toplevel"]) + let repo_root = repo_root(&workspace.root) .context("--since requires the workspace to be inside a git repository")?; - let repo_root = PathBuf::from(repo_root.trim()); let mut files: Vec = Vec::new(); let range = format!("{since}...HEAD"); @@ -41,9 +40,8 @@ pub fn changed_members_between( base: &str, head: &str, ) -> Result> { - let repo_root = git_stdout(&workspace.root, &["rev-parse", "--show-toplevel"]) + let repo_root = repo_root(&workspace.root) .context("changelog check requires the workspace to be inside a git repository")?; - let repo_root = PathBuf::from(repo_root.trim()); let range = format!("{base}...{head}"); let files: Vec = lines(&git_stdout( &workspace.root, @@ -73,22 +71,16 @@ pub fn unchanged_since_merge_base( if candidates.is_empty() { return Ok(HashSet::new()); } - let repo_root = git_stdout(&workspace.root, &["rev-parse", "--show-toplevel"]) + let repo_root = repo_root(&workspace.root) .context("changelog check requires the workspace to be inside a git repository")?; - let repo_root = PathBuf::from(repo_root.trim()) - .canonicalize() - .unwrap_or_else(|_| PathBuf::from(repo_root.trim())); + let repo_root = repo_root.canonicalize().unwrap_or(repo_root); // A `dir` outside the repository has no tracked history to compare // against, so every file in it reads as the branch's own. let Ok(relative) = dir.strip_prefix(&repo_root) else { return Ok(HashSet::new()); }; // git pathspecs are `/`-separated on every platform. - let pathspec = relative - .components() - .map(|component| component.as_os_str().to_string_lossy()) - .collect::>() - .join("/"); + let pathspec = slash_path(relative); let merge_base = git_stdout(&workspace.root, &["merge-base", base, head]) .with_context(|| format!("no merge base between {base} and {head}"))?; @@ -190,6 +182,15 @@ pub fn repo_root(dir: &Path) -> Option { .map(|out| PathBuf::from(out.trim())) } +/// `path`'s components joined by `/` regardless of platform — the form git +/// pathspecs and trellis's own `rel_path` displays use. +pub fn slash_path(path: &Path) -> String { + path.components() + .map(|c| c.as_os_str().to_string_lossy()) + .collect::>() + .join("/") +} + /// Every non-gitignored `gleam.toml` under `cwd` — tracked and untracked /// alike, so freshly created packages are discovered before their first /// commit. Paths are relative to `cwd`. @@ -220,7 +221,7 @@ pub fn ls_gleam_manifests(cwd: &Path) -> Result> { /// `-c user.name=... -c user.email=...` args to prepend to a git command that /// creates a commit or annotated tag, but only when no identity is /// configured (CI runners) — never overriding the user's own config. -pub fn identity_fallback_args(cwd: &Path) -> Vec { +fn identity_fallback_args(cwd: &Path) -> Vec { let has_identity = git_stdout(cwd, &["config", "user.email"]) .map(|email| !email.trim().is_empty()) .unwrap_or(false); @@ -236,6 +237,18 @@ pub fn identity_fallback_args(cwd: &Path) -> Vec { } } +/// Run a git command that writes a commit or annotated tag, with the identity +/// fallback prepended when the user has none configured. +pub(crate) fn git_with_identity(cwd: &Path, args: &[&str]) -> Result { + let identity = identity_fallback_args(cwd); + let full: Vec<&str> = identity + .iter() + .map(String::as_str) + .chain(args.iter().copied()) + .collect(); + git_stdout(cwd, &full) +} + /// The commit a ref names on `url`, from one `ls-remote` — `None` when the /// remote has no such ref. Annotated tags resolve to the peeled commit (the /// object gleam locks), not the tag object. A name matching several refs @@ -244,13 +257,7 @@ pub fn ls_remote_commit(cwd: &Path, url: &str, refname: &str) -> Result {} Some(2) => return Ok(None), @@ -278,12 +285,10 @@ pub fn ls_remote_commit(cwd: &Path, url: &str, refname: &str) -> Result = commits.values().collect(); - distinct.sort(); - distinct.dedup(); + let distinct: std::collections::BTreeSet<&String> = commits.values().collect(); match distinct.len() { 0 => Ok(None), - 1 => Ok(Some(distinct[0].clone())), + 1 => Ok(distinct.into_iter().next().cloned()), _ => bail!( "`{refname}` is ambiguous on {url}: it matches {} — use a full refname", commits.keys().cloned().collect::>().join(", ") @@ -314,13 +319,7 @@ pub fn fetch_ref(cwd: &Path, url: &str, refname: &str) -> Result<()> { /// fetching a tracked ref, a pin absent from its history is exactly the /// drift being tested for. pub fn is_ancestor(cwd: &Path, ancestor: &str, descendant: &str) -> Result { - let args = ["merge-base", "--is-ancestor", ancestor, descendant]; - crate::term::trace_command("git", &args, cwd); - let output = Command::new("git") - .args(args) - .current_dir(cwd) - .output() - .context("failed to run git")?; + let output = git_output(cwd, &["merge-base", "--is-ancestor", ancestor, descendant])?; match output.status.code() { Some(0) => Ok(true), Some(1) => Ok(false), @@ -339,13 +338,21 @@ pub fn is_ancestor(cwd: &Path, ancestor: &str, descendant: &str) -> Result } } -pub(crate) fn git_stdout(cwd: &Path, args: &[&str]) -> Result { +/// Run git in `cwd` and return the raw output, whatever the exit status. +/// Every git invocation goes through here so `--verbose` tracing and the +/// "git missing" error are uniform. +pub(crate) fn git_output(cwd: &Path, args: &[&str]) -> Result { crate::term::trace_command("git", args, cwd); - let output = Command::new("git") + Command::new("git") .args(args) .current_dir(cwd) .output() - .context("failed to run git")?; + .context("failed to run git") +} + +/// [`git_output`] that fails on a non-zero exit, returning stdout. +pub(crate) fn git_stdout(cwd: &Path, args: &[&str]) -> Result { + let output = git_output(cwd, args)?; if !output.status.success() { bail!( "git {} failed: {}", diff --git a/src/github.rs b/src/github.rs index 122e305..c43f563 100644 --- a/src/github.rs +++ b/src/github.rs @@ -43,14 +43,8 @@ impl GitHubClient { /// The number of the open PR whose head is `branch`, if one exists. pub fn find_open_pr(&self, branch: &str) -> Result> { - let url = format!( - "{}/repos/{}/{}/pulls?head={}:{branch}&state=open", - self.base, self.owner, self.repo, self.owner - ); - let (status, body) = self.get(&url)?; - if status != 200 { - bail!("GitHub API GET {url} failed: {}", api_error(status, &body)); - } + let url = self.url(&format!("/pulls?head={}:{branch}&state=open", self.owner)); + let body = expect_status("GET", &url, 200, self.get(&url)?)?; Ok(body .as_array() .and_then(|prs| prs.first()) @@ -59,20 +53,14 @@ impl GitHubClient { /// Open a PR and return its URL. pub fn create_pr(&self, base: &str, head: &str, title: &str, body: &str) -> Result { - let url = format!("{}/repos/{}/{}/pulls", self.base, self.owner, self.repo); + let url = self.url("/pulls"); let payload = serde_json::json!({ "base": base, "head": head, "title": title, "body": body, }); - let (status, response) = self.send("POST", &url, &payload)?; - if status != 201 { - bail!( - "GitHub API POST {url} failed: {}", - api_error(status, &response) - ); - } + let response = expect_status("POST", &url, 201, self.send(Write::Post, &url, &payload)?)?; response["html_url"] .as_str() .map(str::to_string) @@ -81,27 +69,15 @@ impl GitHubClient { /// Retitle and re-body an existing PR. pub fn update_pr(&self, number: u64, title: &str, body: &str) -> Result<()> { - let url = format!( - "{}/repos/{}/{}/pulls/{number}", - self.base, self.owner, self.repo - ); + let url = self.url(&format!("/pulls/{number}")); let payload = serde_json::json!({ "title": title, "body": body }); - let (status, response) = self.send("PATCH", &url, &payload)?; - if status != 200 { - bail!( - "GitHub API PATCH {url} failed: {}", - api_error(status, &response) - ); - } + expect_status("PATCH", &url, 200, self.send(Write::Patch, &url, &payload)?)?; Ok(()) } /// Whether a GitHub Release exists for `tag`. pub fn release_exists(&self, tag: &str) -> Result { - let url = format!( - "{}/repos/{}/{}/releases/tags/{tag}", - self.base, self.owner, self.repo - ); + let url = self.url(&format!("/releases/tags/{tag}")); let (status, body) = self.get(&url)?; match status { 200 => Ok(true), @@ -112,48 +88,47 @@ impl GitHubClient { /// Create a GitHub Release on `tag`. pub fn create_release(&self, tag: &str, title: &str, notes: &str) -> Result<()> { - let url = format!("{}/repos/{}/{}/releases", self.base, self.owner, self.repo); + let url = self.url("/releases"); let payload = serde_json::json!({ "tag_name": tag, "name": title, "body": notes, }); - let (status, response) = self.send("POST", &url, &payload)?; - if status != 201 { - bail!( - "GitHub API POST {url} failed: {}", - api_error(status, &response) - ); - } + expect_status("POST", &url, 201, self.send(Write::Post, &url, &payload)?)?; Ok(()) } + /// `tail` appended to this repository's API root. + fn url(&self, tail: &str) -> String { + format!("{}/repos/{}/{}{tail}", self.base, self.owner, self.repo) + } + fn get(&self, url: &str) -> Result<(u16, serde_json::Value)> { crate::term::trace_http("GET", url); let response = self .headers(self.agent.get(url)) .call() .with_context(|| format!("GitHub API request failed: GET {url}"))?; - read_response(response) + Ok(read_response(response)) } fn send( &self, - method: &str, + method: Write, url: &str, payload: &serde_json::Value, ) -> Result<(u16, serde_json::Value)> { - crate::term::trace_http(method, url); + let name = method.name(); + crate::term::trace_http(name, url); let request = match method { - "POST" => self.agent.post(url), - "PATCH" => self.agent.patch(url), - other => bail!("unsupported HTTP method {other}"), + Write::Post => self.agent.post(url), + Write::Patch => self.agent.patch(url), }; let response = self .headers(request) .send_json(payload) - .with_context(|| format!("GitHub API request failed: {method} {url}"))?; - read_response(response) + .with_context(|| format!("GitHub API request failed: {name} {url}"))?; + Ok(read_response(response)) } fn headers(&self, request: ureq::RequestBuilder) -> ureq::RequestBuilder { @@ -165,9 +140,23 @@ impl GitHubClient { } } -fn read_response( - mut response: ureq::http::Response, -) -> Result<(u16, serde_json::Value)> { +/// The two methods trellis writes with. +#[derive(Clone, Copy)] +enum Write { + Post, + Patch, +} + +impl Write { + fn name(self) -> &'static str { + match self { + Write::Post => "POST", + Write::Patch => "PATCH", + } + } +} + +fn read_response(mut response: ureq::http::Response) -> (u16, serde_json::Value) { let status = response.status().as_u16(); // Error bodies matter as much as success bodies (they carry the API's // "message"), but not every response is JSON — a 502 from a proxy, say. @@ -175,7 +164,24 @@ fn read_response( .body_mut() .read_json() .unwrap_or(serde_json::Value::Null); - Ok((status, body)) + (status, body) +} + +/// The body of a response with the `expected` status; anything else is an +/// error naming the call. +fn expect_status( + method: &str, + url: &str, + expected: u16, + (status, body): (u16, serde_json::Value), +) -> Result { + if status != expected { + bail!( + "GitHub API {method} {url} failed: {}", + api_error(status, &body) + ); + } + Ok(body) } /// A one-line description of a failed API call: the status plus whatever @@ -189,13 +195,13 @@ fn api_error(status: u16, body: &serde_json::Value) -> String { /// The token, from the environment or a logged-in gh CLI. fn resolve_token() -> Result { - for var in ["GITHUB_TOKEN", "GH_TOKEN"] { - if let Ok(token) = std::env::var(var) { - let token = token.trim().to_string(); - if !token.is_empty() { - return Ok(token); - } - } + if let Some(token) = ["GITHUB_TOKEN", "GH_TOKEN"] + .into_iter() + .filter_map(|var| std::env::var(var).ok()) + .map(|token| token.trim().to_string()) + .find(|token| !token.is_empty()) + { + return Ok(token); } let gh = crate::tools::gh_bin(); if let Ok(output) = Command::new(&gh).args(["auth", "token"]).output() @@ -219,21 +225,10 @@ fn resolve_repo(root: &Path) -> Result<(String, String)> { _ => bail!("TRELLIS_GITHUB_REPO must be `owner/repo`, got `{spec}`"), }; } - let args = ["remote", "get-url", "origin"]; - crate::term::trace_command("git", &args, root); - let output = Command::new("git") - .args(args) - .current_dir(root) - .output() - .context("failed to run git")?; - if !output.status.success() { - bail!( - "GitHub operations need an `origin` remote: {}", - String::from_utf8_lossy(&output.stderr).trim() - ); - } - let url = String::from_utf8_lossy(&output.stdout).trim().to_string(); - parse_remote_url(&url).ok_or_else(|| { + let url = crate::git::git_stdout(root, &["remote", "get-url", "origin"]) + .context("GitHub operations need an `origin` remote")?; + let url = url.trim(); + parse_remote_url(url).ok_or_else(|| { anyhow!( "origin remote `{url}` is not a GitHub repository (set TRELLIS_GITHUB_REPO to override)" ) @@ -265,12 +260,8 @@ pub fn parse_remote_url(url: &str) -> Option<(String, String)> { mod tests { use super::parse_remote_url; - fn parsed(url: &str) -> Option<(String, String)> { - parse_remote_url(url) - } - fn owner_repo(url: &str) -> (String, String) { - parsed(url).unwrap() + parse_remote_url(url).unwrap() } #[test] @@ -307,18 +298,18 @@ mod tests { #[test] fn rejects_non_github_urls() { - assert_eq!(parsed("git@gitlab.com:owner/repo.git"), None); - assert_eq!(parsed("https://gitlab.com/owner/repo"), None); - assert_eq!(parsed("/local/bare/repo.git"), None); + assert_eq!(parse_remote_url("git@gitlab.com:owner/repo.git"), None); + assert_eq!(parse_remote_url("https://gitlab.com/owner/repo"), None); + assert_eq!(parse_remote_url("/local/bare/repo.git"), None); } #[test] fn rejects_missing_owner_or_repo() { - assert_eq!(parsed("git@github.com:/repo"), None); - assert_eq!(parsed("git@github.com:owner/"), None); - assert_eq!(parsed("git@github.com:owner"), None); - assert_eq!(parsed("https://github.com//repo"), None); - assert_eq!(parsed("https://github.com/owner/"), None); + assert_eq!(parse_remote_url("git@github.com:/repo"), None); + assert_eq!(parse_remote_url("git@github.com:owner/"), None); + assert_eq!(parse_remote_url("git@github.com:owner"), None); + assert_eq!(parse_remote_url("https://github.com//repo"), None); + assert_eq!(parse_remote_url("https://github.com/owner/"), None); } #[test] diff --git a/src/gleam.rs b/src/gleam.rs index 0c2e3f0..8468038 100644 --- a/src/gleam.rs +++ b/src/gleam.rs @@ -46,8 +46,7 @@ struct RawManifest { dependencies: BTreeMap, #[serde(default, rename = "dev-dependencies")] dev_dependencies: BTreeMap, - #[serde(default)] - tools: Option, + tools: Option, } fn default_version() -> String { @@ -75,21 +74,21 @@ impl GleamManifest { pub fn parse(text: &str) -> Result { let raw: RawManifest = toml::from_str(text)?; let mut dependencies = Vec::new(); - for (deps, dev) in [(&raw.dependencies, false), (&raw.dev_dependencies, true)] { + for (deps, dev) in [(raw.dependencies, false), (raw.dev_dependencies, true)] { for (name, dep) in deps { let requirement = match dep { - RawDep::Requirement(req) => Requirement::Hex(req.clone()), + RawDep::Requirement(req) => Requirement::Hex(req), // `git` wins over `path`: since Gleam 1.18 a git dep may // carry a `path` key selecting a subdirectory of the // remote repo, which is not a workspace-local path. - RawDep::Detailed { git: Some(git), .. } => Requirement::Git(git.clone()), + RawDep::Detailed { git: Some(git), .. } => Requirement::Git(git), RawDep::Detailed { path: Some(path), .. - } => Requirement::Path(path.clone()), + } => Requirement::Path(path), RawDep::Detailed { version: Some(version), .. - } => Requirement::Hex(version.clone()), + } => Requirement::Hex(version), RawDep::Detailed { .. } => { anyhow::bail!( "dependency `{name}` has neither a version, a path, nor a git source" @@ -97,17 +96,18 @@ impl GleamManifest { } }; dependencies.push(Dependency { - name: name.clone(), + name, requirement, dev, }); } } + // The same test root discovery applies: a `trellis` *table*. let has_trellis_config = raw .tools .as_ref() .and_then(|tools| tools.get("trellis")) - .is_some(); + .is_some_and(toml::Value::is_table); Ok(Self { name: raw.name, version: raw.version, diff --git a/src/json.rs b/src/json.rs index 0e9f4db..3a75b5e 100644 --- a/src/json.rs +++ b/src/json.rs @@ -85,6 +85,7 @@ impl Check { /// The serialized identifier, for renderings that are not serde — the /// `title=` of a GitHub annotation, say. Kept beside the `Serialize` derive /// so the two cannot disagree; `check_names_match_serde` asserts it. + // ponytail: as_str mirrors serde names by hand; serde_plain would delete it but adds a dep pub fn as_str(self) -> &'static str { match self { Check::MemberGlob => "member_glob", @@ -558,8 +559,8 @@ pub struct UpdatedDependency<'a> { #[serde(rename_all = "snake_case")] pub struct Bump<'a> { pub name: &'a str, - pub current: &'a str, - pub next: &'a str, + pub current: String, + pub next: String, /// Fragments the package owns on disk. Zero for a package bumping only /// because a dependency did. pub fragments: usize, diff --git a/src/lockfile.rs b/src/lockfile.rs index 10e7772..3695a02 100644 --- a/src/lockfile.rs +++ b/src/lockfile.rs @@ -3,8 +3,10 @@ //! `gleam update` (which would hit Hex and trip rate limits on shared //! runners). toml_edit keeps the rest of the file byte-identical. +use crate::workspace::Member; use anyhow::{Context, Result}; use std::collections::BTreeMap; +use std::path::PathBuf; use toml_edit::{DocumentMut, Value}; #[derive(Debug, PartialEq, Eq)] @@ -14,6 +16,14 @@ pub struct PatchedEntry { pub new: String, } +/// Replace a string value in place, keeping the whitespace and comments +/// around it so the rest of the line is byte-identical. +pub(crate) fn set_str_keep_decor(value: &mut Value, new: &str) { + let mut replacement = Value::from(new); + *replacement.decor_mut() = value.decor().clone(); + *value = replacement; +} + /// Update `packages[].version` for every entry whose name appears in /// `versions` and whose locked version differs. Returns the new text and what /// changed; the text is unchanged when nothing needed patching. @@ -21,75 +31,7 @@ pub fn patch_locked_versions( text: &str, versions: &BTreeMap, ) -> Result<(String, Vec)> { - let mut doc: DocumentMut = text.parse().context("failed to parse manifest.toml")?; - let mut patched = Vec::new(); - - if let Some(array) = doc.get_mut("packages").and_then(|item| item.as_array_mut()) { - // The common gleam shape: packages = [ { name = ..., version = ... }, … ] - for item in array.iter_mut() { - let Some(table) = item.as_inline_table_mut() else { - continue; - }; - let Some(name) = table - .get("name") - .and_then(|v| v.as_str()) - .map(str::to_string) - else { - continue; - }; - let Some(new) = versions.get(&name) else { - continue; - }; - if let Some(value) = table.get_mut("version") { - let old = value.as_str().unwrap_or_default().to_string(); - if old != *new { - let mut replacement = Value::from(new.clone()); - *replacement.decor_mut() = value.decor().clone(); - *value = replacement; - patched.push(PatchedEntry { - name, - old, - new: new.clone(), - }); - } - } - } - } else if let Some(tables) = doc - .get_mut("packages") - .and_then(|item| item.as_array_of_tables_mut()) - { - // The [[packages]] form, for tools that rewrite the file. - for table in tables.iter_mut() { - let Some(name) = table - .get("name") - .and_then(|v| v.as_str()) - .map(str::to_string) - else { - continue; - }; - let Some(new) = versions.get(&name) else { - continue; - }; - if let Some(value) = table - .get_mut("version") - .and_then(|item| item.as_value_mut()) - { - let old = value.as_str().unwrap_or_default().to_string(); - if old != *new { - let mut replacement = Value::from(new.clone()); - *replacement.decor_mut() = value.decor().clone(); - *value = replacement; - patched.push(PatchedEntry { - name, - old, - new: new.clone(), - }); - } - } - } - } - - Ok((doc.to_string(), patched)) + patch_packages(text, "version", false, versions) } /// Update `packages[].commit` for every git-sourced entry whose name appears @@ -99,14 +41,50 @@ pub fn patch_locked_versions( pub fn patch_locked_commits( text: &str, commits: &BTreeMap, +) -> Result<(String, Vec)> { + patch_packages(text, "commit", true, commits) +} + +/// Walk `packages` in either shape gleam writes — the inline-array form +/// `packages = [ { name = ..., version = ... }, … ]` or `[[packages]]` +/// tables — and set `key` from `wanted` on every named entry that differs. +fn patch_packages( + text: &str, + key: &str, + git_only: bool, + wanted: &BTreeMap, ) -> Result<(String, Vec)> { let mut doc: DocumentMut = text.parse().context("failed to parse manifest.toml")?; let mut patched = Vec::new(); + let mut visit = |table: &mut dyn toml_edit::TableLike| { + let Some(name) = table.get("name").and_then(|item| item.as_str()) else { + return; + }; + let Some(new) = wanted.get(name) else { + return; + }; + if git_only && table.get("source").and_then(|item| item.as_str()) != Some("git") { + return; + } + let name = name.to_string(); + let Some(value) = table.get_mut(key).and_then(|item| item.as_value_mut()) else { + return; + }; + let old = value.as_str().unwrap_or_default().to_string(); + if old != *new { + set_str_keep_decor(value, new); + patched.push(PatchedEntry { + name, + old, + new: new.clone(), + }); + } + }; if let Some(array) = doc.get_mut("packages").and_then(|item| item.as_array_mut()) { for item in array.iter_mut() { if let Some(table) = item.as_inline_table_mut() { - patch_commit_entry(table, commits, &mut patched); + visit(table); } } } else if let Some(tables) = doc @@ -114,45 +92,46 @@ pub fn patch_locked_commits( .and_then(|item| item.as_array_of_tables_mut()) { for table in tables.iter_mut() { - patch_commit_entry(table, commits, &mut patched); + visit(table); } } Ok((doc.to_string(), patched)) } -fn patch_commit_entry( - table: &mut dyn toml_edit::TableLike, - commits: &BTreeMap, - patched: &mut Vec, -) { - let Some(name) = table - .get("name") - .and_then(|item| item.as_str()) - .map(str::to_string) - else { - return; - }; - let Some(new) = commits.get(&name) else { - return; - }; - if table.get("source").and_then(|item| item.as_str()) != Some("git") { - return; +/// A member's manifest.toml with its workspace-internal locked versions +/// brought in line with `versions`. +pub struct PatchedLockfile { + pub path: PathBuf, + /// Workspace-relative `/manifest.toml`, for messages. + pub rel_path: String, + pub text: String, + pub patched: Vec, +} + +/// [`patch_locked_versions`] for one member's manifest.toml. `None` when the +/// member has no lockfile yet or nothing in it needs patching. +pub fn patch_member( + member: &Member, + versions: &BTreeMap, +) -> Result> { + let path = member.path.join("manifest.toml"); + if !path.is_file() { + return Ok(None); } - let Some(value) = table.get_mut("commit").and_then(|item| item.as_value_mut()) else { - return; - }; - let old = value.as_str().unwrap_or_default().to_string(); - if old != *new { - let mut replacement = Value::from(new.clone()); - *replacement.decor_mut() = value.decor().clone(); - *value = replacement; - patched.push(PatchedEntry { - name, - old, - new: new.clone(), - }); + let text = std::fs::read_to_string(&path) + .with_context(|| format!("failed to read {}", path.display()))?; + let (text, patched) = patch_locked_versions(&text, versions) + .with_context(|| format!("failed to patch {}", path.display()))?; + if patched.is_empty() { + return Ok(None); } + Ok(Some(PatchedLockfile { + rel_path: format!("{}/manifest.toml", member.rel_path), + path, + text, + patched, + })) } #[cfg(test)] @@ -162,7 +141,7 @@ mod tests { fn versions(pairs: &[(&str, &str)]) -> BTreeMap { pairs .iter() - .map(|(k, v)| (k.to_string(), v.to_string())) + .map(|&(k, v)| (k.to_string(), v.to_string())) .collect() } diff --git a/src/main.rs b/src/main.rs index 06e3003..89a4273 100644 --- a/src/main.rs +++ b/src/main.rs @@ -552,50 +552,48 @@ fn main() -> ExitCode { } fn dispatch(cli: Cli) -> Result { - let start = match &cli.directory { - Some(dir) => dir.clone(), + let start = match cli.directory { + Some(dir) => dir, None => std::env::current_dir()?, }; // Doctor loads leniently so it can report every problem instead of // failing on the first one. // Reference generation needs no workspace — it reflects on the CLI itself. - match &cli.command { + match cli.command { Command::MarkdownHelp => { print!("{}", commands::markdown_help()); return Ok(true); } Command::Completions { shell } => { - commands::generate::completions(*shell)?; + commands::generate::completions(shell)?; return Ok(true); } Command::Man { out } => { - commands::generate::man_pages(out)?; + commands::generate::man_pages(&out)?; return Ok(true); } // `init` creates the workspace root, so it cannot be found by loading // one — it finds the repository root itself. Command::Init => return commands::init::run(&start), + Command::Doctor { + fix, + dry_run, + format, + } => { + let root = Workspace::find_root(&start)?; + return commands::doctor::run( + &root, + &commands::doctor::DoctorOptions { + fix, + dry_run, + format, + }, + ); + } _ => {} } - if let Command::Doctor { - fix, - dry_run, - format, - } = cli.command - { - let root = Workspace::find_root(&start)?; - return commands::doctor::run( - &root, - &commands::doctor::DoctorOptions { - fix, - dry_run, - format, - }, - ); - } - let workspace = Workspace::load(&start)?; match cli.command { Command::List { @@ -722,7 +720,8 @@ fn dispatch(cli: Cli) -> Result { commands::release::pr(&workspace, &commands::release::PrOptions { base, branch }) } ReleaseCommand::Bootstrap { args } => { - commands::release::bootstrap(&workspace, &args.options()) + commands::tag::create(&workspace, &args.options())?; + Ok(true) } }, Command::Tag { command } => match command { diff --git a/src/rewrite.rs b/src/rewrite.rs index bdb538c..e01a9da 100644 --- a/src/rewrite.rs +++ b/src/rewrite.rs @@ -7,7 +7,7 @@ use crate::config::PathDepRequirement; use anyhow::{Context, Result, bail}; use std::collections::BTreeMap; -use toml_edit::{DocumentMut, Value}; +use toml_edit::DocumentMut; #[derive(Debug, PartialEq, Eq)] pub struct Rewrite { @@ -58,28 +58,20 @@ pub fn rewrite_path_deps( else { continue; }; - let dep_names: Vec = table.iter().map(|(key, _)| key.to_string()).collect(); - for name in dep_names { - let Some(item) = table.get_mut(&name) else { - continue; - }; + for (key, item) in table.iter_mut() { // A `path` key alongside `git` selects a subdirectory of the // remote repo (Gleam 1.18+), not a workspace-local path dep. - let is_path_dep = item - .as_value() - .and_then(|value| value.as_inline_table()) - .is_some_and(|dep| dep.contains_key("path") && !dep.contains_key("git")); - if !is_path_dep { + let Some(dep) = item.as_value_mut().filter(|v| { + v.as_inline_table() + .is_some_and(|t| t.contains_key("path") && !t.contains_key("git")) + }) else { continue; - } + }; + let name = key.get().to_string(); match hex_versions.get(&name) { Some(version) => { let requirement = hex_requirement(version, mode)?; - let mut value = Value::from(requirement.clone()); - if let Some(old) = item.as_value() { - *value.decor_mut() = old.decor().clone(); - } - *item = toml_edit::Item::Value(value); + crate::lockfile::set_str_keep_decor(dep, &requirement); rewrites.push(Rewrite { name, requirement }); } None if section == "dependencies" => bail!( @@ -101,7 +93,7 @@ mod tests { fn versions(pairs: &[(&str, &str)]) -> BTreeMap { pairs .iter() - .map(|(k, v)| (k.to_string(), v.to_string())) + .map(|&(k, v)| (k.to_string(), v.to_string())) .collect() } diff --git a/src/runner.rs b/src/runner.rs index a6672c6..238ffa6 100644 --- a/src/runner.rs +++ b/src/runner.rs @@ -36,9 +36,10 @@ impl CommandSpec { if self.program == "sh" && self.args.first().map(String::as_str) == Some("-c") { return self.args.get(1).cloned().unwrap_or_default(); } - let mut parts = vec![self.program.clone()]; - parts.extend(self.args.iter().cloned()); - parts.join(" ") + std::iter::once(self.program.as_str()) + .chain(self.args.iter().map(String::as_str)) + .collect::>() + .join(" ") } } @@ -79,6 +80,20 @@ pub struct RunOptions { pub json: bool, } +impl RunOptions { + /// `--serial` wins; then `--jobs N`; then one job per available core. + pub fn parallelism(serial: bool, jobs: Option) -> usize { + if serial { + return 1; + } + jobs.unwrap_or_else(|| { + std::thread::available_parallelism() + .map(std::num::NonZero::get) + .unwrap_or(4) + }) + } +} + #[derive(Clone)] struct Output { progress: Option>, @@ -171,12 +186,16 @@ impl JobDisplay { progress.set_style( ProgressStyle::with_template("{prefix} {msg}").expect("progress template is valid"), ); - let status = match status { - JobStatus::Success => crate::term::ok("✓ ok"), - JobStatus::Failed(_) => crate::term::err("✗ FAILED"), - JobStatus::Skipped => crate::term::warn("- skipped"), + let text = match status { + JobStatus::Success => "✓ ok", + JobStatus::Failed(_) => "✗ FAILED", + JobStatus::Skipped => "- skipped", }; - progress.finish_with_message(format!("{status} {:.1}s", duration.as_secs_f64())); + progress.finish_with_message(format!( + "{} {:.1}s", + paint_status(status, text), + duration.as_secs_f64() + )); } } @@ -192,7 +211,7 @@ impl JobDisplay { /// sequence. pub fn run_jobs( workspace: &Workspace, - jobs: Vec, + jobs: &[Job], options: &RunOptions, ) -> Result> { if jobs.is_empty() { @@ -250,7 +269,8 @@ pub fn run_jobs( // Sparse until the run ends: a slot stays `None` if that job never started, // which is what distinguishes "skipped" from "ran" in the final pass. let mut results: Vec> = (0..jobs.len()).map(|_| None).collect(); - let (sender, receiver) = mpsc::channel::(); + // Results arrive in completion order, so each carries its job index. + let (sender, receiver) = mpsc::channel::<(usize, JobOutcome)>(); let mut running = 0usize; // Set once a job fails without `--keep-going`. It stops *new* starts only; // jobs already in flight are still awaited, never killed. @@ -281,17 +301,7 @@ pub fn run_jobs( scope.spawn(move || { let outcome = execute_job(job, &name, prefix_width, &output, &display); display.finish(&outcome.status, outcome.duration); - // `member` smuggles the *job* index back over the channel so - // the receiver can tell which job this is — results arrive in - // completion order, not job order. It is overwritten with the - // real member index on receipt. - let _ = sender.send(JobResult { - member: job_idx, // job index in-flight; remapped below - status: outcome.status, - duration: outcome.duration, - exit_code: outcome.exit_code, - failed_command: outcome.failed_command, - }); + let _ = sender.send((job_idx, outcome)); }); } // Nothing running and nothing startable: either every job finished, or @@ -303,13 +313,15 @@ pub fn run_jobs( } // Reap phase. Blocking on one completion is what bounds the pool — // control returns to the fill phase with exactly one free slot. - let done = receiver.recv().expect("worker threads outlive the loop"); + let (job_idx, outcome) = receiver.recv().expect("worker threads outlive the loop"); running -= 1; - let job_idx = done.member; // see the send above: job index, not member - let failed = matches!(done.status, JobStatus::Failed(_)); + let failed = matches!(outcome.status, JobStatus::Failed(_)); results[job_idx] = Some(JobResult { - member: jobs[job_idx].member, // now the real member index - ..done + member: jobs[job_idx].member, + status: outcome.status, + duration: outcome.duration, + exit_code: outcome.exit_code, + failed_command: outcome.failed_command, }); if failed && !options.keep_going { halted = true; @@ -346,10 +358,38 @@ pub fn run_jobs( .collect(); output.clear_live(); - print_summary(workspace, &results, &output); + print_summary(workspace, &results, &output, prefix_width); Ok(results) } +/// [`run_jobs`], then under `--json` print the document `render` builds from +/// the outcome. Returns whether every job succeeded. +pub fn run_and_report( + workspace: &Workspace, + jobs: &[Job], + options: &RunOptions, + render: impl FnOnce(bool, Vec>) -> serde_json::Result, +) -> Result { + let results = run_jobs(workspace, jobs, options)?; + let ok = all_succeeded(&results); + if options.json { + let results = results + .iter() + .map(|result| crate::json::TaskResult::new(workspace, result)) + .collect(); + println!("{}", render(ok, results)?); + } + Ok(ok) +} + +fn paint_status(status: &JobStatus, text: &str) -> String { + match status { + JobStatus::Success => crate::term::ok(text), + JobStatus::Failed(_) => crate::term::err(text), + JobStatus::Skipped => crate::term::warn(text), + } +} + /// What one job produced, before it is paired back up with its member index. struct JobOutcome { status: JobStatus, @@ -446,17 +486,12 @@ fn run_streaming( Ok(child.wait()?) } -fn print_summary(workspace: &Workspace, results: &[JobResult], output: &Output) { +fn print_summary(workspace: &Workspace, results: &[JobResult], output: &Output, name_width: usize) { // `-q` drops it; `--json` replaces it with the payload the caller prints. if output.quiet || output.to_stderr { return; } - let width = results - .iter() - .map(|r| workspace.members[r.member].name.len()) - .max() - .unwrap_or(0) - .max("package".len()); + let width = name_width.max("package".len()); println!(); println!( "{}", @@ -472,12 +507,7 @@ fn print_summary(workspace: &Workspace, results: &[JobResult], output: &Output) JobStatus::Skipped => ("skipped", String::new()), }; // Padded before painting: ANSI codes inside `{:8}` would defeat it. - let status = format!("{status:8}"); - let status = match &result.status { - JobStatus::Success => crate::term::ok(&status), - JobStatus::Failed(_) => crate::term::err(&status), - JobStatus::Skipped => crate::term::warn(&status), - }; + let status = paint_status(&result.status, &format!("{status:8}")); let time = if result.status == JobStatus::Skipped { String::new() } else { diff --git a/src/term.rs b/src/term.rs index 69a1f12..38360b1 100644 --- a/src/term.rs +++ b/src/term.rs @@ -178,8 +178,8 @@ fn name_color_code(name: &str) -> u8 { /// FNV-1a, so a package keeps its color across runs, machines, and releases. fn stable_name_hash(name: &str) -> u64 { - name.bytes().fold(0xcbf29ce484222325, |hash, byte| { - (hash ^ u64::from(byte)).wrapping_mul(0x100000001b3) + name.bytes().fold(0xcbf2_9ce4_8422_2325, |hash, byte| { + (hash ^ u64::from(byte)).wrapping_mul(0x0100_0000_01b3) }) } diff --git a/src/tools.rs b/src/tools.rs index 6d315b6..5bf5c63 100644 --- a/src/tools.rs +++ b/src/tools.rs @@ -45,21 +45,20 @@ pub fn with_retry( ) -> Result { let attempts = policy.attempts.max(1); let mut delay = parse_duration(&policy.initial_delay)?; - let mut last_attempt_error = None; - for attempt in 1..=attempts { + for attempt in 1..attempts { match operation() { Ok(value) => return Ok(value), - Err(err) if attempt < attempts => { + Err(err) => { eprintln!( "warning: {what} failed (attempt {attempt}/{attempts}): {err:#}; retrying in {delay:?}" ); std::thread::sleep(delay); delay *= policy.multiplier; } - Err(err) => last_attempt_error = Some(err), } } - Err(last_attempt_error.expect("loop ends with an error")) + // The last attempt is not retried, so its error is the caller's. + operation() } #[cfg(test)] diff --git a/src/workspace.rs b/src/workspace.rs index 8a0b872..af01a0d 100644 --- a/src/workspace.rs +++ b/src/workspace.rs @@ -6,7 +6,7 @@ use crate::config::{ConfigFile, ReleaseLifecycle, TagLevel}; use crate::gleam::GleamManifest; use crate::json::{Check, Finding}; use anyhow::{Context, Result, bail}; -use std::collections::{BTreeSet, HashMap, HashSet}; +use std::collections::{BTreeMap, BTreeSet, HashMap, HashSet}; use std::path::{Component, Path, PathBuf}; pub const GLEAM_TOML: &str = "gleam.toml"; @@ -29,6 +29,17 @@ pub struct Member { } impl Member { + /// `/`, for messages that cite a file in this member. + pub fn rel_file(&self, name: &str) -> String { + format!("{}/{name}", self.rel_path) + } + + /// True when a release maintains at least one moving series tag for this + /// member (any level but [`TagLevel::Exact`]). + pub fn has_series_tag(&self) -> bool { + self.tags.iter().any(|level| level.is_series()) + } + /// True when this member is published to Hex (`lifecycle == hex`). pub fn publishes_to_hex(&self) -> bool { self.lifecycle == ReleaseLifecycle::Hex @@ -85,6 +96,16 @@ impl Diagnostics { self.findings.push(finding); } + /// An error about the root manifest's `[tools.trellis]` table. + fn config_error(&mut self, message: impl Into) { + self.push(Finding::error(Check::WorkspaceConfig, message).at(GLEAM_TOML)); + } + + /// A warning about the root manifest's `[tools.trellis]` table. + fn config_warning(&mut self, message: impl Into) { + self.push(Finding::warning(Check::WorkspaceConfig, message).at(GLEAM_TOML)); + } + /// Error messages only, in the order they were found. pub fn errors(&self) -> impl Iterator { self.findings @@ -171,37 +192,35 @@ impl Workspace { // configuration, its absence (or a missing manifest — a configless // git-root workspace) means everything is defaulted and discovered. let manifest_path = root.join(GLEAM_TOML); - let (configless, root_is_package) = match std::fs::read_to_string(&manifest_path) { + let document = match std::fs::read_to_string(&manifest_path) { Ok(text) => match toml::from_str::(&text) { - Ok(document) => ( - !crate::config::has_trellis_table(&document), - document.get("name").is_some(), - ), + Ok(document) => Some(document), Err(err) => { - diagnostics.push( - Finding::error( - Check::WorkspaceConfig, - format!("failed to parse {}: {err}", manifest_path.display()), - ) - .at(GLEAM_TOML), - ); + diagnostics.config_error(format!( + "failed to parse {}: {err}", + manifest_path.display() + )); return Ok((None, diagnostics)); } }, - Err(_) => (true, false), + Err(_) => None, }; - let config = if configless { - ConfigFile::configless() - } else { - match ConfigFile::load(&manifest_path) { + let root_is_package = document.as_ref().is_some_and(|d| d.get("name").is_some()); + let trellis_table = document + .as_ref() + .filter(|d| crate::config::has_trellis_table(d)); + let configless = trellis_table.is_none(); + let config = match trellis_table { + None => ConfigFile::configless(), + Some(document) => match ConfigFile::from_document(document) + .with_context(|| format!("in {}", manifest_path.display())) + { Ok(config) => config, Err(err) => { - diagnostics.push( - Finding::error(Check::WorkspaceConfig, format!("{err:#}")).at(GLEAM_TOML), - ); + diagnostics.config_error(format!("{err:#}")); return Ok((None, diagnostics)); } - } + }, }; report_unknown_config_keys(&config, &mut diagnostics); @@ -218,13 +237,7 @@ impl Workspace { // Parse each member manifest; unparseable members are reported and dropped. for (task, patterns) in &config.exclude { if let Err(err) = build_globset(patterns) { - diagnostics.push( - Finding::error( - Check::WorkspaceConfig, - format!("invalid `{task}` exclusion glob: {err:#}"), - ) - .at(GLEAM_TOML), - ); + diagnostics.config_error(format!("invalid `{task}` exclusion glob: {err:#}")); } } @@ -239,72 +252,33 @@ impl Workspace { } } if member_dirs.is_empty() && !diagnostics.has_errors() { - diagnostics.push( - Finding::error( - Check::WorkspaceConfig, - format!( - "no workspace members left after `{}` exclusions", - crate::config::MEMBERS_EXCLUDE_KEY - ), - ) - .at(GLEAM_TOML), - ); + diagnostics.config_error(format!( + "no workspace members left after `{}` exclusions", + crate::config::MEMBERS_EXCLUDE_KEY + )); } - let release_exclusions = config + // An invalid glob was already reported by the `exclude` sweep above. + let release_excludes = config .exclude .get(crate::config::RELEASE_EXCLUDE_KEY) - .cloned() - .unwrap_or_default(); - let release_excludes = build_globset(&release_exclusions) - .map_err(|err| { - diagnostics.push( - Finding::error( - Check::WorkspaceConfig, - format!("invalid release exclusion glob: {err:#}"), - ) - .at(GLEAM_TOML), - ); - }) - .ok(); + .and_then(|patterns| build_globset(patterns).ok()); // Keyed by one member-path glob each, like `publish.lifecycle.packages` // below, so a member can match several with different lists — the case // `resolve_package_tags` must reject. - let mut package_tags_overrides: Vec<(Vec, globset::GlobMatcher)> = Vec::new(); - for (pattern, levels) in &config.publish.package_tags_overrides { - match globset::Glob::new(pattern) { - Ok(glob) => package_tags_overrides.push((levels.clone(), glob.compile_matcher())), - Err(err) => { - diagnostics.push( - Finding::error( - Check::WorkspaceConfig, - format!("invalid `package_tags_overrides` glob `{pattern}`: {err:#}"), - ) - .at(GLEAM_TOML), - ); - } - } - } + let package_tags_overrides = compile_overrides( + &config.publish.package_tags_overrides, + "package_tags_overrides", + &mut diagnostics, + ); // `publish.lifecycle.packages` globs, compiled individually — each key // names exactly one glob, so a member can match several with different // targets, which is the case `resolve_lifecycle` must reject. - let mut lifecycle_overrides: Vec<(ReleaseLifecycle, globset::GlobMatcher)> = Vec::new(); - for (pattern, lifecycle) in &config.publish.lifecycle.packages { - match globset::Glob::new(pattern) { - Ok(glob) => lifecycle_overrides.push((*lifecycle, glob.compile_matcher())), - Err(err) => { - diagnostics.push( - Finding::error( - Check::WorkspaceConfig, - format!( - "invalid `publish.lifecycle.packages` glob `{pattern}`: {err:#}" - ), - ) - .at(GLEAM_TOML), - ); - } - } - } + let lifecycle_overrides = compile_overrides( + &config.publish.lifecycle.packages, + "publish.lifecycle.packages", + &mut diagnostics, + ); let mut members = Vec::new(); for dir in member_dirs { let rel_path = rel_path_string(root, &dir); @@ -392,13 +366,9 @@ impl Workspace { if let Some(anchor) = &config.publish.repository_tag_package { match members.iter().find(|member| &member.name == anchor) { - None => diagnostics.push( - Finding::error( - Check::WorkspaceConfig, - format!("`repository_tag_package` `{anchor}` is not a workspace member"), - ) - .at(GLEAM_TOML), - ), + None => diagnostics.config_error(format!( + "`repository_tag_package` `{anchor}` is not a workspace member" + )), Some(member) if !member.releasable() => diagnostics.push( Finding::error( Check::ReleaseBoundary, @@ -467,7 +437,7 @@ impl Workspace { } } - let names: Vec = members.iter().map(|m| m.name.clone()).collect(); + let names: Vec<&str> = members.iter().map(|m| m.name.as_str()).collect(); let edge_list: Vec<(usize, usize)> = edges.iter().copied().collect(); let order = match toposort(members.len(), &names, &edge_list) { Ok(order) => order, @@ -564,14 +534,14 @@ impl Workspace { } pub fn transitive_deps(&self, idx: usize) -> HashSet { - self.closure(idx, &self.deps) + Self::closure(idx, &self.deps) } pub fn transitive_dependents(&self, idx: usize) -> HashSet { - self.closure(idx, &self.dependents) + Self::closure(idx, &self.dependents) } - fn closure(&self, start: usize, adjacency: &[Vec]) -> HashSet { + fn closure(start: usize, adjacency: &[Vec]) -> HashSet { let mut seen = HashSet::new(); let mut stack = adjacency[start].clone(); while let Some(next) = stack.pop() { @@ -582,26 +552,35 @@ impl Workspace { seen } + /// The series tags a release of member `idx` maintains at its current + /// version — empty unless [`Member::has_series_tag`]. + pub fn series_tags_of(&self, idx: usize) -> Vec { + let member = &self.members[idx]; + self.config + .series_tags(&member.name, member.version(), &member.tags) + } + /// Resolve a set of member names/filters into topologically ordered indices. pub fn select(&self, filter: &SelectionFilter) -> Result> { let mut selected: HashSet = if filter.names.is_empty() { (0..self.members.len()).collect() } else { - let mut set = HashSet::new(); - for name in &filter.names { - let idx = self.member_index(name).with_context(|| { - format!( - "unknown package `{name}` (members: {})", - self.members - .iter() - .map(|m| m.name.as_str()) - .collect::>() - .join(", ") - ) - })?; - set.insert(idx); - } - set + filter + .names + .iter() + .map(|name| { + self.member_index(name).with_context(|| { + format!( + "unknown package `{name}` (members: {})", + self.members + .iter() + .map(|m| m.name.as_str()) + .collect::>() + .join(", ") + ) + }) + }) + .collect::>()? }; if let Some(since) = &filter.since { @@ -643,7 +622,7 @@ pub struct SelectionFilter { /// dependency order, or one cycle (as names) on failure. pub fn toposort( n: usize, - names: &[String], + names: &[&str], edges: &[(usize, usize)], ) -> Result, Vec> { use std::cmp::Reverse; @@ -657,7 +636,7 @@ pub fn toposort( } let mut ready: BinaryHeap> = (0..n) .filter(|&idx| in_degree[idx] == 0) - .map(|idx| Reverse((names[idx].as_str(), idx))) + .map(|idx| Reverse((names[idx], idx))) .collect(); let mut order = Vec::with_capacity(n); while let Some(Reverse((_, idx))) = ready.pop() { @@ -665,7 +644,7 @@ pub fn toposort( for &next in &adjacency[idx] { in_degree[next] -= 1; if in_degree[next] == 0 { - ready.push(Reverse((names[next].as_str(), next))); + ready.push(Reverse((names[next], next))); } } } @@ -689,9 +668,9 @@ pub fn toposort( let cycle_start = path.iter().position(|&idx| idx == next).unwrap_or(0); let mut cycle: Vec = path[cycle_start..] .iter() - .map(|&idx| names[idx].clone()) + .map(|&idx| names[idx].to_string()) .collect(); - cycle.push(names[next].clone()); + cycle.push(names[next].to_string()); return Err(cycle); } path.push(next); @@ -708,32 +687,40 @@ pub fn toposort( /// break working repositories for a spelling change. fn report_unknown_config_keys(config: &ConfigFile, diagnostics: &mut Diagnostics) { for key in &config.deprecated_keys { - diagnostics.push( - Finding::warning( - Check::WorkspaceConfig, - format!( - "[tools.trellis] key `{}` is deprecated; rename it to `{}` \ + diagnostics.config_warning(format!( + "[tools.trellis] key `{}` is deprecated; rename it to `{}` \ (trellis config keys are snake_case)", - key.path, key.replacement - ), - ) - .at(GLEAM_TOML), - ); + key.path, key.replacement + )); } for path in &config.unknown_keys { - diagnostics.push( - Finding::warning( - Check::WorkspaceConfig, - format!( - "[tools.trellis] key `{path}` is not recognized and is being ignored; \ + diagnostics.config_warning(format!( + "[tools.trellis] key `{path}` is not recognized and is being ignored; \ it may belong to a newer trellis" - ), - ) - .at(GLEAM_TOML), - ); + )); } } +/// Compile a `{ glob = value }` override table, one matcher per key. Each key +/// is exactly one glob, so a member can match several with different values +/// — the case the resolvers must reject. +fn compile_overrides( + overrides: &BTreeMap, + key: &str, + diagnostics: &mut Diagnostics, +) -> Vec<(T, globset::GlobMatcher)> { + overrides + .iter() + .filter_map(|(pattern, value)| match globset::Glob::new(pattern) { + Ok(glob) => Some((value.clone(), glob.compile_matcher())), + Err(err) => { + diagnostics.config_error(format!("invalid `{key}` glob `{pattern}`: {err:#}")); + None + } + }) + .collect() +} + /// Validates `@members` exclusion globs against the pre-filter candidate set /// — the same globs are applied as a `retain` right after this runs, so /// checking them afterward against the survivors would mean a working @@ -749,28 +736,19 @@ fn check_members_exclude_globs( .map(|dir| rel_path_string(root, dir)) .collect(); for pattern in patterns { - match globset::Glob::new(pattern) { - Ok(glob) => { - let matcher = glob.compile_matcher(); - if !rel_paths.iter().any(|rel| matcher.is_match(rel)) { - diagnostics.push( - Finding::error( - Check::ExclusionGlob, - format!( - "`@members` exclusion glob `{pattern}` matches no member (typo?)" - ), - ) - .at(GLEAM_TOML), - ); - } - } - Err(_) => diagnostics.push( + // An invalid glob was already reported by the `exclude` sweep. + let Ok(glob) = globset::Glob::new(pattern) else { + continue; + }; + let matcher = glob.compile_matcher(); + if !rel_paths.iter().any(|rel| matcher.is_match(rel)) { + diagnostics.push( Finding::error( Check::ExclusionGlob, - format!("`@members` exclusion glob `{pattern}` is invalid"), + format!("`@members` exclusion glob `{pattern}` matches no member (typo?)"), ) .at(GLEAM_TOML), - ), + ); } } } @@ -945,33 +923,27 @@ fn resolve_package_tags( .filter(|(_, glob)| glob.is_match(rel_path)) .map(|(levels, _)| levels) .collect(); - matched.dedup_by(|a, b| a == b); + matched.dedup(); match matched.as_slice() { [] => default.to_vec(), [levels] => (*levels).clone(), lists => { - diagnostics.push( - Finding::error( - Check::WorkspaceConfig, - format!( - "member `{rel_path}` matches `package_tags_overrides` globs resolving \ + diagnostics.config_error(format!( + "member `{rel_path}` matches `package_tags_overrides` globs resolving \ to {}; a member may have only one tag list", - lists + lists + .iter() + .map(|levels| format!( + "[{}]", + levels .iter() - .map(|levels| format!( - "[{}]", - levels - .iter() - .map(|level| format!("`{}`", level.key())) - .collect::>() - .join(", ") - )) + .map(|level| format!("`{}`", level.key())) .collect::>() - .join(" and ") - ), - ) - .at(GLEAM_TOML), - ); + .join(", ") + )) + .collect::>() + .join(" and ") + )); default.to_vec() } } @@ -1010,30 +982,21 @@ fn resolve_lifecycle( } Some((&(first, _), rest)) if rest.iter().all(|&(lifecycle, _)| lifecycle == first) => first, Some(_) => { - diagnostics.push( - Finding::error( - Check::WorkspaceConfig, - format!( - "member `{rel_path}` matches `publish.lifecycle.packages` globs for \ + diagnostics.config_error(format!( + "member `{rel_path}` matches `publish.lifecycle.packages` globs for \ conflicting lifecycles: {}", - matched - .iter() - .map(|(lifecycle, pattern)| format!( - "`{pattern}` => `{}`", - lifecycle.key() - )) - .collect::>() - .join(", "), - ), - ) - .at(GLEAM_TOML), - ); + matched + .iter() + .map(|(lifecycle, pattern)| format!("`{pattern}` => `{}`", lifecycle.key())) + .collect::>() + .join(", "), + )); default } } } -fn build_globset(patterns: &[String]) -> Result { +pub(crate) fn build_globset(patterns: &[String]) -> Result { let mut builder = globset::GlobSetBuilder::new(); for pattern in patterns { builder.add(globset::Glob::new(pattern)?); @@ -1060,12 +1023,7 @@ pub fn normalize_path(path: &Path) -> PathBuf { } fn rel_path_string(root: &Path, path: &Path) -> String { - let rel = path.strip_prefix(root).unwrap_or(path); - let joined = rel - .components() - .map(|c| c.as_os_str().to_string_lossy()) - .collect::>() - .join("/"); + let joined = crate::git::slash_path(path.strip_prefix(root).unwrap_or(path)); // The root itself can be a member (a single-package repo under // auto-discovery); "." keeps `{rel_path}/...` displays working. if joined.is_empty() { @@ -1079,8 +1037,8 @@ fn rel_path_string(root: &Path, path: &Path) -> String { mod tests { use super::*; - fn names(items: &[&str]) -> Vec { - items.iter().map(|s| s.to_string()).collect() + fn names<'a>(items: &[&'a str]) -> Vec<&'a str> { + items.to_vec() } #[test] @@ -1110,7 +1068,8 @@ mod tests { } fn globs(patterns: &[&str]) -> globset::GlobSet { - build_globset(&names(patterns)).unwrap() + let patterns: Vec = patterns.iter().map(|&s| s.to_string()).collect(); + build_globset(&patterns).unwrap() } fn tag_overrides( diff --git a/tests/phase2.rs b/tests/changelog_version.rs similarity index 88% rename from tests/phase2.rs rename to tests/changelog_version.rs index 08f5d9b..bcd51f5 100644 --- a/tests/phase2.rs +++ b/tests/changelog_version.rs @@ -2,89 +2,21 @@ //! check, plan, apply, and template rendering. No external changie binary — //! trellis is the engine. -use assert_cmd::Command; -use predicates::prelude::*; -use std::fs; -use std::path::{Path, PathBuf}; - -fn fixture(name: &str) -> PathBuf { - Path::new(env!("CARGO_MANIFEST_DIR")) - .join("tests/fixtures") - .join(name) -} - -fn trellis(dir: &Path) -> Command { - let mut cmd = Command::cargo_bin("trellis").unwrap(); - cmd.current_dir(dir); - // Deterministic dates in rendered changelogs: 2026-07-11. - cmd.env("SOURCE_DATE_EPOCH", "1783728000"); - cmd -} - -fn write(path: &Path, content: &str) { - fs::create_dir_all(path.parent().unwrap()).unwrap(); - fs::write(path, content).unwrap(); -} - -fn copy_fixture_to(root: &Path) { - fn walk(dir: &Path, files: &mut Vec) { - for entry in fs::read_dir(dir).unwrap() { - let path = entry.unwrap().path(); - if path.is_dir() { - walk(&path, files); - } else { - files.push(path); - } - } - } - let from = fixture("basic"); - let mut files = Vec::new(); - walk(&from, &mut files); - for file in files { - let dest = root.join(file.strip_prefix(&from).unwrap()); - fs::create_dir_all(dest.parent().unwrap()).unwrap(); - fs::copy(&file, &dest).unwrap(); - } -} +// ponytail: changelog/gleam-log assertions use contains(); convert to insta::assert_snapshot! when next touched -fn add_fragment(root: &Path, project: &str, kind: &str, body: &str) { - let dir = root.join(".changes/unreleased"); - fs::create_dir_all(&dir).unwrap(); - for n in 1u32.. { - let path = dir.join(format!("{project}-{n}.toml")); - if !path.exists() { - write( - &path, - &format!("project = \"{project}\"\nkind = \"{kind}\"\nbody = \"{body}\"\n"), - ); - return; - } - } -} +mod common; -fn git(root: &Path, args: &[&str]) { - let status = std::process::Command::new("git") - .args(args) - .current_dir(root) - .env("GIT_AUTHOR_NAME", "t") - .env("GIT_AUTHOR_EMAIL", "t@t") - .env("GIT_COMMITTER_NAME", "t") - .env("GIT_COMMITTER_EMAIL", "t@t") - .stdout(std::process::Stdio::null()) - .stderr(std::process::Stdio::null()) - .status() - .unwrap(); - assert!(status.success(), "git {args:?} failed"); -} +use common::*; +use predicates::prelude::*; +use std::fs; +use std::path::Path; /// Commit the fixture on `main`, branch, then touch two releasable packages and /// give only `lat_core` a fragment. Every strictness and `--format github` case /// wants the same shape: one package satisfied, one (`lat_mid`) missing. fn workspace_with_one_missing_fragment(root: &Path) { copy_fixture_to(root); - git(root, &["init", "-q", "-b", "main"]); - git(root, &["add", "."]); - git(root, &["commit", "-q", "-m", "init"]); + init_repo(root); git(root, &["checkout", "-q", "-b", "feature"]); write(&root.join("packages/lat_core/src/new.gleam"), "// x\n"); write(&root.join("packages/lat_mid/src/new.gleam"), "// x\n"); @@ -448,9 +380,7 @@ fn changelog_check_maps_diff_to_missing_fragments() { let root = tmp.path(); copy_fixture_to(root); - git(root, &["init", "-q", "-b", "main"]); - git(root, &["add", "."]); - git(root, &["commit", "-q", "-m", "init"]); + init_repo(root); git(root, &["checkout", "-q", "-b", "feature"]); // Change two releasable packages and the example package; add a fragment for one. write(&root.join("packages/lat_core/src/new.gleam"), "// x\n"); @@ -460,12 +390,11 @@ fn changelog_check_maps_diff_to_missing_fragments() { git(root, &["add", "."]); git(root, &["commit", "-q", "-m", "change"]); - let output = trellis(root) - .args(["changelog", "check", "--base", "main", "--json"]) - .output() - .unwrap(); - assert!(!output.status.success(), "lat_mid lacks a fragment"); - let payload: serde_json::Value = serde_json::from_slice(&output.stdout).unwrap(); + let payload = json_output( + root, + &["changelog", "check", "--base", "main", "--json"], + false, + ); assert_eq!(payload["has_entries"], true); assert_eq!(payload["needs_entry"], true); let packages = payload["packages"].as_array().unwrap(); @@ -492,9 +421,7 @@ fn changelog_check_rows_a_package_the_branch_wrote_a_fragment_for() { let root = tmp.path(); copy_fixture_to(root); - git(root, &["init", "-q", "-b", "main"]); - git(root, &["add", "."]); - git(root, &["commit", "-q", "-m", "init"]); + init_repo(root); git(root, &["checkout", "-q", "-b", "feature"]); // Only `lat_core` has changed files. `lat_mid` is documented by a fragment // this branch wrote — a break that propagates to it without touching its @@ -506,12 +433,11 @@ fn changelog_check_rows_a_package_the_branch_wrote_a_fragment_for() { git(root, &["add", "."]); git(root, &["commit", "-q", "-m", "change"]); - let output = trellis(root) - .args(["changelog", "check", "--base", "main", "--format", "json"]) - .output() - .unwrap(); - assert!(output.status.success(), "both packages are documented"); - let payload: serde_json::Value = serde_json::from_slice(&output.stdout).unwrap(); + let payload = json_output( + root, + &["changelog", "check", "--base", "main", "--format", "json"], + true, + ); let packages = payload["packages"].as_array().unwrap(); assert_eq!(packages.len(), 2); let core = packages.iter().find(|p| p["name"] == "lat_core").unwrap(); @@ -575,12 +501,11 @@ fn strictness_off_drops_the_verdict_but_still_reports_counts() { .stdout(predicate::str::contains("lat_mid: no entries")) .stdout(predicate::str::contains("needs a changelog entry").not()); - let output = trellis(root) - .args(["changelog", "check", "--base", "main", "--format", "json"]) - .output() - .unwrap(); - assert!(output.status.success()); - let payload: serde_json::Value = serde_json::from_slice(&output.stdout).unwrap(); + let payload = json_output( + root, + &["changelog", "check", "--base", "main", "--format", "json"], + true, + ); assert_eq!(payload["needs_entry"], false); assert_eq!(payload["ok"], true); // The rows survive — `off` means "don't gate", not "don't report". @@ -635,26 +560,6 @@ fn the_strictness_flag_overrides_the_configured_value() { .success(); } -#[test] -fn invalid_fragments_fail_at_every_strictness() { - let tmp = tempfile::tempdir().unwrap(); - let root = tmp.path(); - workspace_with_one_missing_fragment(root); - set_changelog_config(root, "strictness = \"off\""); - write( - &root.join(".changes/unreleased/broken-1.toml"), - "not toml at all {{{\n", - ); - - // Strictness is a policy about missing entries. A fragment that does not - // parse is malformed input, and no policy setting excuses it. - trellis(root) - .args(["changelog", "check", "--base", "main"]) - .assert() - .failure() - .stdout(predicate::str::contains("broken-1.toml")); -} - // ---- changelog check: --format github -------------------------------------- #[test] @@ -721,12 +626,11 @@ fn the_preview_reports_next_versions_and_fragment_contents() { workspace_with_one_missing_fragment(root); add_fragment(root, "lat_mid", "Fixed", "more"); - let output = trellis(root) - .args(["changelog", "check", "--base", "main", "--format", "json"]) - .output() - .unwrap(); - assert!(output.status.success()); - let payload: serde_json::Value = serde_json::from_slice(&output.stdout).unwrap(); + let payload = json_output( + root, + &["changelog", "check", "--base", "main", "--format", "json"], + true, + ); let preview = payload["preview"].as_str().unwrap(); // The table carries each package's planned bump alongside its counts. assert!( @@ -759,21 +663,18 @@ fn the_release_preview_ignores_fragments_already_on_the_base_branch() { // Unreleased on `main` already: an earlier PR's fragment, which this PR // neither added nor is answerable for. add_fragment(root, "lat_mid", "Fixed", "from an earlier pr"); - git(root, &["init", "-q", "-b", "main"]); - git(root, &["add", "."]); - git(root, &["commit", "-q", "-m", "init"]); + init_repo(root); git(root, &["checkout", "-q", "-b", "feature"]); write(&root.join("packages/lat_core/src/new.gleam"), "// x\n"); add_fragment(root, "lat_core", "Added", "from this pr"); git(root, &["add", "."]); git(root, &["commit", "-q", "-m", "change"]); - let output = trellis(root) - .args(["changelog", "check", "--base", "main", "--format", "json"]) - .output() - .unwrap(); - assert!(output.status.success()); - let payload: serde_json::Value = serde_json::from_slice(&output.stdout).unwrap(); + let payload = json_output( + root, + &["changelog", "check", "--base", "main", "--format", "json"], + true, + ); let preview = payload["preview"].as_str().unwrap(); assert!(preview.contains("- from this pr"), "{preview}"); assert!(!preview.contains("from an earlier pr"), "{preview}"); @@ -792,9 +693,7 @@ fn the_release_preview_ignores_fragments_already_on_the_base_branch() { fn workspace_with_a_base_branch_fragment(root: &Path) { copy_fixture_to(root); add_fragment(root, "lat_core", "Added", "from an earlier pr"); - git(root, &["init", "-q", "-b", "main"]); - git(root, &["add", "."]); - git(root, &["commit", "-q", "-m", "init"]); + init_repo(root); git(root, &["checkout", "-q", "-b", "feature"]); write(&root.join("packages/lat_core/src/new.gleam"), "// x\n"); git(root, &["add", "."]); @@ -807,12 +706,11 @@ fn a_base_branch_fragment_does_not_satisfy_a_later_pr() { let root = tmp.path(); workspace_with_a_base_branch_fragment(root); - let output = trellis(root) - .args(["changelog", "check", "--base", "main", "--format", "json"]) - .output() - .unwrap(); - assert!(!output.status.success()); - let payload: serde_json::Value = serde_json::from_slice(&output.stdout).unwrap(); + let payload = json_output( + root, + &["changelog", "check", "--base", "main", "--format", "json"], + false, + ); assert_eq!(payload["needs_entry"], true); assert_eq!(payload["ok"], false); // `has_entries` follows the counts: this PR wrote nothing, so the CI recipe @@ -842,12 +740,11 @@ fn editing_a_base_branch_fragment_counts_as_this_prs_entry() { git(root, &["add", "."]); git(root, &["commit", "-q", "-m", "reword"]); - let output = trellis(root) - .args(["changelog", "check", "--base", "main", "--format", "json"]) - .output() - .unwrap(); - assert!(output.status.success()); - let payload: serde_json::Value = serde_json::from_slice(&output.stdout).unwrap(); + let payload = json_output( + root, + &["changelog", "check", "--base", "main", "--format", "json"], + true, + ); assert_eq!(payload["needs_entry"], false); let preview = payload["preview"].as_str().unwrap(); assert!(preview.contains("- reworded by this pr"), "{preview}"); @@ -866,12 +763,11 @@ fn an_uncommitted_fragment_satisfies_the_check_before_it_is_committed() { // answer CI will give once it is. add_fragment(root, "lat_core", "Fixed", "not committed yet"); - let output = trellis(root) - .args(["changelog", "check", "--base", "main", "--format", "json"]) - .output() - .unwrap(); - assert!(output.status.success()); - let payload: serde_json::Value = serde_json::from_slice(&output.stdout).unwrap(); + let payload = json_output( + root, + &["changelog", "check", "--base", "main", "--format", "json"], + true, + ); assert_eq!(payload["needs_entry"], false); let preview = payload["preview"].as_str().unwrap(); assert!(preview.contains("- not committed yet"), "{preview}"); @@ -889,21 +785,18 @@ fn an_invalid_base_branch_fragment_still_fails_the_check() { &root.join(".changes/unreleased/broken-1.toml"), "not toml at all {{{\n", ); - git(root, &["init", "-q", "-b", "main"]); - git(root, &["add", "."]); - git(root, &["commit", "-q", "-m", "init"]); + init_repo(root); git(root, &["checkout", "-q", "-b", "feature"]); write(&root.join("packages/lat_core/src/new.gleam"), "// x\n"); add_fragment(root, "lat_core", "Added", "from this pr"); git(root, &["add", "."]); git(root, &["commit", "-q", "-m", "change"]); - let output = trellis(root) - .args(["changelog", "check", "--base", "main", "--format", "json"]) - .output() - .unwrap(); - assert!(!output.status.success()); - let payload: serde_json::Value = serde_json::from_slice(&output.stdout).unwrap(); + let payload = json_output( + root, + &["changelog", "check", "--base", "main", "--format", "json"], + false, + ); assert_eq!(payload["needs_entry"], false); assert_eq!(payload["ok"], false); assert_eq!(payload["invalid_fragments"].as_array().unwrap().len(), 1); @@ -922,12 +815,11 @@ fn invalid_fragments_leave_the_preview_without_versions() { // A plan cannot be computed over a fragment that does not parse, so the // preview falls back to counts alone — the problem itself is reported. - let output = trellis(root) - .args(["changelog", "check", "--base", "main", "--format", "json"]) - .output() - .unwrap(); - assert!(!output.status.success()); - let payload: serde_json::Value = serde_json::from_slice(&output.stdout).unwrap(); + let payload = json_output( + root, + &["changelog", "check", "--base", "main", "--format", "json"], + false, + ); let preview = payload["preview"].as_str().unwrap(); assert!(!preview.contains("### Release preview"), "{preview}"); assert!(preview.contains("| lat_core | ✅ 1 | — |"), "{preview}"); @@ -950,10 +842,11 @@ fn json_stays_an_alias_for_format_json() { } #[test] -fn invalid_fragments_fail_check_and_doctor() { +fn invalid_fragments_fail_check_doctor_and_plan_at_every_strictness() { let tmp = tempfile::tempdir().unwrap(); let root = tmp.path(); - copy_fixture_to(root); + workspace_with_one_missing_fragment(root); + set_changelog_config(root, "strictness = \"off\""); add_fragment(root, "lat_typo", "Added", "x"); // unknown project add_fragment(root, "lat_core", "Invented", "x"); // unknown kind write( @@ -961,6 +854,14 @@ fn invalid_fragments_fail_check_and_doctor() { "not toml at all {{{\n", ); + // Strictness is a policy about missing entries. A fragment that does not + // parse is malformed input, and no policy setting excuses it. + trellis(root) + .args(["changelog", "check", "--base", "main"]) + .assert() + .failure() + .stdout(predicate::str::contains("broken-1.toml")); + trellis(root) .arg("doctor") .assert() @@ -990,12 +891,7 @@ fn version_plan_bumps_by_the_largest_kind() { add_fragment(root, "lat_core", "Added", "minor-level change"); add_fragment(root, "lat_mid", "Breaking", "major-level change"); - let output = trellis(root) - .args(["version", "plan", "--json"]) - .output() - .unwrap(); - assert!(output.status.success()); - let plan: serde_json::Value = serde_json::from_slice(&output.stdout).unwrap(); + let plan = json_output(root, &["version", "plan", "--json"], true); // lat_cli owns no fragment. lat_core's minor bump is inside its requirement // and ripples; lat_mid's major bump is outside and does not. assert_eq!( @@ -1025,12 +921,7 @@ fn major_bump_outside_path_dep_requirement_does_not_ripple() { copy_fixture_to(root); add_fragment(root, "lat_core", "Breaking", "major-level change"); - let output = trellis(root) - .args(["version", "plan", "--json"]) - .output() - .unwrap(); - assert!(output.status.success()); - let plan: serde_json::Value = serde_json::from_slice(&output.stdout).unwrap(); + let plan = json_output(root, &["version", "plan", "--json"], true); assert_eq!( plan["bumped"], serde_json::json!([ @@ -1295,12 +1186,7 @@ fn version_apply_adopts_existing_changelog_history() { copy_fixture_to(root); add_fragment(root, "lat_core", "Added", "grow more vines"); - let output = trellis(root) - .args(["version", "apply", "--json"]) - .output() - .unwrap(); - assert!(output.status.success()); - let payload: serde_json::Value = serde_json::from_slice(&output.stdout).unwrap(); + let payload = json_output(root, &["version", "apply", "--json"], true); // Every package this release touches keeps its history, in topological // order — including the two that only bumped because lat_core did. assert_eq!( @@ -1407,7 +1293,7 @@ fn changelog_without_a_parseable_heading_is_dated_by_current_version() { } /// The ripple makes adoption matter for packages that never had a fragment of -/// their own — before this, releasing lat_core would wipe lat_cli's history. +/// their own — before this, releasing `lat_core` would wipe `lat_cli`'s history. #[test] fn rippled_packages_keep_their_changelog_history() { let tmp = tempfile::tempdir().unwrap(); @@ -1546,41 +1432,6 @@ fn custom_minijinja_templates_shape_the_output() { // ---- version overrides (--bump, --set) ------------------------------------- -/// A package's version straight from its gleam.toml, so a test asserts on what -/// actually landed on disk rather than on what `apply` said it did. -fn version_of(root: &Path, package: &str) -> String { - let manifest = fs::read_to_string(root.join("packages").join(package).join("gleam.toml")) - .unwrap_or_else(|err| panic!("no gleam.toml for {package}: {err}")); - manifest - .lines() - .find_map(|line| line.trim().strip_prefix("version = ")) - .unwrap_or_else(|| panic!("no version in {package}'s gleam.toml")) - .trim_matches('"') - .to_string() -} - -/// A committed repository, for the commands that read git state. -fn init_repo(root: &Path) { - for args in [ - &["init", "-q", "-b", "main"][..], - &["add", "."], - &["commit", "-q", "-m", "init"], - ] { - let status = std::process::Command::new("git") - .args(args) - .current_dir(root) - .env("GIT_AUTHOR_NAME", "t") - .env("GIT_AUTHOR_EMAIL", "t@t") - .env("GIT_COMMITTER_NAME", "t") - .env("GIT_COMMITTER_EMAIL", "t@t") - .stdout(std::process::Stdio::null()) - .stderr(std::process::Stdio::null()) - .status() - .unwrap(); - assert!(status.success(), "git {args:?} failed"); - } -} - fn unreleased_fragments(root: &Path) -> usize { fs::read_dir(root.join(".changes/unreleased")) .map(|entries| { diff --git a/tests/cli.rs b/tests/cli.rs index e060257..c0aa1bd 100644 --- a/tests/cli.rs +++ b/tests/cli.rs @@ -1,21 +1,13 @@ //! End-to-end tests running the trellis binary against fixture workspaces. +mod common; + use assert_cmd::Command; +use common::*; use predicates::prelude::*; use std::fs; -use std::path::{Path, PathBuf}; - -fn fixture(name: &str) -> PathBuf { - Path::new(env!("CARGO_MANIFEST_DIR")) - .join("tests/fixtures") - .join(name) -} - -fn trellis(dir: &Path) -> Command { - let mut cmd = Command::cargo_bin("trellis").unwrap(); - cmd.current_dir(dir); - cmd -} +use std::os::unix::fs::PermissionsExt; +use std::path::Path; // ---- list ------------------------------------------------------------ @@ -59,29 +51,6 @@ fn list_releasable_excludes_release_excluded_members() { .stdout("lat_core hex\nlat_mid hex\nlat_cli hex\n"); } -#[test] -fn list_json_includes_graph_facts() { - let output = trellis(&fixture("basic")) - .args(["list", "--json"]) - .output() - .unwrap(); - assert!(output.status.success()); - let document: serde_json::Value = serde_json::from_slice(&output.stdout).unwrap(); - assert_eq!(document["schema"], "trellis.list/1"); - let items = document["packages"].as_array().unwrap(); - assert_eq!(items.len(), 4); - let mid = items.iter().find(|i| i["name"] == "lat_mid").unwrap(); - assert_eq!(mid["version"], "0.5.0"); - assert_eq!(mid["path"], "packages/lat_mid"); - assert_eq!(mid["lifecycle"], "hex"); - assert_eq!(mid["releasable"], true); - assert_eq!(mid["dependencies"], serde_json::json!(["lat_core"])); - assert_eq!(mid["dependents"], serde_json::json!(["lat_cli"])); - let package_a = items.iter().find(|i| i["name"] == "package_a").unwrap(); - assert_eq!(package_a["lifecycle"], "workspace"); - assert_eq!(package_a["releasable"], false); -} - // ---- graph ----------------------------------------------------------- #[test] @@ -94,19 +63,6 @@ fn graph_mermaid_shows_edges() { .stdout(predicate::str::contains("lat_cli --> lat_mid")); } -#[test] -fn graph_json_lists_nodes_and_edges() { - let output = trellis(&fixture("basic")) - .args(["graph", "--format", "json"]) - .output() - .unwrap(); - let graph: serde_json::Value = serde_json::from_slice(&output.stdout).unwrap(); - assert_eq!(graph["nodes"].as_array().unwrap().len(), 4); - // lat_mid->lat_core, lat_cli->lat_mid, lat_cli->lat_core (dev), - // package_a->lat_cli - assert_eq!(graph["edges"].as_array().unwrap().len(), 4); -} - // ---- info ------------------------------------------------------------ #[test] @@ -252,31 +208,6 @@ fn exec_keep_going_runs_everything_despite_failures() { // ---- run / exec --json ----------------------------------------------- -#[test] -fn run_json_reports_one_record_per_package() { - let output = trellis(&fixture("basic")) - .args(["run", "hello", "--json"]) - .output() - .unwrap(); - assert!(output.status.success()); - let document: serde_json::Value = serde_json::from_slice(&output.stdout).unwrap(); - assert_eq!(document["schema"], "trellis.run/1"); - assert_eq!(document["ok"], true); - assert_eq!(document["task"], "hello"); - // `--target` was not given, so the field is absent rather than null. - assert!(document.get("target").is_none()); - let results = document["results"].as_array().unwrap(); - // package_a is excluded from `hello` by the fixture's [tools.trellis.exclude]. - assert_eq!(results.len(), 3); - let core = results.iter().find(|r| r["package"] == "lat_core").unwrap(); - assert_eq!(core["path"], "packages/lat_core"); - assert_eq!(core["status"], "success"); - assert!(core["duration_ms"].is_u64()); - // Nothing failed, so neither failure field is present. - assert!(core.get("exit_code").is_none()); - assert!(core.get("command").is_none()); -} - #[test] fn run_json_carries_the_target_flag_as_given() { let output = trellis(&fixture("basic")) @@ -289,40 +220,15 @@ fn run_json_carries_the_target_flag_as_given() { assert_eq!(document["target"], "all"); } -#[test] -fn exec_json_records_the_exit_code_of_the_failing_command() { - let output = trellis(&fixture("basic")) - .args(["exec", "lat_core", "--json", "--", "sh", "-c", "exit 3"]) - .output() - .unwrap(); - assert!(!output.status.success()); - let document: serde_json::Value = serde_json::from_slice(&output.stdout).unwrap(); - assert_eq!(document["schema"], "trellis.exec/1"); - assert_eq!(document["ok"], false); - // argv, not a re-splittable string. - assert_eq!( - document["command"], - serde_json::json!(["sh", "-c", "exit 3"]) - ); - let results = document["results"].as_array().unwrap(); - assert_eq!(results.len(), 1); - assert_eq!(results[0]["status"], "failed"); - assert_eq!(results[0]["exit_code"], 3); - // `sh -c` is unwrapped back to the script, matching what the summary table - // and the `$ ...` echo have always shown. - assert_eq!(results[0]["command"], "exit 3"); -} - #[test] fn exec_json_distinguishes_skipped_from_failed() { // lat_core fails first, so the other three never run. Skipped is not a // pass: it carries no exit code and still fails the command. - let output = trellis(&fixture("basic")) - .args(["exec", "--serial", "--json", "--", "sh", "-c", "exit 1"]) - .output() - .unwrap(); - assert!(!output.status.success()); - let document: serde_json::Value = serde_json::from_slice(&output.stdout).unwrap(); + let document = json_output( + &fixture("basic"), + &["exec", "--serial", "--json", "--", "sh", "-c", "exit 1"], + false, + ); let results = document["results"].as_array().unwrap(); let statuses: Vec<&str> = results .iter() @@ -355,12 +261,11 @@ fn json_keeps_stdout_clean_and_moves_package_output_to_stderr() { fn json_emits_a_document_even_when_nothing_is_selected() { // The "no packages selected" notice would otherwise be the one thing on // stdout that is not JSON. - let output = trellis(&fixture("basic")) - .args(["run", "hello", "package_a", "--json"]) - .output() - .unwrap(); - assert!(output.status.success()); - let document: serde_json::Value = serde_json::from_slice(&output.stdout).unwrap(); + let document = json_output( + &fixture("basic"), + &["run", "hello", "package_a", "--json"], + true, + ); assert_eq!(document["ok"], true); assert_eq!(document["results"], serde_json::json!([])); } @@ -509,11 +414,6 @@ fn doctor_passes_on_healthy_workspace() { .stdout(predicate::str::contains("ok: 4 package(s)")); } -fn write(path: &Path, content: &str) { - fs::create_dir_all(path.parent().unwrap()).unwrap(); - fs::write(path, content).unwrap(); -} - #[test] fn doctor_reports_all_problems_at_once() { let dir = tempfile::tempdir().unwrap(); @@ -932,21 +832,6 @@ fn doctor_github_format_escapes_multiline_messages() { assert_eq!(stdout.lines().count(), 1, "annotation spilled: {stdout}"); } -/// `--json` owns stdout the same way, and still reports through the exit code. -#[test] -fn doctor_json_format_emits_only_the_payload() { - let assert = trellis(&fixture("basic")) - .args(["doctor", "--format", "json"]) - .assert() - .success() - .stdout(predicate::str::contains("checked:").not()); - - let stdout = String::from_utf8(assert.get_output().stdout.clone()).unwrap(); - let payload: serde_json::Value = serde_json::from_str(&stdout).unwrap(); - assert_eq!(payload["schema"], "trellis.doctor/1"); - assert_eq!(payload["ok"], true); -} - /// `--fix` in a structured format reports what it wrote through `applied` /// rather than through the `fixed:` prose it suppresses. #[test] @@ -1010,42 +895,42 @@ fn member_glob_skips_directories_without_gleam_toml() { std::fs::create_dir_all(root.join("pkgs/node_modules")).unwrap(); let output = trellis(root).arg("list").assert().success(); let stdout = String::from_utf8_lossy(&output.get_output().stdout).to_string(); - assert!(stdout.contains("a")); + assert!(stdout.contains('a')); assert!(!stdout.contains("node_modules")); } -// ---- ci -------------------------------------------------------------- - #[test] -fn ci_matrix_emits_github_actions_shape() { - let output = trellis(&fixture("basic")) - .args(["ci", "matrix"]) - .output() - .unwrap(); - assert!(output.status.success()); - let matrix: serde_json::Value = serde_json::from_slice(&output.stdout).unwrap(); - let include = matrix["include"].as_array().unwrap(); - assert_eq!(include.len(), 4); - assert_eq!(include[0]["name"], "lat_core"); - assert_eq!(include[0]["path"], "packages/lat_core"); - assert_eq!(include[0]["version"], "1.2.0"); -} +fn doctor_warns_on_tool_versions_mismatch() { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path(); + copy_fixture_to(root); + write(&root.join(".tool-versions"), "erlang 27.0\ngleam 1.5.0\n"); + let gleam = root.join("fake-gleam.sh"); + write(&gleam, "#!/bin/sh\necho 'gleam 1.4.1'\n"); + fs::set_permissions(&gleam, fs::Permissions::from_mode(0o755)).unwrap(); -#[test] -fn ci_outputs_emits_key_value_lines() { - trellis(&fixture("basic")) - .args(["ci", "outputs"]) + // Mismatch is a warning, not an error: doctor still succeeds. + trellis(root) + .env("TRELLIS_GLEAM_BIN", &gleam) + .arg("doctor") .assert() .success() .stdout(predicate::str::contains( - "projects=[\"lat_core\",\"lat_mid\",\"lat_cli\",\"package_a\"]", - )) - .stdout(predicate::str::contains( - "releasable=[\"lat_core\",\"lat_mid\",\"lat_cli\"]", - )) - .stdout(predicate::str::contains("lat_core-v1.2.0")); + "gleam on PATH is 1.4.1 but .tool-versions pins 1.5.0", + )); + + // Matching versions: no warning. + write(&root.join(".tool-versions"), "gleam 1.4.1\n"); + trellis(root) + .env("TRELLIS_GLEAM_BIN", &gleam) + .arg("doctor") + .assert() + .success() + .stdout(predicate::str::contains("gleam on PATH is").not()); } +// ---- ci -------------------------------------------------------------- + // ---- markdown reference ---------------------------------------------- #[test] @@ -1225,29 +1110,13 @@ fn since_selects_changed_packages_and_dependents() { let dir = tempfile::tempdir().unwrap(); let root = dir.path(); // Copy the basic fixture into a real git repo. - copy_dir(&fixture("basic"), root); - - let git = |args: &[&str]| { - let status = std::process::Command::new("git") - .args(args) - .current_dir(root) - .env("GIT_AUTHOR_NAME", "t") - .env("GIT_AUTHOR_EMAIL", "t@t") - .env("GIT_COMMITTER_NAME", "t") - .env("GIT_COMMITTER_EMAIL", "t@t") - .stdout(std::process::Stdio::null()) - .stderr(std::process::Stdio::null()) - .status() - .unwrap(); - assert!(status.success(), "git {args:?} failed"); - }; - git(&["init", "-q", "-b", "main"]); - git(&["add", "."]); - git(&["commit", "-q", "-m", "init"]); - git(&["checkout", "-q", "-b", "feature"]); + copy_fixture_to(root); + + init_repo(root); + git(root, &["checkout", "-q", "-b", "feature"]); write(&root.join("packages/lat_mid/src/new.gleam"), "// change\n"); - git(&["add", "."]); - git(&["commit", "-q", "-m", "touch mid"]); + git(root, &["add", "."]); + git(root, &["commit", "-q", "-m", "touch mid"]); trellis(root) .args(["list", "--since", "main"]) @@ -1297,31 +1166,9 @@ fn version_appends_git_describe_on_dev_builds() { } } -fn copy_dir(from: &Path, to: &Path) { - for entry in walk(from) { - let rel = entry.strip_prefix(from).unwrap(); - let dest = to.join(rel); - fs::create_dir_all(dest.parent().unwrap()).unwrap(); - fs::copy(&entry, &dest).unwrap(); - } -} - -fn walk(dir: &Path) -> Vec { - let mut files = Vec::new(); - for entry in fs::read_dir(dir).unwrap() { - let path = entry.unwrap().path(); - if path.is_dir() { - files.extend(walk(&path)); - } else { - files.push(path); - } - } - files -} - // ---- doctor: unrecognized config keys --------------------------------- -/// A two-member workspace, so the shared_dependency check has something to +/// A two-member workspace, so the `shared_dependency` check has something to /// compare. `config` is spliced in under `[tools.trellis]`. fn workspace_with(root: &Path, config: &str, a_deps: &str, b_deps: &str) { write( diff --git a/tests/common/mod.rs b/tests/common/mod.rs index 5bf36d7..f410f85 100644 --- a/tests/common/mod.rs +++ b/tests/common/mod.rs @@ -1,7 +1,9 @@ -//! Shared e2e helpers: a mock GitHub API served from a background thread. +#![allow(dead_code)] +//! Shared e2e helpers: fixture/process/git utilities and a mock GitHub API +//! served from a background thread. //! -//! Point TRELLIS_GITHUB_API_URL at the returned base URL and set -//! TRELLIS_GITHUB_REPO plus GITHUB_TOKEN; every request the binary makes is +//! Point `TRELLIS_GITHUB_API_URL` at the returned base URL and set +//! `TRELLIS_GITHUB_REPO` plus `GITHUB_TOKEN`; every request the binary makes is //! appended to `.fake/github-log` as `METHOD path?query`, then the JSON body, //! then `---`, so tests assert on the log the way they asserted on the old //! fake-gh log. State lives in `.fake/`: a created release becomes a @@ -12,6 +14,168 @@ use std::io::{Read, Write}; use std::net::{TcpListener, TcpStream}; use std::path::{Path, PathBuf}; +use assert_cmd::Command; +use std::fs; + +pub fn fixture(name: &str) -> PathBuf { + Path::new(env!("CARGO_MANIFEST_DIR")) + .join("tests/fixtures") + .join(name) +} + +/// A trellis invocation in `dir` with deterministic changelog dates +/// (`SOURCE_DATE_EPOCH` = 2026-07-11) and proxy variables stripped, so requests +/// reach the localhost mocks instead of an agent proxy. +pub fn trellis(dir: &Path) -> Command { + let mut cmd = Command::cargo_bin("trellis").unwrap(); + cmd.current_dir(dir); + cmd.env("SOURCE_DATE_EPOCH", "1783728000"); + for var in [ + "HTTP_PROXY", + "HTTPS_PROXY", + "http_proxy", + "https_proxy", + "ALL_PROXY", + "all_proxy", + ] { + cmd.env_remove(var); + } + cmd +} + +/// A trellis invocation aimed at the mock GitHub API: base URL and repo +/// overridden, and a token in the environment. +pub fn trellis_github(dir: &Path, api: &str) -> Command { + let mut cmd = trellis(dir); + cmd.env("TRELLIS_GITHUB_API_URL", api) + .env("TRELLIS_GITHUB_REPO", "example/repo") + .env("GITHUB_TOKEN", "test-token"); + cmd +} + +/// Run a command and parse its stdout as JSON. Takes the expected exit status +/// because `changelog check` reports failure through it while still emitting a +/// well-formed payload. +pub fn json_output(dir: &Path, args: &[&str], expect_success: bool) -> serde_json::Value { + let output = trellis(dir).args(args).output().unwrap(); + assert_eq!( + output.status.success(), + expect_success, + "unexpected exit for {args:?}: {}", + String::from_utf8_lossy(&output.stderr) + ); + serde_json::from_slice(&output.stdout) + .unwrap_or_else(|err| panic!("{args:?} did not emit JSON: {err}")) +} + +pub fn write(path: &Path, content: &str) { + fs::create_dir_all(path.parent().unwrap()).unwrap(); + fs::write(path, content).unwrap(); +} + +/// Copy the `basic` fixture into `root`. +pub fn copy_fixture_to(root: &Path) { + fn walk(dir: &Path, files: &mut Vec) { + for entry in fs::read_dir(dir).unwrap() { + let path = entry.unwrap().path(); + if path.is_dir() { + walk(&path, files); + } else { + files.push(path); + } + } + } + let from = fixture("basic"); + let mut files = Vec::new(); + walk(&from, &mut files); + for file in files { + let dest = root.join(file.strip_prefix(&from).unwrap()); + fs::create_dir_all(dest.parent().unwrap()).unwrap(); + fs::copy(&file, &dest).unwrap(); + } +} + +pub fn git(root: &Path, args: &[&str]) { + let status = std::process::Command::new("git") + .args(args) + .current_dir(root) + .env("GIT_AUTHOR_NAME", "t") + .env("GIT_AUTHOR_EMAIL", "t@t") + .env("GIT_COMMITTER_NAME", "t") + .env("GIT_COMMITTER_EMAIL", "t@t") + .stdout(std::process::Stdio::null()) + .stderr(std::process::Stdio::null()) + .status() + .unwrap(); + assert!(status.success(), "git {args:?} failed"); +} + +pub fn git_stdout(dir: &Path, args: &[&str]) -> String { + let output = std::process::Command::new("git") + .args(args) + .current_dir(dir) + .output() + .unwrap(); + assert!( + output.status.success(), + "git {args:?} failed in {}: {}", + dir.display(), + String::from_utf8_lossy(&output.stderr) + ); + String::from_utf8(output.stdout).unwrap().trim().to_string() +} + +/// A committed repository on `main`, for the commands that read git state. +pub fn init_repo(root: &Path) { + git(root, &["init", "-q", "-b", "main"]); + git(root, &["add", "."]); + git(root, &["commit", "-q", "-m", "init"]); +} + +/// Write the next unreleased fragment for `project`. Uses the pre-1.0 +/// `project` key on purpose, so the alias stays exercised across the suite. +pub fn add_fragment(root: &Path, project: &str, kind: &str, body: &str) { + let dir = root.join(".changes/unreleased"); + fs::create_dir_all(&dir).unwrap(); + for n in 1u32.. { + let path = dir.join(format!("{project}-{n}.toml")); + if !path.exists() { + write( + &path, + &format!("project = \"{project}\"\nkind = \"{kind}\"\nbody = \"{body}\"\n"), + ); + return; + } + } +} + +pub fn version_of(root: &Path, package: &str) -> String { + let manifest = fs::read_to_string(root.join("packages").join(package).join("gleam.toml")) + .unwrap_or_else(|err| panic!("no gleam.toml for {package}: {err}")); + manifest + .lines() + .find_map(|line| line.trim().strip_prefix("version = ")) + .unwrap_or_else(|| panic!("no version in {package}'s gleam.toml")) + .trim_matches('"') + .to_string() +} + +pub fn set_version(root: &Path, package: &str, version: &str) { + let path = root.join("packages").join(package).join("gleam.toml"); + let text = fs::read_to_string(&path).unwrap(); + let text: Vec = text + .lines() + .map(|line| { + if line.starts_with("version = ") { + format!("version = \"{version}\"") + } else { + line.to_string() + } + }) + .collect(); + fs::write(&path, text.join("\n") + "\n").unwrap(); +} + pub fn mock_github(root: &Path) -> String { let listener = TcpListener::bind("127.0.0.1:0").unwrap(); let base = format!("http://{}", listener.local_addr().unwrap()); @@ -31,14 +195,13 @@ fn handle(stream: &mut TcpStream, root: &Path) { let mut chunk = [0u8; 4096]; let header_end = loop { match stream.read(&mut chunk) { - Ok(0) => return, + Ok(0) | Err(_) => return, Ok(n) => { buf.extend_from_slice(&chunk[..n]); if let Some(pos) = buf.windows(4).position(|w| w == b"\r\n\r\n") { break pos + 4; } } - Err(_) => return, } }; let headers = String::from_utf8_lossy(&buf[..header_end]).to_string(); @@ -52,9 +215,8 @@ fn handle(stream: &mut TcpStream, root: &Path) { .unwrap_or(0); while buf.len() < header_end + content_length { match stream.read(&mut chunk) { - Ok(0) => break, + Ok(0) | Err(_) => break, Ok(n) => buf.extend_from_slice(&chunk[..n]), - Err(_) => break, } } let body = String::from_utf8_lossy(&buf[header_end..]).to_string(); diff --git a/tests/configless.rs b/tests/configless.rs index efad88a..67e40c4 100644 --- a/tests/configless.rs +++ b/tests/configless.rs @@ -1,34 +1,14 @@ //! End-to-end tests for member auto-discovery: fully configless workspaces //! (no [tools.trellis] anywhere, root inferred from git), configured -//! workspaces without `members`, and the `@members` exclusion key. +//! workspaces without `members`, the `@members` exclusion key, and how +//! member globs honor git ignore rules. -use assert_cmd::Command; +mod common; + +use common::*; use predicates::prelude::*; -use std::fs; use std::path::Path; -fn trellis(dir: &Path) -> Command { - let mut cmd = Command::cargo_bin("trellis").unwrap(); - cmd.current_dir(dir); - cmd -} - -fn write(path: &Path, content: &str) { - fs::create_dir_all(path.parent().unwrap()).unwrap(); - fs::write(path, content).unwrap(); -} - -fn git_init(root: &Path) { - let status = std::process::Command::new("git") - .args(["init", "--quiet"]) - .current_dir(root) - .stdout(std::process::Stdio::null()) - .stderr(std::process::Stdio::null()) - .status() - .unwrap(); - assert!(status.success(), "git init failed"); -} - /// Two packages with a path dependency between them, no config anywhere. fn scaffold_two_packages(root: &Path) { write( @@ -47,7 +27,7 @@ fn scaffold_two_packages(root: &Path) { fn configless_list_discovers_members_from_the_git_root() { let tmp = tempfile::tempdir().unwrap(); let root = tmp.path(); - git_init(root); + git(root, &["init", "-q"]); scaffold_two_packages(root); // Nothing is committed: discovery must see untracked packages too. @@ -62,7 +42,7 @@ fn configless_list_discovers_members_from_the_git_root() { fn configless_works_from_inside_a_package() { let tmp = tempfile::tempdir().unwrap(); let root = tmp.path(); - git_init(root); + git(root, &["init", "-q"]); scaffold_two_packages(root); trellis(&root.join("packages/cli")) @@ -76,15 +56,13 @@ fn configless_works_from_inside_a_package() { fn configless_single_package_repo_has_the_root_as_member() { let tmp = tempfile::tempdir().unwrap(); let root = tmp.path(); - git_init(root); + git(root, &["init", "-q"]); write( &root.join("gleam.toml"), "name = \"solo\"\nversion = \"2.0.0\"\n", ); - let output = trellis(root).args(["list", "--json"]).output().unwrap(); - assert!(output.status.success()); - let document: serde_json::Value = serde_json::from_slice(&output.stdout).unwrap(); + let document = json_output(root, &["list", "--json"], true); let items = document["packages"].as_array().unwrap(); assert_eq!(items.len(), 1); assert_eq!(items[0]["name"], "solo"); @@ -95,7 +73,7 @@ fn configless_single_package_repo_has_the_root_as_member() { fn configless_skips_gitignored_paths_and_build() { let tmp = tempfile::tempdir().unwrap(); let root = tmp.path(); - git_init(root); + git(root, &["init", "-q"]); scaffold_two_packages(root); write(&root.join(".gitignore"), "vendor/\n"); write( @@ -120,7 +98,7 @@ fn configless_skips_gitignored_paths_and_build() { fn configless_doctor_announces_the_inference() { let tmp = tempfile::tempdir().unwrap(); let root = tmp.path(); - git_init(root); + git(root, &["init", "-q"]); scaffold_two_packages(root); trellis(root) @@ -136,7 +114,7 @@ fn configless_doctor_announces_the_inference() { fn configless_errors_on_a_stray_trellis_table() { let tmp = tempfile::tempdir().unwrap(); let root = tmp.path(); - git_init(root); + git(root, &["init", "-q"]); scaffold_two_packages(root); write( &root.join("nested/gleam.toml"), @@ -168,7 +146,7 @@ fn no_config_outside_a_git_repo_is_an_error() { fn unparseable_ancestor_manifest_blocks_the_configless_fallback() { let tmp = tempfile::tempdir().unwrap(); let root = tmp.path(); - git_init(root); + git(root, &["init", "-q"]); scaffold_two_packages(root); write(&root.join("gleam.toml"), "name = \"broken\nversion=\n"); @@ -185,7 +163,7 @@ fn unparseable_ancestor_manifest_blocks_the_configless_fallback() { fn table_without_members_auto_discovers_and_keeps_exclusions() { let tmp = tempfile::tempdir().unwrap(); let root = tmp.path(); - git_init(root); + git(root, &["init", "-q"]); scaffold_two_packages(root); write( &root.join("examples/demo/gleam.toml"), @@ -212,7 +190,7 @@ fn table_without_members_auto_discovers_and_keeps_exclusions() { fn at_members_excludes_directories_from_membership() { let tmp = tempfile::tempdir().unwrap(); let root = tmp.path(); - git_init(root); + git(root, &["init", "-q"]); scaffold_two_packages(root); // A committed fixture package: gitignore cannot exclude it, @members can. write( @@ -249,3 +227,88 @@ fn at_members_also_filters_explicit_member_globs() { .success() .stdout("core hex\n"); } + +// ---- member globs and git ignores --------------------------------------- + +fn write_package(root: &Path, path: &str, name: &str) { + write( + &root.join(path).join("gleam.toml"), + &format!("name = \"{name}\"\nversion = \"0.1.0\"\n"), + ); +} + +#[test] +fn recursive_member_glob_respects_repository_git_ignores() { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path(); + git(root, &["init", "-q"]); + + write( + &root.join("gleam.toml"), + "[tools.trellis]\nmembers = [\"examples/**\"]\n", + ); + write(&root.join(".gitignore"), "build/\n"); + write(&root.join(".git/info/exclude"), "scratch/\n"); + write( + &root.join("examples/collab_docs/.gitignore"), + "generated/\n", + ); + + write_package(root, "examples/chatrooms", "chatrooms"); + write_package(root, "examples/collab_docs/client", "collab_docs_client"); + write_package(root, "examples/scratch", "scratch"); + write_package(root, "examples/collab_docs/generated", "generated"); + + // These duplicate vendored packages reproduce issue #21 when build/ + // directories are traversed. + write_package(root, "examples/chatrooms/build/packages/vendor", "vendor"); + write_package(root, "examples/collab_docs/build/packages/vendor", "vendor"); + + trellis(root) + .arg("list") + .assert() + .success() + .stdout("chatrooms hex\ncollab_docs_client hex\n"); +} + +#[test] +fn literal_member_path_includes_an_ignored_package() { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path(); + git(root, &["init", "-q"]); + + write( + &root.join("gleam.toml"), + "[tools.trellis]\nmembers = [\"generated/package\"]\n", + ); + write(&root.join(".gitignore"), "generated/\n"); + write_package(root, "generated/package", "generated_package"); + + trellis(root) + .arg("list") + .assert() + .success() + .stdout("generated_package hex\n"); +} + +#[test] +fn wildcard_with_only_ignored_packages_reports_no_matches() { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path(); + git(root, &["init", "-q"]); + + write( + &root.join("gleam.toml"), + "[tools.trellis]\nmembers = [\"generated/**\"]\n", + ); + write(&root.join(".gitignore"), "generated/\n"); + write_package(root, "generated/package", "generated_package"); + + trellis(root) + .arg("list") + .assert() + .failure() + .stderr(predicate::str::contains( + "member glob `generated/**` matches no packages", + )); +} diff --git a/tests/exit_codes.rs b/tests/exit_codes.rs index 2a78edb..27c728d 100644 --- a/tests/exit_codes.rs +++ b/tests/exit_codes.rs @@ -8,27 +8,11 @@ //! | 2 | usage error (clap's default) | //! | 3 | internal/environment error — bad config, no git repo, no tool | -use assert_cmd::Command; -use predicates::prelude::*; -use std::fs; -use std::path::{Path, PathBuf}; - -fn fixture(name: &str) -> PathBuf { - Path::new(env!("CARGO_MANIFEST_DIR")) - .join("tests/fixtures") - .join(name) -} +mod common; -fn trellis(dir: &Path) -> Command { - let mut cmd = Command::cargo_bin("trellis").unwrap(); - cmd.current_dir(dir); - cmd -} - -fn write(path: &Path, content: &str) { - fs::create_dir_all(path.parent().unwrap()).unwrap(); - fs::write(path, content).unwrap(); -} +use common::*; +use predicates::prelude::*; +use std::path::Path; /// A workspace whose only problem is an unfixable one, so `doctor` reports a /// finding rather than failing to load. diff --git a/tests/init.rs b/tests/init.rs index 16e4fe6..1719ace 100644 --- a/tests/init.rs +++ b/tests/init.rs @@ -1,32 +1,12 @@ //! End-to-end tests for `trellis init` — bootstrapping a workspace. -use assert_cmd::Command; +mod common; + +use common::*; use predicates::prelude::*; use std::fs; use std::path::Path; -fn trellis(dir: &Path) -> Command { - let mut cmd = Command::cargo_bin("trellis").unwrap(); - cmd.current_dir(dir); - cmd -} - -fn write(path: &Path, content: &str) { - fs::create_dir_all(path.parent().unwrap()).unwrap(); - fs::write(path, content).unwrap(); -} - -fn git(root: &Path, args: &[&str]) { - let status = std::process::Command::new("git") - .args(args) - .current_dir(root) - .stdout(std::process::Stdio::null()) - .stderr(std::process::Stdio::null()) - .status() - .unwrap(); - assert!(status.success(), "git {args:?} failed"); -} - /// Two packages in a git repo, with no workspace configuration at all — the /// state someone adopting trellis actually starts from. fn repo_with_packages(root: &Path) { diff --git a/tests/json_contract.rs b/tests/json_contract.rs index 97bb8b8..c0640e9 100644 --- a/tests/json_contract.rs +++ b/tests/json_contract.rs @@ -2,108 +2,18 @@ //! //! These exist so that a breaking change to a documented shape fails *here* //! rather than in a consumer's workflow. They assert the wire format and -//! nothing else — behavior is covered by `cli.rs`, `phase2.rs`, and -//! `phase3.rs`. +//! nothing else — behavior is covered by `cli.rs`, `changelog_version.rs`, +//! and `tag_publish.rs`. //! //! A failing snapshot is not automatically a bug: adding a field is permitted //! by the contract. Renaming, removing, or retyping one is not, and needs the //! `schema` identifier bumped along with it. See //! `website/src/content/docs/docs/json-output.mdx`. -use assert_cmd::Command; -use std::fs; -use std::path::{Path, PathBuf}; - -fn fixture(name: &str) -> PathBuf { - Path::new(env!("CARGO_MANIFEST_DIR")) - .join("tests/fixtures") - .join(name) -} - -fn trellis(dir: &Path) -> Command { - let mut cmd = Command::cargo_bin("trellis").unwrap(); - cmd.current_dir(dir); - // Deterministic dates in rendered changelogs: 2026-07-11. - cmd.env("SOURCE_DATE_EPOCH", "1783728000"); - cmd -} - -/// Run a command and parse its stdout as JSON. Takes the expected exit status -/// because `changelog check` reports failure through it while still emitting a -/// well-formed payload. -fn json_output(dir: &Path, args: &[&str], expect_success: bool) -> serde_json::Value { - let output = trellis(dir).args(args).output().unwrap(); - assert_eq!( - output.status.success(), - expect_success, - "unexpected exit for {args:?}: {}", - String::from_utf8_lossy(&output.stderr) - ); - serde_json::from_slice(&output.stdout) - .unwrap_or_else(|err| panic!("{args:?} did not emit JSON: {err}")) -} - -fn write(path: &Path, content: &str) { - fs::create_dir_all(path.parent().unwrap()).unwrap(); - fs::write(path, content).unwrap(); -} - -fn copy_fixture_to(root: &Path) { - fn walk(dir: &Path, files: &mut Vec) { - for entry in fs::read_dir(dir).unwrap() { - let path = entry.unwrap().path(); - if path.is_dir() { - walk(&path, files); - } else { - files.push(path); - } - } - } - let from = fixture("basic"); - let mut files = Vec::new(); - walk(&from, &mut files); - for file in files { - let dest = root.join(file.strip_prefix(&from).unwrap()); - fs::create_dir_all(dest.parent().unwrap()).unwrap(); - fs::copy(&file, &dest).unwrap(); - } -} - -fn git(root: &Path, args: &[&str]) { - let status = std::process::Command::new("git") - .args(args) - .current_dir(root) - .env("GIT_AUTHOR_NAME", "t") - .env("GIT_AUTHOR_EMAIL", "t@t") - .env("GIT_COMMITTER_NAME", "t") - .env("GIT_COMMITTER_EMAIL", "t@t") - .stdout(std::process::Stdio::null()) - .stderr(std::process::Stdio::null()) - .status() - .unwrap(); - assert!(status.success(), "git {args:?} failed"); -} +mod common; -fn init_repo(root: &Path) { - git(root, &["init", "-q", "-b", "main"]); - git(root, &["add", "."]); - git(root, &["commit", "-q", "-m", "init"]); -} - -fn add_fragment(root: &Path, project: &str, kind: &str, body: &str) { - let dir = root.join(".changes/unreleased"); - fs::create_dir_all(&dir).unwrap(); - for n in 1u32.. { - let path = dir.join(format!("{project}-{n}.toml")); - if !path.exists() { - write( - &path, - &format!("project = \"{project}\"\nkind = \"{kind}\"\nbody = \"{body}\"\n"), - ); - return; - } - } -} +use common::*; +use std::fs; // ---- introspection ------------------------------------------------------- diff --git a/tests/lifecycle.rs b/tests/lifecycle.rs index 29c7cd8..780a832 100644 --- a/tests/lifecycle.rs +++ b/tests/lifecycle.rs @@ -9,28 +9,10 @@ //! `workspace` through the legacy `@release` mapping instead of an explicit //! rule. -use assert_cmd::Command; -use predicates::prelude::*; -use std::fs; -use std::path::{Path, PathBuf}; - -fn fixture(name: &str) -> PathBuf { - Path::new(env!("CARGO_MANIFEST_DIR")) - .join("tests/fixtures") - .join(name) -} +mod common; -fn trellis(dir: &Path) -> Command { - let mut cmd = Command::cargo_bin("trellis").unwrap(); - cmd.current_dir(dir); - cmd.env("SOURCE_DATE_EPOCH", "1783728000"); - cmd -} - -fn write(path: &Path, content: &str) { - fs::create_dir_all(path.parent().unwrap()).unwrap(); - fs::write(path, content).unwrap(); -} +use common::*; +use predicates::prelude::*; // ---- introspection over the mixed-lifecycle fixture ------------------- @@ -47,12 +29,7 @@ fn doctor_passes_the_mixed_lifecycle_workspace() { #[test] fn list_json_reports_all_three_lifecycle_states() { - let output = trellis(&fixture("lifecycle")) - .args(["list", "--json"]) - .output() - .unwrap(); - assert!(output.status.success()); - let document: serde_json::Value = serde_json::from_slice(&output.stdout).unwrap(); + let document = json_output(&fixture("lifecycle"), &["list", "--json"], true); let items = document["packages"].as_array().unwrap(); let lifecycle_of = |name: &str| { items @@ -242,6 +219,20 @@ fn publish_all_untagged_selects_hex_packages_only() { .stdout(predicate::str::contains("demo").not()); } +#[test] +fn publish_rejects_unreleasable_package() { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path(); + copy_fixture_to(root); + trellis(root) + .args(["publish", "package_a"]) + .assert() + .failure() + .stderr(predicate::str::contains( + "release lifecycle `workspace`, not `hex`", + )); +} + // ---- configuration: parsing, precedence, and conflicts ----------------- #[test] diff --git a/tests/member_discovery.rs b/tests/member_discovery.rs deleted file mode 100644 index 921d1ec..0000000 --- a/tests/member_discovery.rs +++ /dev/null @@ -1,109 +0,0 @@ -//! End-to-end tests for workspace member discovery. - -use assert_cmd::Command; -use predicates::prelude::*; -use std::fs; -use std::path::Path; - -fn trellis(dir: &Path) -> Command { - let mut cmd = Command::cargo_bin("trellis").unwrap(); - cmd.current_dir(dir); - cmd -} - -fn write(path: &Path, content: &str) { - fs::create_dir_all(path.parent().unwrap()).unwrap(); - fs::write(path, content).unwrap(); -} - -fn write_package(root: &Path, path: &str, name: &str) { - write( - &root.join(path).join("gleam.toml"), - &format!("name = \"{name}\"\nversion = \"0.1.0\"\n"), - ); -} - -fn init_git(root: &Path) { - let status = std::process::Command::new("git") - .args(["init", "--quiet"]) - .current_dir(root) - .status() - .unwrap(); - assert!(status.success()); -} - -#[test] -fn recursive_member_glob_respects_repository_git_ignores() { - let tmp = tempfile::tempdir().unwrap(); - let root = tmp.path(); - init_git(root); - - write( - &root.join("gleam.toml"), - "[tools.trellis]\nmembers = [\"examples/**\"]\n", - ); - write(&root.join(".gitignore"), "build/\n"); - write(&root.join(".git/info/exclude"), "scratch/\n"); - write( - &root.join("examples/collab_docs/.gitignore"), - "generated/\n", - ); - - write_package(root, "examples/chatrooms", "chatrooms"); - write_package(root, "examples/collab_docs/client", "collab_docs_client"); - write_package(root, "examples/scratch", "scratch"); - write_package(root, "examples/collab_docs/generated", "generated"); - - // These duplicate vendored packages reproduce issue #21 when build/ - // directories are traversed. - write_package(root, "examples/chatrooms/build/packages/vendor", "vendor"); - write_package(root, "examples/collab_docs/build/packages/vendor", "vendor"); - - trellis(root) - .arg("list") - .assert() - .success() - .stdout("chatrooms hex\ncollab_docs_client hex\n"); -} - -#[test] -fn literal_member_path_includes_an_ignored_package() { - let tmp = tempfile::tempdir().unwrap(); - let root = tmp.path(); - init_git(root); - - write( - &root.join("gleam.toml"), - "[tools.trellis]\nmembers = [\"generated/package\"]\n", - ); - write(&root.join(".gitignore"), "generated/\n"); - write_package(root, "generated/package", "generated_package"); - - trellis(root) - .arg("list") - .assert() - .success() - .stdout("generated_package hex\n"); -} - -#[test] -fn wildcard_with_only_ignored_packages_reports_no_matches() { - let tmp = tempfile::tempdir().unwrap(); - let root = tmp.path(); - init_git(root); - - write( - &root.join("gleam.toml"), - "[tools.trellis]\nmembers = [\"generated/**\"]\n", - ); - write(&root.join(".gitignore"), "generated/\n"); - write_package(root, "generated/package", "generated_package"); - - trellis(root) - .arg("list") - .assert() - .failure() - .stderr(predicate::str::contains( - "member glob `generated/**` matches no packages", - )); -} diff --git a/tests/pin.rs b/tests/pin.rs index 04a490e..302a7d2 100644 --- a/tests/pin.rs +++ b/tests/pin.rs @@ -2,46 +2,12 @@ //! remotes — `ls-remote`, `fetch`, and ancestry all work against a local //! path, so no network is touched. -use assert_cmd::Command; +mod common; + +use common::*; use predicates::prelude::*; use std::fs; -use std::path::{Path, PathBuf}; - -fn trellis(dir: &Path) -> Command { - let mut cmd = Command::cargo_bin("trellis").unwrap(); - cmd.current_dir(dir); - cmd -} - -fn write(path: &Path, content: &str) { - fs::create_dir_all(path.parent().unwrap()).unwrap(); - fs::write(path, content).unwrap(); -} - -fn git(root: &Path, args: &[&str]) { - let status = std::process::Command::new("git") - .args(args) - .current_dir(root) - .env("GIT_AUTHOR_NAME", "t") - .env("GIT_AUTHOR_EMAIL", "t@t") - .env("GIT_COMMITTER_NAME", "t") - .env("GIT_COMMITTER_EMAIL", "t@t") - .stdout(std::process::Stdio::null()) - .stderr(std::process::Stdio::null()) - .status() - .unwrap(); - assert!(status.success(), "git {args:?} failed"); -} - -fn git_stdout(dir: &Path, args: &[&str]) -> String { - let output = std::process::Command::new("git") - .args(args) - .current_dir(dir) - .output() - .unwrap(); - assert!(output.status.success(), "git {args:?} failed"); - String::from_utf8(output.stdout).unwrap().trim().to_string() -} +use std::path::PathBuf; /// A repository serving as the git remote: one commit on `main`, with an /// annotated tag `v1` on it. Returns the tempdir and the commit SHA. @@ -77,9 +43,7 @@ fn workspace(url: &str) -> (tempfile::TempDir, PathBuf) { dep_b = {{ git = \"{url}\", ref = \"main\" }} # keep\n" ), ); - git(root.path(), &["init", "-q", "-b", "main"]); - git(root.path(), &["add", "."]); - git(root.path(), &["commit", "-q", "-m", "init"]); + init_repo(root.path()); (root, manifest) } diff --git a/tests/new_and_release.rs b/tests/release_pr.rs similarity index 61% rename from tests/new_and_release.rs rename to tests/release_pr.rs index a4dfd2d..429d733 100644 --- a/tests/new_and_release.rs +++ b/tests/release_pr.rs @@ -1,109 +1,14 @@ //! End-to-end tests for `trellis release pr` (release-PR management via a -//! mock GitHub API) and doctor's .tool-versions advisory. +//! mock GitHub API). mod common; -use assert_cmd::Command; +use common::*; use predicates::prelude::*; use std::fs; -use std::os::unix::fs::PermissionsExt; -use std::path::{Path, PathBuf}; - -fn fixture(name: &str) -> PathBuf { - Path::new(env!("CARGO_MANIFEST_DIR")) - .join("tests/fixtures") - .join(name) -} - -fn trellis(dir: &Path) -> Command { - let mut cmd = Command::cargo_bin("trellis").unwrap(); - cmd.current_dir(dir); - cmd -} - -fn write(path: &Path, content: &str) { - fs::create_dir_all(path.parent().unwrap()).unwrap(); - fs::write(path, content).unwrap(); -} - -fn copy_fixture_to(root: &Path) { - fn walk(dir: &Path, files: &mut Vec) { - for entry in fs::read_dir(dir).unwrap() { - let path = entry.unwrap().path(); - if path.is_dir() { - walk(&path, files); - } else { - files.push(path); - } - } - } - let from = fixture("basic"); - let mut files = Vec::new(); - walk(&from, &mut files); - for file in files { - let dest = root.join(file.strip_prefix(&from).unwrap()); - fs::create_dir_all(dest.parent().unwrap()).unwrap(); - fs::copy(&file, &dest).unwrap(); - } -} - -fn git(root: &Path, args: &[&str]) { - let status = std::process::Command::new("git") - .args(args) - .current_dir(root) - .env("GIT_AUTHOR_NAME", "t") - .env("GIT_AUTHOR_EMAIL", "t@t") - .env("GIT_COMMITTER_NAME", "t") - .env("GIT_COMMITTER_EMAIL", "t@t") - .stdout(std::process::Stdio::null()) - .stderr(std::process::Stdio::null()) - .status() - .unwrap(); - assert!(status.success(), "git {args:?} failed"); -} - -fn make_executable(path: &Path) { - fs::set_permissions(path, fs::Permissions::from_mode(0o755)).unwrap(); -} // ---- trellis release pr ---------------------------------------------------- -fn add_fragment(root: &Path, project: &str, kind: &str, body: &str) { - let dir = root.join(".changes/unreleased"); - fs::create_dir_all(&dir).unwrap(); - for n in 1u32.. { - let path = dir.join(format!("{project}-{n}.toml")); - if !path.exists() { - write( - &path, - &format!("project = \"{project}\"\nkind = \"{kind}\"\nbody = \"{body}\"\n"), - ); - return; - } - } -} - -/// A trellis invocation aimed at the mock GitHub API: base URL and repo -/// overridden, a token in the environment, and proxy variables stripped so -/// requests reach the localhost mock. -fn trellis_github(dir: &Path, api: &str) -> Command { - let mut cmd = trellis(dir); - cmd.env("TRELLIS_GITHUB_API_URL", api) - .env("TRELLIS_GITHUB_REPO", "example/repo") - .env("GITHUB_TOKEN", "test-token"); - for var in [ - "HTTP_PROXY", - "HTTPS_PROXY", - "http_proxy", - "https_proxy", - "ALL_PROXY", - "all_proxy", - ] { - cmd.env_remove(var); - } - cmd -} - #[test] fn release_pr_creates_then_updates_the_pull_request() { let tmp = tempfile::tempdir().unwrap(); @@ -111,9 +16,7 @@ fn release_pr_creates_then_updates_the_pull_request() { copy_fixture_to(root); let api = common::mock_github(root); - git(root, &["init", "-q", "-b", "main"]); - git(root, &["add", "."]); - git(root, &["commit", "-q", "-m", "init"]); + init_repo(root); let remote = tempfile::tempdir().unwrap(); git(remote.path(), &["init", "-q", "--bare"]); git( @@ -216,9 +119,7 @@ fn release_pr_requires_a_clean_tree_and_pending_fragments() { let tmp = tempfile::tempdir().unwrap(); let root = tmp.path(); copy_fixture_to(root); - git(root, &["init", "-q", "-b", "main"]); - git(root, &["add", "."]); - git(root, &["commit", "-q", "-m", "init"]); + init_repo(root); // No fragments: a clean no-op. Both paths stop before any GitHub call, // so no API mock (or token) is needed. @@ -242,9 +143,7 @@ fn release_pr_noop_preserves_an_existing_release_branch() { let tmp = tempfile::tempdir().unwrap(); let root = tmp.path(); copy_fixture_to(root); - git(root, &["init", "-q", "-b", "main"]); - git(root, &["add", "."]); - git(root, &["commit", "-q", "-m", "init"]); + init_repo(root); git(root, &["checkout", "-q", "-b", "release/pending"]); write(&root.join("existing-release.txt"), "keep this commit\n"); git(root, &["add", "."]); @@ -273,9 +172,7 @@ fn release_pr_failure_preserves_an_existing_release_branch() { let tmp = tempfile::tempdir().unwrap(); let root = tmp.path(); copy_fixture_to(root); - git(root, &["init", "-q", "-b", "main"]); - git(root, &["add", "."]); - git(root, &["commit", "-q", "-m", "init"]); + init_repo(root); add_fragment(root, "lat_core", "Added", "pending change"); git(root, &["add", "."]); git(root, &["commit", "-q", "-m", "fragment"]); @@ -308,35 +205,3 @@ fn release_pr_failure_preserves_an_existing_release_branch() { "a failed release must not move the existing local release branch" ); } - -// ---- doctor .tool-versions advisory ---------------------------------------- - -#[test] -fn doctor_warns_on_tool_versions_mismatch() { - let tmp = tempfile::tempdir().unwrap(); - let root = tmp.path(); - copy_fixture_to(root); - write(&root.join(".tool-versions"), "erlang 27.0\ngleam 1.5.0\n"); - let gleam = root.join("fake-gleam.sh"); - write(&gleam, "#!/bin/sh\necho 'gleam 1.4.1'\n"); - make_executable(&gleam); - - // Mismatch is a warning, not an error: doctor still succeeds. - trellis(root) - .env("TRELLIS_GLEAM_BIN", &gleam) - .arg("doctor") - .assert() - .success() - .stdout(predicate::str::contains( - "gleam on PATH is 1.4.1 but .tool-versions pins 1.5.0", - )); - - // Matching versions: no warning. - write(&root.join(".tool-versions"), "gleam 1.4.1\n"); - trellis(root) - .env("TRELLIS_GLEAM_BIN", &gleam) - .arg("doctor") - .assert() - .success() - .stdout(predicate::str::contains("gleam on PATH is").not()); -} diff --git a/tests/phase3.rs b/tests/tag_publish.rs similarity index 91% rename from tests/phase3.rs rename to tests/tag_publish.rs index 987f28f..4bd3b4f 100644 --- a/tests/phase3.rs +++ b/tests/tag_publish.rs @@ -1,11 +1,14 @@ //! End-to-end tests for the tag/publish layer, using a fake gleam binary -//! (TRELLIS_GLEAM_BIN), a mock GitHub API (TRELLIS_GITHUB_API_URL), a mock -//! Hex API served from a local thread (TRELLIS_HEX_API_URL), and real git +//! (`TRELLIS_GLEAM_BIN`), a mock GitHub API (`TRELLIS_GITHUB_API_URL`), a mock +//! Hex API served from a local thread (`TRELLIS_HEX_API_URL`), and real git //! repos. +// ponytail: changelog/gleam-log assertions use contains(); convert to insta::assert_snapshot! when next touched + mod common; -use assert_cmd::Command; +use common::*; + use predicates::prelude::*; use std::fs; use std::io::{Read, Write as IoWrite}; @@ -13,77 +16,6 @@ use std::net::TcpListener; use std::os::unix::fs::PermissionsExt; use std::path::{Path, PathBuf}; -fn fixture(name: &str) -> PathBuf { - Path::new(env!("CARGO_MANIFEST_DIR")) - .join("tests/fixtures") - .join(name) -} - -/// The mock Hex server binds localhost, so the agent proxy configured in -/// some environments must not intercept requests. -fn trellis(dir: &Path) -> Command { - let mut cmd = Command::cargo_bin("trellis").unwrap(); - cmd.current_dir(dir); - for var in [ - "HTTP_PROXY", - "HTTPS_PROXY", - "http_proxy", - "https_proxy", - "ALL_PROXY", - "all_proxy", - ] { - cmd.env_remove(var); - } - cmd -} - -fn write(path: &Path, content: &str) { - fs::create_dir_all(path.parent().unwrap()).unwrap(); - fs::write(path, content).unwrap(); -} - -fn copy_fixture_to(root: &Path) { - fn walk(dir: &Path, files: &mut Vec) { - for entry in fs::read_dir(dir).unwrap() { - let path = entry.unwrap().path(); - if path.is_dir() { - walk(&path, files); - } else { - files.push(path); - } - } - } - let from = fixture("basic"); - let mut files = Vec::new(); - walk(&from, &mut files); - for file in files { - let dest = root.join(file.strip_prefix(&from).unwrap()); - fs::create_dir_all(dest.parent().unwrap()).unwrap(); - fs::copy(&file, &dest).unwrap(); - } -} - -fn git(root: &Path, args: &[&str]) { - let status = std::process::Command::new("git") - .args(args) - .current_dir(root) - .env("GIT_AUTHOR_NAME", "t") - .env("GIT_AUTHOR_EMAIL", "t@t") - .env("GIT_COMMITTER_NAME", "t") - .env("GIT_COMMITTER_EMAIL", "t@t") - .stdout(std::process::Stdio::null()) - .stderr(std::process::Stdio::null()) - .status() - .unwrap(); - assert!(status.success(), "git {args:?} failed"); -} - -fn init_repo(root: &Path) { - git(root, &["init", "-q", "-b", "main"]); - git(root, &["add", "."]); - git(root, &["commit", "-q", "-m", "init"]); -} - /// A fake gleam that logs every invocation (cwd + args) to `.fake/gleam-log` /// and snapshots gleam.toml at publish time so tests can observe the rewrite. /// @@ -114,16 +46,6 @@ fn install_fake_gleam(root: &Path) -> PathBuf { script } -/// A trellis invocation aimed at the mock GitHub API: base URL and repo -/// overridden, and a token in the environment. -fn trellis_github(dir: &Path, api: &str) -> Command { - let mut cmd = trellis(dir); - cmd.env("TRELLIS_GITHUB_API_URL", api) - .env("TRELLIS_GITHUB_REPO", "example/repo") - .env("GITHUB_TOKEN", "test-token"); - cmd -} - /// Serve a canned Hex API from a background thread: `versions` maps package /// name → published versions; unknown packages get a 404, like Hex. fn mock_hex(versions: Vec<(&'static str, Vec<&'static str>)>) -> String { @@ -182,12 +104,7 @@ fn tag_plan_lists_untagged_versions_and_create_tags_them() { // lat_core 1.2.0 is already tagged; lat_mid and lat_cli are not. git(root, &["tag", "-a", "lat_core-v1.2.0", "-m", "existing"]); - let output = trellis(root) - .args(["tag", "plan", "--json"]) - .output() - .unwrap(); - assert!(output.status.success()); - let document: serde_json::Value = serde_json::from_slice(&output.stdout).unwrap(); + let document = json_output(root, &["tag", "plan", "--json"], true); assert_eq!(document["schema"], "trellis.tag_plan/2"); let plan = document["tags"].as_array().unwrap(); let names: Vec<&str> = plan.iter().map(|p| p["name"].as_str().unwrap()).collect(); @@ -401,12 +318,11 @@ fn ci_tag_package_resolves_tag_to_package() { .success() .stdout("lat_core\n"); - let output = trellis(&fixture("basic")) - .args(["ci", "tag-package", "lat_mid-v9.9.9", "--json"]) - .output() - .unwrap(); - assert!(output.status.success()); - let info: serde_json::Value = serde_json::from_slice(&output.stdout).unwrap(); + let info = json_output( + &fixture("basic"), + &["ci", "tag-package", "lat_mid-v9.9.9", "--json"], + true, + ); assert_eq!(info["name"], "lat_mid"); assert_eq!(info["version"], "0.5.0"); assert_eq!(info["tag_version"], "9.9.9"); @@ -667,20 +583,6 @@ fn publish_restores_manifest_even_when_publish_fails() { ); } -#[test] -fn publish_rejects_unreleasable_package() { - let tmp = tempfile::tempdir().unwrap(); - let root = tmp.path(); - copy_fixture_to(root); - trellis(root) - .args(["publish", "package_a"]) - .assert() - .failure() - .stderr(predicate::str::contains( - "release lifecycle `workspace`, not `hex`", - )); -} - // ---- series tags ----------------------------------------------------------- /// The repository-series config the tag tests share: `lat_cli` anchoring @@ -716,37 +618,6 @@ fn series_repo(root: &Path, publish: &str) -> tempfile::TempDir { bare_origin(root) } -fn set_version(root: &Path, package: &str, version: &str) { - let path = root.join("packages").join(package).join("gleam.toml"); - let text = fs::read_to_string(&path).unwrap(); - let text: Vec = text - .lines() - .map(|line| { - if line.starts_with("version = ") { - format!("version = \"{version}\"") - } else { - line.to_string() - } - }) - .collect(); - fs::write(&path, text.join("\n") + "\n").unwrap(); -} - -fn git_stdout(dir: &Path, args: &[&str]) -> String { - let output = std::process::Command::new("git") - .args(args) - .current_dir(dir) - .output() - .unwrap(); - assert!( - output.status.success(), - "git {args:?} failed in {}: {}", - dir.display(), - String::from_utf8_lossy(&output.stderr) - ); - String::from_utf8_lossy(&output.stdout).trim().to_string() -} - fn commit_of(dir: &Path, revision: &str) -> String { git_stdout(dir, &["rev-parse", &format!("{revision}^{{commit}}")]) } @@ -1232,12 +1103,7 @@ fn repository_series_moves_only_when_the_anchor_manifest_version_changes() { set_version(root, "lat_cli", "0.3.2"); git(root, &["commit", "-qam", "release anchor"]); - let output = trellis(root) - .args(["tag", "plan", "--json"]) - .output() - .unwrap(); - assert!(output.status.success()); - let document: serde_json::Value = serde_json::from_slice(&output.stdout).unwrap(); + let document = json_output(root, &["tag", "plan", "--json"], true); let repository = document["tags"] .as_array() .unwrap() @@ -1537,19 +1403,6 @@ fn doctor_rejects_a_package_tags_override_that_matches_nothing() { // ---- release bootstrap ----------------------------------------------------- -// Mirrors `version_of` in phase2.rs — the same manifest-version read, kept in -// step until the test binaries grow a shared support module. -fn version_of(root: &Path, package: &str) -> String { - let manifest = fs::read_to_string(root.join("packages").join(package).join("gleam.toml")) - .unwrap_or_else(|err| panic!("no gleam.toml for {package}: {err}")); - manifest - .lines() - .find_map(|line| line.trim().strip_prefix("version = ")) - .unwrap_or_else(|| panic!("no version in {package}'s gleam.toml")) - .trim_matches('"') - .to_string() -} - #[test] fn bootstrap_uses_current_versions_with_no_fragments_required() { let tmp = tempfile::tempdir().unwrap();