diff --git a/.sampo/changesets/somber-iceseeker-marjatta.md b/.sampo/changesets/somber-iceseeker-marjatta.md new file mode 100644 index 0000000..9ae5e2f --- /dev/null +++ b/.sampo/changesets/somber-iceseeker-marjatta.md @@ -0,0 +1,9 @@ +--- +cargo/sampo-core: minor +cargo/sampo: minor +cargo/sampo-github-action: minor +--- + +Made git tag format configurable via new `tag_format` and `short_tags_format` options under `[git]`. Templates accept `{ecosystem}`, `{package_name}`, and `{version}`. + +`sampo publish` now also detects cross-ecosystem tag conflicts: it errors when two packages would produce the same git tag for the release in flight, and warns when packages share a name across ecosystems with a template that doesn't include `{ecosystem}` (a future version bump would silently collide). Both diagnostics suggest setting `tag_format = "{ecosystem}-{package_name}-v{version}"` as the fix. diff --git a/crates/sampo-core/src/config.rs b/crates/sampo-core/src/config.rs index 031cbdf..c4abbea 100644 --- a/crates/sampo-core/src/config.rs +++ b/crates/sampo-core/src/config.rs @@ -1,9 +1,17 @@ use crate::errors::SampoError; +use crate::tag_template::TagTemplate; +use crate::types::PackageKind; use rustc_hash::FxHashSet; -use semver::Version; use std::collections::BTreeSet; use std::path::Path; +/// Stays narrow by default; cross-ecosystem same-name conflicts are caught at +/// publish time and surface a hint suggesting `{ecosystem}` in the template. +pub const DEFAULT_TAG_FORMAT: &str = "{package_name}-v{version}"; + +/// Used by packages selected via `git.short_tags` (Packagist requires `vX.Y.Z`). +pub const DEFAULT_SHORT_TAGS_FORMAT: &str = "v{version}"; + /// Configuration for Sampo #[derive(Debug, Clone)] pub struct Config { @@ -24,8 +32,10 @@ pub struct Config { pub ignore: Vec, pub git_default_branch: Option, pub git_release_branches: Vec, - /// Package using short tag format (`v{version}`) for Packagist compatibility. + /// Package using short tag format for Packagist compatibility. pub git_short_tags: Option, + pub git_tag_format: TagTemplate, + pub git_short_tags_format: TagTemplate, } impl Default for Config { @@ -46,6 +56,8 @@ impl Default for Config { git_default_branch: None, git_release_branches: Vec::new(), git_short_tags: None, + git_tag_format: TagTemplate::from_static(DEFAULT_TAG_FORMAT), + git_short_tags_format: TagTemplate::from_static(DEFAULT_SHORT_TAGS_FORMAT), } } } @@ -274,40 +286,54 @@ impl Config { } } - let (git_default_branch, git_release_branches, git_short_tags) = value - .get("git") - .and_then(|v| v.as_table()) - .map(|git_table| { - let default_branch = git_table - .get("default_branch") - .and_then(|v| v.as_str()) - .map(|s| s.trim()) - .filter(|s| !s.is_empty()) - .map(|s| s.to_string()); - - let release_branches = git_table - .get("release_branches") - .and_then(|v| v.as_array()) - .map(|arr| { - arr.iter() - .filter_map(|item| item.as_str()) - .map(|s| s.trim()) - .filter(|s| !s.is_empty()) - .map(|s| s.to_string()) - .collect::>() - }) - .unwrap_or_default(); + let git_table = value.get("git").and_then(|v| v.as_table()); - let short_tags = git_table - .get("short_tags") - .and_then(|v| v.as_str()) + let git_default_branch = git_table + .and_then(|t| t.get("default_branch")) + .and_then(|v| v.as_str()) + .map(|s| s.trim()) + .filter(|s| !s.is_empty()) + .map(|s| s.to_string()); + + let git_release_branches = git_table + .and_then(|t| t.get("release_branches")) + .and_then(|v| v.as_array()) + .map(|arr| { + arr.iter() + .filter_map(|item| item.as_str()) .map(|s| s.trim()) .filter(|s| !s.is_empty()) - .map(|s| s.to_string()); - - (default_branch, release_branches, short_tags) + .map(|s| s.to_string()) + .collect::>() }) - .unwrap_or((None, Vec::new(), None)); + .unwrap_or_default(); + + let git_short_tags = git_table + .and_then(|t| t.get("short_tags")) + .and_then(|v| v.as_str()) + .map(|s| s.trim()) + .filter(|s| !s.is_empty()) + .map(|s| s.to_string()); + + let git_tag_format = match git_table.and_then(|t| t.get("tag_format")) { + Some(value) => { + let raw = value + .as_str() + .ok_or_else(|| SampoError::Config("git.tag_format must be a string".into()))?; + TagTemplate::parse(raw)? + } + None => TagTemplate::from_static(DEFAULT_TAG_FORMAT), + }; + + let git_short_tags_format = match git_table.and_then(|t| t.get("short_tags_format")) { + Some(value) => { + let raw = value.as_str().ok_or_else(|| { + SampoError::Config("git.short_tags_format must be a string".into()) + })?; + TagTemplate::parse(raw)? + } + None => TagTemplate::from_static(DEFAULT_SHORT_TAGS_FORMAT), + }; Ok(Self { version, @@ -325,6 +351,8 @@ impl Config { git_default_branch, git_release_branches, git_short_tags, + git_tag_format, + git_short_tags_format, }) } @@ -347,45 +375,47 @@ impl Config { self.release_branches().contains(branch) } - /// Returns true if the given package should use short tag format (`v{version}`). + /// Returns true if the given package should use the short tag format. pub fn uses_short_tags(&self, package_name: &str) -> bool { self.git_short_tags .as_ref() .is_some_and(|name| name == package_name) } - /// Builds a git tag name for the given package and version. - pub fn build_tag_name(&self, package_name: &str, version: &str) -> String { + /// Returns the template that applies to the given package. + pub fn tag_template_for(&self, package_name: &str) -> &TagTemplate { if self.uses_short_tags(package_name) { - format!("v{}", version) + &self.git_short_tags_format } else { - format!("{}-v{}", package_name, version) + &self.git_tag_format } } - /// Parses a tag and returns (package_name, version). + /// Builds a git tag name for the given package and version. + pub fn build_tag_name(&self, kind: PackageKind, package_name: &str, version: &str) -> String { + self.tag_template_for(package_name) + .render(kind, package_name, version) + } + + /// Parses a tag and returns `(package_name, version)`. + /// + /// Tries the short-tag template first (it typically lacks `{package_name}` + /// and only matches the one configured package), then the regular template. pub fn parse_tag(&self, tag: &str) -> Option<(String, String)> { - if let Some(short_pkg) = self - .git_short_tags - .as_ref() - .filter(|_| tag.starts_with('v')) + if let Some(short_pkg) = self.git_short_tags.as_deref() + && let Some(captured) = self.git_short_tags_format.match_tag(tag) + && let Some(version) = captured.version { - let version_str = tag.trim_start_matches('v'); - if Version::parse(version_str).is_ok() { - return Some((short_pkg.clone(), version_str.to_string())); - } + let name = captured + .package_name + .unwrap_or_else(|| short_pkg.to_string()); + return Some((name, version)); } - // Iterate over all "-v" positions to handle prereleases containing "-v" (e.g., "pkg-v1.2.3-v1"). - for (idx, _) in tag.match_indices("-v") { - let name = &tag[..idx]; - let version = &tag[idx + 2..]; - if name.is_empty() || version.is_empty() { - continue; - } - if Version::parse(version).is_ok() { - return Some((name.to_string(), version.to_string())); - } + if let Some(captured) = self.git_tag_format.match_tag(tag) + && let (Some(name), Some(version)) = (captured.package_name, captured.version) + { + return Some((name, version)); } None @@ -765,13 +795,61 @@ mod tests { .unwrap(); let config = Config::load(temp.path()).unwrap(); - assert_eq!(config.build_tag_name("my-package", "1.2.3"), "v1.2.3"); assert_eq!( - config.build_tag_name("other-package", "1.2.3"), + config.build_tag_name(PackageKind::Packagist, "my-package", "1.2.3"), + "v1.2.3" + ); + assert_eq!( + config.build_tag_name(PackageKind::Cargo, "other-package", "1.2.3"), "other-package-v1.2.3" ); } + #[test] + fn build_tag_name_default_omits_ecosystem() { + let temp = tempfile::tempdir().unwrap(); + let config = Config::load(temp.path()).unwrap(); + assert_eq!( + config.build_tag_name(PackageKind::Cargo, "sampo-core", "0.1.0"), + "sampo-core-v0.1.0" + ); + assert_eq!( + config.build_tag_name(PackageKind::Npm, "sampo-core", "0.1.0"), + "sampo-core-v0.1.0" + ); + } + + #[test] + fn build_tag_name_honours_custom_format() { + let temp = tempfile::tempdir().unwrap(); + fs::create_dir_all(temp.path().join(".sampo")).unwrap(); + fs::write( + temp.path().join(".sampo/config.toml"), + "[git]\ntag_format = \"{ecosystem}-{package_name}-v{version}\"\n", + ) + .unwrap(); + + let config = Config::load(temp.path()).unwrap(); + assert_eq!( + config.build_tag_name(PackageKind::Cargo, "my-crate", "1.2.3"), + "cargo-my-crate-v1.2.3" + ); + } + + #[test] + fn rejects_invalid_tag_format() { + let temp = tempfile::tempdir().unwrap(); + fs::create_dir_all(temp.path().join(".sampo")).unwrap(); + fs::write( + temp.path().join(".sampo/config.toml"), + "[git]\ntag_format = \"{kind}-v{version}\"\n", + ) + .unwrap(); + + let err = Config::load(temp.path()).unwrap_err(); + assert!(format!("{err}").contains("unknown placeholder")); + } + #[test] fn parse_tag_handles_short_format() { let temp = tempfile::tempdir().unwrap(); @@ -791,7 +869,6 @@ mod tests { config.parse_tag("v1.2.3-alpha.1"), Some(("my-package".to_string(), "1.2.3-alpha.1".to_string())) ); - // Standard format still works assert_eq!( config.parse_tag("other-package-v1.2.3"), Some(("other-package".to_string(), "1.2.3".to_string())) @@ -855,11 +932,10 @@ mod tests { } #[test] - fn parse_tag_without_short_tags_config() { + fn parse_tag_with_default_format() { let temp = tempfile::tempdir().unwrap(); let config = Config::load(temp.path()).unwrap(); - assert_eq!(config.parse_tag("v1.2.3"), None); assert_eq!( config.parse_tag("my-package-v1.2.3"), Some(("my-package".to_string(), "1.2.3".to_string())) @@ -868,12 +944,65 @@ mod tests { config.parse_tag("my-package-v1.2.3-alpha.1"), Some(("my-package".to_string(), "1.2.3-alpha.1".to_string())) ); - // -v in prerelease requires semver validation to parse correctly + // -v in prerelease still parses correctly thanks to semver validation. assert_eq!( config.parse_tag("my-package-v1.2.3-v1"), Some(("my-package".to_string(), "1.2.3-v1".to_string())) ); + // Tags without `{package_name}` are rejected by the default template. + assert_eq!(config.parse_tag("v1.2.3"), None); assert_eq!(config.parse_tag("my-package-vfoo"), None); assert_eq!(config.parse_tag("my-package-v1.2"), None); } + + #[test] + fn build_tag_name_combines_tag_format_and_short_tags_format() { + let temp = tempfile::tempdir().unwrap(); + fs::create_dir_all(temp.path().join(".sampo")).unwrap(); + fs::write( + temp.path().join(".sampo/config.toml"), + "[git]\n\ + tag_format = \"{package_name}-v{version}\"\n\ + short_tags = \"my-php-pkg\"\n\ + short_tags_format = \"release-{version}\"\n", + ) + .unwrap(); + + let config = Config::load(temp.path()).unwrap(); + assert_eq!( + config.build_tag_name(PackageKind::Cargo, "core", "1.0.0"), + "core-v1.0.0" + ); + assert_eq!( + config.build_tag_name(PackageKind::Packagist, "my-php-pkg", "1.0.0"), + "release-1.0.0" + ); + assert_eq!( + config.parse_tag("core-v1.0.0"), + Some(("core".to_string(), "1.0.0".to_string())) + ); + assert_eq!( + config.parse_tag("release-1.0.0"), + Some(("my-php-pkg".to_string(), "1.0.0".to_string())) + ); + } + + #[test] + fn parse_tag_with_ecosystem_format_via_config() { + let temp = tempfile::tempdir().unwrap(); + fs::create_dir_all(temp.path().join(".sampo")).unwrap(); + fs::write( + temp.path().join(".sampo/config.toml"), + "[git]\ntag_format = \"{ecosystem}-{package_name}-v{version}\"\n", + ) + .unwrap(); + + let config = Config::load(temp.path()).unwrap(); + assert_eq!( + config.parse_tag("cargo-my-package-v1.2.3"), + Some(("my-package".to_string(), "1.2.3".to_string())) + ); + // Legacy-shape tags are rejected once the user opts into the disambiguating template. + assert_eq!(config.parse_tag("my-package-v1.2.3"), None); + } } diff --git a/crates/sampo-core/src/lib.rs b/crates/sampo-core/src/lib.rs index 6d9634c..1bd59ec 100644 --- a/crates/sampo-core/src/lib.rs +++ b/crates/sampo-core/src/lib.rs @@ -10,6 +10,7 @@ pub mod prerelease; pub mod process; pub mod publish; pub mod release; +pub mod tag_template; pub mod types; pub mod workspace; diff --git a/crates/sampo-core/src/publish.rs b/crates/sampo-core/src/publish.rs index fcebac6..8273813 100644 --- a/crates/sampo-core/src/publish.rs +++ b/crates/sampo-core/src/publish.rs @@ -1,4 +1,5 @@ use crate::adapters::PackageAdapter; +use crate::tag_template::Placeholder; use crate::types::{PackageInfo, PackageKind, PublishOutput}; use crate::{ Config, current_branch, discover_workspace, @@ -130,6 +131,10 @@ pub fn run_publish( }); } + for warning in check_tag_conflicts(&config, &all_non_ignored)? { + eprintln!("Warning: {warning}"); + } + // Validate internal deps do not include non-publishable packages let mut errors: Vec = Vec::new(); for identifier in &publishable { @@ -242,12 +247,17 @@ pub fn run_publish( adapter.publish(manifest.as_path(), dry_run, &args)?; any_published = true; - let tag = config.build_tag_name(&package.name, &package.version); + let tag = config.build_tag_name(package.kind, &package.name, &package.version); // Tag immediately after successful publish to ensure partial failures still tag what succeeded if !dry_run { - if let Err(e) = tag_published_crate(&ws.root, &config, &package.name, &package.version) - { + if let Err(e) = tag_published_crate( + &ws.root, + &config, + package.kind, + &package.name, + &package.version, + ) { eprintln!( "Warning: failed to create tag for {}@{}: {}", package.name, package.version, e @@ -255,7 +265,13 @@ pub fn run_publish( } else { tags_to_create.push(tag); } - } else if !package_tag_exists(&ws.root, &config, &package.name, &package.version)? { + } else if !package_tag_exists( + &ws.root, + &config, + package.kind, + &package.name, + &package.version, + )? { tags_to_create.push(tag); } } @@ -270,18 +286,28 @@ pub fn run_publish( if publishable.contains(package.canonical_identifier()) { continue; } - if !package_tag_exists(&ws.root, &config, &package.name, &package.version)? { + if !package_tag_exists( + &ws.root, + &config, + package.kind, + &package.name, + &package.version, + )? { private_packages_to_tag.push(*package); } } if any_published || !private_packages_to_tag.is_empty() { for package in &private_packages_to_tag { - let tag = config.build_tag_name(&package.name, &package.version); + let tag = config.build_tag_name(package.kind, &package.name, &package.version); if !dry_run { - if let Err(e) = - tag_published_crate(&ws.root, &config, &package.name, &package.version) - { + if let Err(e) = tag_published_crate( + &ws.root, + &config, + package.kind, + &package.name, + &package.version, + ) { eprintln!( "Warning: failed to create tag for {}@{}: {}", package.name, package.version, e @@ -307,9 +333,107 @@ pub fn run_publish( }) } +/// Two-tier tag-conflict diagnostics, scoped to all non-ignored packages +/// (private ones get tagged too): +/// +/// - Errors when two packages currently render to the *same* git tag (a release +/// right now would collide). +/// - Returns warnings when ≥2 packages share a name across ≥2 ecosystems and +/// the active template doesn't include `{ecosystem}` — they happen to render +/// distinct tags only because their versions differ today, and a future bump +/// that aligns the versions will collide silently. +/// +/// Both diagnostics suggest adding `{ecosystem}` to the user's `tag_format`. +/// Callers must surface the returned warnings — losing them silently defeats +/// the point of running this check. +#[must_use = "tag-conflict warnings must be surfaced to the user"] +fn check_tag_conflicts(config: &Config, packages: &[&PackageInfo]) -> Result> { + let mut by_tag: BTreeMap = BTreeMap::new(); + for package in packages { + let tag = config.build_tag_name(package.kind, &package.name, &package.version); + if let Some(existing) = by_tag.insert(tag.clone(), package) { + return Err(SampoError::Publish(format!( + "tag conflict: '{}' and '{}' both render to git tag '{}'. {}", + existing.canonical_identifier(), + package.canonical_identifier(), + tag, + tag_conflict_hint(config, &[existing, package]) + ))); + } + } + + let mut by_name: BTreeMap<&str, Vec<&PackageInfo>> = BTreeMap::new(); + for package in packages { + by_name + .entry(package.name.as_str()) + .or_default() + .push(*package); + } + + let mut warnings = Vec::new(); + for (name, group) in &by_name { + if group.len() < 2 { + continue; + } + let mut kinds: BTreeSet = BTreeSet::new(); + for package in group { + kinds.insert(package.kind); + } + if kinds.len() < 2 { + continue; + } + let any_template_disambiguates = group.iter().any(|package| { + config + .tag_template_for(&package.name) + .contains(Placeholder::Ecosystem) + }); + if any_template_disambiguates { + continue; + } + let mut identifiers: Vec = group + .iter() + .map(|package| format!("'{}'", package.canonical_identifier())) + .collect(); + identifiers.sort(); + warnings.push(format!( + "packages {} share the name '{}' across ecosystems; the current `tag_format` will collide once their versions match. {}", + join_with_and(&identifiers), + name, + tag_conflict_hint(config, group) + )); + } + + Ok(warnings) +} + +fn join_with_and(items: &[String]) -> String { + match items { + [] => String::new(), + [only] => only.clone(), + [first, second] => format!("{first} and {second}"), + [head @ .., last] => format!("{}, and {last}", head.join(", ")), + } +} + +/// Picks the right hint depending on whether any package in `group` is on the +/// short-tag path (so we mention `short_tags_format` too). +fn tag_conflict_hint(config: &Config, group: &[&PackageInfo]) -> String { + let touches_short_tags = group + .iter() + .any(|package| config.uses_short_tags(&package.name)); + if touches_short_tags { + "Add `{ecosystem}` to `git.tag_format` and/or `git.short_tags_format` in `.sampo/config.toml` to disambiguate (e.g. `tag_format = \"{ecosystem}-{package_name}-v{version}\"`)." + .to_string() + } else { + "Add `{ecosystem}` to `git.tag_format` in `.sampo/config.toml` to disambiguate (e.g. `tag_format = \"{ecosystem}-{package_name}-v{version}\"`)." + .to_string() + } +} + fn package_tag_exists( repo_root: &Path, config: &Config, + kind: PackageKind, crate_name: &str, version: &str, ) -> Result { @@ -317,7 +441,7 @@ fn package_tag_exists( return Ok(false); } - let tag = config.build_tag_name(crate_name, version); + let tag = config.build_tag_name(kind, crate_name, version); let out = Command::new("git") .arg("-C") .arg(repo_root) @@ -341,6 +465,7 @@ fn package_tag_exists( pub fn tag_published_crate( repo_root: &Path, config: &Config, + kind: PackageKind, crate_name: &str, version: &str, ) -> Result { @@ -348,10 +473,10 @@ pub fn tag_published_crate( // Not a git repo, skip return Ok(false); } - if package_tag_exists(repo_root, config, crate_name, version)? { + if package_tag_exists(repo_root, config, kind, crate_name, version)? { return Ok(false); } - let tag = config.build_tag_name(crate_name, version); + let tag = config.build_tag_name(kind, crate_name, version); let msg = format!("Release {} {}", crate_name, version); let status = Command::new("git") @@ -467,6 +592,112 @@ mod tests { sync::{Mutex, MutexGuard, OnceLock}, }; + fn make_package(kind: PackageKind, name: &str, version: &str) -> PackageInfo { + PackageInfo { + name: name.to_string(), + identifier: PackageInfo::dependency_identifier(kind, name), + version: version.to_string(), + path: PathBuf::from(format!("crates/{name}")), + internal_deps: std::collections::BTreeSet::new(), + internal_dev_deps: std::collections::BTreeSet::new(), + kind, + } + } + + #[test] + fn check_tag_conflicts_errors_on_same_rendered_tag() { + // Default template drops `{ecosystem}`, so two same-name same-version + // packages from different ecosystems collide right now. + let cargo_pkg = make_package(PackageKind::Cargo, "shared", "1.0.0"); + let npm_pkg = make_package(PackageKind::Npm, "shared", "1.0.0"); + let packages = vec![&cargo_pkg, &npm_pkg]; + let config = Config::default(); + let err = check_tag_conflicts(&config, &packages).unwrap_err(); + let msg = format!("{err}"); + assert!(msg.contains("tag conflict"), "{msg}"); + assert!(msg.contains("cargo/shared"), "{msg}"); + assert!(msg.contains("npm/shared"), "{msg}"); + assert!(msg.contains("shared-v1.0.0"), "{msg}"); + assert!(msg.contains("{ecosystem}"), "{msg}"); + } + + #[test] + fn check_tag_conflicts_warns_when_cross_ecosystem_names_match() { + // Different versions today → no immediate collision, but a future + // alignment would. Default template lacks `{ecosystem}`, so warn. + let cargo_pkg = make_package(PackageKind::Cargo, "shared", "1.0.0"); + let npm_pkg = make_package(PackageKind::Npm, "shared", "2.3.4"); + let packages = vec![&cargo_pkg, &npm_pkg]; + let config = Config::default(); + let warnings = check_tag_conflicts(&config, &packages).expect("no immediate collision"); + assert_eq!(warnings.len(), 1, "{warnings:?}"); + let warning = &warnings[0]; + assert!(warning.contains("'cargo/shared'"), "{warning}"); + assert!(warning.contains("'npm/shared'"), "{warning}"); + assert!(warning.contains("share the name 'shared'"), "{warning}"); + assert!(warning.contains("{ecosystem}"), "{warning}"); + } + + #[test] + fn check_tag_conflicts_silent_when_template_includes_ecosystem() { + let cargo_pkg = make_package(PackageKind::Cargo, "shared", "1.0.0"); + let npm_pkg = make_package(PackageKind::Npm, "shared", "1.0.0"); + let packages = vec![&cargo_pkg, &npm_pkg]; + let config = Config { + git_tag_format: crate::tag_template::TagTemplate::parse( + "{ecosystem}-{package_name}-v{version}", + ) + .unwrap(), + ..Config::default() + }; + let warnings = check_tag_conflicts(&config, &packages) + .expect("ecosystem in template prevents collisions"); + assert!(warnings.is_empty(), "{warnings:?}"); + } + + #[test] + fn check_tag_conflicts_silent_when_names_disjoint() { + let cargo_pkg = make_package(PackageKind::Cargo, "alpha", "1.0.0"); + let npm_pkg = make_package(PackageKind::Npm, "beta", "1.0.0"); + let packages = vec![&cargo_pkg, &npm_pkg]; + let config = Config::default(); + let warnings = check_tag_conflicts(&config, &packages).expect("no shared names"); + assert!(warnings.is_empty(), "{warnings:?}"); + } + + #[test] + fn check_tag_conflicts_short_tags_hint_mentions_short_format() { + // When at least one conflicting package sits on the short-tag path, + // the hint must mention `short_tags_format` too. + let cargo_pkg = make_package(PackageKind::Cargo, "alpha", "1.0.0"); + let php_pkg = make_package(PackageKind::Packagist, "beta", "1.0.0"); + let packages = vec![&cargo_pkg, &php_pkg]; + let config = Config { + git_tag_format: crate::tag_template::TagTemplate::parse("v{version}").unwrap(), + git_short_tags: Some("alpha".to_string()), + ..Config::default() + }; + let err = check_tag_conflicts(&config, &packages).unwrap_err(); + let msg = format!("{err}"); + assert!(msg.contains("short_tags_format"), "{msg}"); + } + + #[test] + fn check_tag_conflicts_warning_uses_oxford_comma_for_three_groups() { + let cargo_pkg = make_package(PackageKind::Cargo, "shared", "1.0.0"); + let npm_pkg = make_package(PackageKind::Npm, "shared", "2.0.0"); + let hex_pkg = make_package(PackageKind::Hex, "shared", "3.0.0"); + let packages = vec![&cargo_pkg, &npm_pkg, &hex_pkg]; + let config = Config::default(); + let warnings = check_tag_conflicts(&config, &packages).expect("no immediate collision"); + assert_eq!(warnings.len(), 1, "{warnings:?}"); + let warning = &warnings[0]; + assert!( + warning.contains("'cargo/shared', 'hex/shared', and 'npm/shared'"), + "{warning}" + ); + } + #[test] fn args_for_kind_returns_universal_only_when_no_ecosystem_args() { let extra = PublishExtraArgs { diff --git a/crates/sampo-core/src/release_tests.rs b/crates/sampo-core/src/release_tests.rs index 9cfb923..0d776f6 100644 --- a/crates/sampo-core/src/release_tests.rs +++ b/crates/sampo-core/src/release_tests.rs @@ -1603,21 +1603,8 @@ tempfile = "3.0" // Create config with fixed dependencies let config = Config { - version: 1, - github_repository: None, - changelog_show_commit_hash: true, - changelog_show_acknowledgments: true, - changelog_show_release_date: true, - changelog_release_date_format: "%Y-%m-%d".to_string(), - changelog_release_date_timezone: None, - changesets_tags: vec![], fixed_dependencies: vec![vec!["pkg-a".to_string(), "pkg-c".to_string()]], - linked_dependencies: vec![], - ignore_unpublished: false, - ignore: vec![], - git_default_branch: None, - git_release_branches: Vec::new(), - git_short_tags: None, + ..Config::default() }; // Create changeset that affects pkg-b only diff --git a/crates/sampo-core/src/tag_template.rs b/crates/sampo-core/src/tag_template.rs new file mode 100644 index 0000000..3eca3c2 --- /dev/null +++ b/crates/sampo-core/src/tag_template.rs @@ -0,0 +1,435 @@ +use crate::errors::SampoError; +use crate::types::PackageKind; +use semver::Version; + +/// Placeholder tokens supported by tag templates. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum Placeholder { + Ecosystem, + PackageName, + Version, +} + +impl Placeholder { + fn from_token(token: &str) -> Option { + match token { + "ecosystem" => Some(Self::Ecosystem), + "package_name" => Some(Self::PackageName), + "version" => Some(Self::Version), + _ => None, + } + } +} + +#[derive(Debug, Clone, PartialEq, Eq)] +enum Segment { + Literal(String), + Placeholder(Placeholder), +} + +/// A parsed git tag template (e.g. `"{ecosystem}-{package_name}-v{version}"`). +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct TagTemplate { + raw: String, + segments: Vec, +} + +/// Captured fields from a tag string matched against a template. +#[derive(Debug, Clone, PartialEq, Eq, Default)] +pub struct TagMatch { + pub ecosystem: Option, + pub package_name: Option, + pub version: Option, +} + +impl TagTemplate { + /// Recognised placeholders: `{ecosystem}`, `{package_name}`, `{version}`. + /// `{` / `}` are not escapable. + pub fn parse(input: &str) -> Result { + let trimmed = input.trim(); + if trimmed.is_empty() { + return Err(SampoError::Config( + "tag format template cannot be empty".to_string(), + )); + } + + let mut segments = Vec::new(); + let mut buffer = String::new(); + let mut chars = trimmed.char_indices().peekable(); + while let Some((i, c)) = chars.next() { + match c { + '{' => { + let rest = &trimmed[i + 1..]; + let close_offset = rest.find('}').ok_or_else(|| { + SampoError::Config(format!( + "tag format template '{trimmed}' has an unterminated placeholder" + )) + })?; + let token = &rest[..close_offset]; + let placeholder = Placeholder::from_token(token).ok_or_else(|| { + SampoError::Config(format!( + "tag format template '{trimmed}' contains unknown placeholder '{{{token}}}'. \ + Allowed placeholders: {{ecosystem}}, {{package_name}}, {{version}}." + )) + })?; + if !buffer.is_empty() { + segments.push(Segment::Literal(std::mem::take(&mut buffer))); + } + segments.push(Segment::Placeholder(placeholder)); + let resume = i + 1 + close_offset + 1; + while let Some(&(idx, _)) = chars.peek() { + if idx < resume { + chars.next(); + } else { + break; + } + } + } + '}' => { + return Err(SampoError::Config(format!( + "tag format template '{trimmed}' has an unmatched '}}'" + ))); + } + _ => buffer.push(c), + } + } + if !buffer.is_empty() { + segments.push(Segment::Literal(buffer)); + } + + if !segments + .iter() + .any(|s| matches!(s, Segment::Placeholder(Placeholder::Version))) + { + return Err(SampoError::Config(format!( + "tag format template '{trimmed}' must include the {{version}} placeholder" + ))); + } + + for window in segments.windows(2) { + if matches!(window[0], Segment::Placeholder(_)) + && matches!(window[1], Segment::Placeholder(_)) + { + return Err(SampoError::Config(format!( + "tag format template '{trimmed}' has adjacent placeholders without a separator; \ + insert a literal character (e.g. '-') between them" + ))); + } + } + + Ok(Self { + raw: trimmed.to_string(), + segments, + }) + } + + pub fn as_str(&self) -> &str { + &self.raw + } + + /// Panics on invalid input. For hard-coded defaults only. + pub(crate) fn from_static(input: &'static str) -> Self { + Self::parse(input) + .unwrap_or_else(|err| panic!("invalid built-in tag template '{input}': {err}")) + } + + pub fn contains(&self, placeholder: Placeholder) -> bool { + self.segments + .iter() + .any(|s| matches!(s, Segment::Placeholder(p) if *p == placeholder)) + } + + pub fn render(&self, kind: PackageKind, package_name: &str, version: &str) -> String { + let mut out = String::new(); + for segment in &self.segments { + match segment { + Segment::Literal(text) => out.push_str(text), + Segment::Placeholder(Placeholder::Ecosystem) => out.push_str(kind.as_str()), + Segment::Placeholder(Placeholder::PackageName) => out.push_str(package_name), + Segment::Placeholder(Placeholder::Version) => out.push_str(version), + } + } + out + } + + /// Returns `None` when the tag doesn't fit the template, the `{ecosystem}` + /// capture isn't a known kind, or `{version}` isn't valid semver. + pub fn match_tag(&self, tag: &str) -> Option { + let mut state = MatchState::default(); + if match_segments(&self.segments, tag, &mut state) { + Some(TagMatch { + ecosystem: state.ecosystem, + package_name: state.package_name, + version: state.version, + }) + } else { + None + } + } +} + +#[derive(Default, Clone)] +struct MatchState { + ecosystem: Option, + package_name: Option, + version: Option, +} + +fn match_segments(segments: &[Segment], input: &str, state: &mut MatchState) -> bool { + if segments.is_empty() { + return input.is_empty(); + } + + match &segments[0] { + Segment::Literal(text) => { + if let Some(rest) = input.strip_prefix(text.as_str()) { + match_segments(&segments[1..], rest, state) + } else { + false + } + } + Segment::Placeholder(placeholder) => { + // `parse()` rejects adjacent placeholders, so the next segment is + // either a bounding literal or absent (placeholder takes the rest). + let next_literal = match segments.get(1) { + Some(Segment::Literal(text)) => Some(text.as_str()), + _ => None, + }; + + let (candidates, advance_after_capture) = match next_literal { + Some(literal) => ( + enumerate_literal_positions(input, literal), + Some(literal.len()), + ), + None => (vec![input.len()], None), + }; + + for end in candidates { + let captured = &input[..end]; + let rest_start = end + advance_after_capture.unwrap_or(0); + if rest_start > input.len() { + continue; + } + let rest = &input[rest_start..]; + + let mut local = state.clone(); + if !apply_capture(&mut local, *placeholder, captured) { + continue; + } + + let next_segments = if advance_after_capture.is_some() { + &segments[2..] + } else { + &segments[1..] + }; + + if match_segments(next_segments, rest, &mut local) { + *state = local; + return true; + } + } + + false + } + } +} + +/// Non-overlapping byte offsets of `literal` in `input`. Realistic tag +/// separators (`-`, `-v`, `/`) don't self-overlap, so this is fine. +fn enumerate_literal_positions(input: &str, literal: &str) -> Vec { + if literal.is_empty() { + return vec![input.len()]; + } + let mut positions = Vec::new(); + let mut start = 0; + while let Some(pos) = input[start..].find(literal) { + let absolute = start + pos; + positions.push(absolute); + start = absolute + literal.len(); + } + positions +} + +fn apply_capture(state: &mut MatchState, placeholder: Placeholder, captured: &str) -> bool { + if captured.is_empty() { + return false; + } + match placeholder { + Placeholder::Ecosystem => match PackageKind::parse(captured) { + Some(kind) => { + if let Some(existing) = state.ecosystem + && existing != kind + { + return false; + } + state.ecosystem = Some(kind); + true + } + None => false, + }, + Placeholder::PackageName => { + if let Some(existing) = &state.package_name + && existing != captured + { + return false; + } + state.package_name = Some(captured.to_string()); + true + } + Placeholder::Version => { + if Version::parse(captured).is_err() { + return false; + } + if let Some(existing) = &state.version + && existing != captured + { + return false; + } + state.version = Some(captured.to_string()); + true + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn template(s: &str) -> TagTemplate { + TagTemplate::parse(s).unwrap() + } + + #[test] + fn parses_default_template() { + let t = template("{ecosystem}-{package_name}-v{version}"); + assert_eq!(t.as_str(), "{ecosystem}-{package_name}-v{version}"); + assert!(t.contains(Placeholder::Ecosystem)); + assert!(t.contains(Placeholder::PackageName)); + assert!(t.contains(Placeholder::Version)); + } + + #[test] + fn rejects_unknown_placeholder() { + let err = TagTemplate::parse("{kind}-{version}").unwrap_err(); + assert!(format!("{err}").contains("unknown placeholder")); + } + + #[test] + fn rejects_template_without_version() { + let err = TagTemplate::parse("{package_name}").unwrap_err(); + assert!(format!("{err}").contains("{version}")); + } + + #[test] + fn rejects_unterminated_placeholder() { + let err = TagTemplate::parse("{version").unwrap_err(); + assert!(format!("{err}").contains("unterminated")); + } + + #[test] + fn rejects_unmatched_close_brace() { + let err = TagTemplate::parse("v}{version}").unwrap_err(); + assert!(format!("{err}").contains("unmatched")); + } + + #[test] + fn renders_default_template() { + let t = template("{ecosystem}-{package_name}-v{version}"); + assert_eq!( + t.render(PackageKind::Cargo, "sampo-core", "1.2.3"), + "cargo-sampo-core-v1.2.3" + ); + assert_eq!( + t.render(PackageKind::Npm, "@scope/foo", "0.1.0"), + "npm-@scope/foo-v0.1.0" + ); + } + + #[test] + fn renders_short_template() { + let t = template("v{version}"); + assert_eq!( + t.render(PackageKind::Packagist, "ignored", "1.0.0"), + "v1.0.0" + ); + } + + #[test] + fn matches_default_template() { + let t = template("{ecosystem}-{package_name}-v{version}"); + let m = t.match_tag("cargo-my-pkg-v1.2.3").unwrap(); + assert_eq!(m.ecosystem, Some(PackageKind::Cargo)); + assert_eq!(m.package_name.as_deref(), Some("my-pkg")); + assert_eq!(m.version.as_deref(), Some("1.2.3")); + } + + #[test] + fn matches_legacy_template_with_v_in_prerelease() { + let t = template("{package_name}-v{version}"); + let m = t.match_tag("my-pkg-v1.2.3-v1").unwrap(); + assert_eq!(m.package_name.as_deref(), Some("my-pkg")); + assert_eq!(m.version.as_deref(), Some("1.2.3-v1")); + } + + #[test] + fn matches_short_template() { + let t = template("v{version}"); + let m = t.match_tag("v1.2.3").unwrap(); + assert_eq!(m.version.as_deref(), Some("1.2.3")); + assert!(m.package_name.is_none()); + assert!(m.ecosystem.is_none()); + } + + #[test] + fn rejects_invalid_semver() { + let t = template("v{version}"); + assert!(t.match_tag("vfoo").is_none()); + assert!(t.match_tag("v1.2").is_none()); + } + + #[test] + fn rejects_unknown_ecosystem() { + let t = template("{ecosystem}-{package_name}-v{version}"); + assert!(t.match_tag("ruby-foo-v1.2.3").is_none()); + } + + #[test] + fn matches_with_unusual_separator() { + let t = template("{ecosystem}/{package_name}@{version}"); + let m = t.match_tag("npm/foo@1.0.0").unwrap(); + assert_eq!(m.ecosystem, Some(PackageKind::Npm)); + assert_eq!(m.package_name.as_deref(), Some("foo")); + assert_eq!(m.version.as_deref(), Some("1.0.0")); + } + + #[test] + fn preserves_non_ascii_literal_segments() { + let t = template("{package_name}—v{version}"); + assert_eq!(t.render(PackageKind::Cargo, "café", "1.2.3"), "café—v1.2.3"); + let m = t.match_tag("café—v1.2.3").unwrap(); + assert_eq!(m.package_name.as_deref(), Some("café")); + assert_eq!(m.version.as_deref(), Some("1.2.3")); + } + + #[test] + fn rejects_empty_capture() { + let t = template("{package_name}-v{version}"); + assert!(t.match_tag("-v1.2.3").is_none()); + } + + #[test] + fn rejects_template_without_version_placeholder() { + assert!(TagTemplate::parse("{package_name}-stable").is_err()); + } + + #[test] + fn template_can_omit_optional_placeholders() { + let t = template("release-{version}"); + assert_eq!( + t.render(PackageKind::Cargo, "anything", "9.9.9"), + "release-9.9.9" + ); + let m = t.match_tag("release-9.9.9").unwrap(); + assert_eq!(m.version.as_deref(), Some("9.9.9")); + } +} diff --git a/crates/sampo-github-action/src/main.rs b/crates/sampo-github-action/src/main.rs index a2c6f65..2ca1f8a 100644 --- a/crates/sampo-github-action/src/main.rs +++ b/crates/sampo-github-action/src/main.rs @@ -919,25 +919,13 @@ fn build_release_body_from_changelog(workspace: &Path, tag: &str) -> Option) -> Option<(String, String)> { - if let Some(cfg) = config { - cfg.parse_tag(tag) - } else { - // Fallback to standard format only when no config is available - parse_tag(tag) - } -} - -/// Parse tags in standard format `-v`. -fn parse_tag(tag: &str) -> Option<(String, String)> { - let idx = tag.rfind("-v")?; - let (name, ver) = tag.split_at(idx); - let version = ver.trim_start_matches("-v").to_string(); - if name.is_empty() || version.is_empty() { - return None; + match config { + Some(cfg) => cfg.parse_tag(tag), + None => SampoConfig::default().parse_tag(tag), } - Some((name.to_string(), version)) } fn tag_is_prerelease_with_config(tag: &str, config: Option<&SampoConfig>) -> bool { @@ -1380,21 +1368,23 @@ mod tests { } #[test] - fn test_parse_tag() { + fn parse_tag_with_default_template() { assert_eq!( - parse_tag("my-crate-v1.2.3"), - Some(("my-crate".into(), "1.2.3".into())) + parse_tag_with_config("my-crate-v1.2.3", None), + Some(("my-crate".to_string(), "1.2.3".to_string())) ); assert_eq!( - parse_tag("sampo-v0.9.0"), - Some(("sampo".into(), "0.9.0".into())) + parse_tag_with_config("sampo-v0.9.0", None), + Some(("sampo".to_string(), "0.9.0".to_string())) ); assert_eq!( - parse_tag("sampo-github-action-v0.8.2"), - Some(("sampo-github-action".into(), "0.8.2".into())) + parse_tag_with_config("sampo-github-action-v0.8.2", None), + Some(("sampo-github-action".to_string(), "0.8.2".to_string())) ); - assert_eq!(parse_tag("nope"), None); - assert_eq!(parse_tag("-v1.0.0"), None); + assert_eq!(parse_tag_with_config("nope", None), None); + // Short tags require explicit `git.short_tags` config; under the + // default template a bare `v…` doesn't match the `{package_name}` slot. + assert_eq!(parse_tag_with_config("v1.2.3", None), None); } #[test] @@ -1420,7 +1410,6 @@ mod tests { Some(("my-package".to_string(), "1.2.3-alpha.1".to_string())) ); - // Standard format still works for other packages assert_eq!( parse_tag_with_config("other-package-v2.0.0", Some(&config)), Some(("other-package".to_string(), "2.0.0".to_string())) @@ -1431,13 +1420,27 @@ mod tests { } #[test] - fn parse_tag_with_config_falls_back_without_config() { + fn parse_tag_honours_custom_tag_format() { + use std::fs; + let temp = tempfile::tempdir().unwrap(); + fs::create_dir_all(temp.path().join(".sampo")).unwrap(); + fs::write( + temp.path().join(".sampo/config.toml"), + "[git]\ntag_format = \"{ecosystem}-{package_name}-v{version}\"\n", + ) + .unwrap(); + + let config = SampoConfig::load(temp.path()).unwrap(); assert_eq!( - parse_tag_with_config("my-crate-v1.2.3", None), + parse_tag_with_config("cargo-my-crate-v1.2.3", Some(&config)), Some(("my-crate".to_string(), "1.2.3".to_string())) ); - - assert_eq!(parse_tag_with_config("v1.2.3", None), None); + // Default-shape tags no longer match once the user opts into the + // disambiguating template. + assert_eq!( + parse_tag_with_config("my-crate-v1.2.3", Some(&config)), + None + ); } #[test] diff --git a/crates/sampo/README.md b/crates/sampo/README.md index 0ee1ede..46f6001 100644 --- a/crates/sampo/README.md +++ b/crates/sampo/README.md @@ -162,10 +162,21 @@ linked = [["cargo/pkg-e", "cargo/pkg-f"], ["cargo/pkg-g", "cargo/pkg-h"]] > [!TIP] > At runtime you can override the detected branch with the `SAMPO_RELEASE_BRANCH` environment variable, which is useful for local testing or custom CI setups. -`short_tags`: Optional package name that should use short tag format (`v{version}`) instead of the standard format (`{package}-v{version}`). +`tag_format`: Template used for git tags created by `sampo publish` (default: `"{package_name}-v{version}"`). Supported placeholders: + +- `{ecosystem}` — `cargo`, `npm`, `hex`, `pypi`, or `packagist`. +- `{package_name}` — the package's local name. +- `{version}` — the released version (required). > [!IMPORTANT] -> For publishable PHP packages, this option is required as Packagist only detects short version tags. Sadly, the Packagist adapter does not support monorepos with multiple publishable PHP packages, as `vX.Y.Z` tags cannot distinguish between packages. +> If your workspace ships packages with the same name across multiple ecosystems (e.g. a Cargo crate and an npm package both called `foo`), `sampo publish` warns you that the default template will eventually collide and suggests `tag_format = "{ecosystem}-{package_name}-v{version}"`. If two packages would already produce the same tag for the release in flight, the publish errors out with the same hint. + +`short_tags`: Optional package name that should use the short tag format (see `short_tags_format`) instead of `tag_format`. + +> [!IMPORTANT] +> For publishable PHP packages, the short option is required as Packagist only detects short version tags. Sadly, the Packagist adapter does not support monorepos with multiple publishable PHP packages, since their short tag format cannot distinguish between them. + +`short_tags_format`: Template applied to the package selected by `short_tags` (default: `"v{version}"`). Same placeholders as `tag_format`. ### `[github]` section