From 97ca1696979f0efe3dc0d6701796f3a37863a235 Mon Sep 17 00:00:00 2001 From: Brendan Ryan Date: Tue, 17 Feb 2026 16:27:17 -0800 Subject: [PATCH 1/2] fix: improve error visibility across publish pipeline --- Cargo.lock | 2 +- action.yml | 13 ++++--- src/cli/publish.rs | 10 +++-- src/ecosystems/python.rs | 81 +++++++++++++++++++++++++++++++++++++--- src/ecosystems/rust.rs | 69 ++++++++++++++++++++++++++++++++-- 5 files changed, 157 insertions(+), 18 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index af12f969..1d78c293 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -156,7 +156,7 @@ checksum = "9330f8b2ff13f34540b44e946ef35111825727b38d33286ef986142615121801" [[package]] name = "changelogs" -version = "0.6.0" +version = "0.6.1" dependencies = [ "anyhow", "cargo_metadata", diff --git a/action.yml b/action.yml index 908bddb5..5e535ad5 100644 --- a/action.yml +++ b/action.yml @@ -106,8 +106,12 @@ runs: if [ -n "${{ inputs.ecosystem }}" ]; then ECOSYSTEM_FLAG="--ecosystem ${{ inputs.ecosystem }}" fi - output=$(changelogs $ECOSYSTEM_FLAG version 2>&1) + output=$(changelogs $ECOSYSTEM_FLAG version 2>&1) && version_exit=0 || version_exit=$? echo "$output" + if [ $version_exit -ne 0 ]; then + echo "::error::changelogs version failed with exit code $version_exit" + exit $version_exit + fi # Extract versions from output (e.g., "changelogs 0.0.1 → 0.1.0" -> "changelogs@0.1.0") all_versions=$(echo "$output" | grep -oE '[a-zA-Z0-9_-]+ [0-9]+\.[0-9]+\.[0-9]+ → [0-9]+\.[0-9]+\.[0-9]+' | sed 's/ .* → /@/') @@ -195,8 +199,7 @@ runs: if [ -n "${{ inputs.ecosystem }}" ]; then ECOSYSTEM_FLAG="--ecosystem ${{ inputs.ecosystem }}" fi - output=$(changelogs $ECOSYSTEM_FLAG publish 2>&1) - publish_exit=$? + output=$(changelogs $ECOSYSTEM_FLAG publish 2>&1) && publish_exit=0 || publish_exit=$? echo "$output" # Parse published packages from output (✓ = published, ⊘ = skipped/tags-only) @@ -209,9 +212,9 @@ runs: echo "published=false" >> $GITHUB_OUTPUT echo "publishedPackages=[]" >> $GITHUB_OUTPUT fi - + if [ $publish_exit -ne 0 ]; then - echo "::error::Package publishing failed with exit code $publish_exit" + echo "::error::changelogs publish failed with exit code $publish_exit" exit $publish_exit fi diff --git a/src/cli/publish.rs b/src/cli/publish.rs index 42e6fc97..f5df4c51 100644 --- a/src/cli/publish.rs +++ b/src/cli/publish.rs @@ -85,12 +85,16 @@ fn create_git_tags(workspace: &Workspace, packages: &[&Package]) -> Result<()> { for pkg in packages { let tag = workspace.tag_name(pkg); - let status = Command::new("git") + let output = Command::new("git") .args(["tag", "-a", &tag, "-m", &format!("Release {}", tag)]) - .status()?; + .output() + .map_err(|e| anyhow::anyhow!("failed to run 'git tag': {}", e))?; - if status.success() { + if output.status.success() { println!("Created git tag: {}", tag); + } else { + let stderr = String::from_utf8_lossy(&output.stderr); + eprintln!("Failed to create git tag {}: {}", tag, stderr.trim()); } } diff --git a/src/ecosystems/python.rs b/src/ecosystems/python.rs index 16280104..aa0e30ed 100644 --- a/src/ecosystems/python.rs +++ b/src/ecosystems/python.rs @@ -193,13 +193,19 @@ impl EcosystemAdapter for PythonAdapter { let build_output = Command::new("python") .args(["-m", "build"]) .current_dir(&pkg_path) - .output()?; + .output() + .map_err(|e| { + Error::PublishFailed(format!("failed to run 'python -m build': {}", e)) + })?; if !build_output.status.success() { + let stdout = String::from_utf8_lossy(&build_output.stdout); let stderr = String::from_utf8_lossy(&build_output.stderr); return Err(Error::PublishFailed(format!( - "python -m build failed: {}", - stderr + "python -m build failed (exit code {}):\nstdout: {}\nstderr: {}", + build_output.status, + stdout.trim(), + stderr.trim(), ))); } @@ -241,20 +247,25 @@ impl EcosystemAdapter for PythonAdapter { } cmd.current_dir(&pkg_path); - let upload_output = cmd.output()?; + let upload_output = cmd.output().map_err(|e| { + Error::PublishFailed(format!("failed to run 'twine upload': {}", e)) + })?; if upload_output.status.success() { return Ok(PublishResult::Success); } + let stdout = String::from_utf8_lossy(&upload_output.stdout); let stderr = String::from_utf8_lossy(&upload_output.stderr); if stderr.contains("already exists") || stderr.contains("File already exists") { return Ok(PublishResult::Success); } Err(Error::PublishFailed(format!( - "twine upload failed: {}", - stderr + "twine upload failed (exit code {}):\nstdout: {}\nstderr: {}", + upload_output.status, + stdout.trim(), + stderr.trim(), ))) } } @@ -890,4 +901,62 @@ version = "2.0.0" assert_eq!(packages[0].name, "pep621-pkg"); assert_eq!(packages[0].version.to_string(), "1.0.0"); } + + #[test] + fn publish_dry_run_returns_success() { + let tmp = TempDir::new().unwrap(); + create_pyproject( + tmp.path(), + r#" +[project] +name = "test-pkg" +version = "1.0.0" +"#, + ); + let pkg = &PythonAdapter::discover(tmp.path()).unwrap()[0]; + let result = PythonAdapter::publish(pkg, true, None).unwrap(); + assert_eq!(result, PublishResult::Success); + } + + #[test] + fn publish_skipped_without_tokens() { + let tmp = TempDir::new().unwrap(); + create_pyproject( + tmp.path(), + r#" +[project] +name = "test-pkg" +version = "1.0.0" +"#, + ); + let pkg = &PythonAdapter::discover(tmp.path()).unwrap()[0]; + // Ensure tokens are not set + // SAFETY: test-only, no concurrent access to these env vars + unsafe { + std::env::remove_var("TWINE_PASSWORD"); + std::env::remove_var("TWINE_USERNAME"); + } + let result = PythonAdapter::publish(pkg, false, None).unwrap(); + assert_eq!(result, PublishResult::Skipped); + } + + #[test] + fn publish_failed_error_includes_context() { + let err = Error::PublishFailed("python -m build failed (exit code 1):\nstdout: \nstderr: No module named build".to_string()); + let msg = err.to_string(); + assert!(msg.contains("publish failed")); + assert!(msg.contains("exit code 1")); + assert!(msg.contains("No module named build")); + } + + #[test] + fn publish_failed_command_not_found_error() { + let err = Error::PublishFailed( + "failed to run 'python -m build': No such file or directory (os error 2)".to_string(), + ); + let msg = err.to_string(); + assert!(msg.contains("publish failed")); + assert!(msg.contains("failed to run")); + assert!(msg.contains("No such file or directory")); + } } diff --git a/src/ecosystems/rust.rs b/src/ecosystems/rust.rs index 479990cb..10610290 100644 --- a/src/ecosystems/rust.rs +++ b/src/ecosystems/rust.rs @@ -145,20 +145,25 @@ impl EcosystemAdapter for RustAdapter { cmd.env("CARGO_REGISTRY_DEFAULT", reg); } - let output = cmd.output()?; + let output = cmd.output().map_err(|e| { + crate::error::Error::PublishFailed(format!("failed to run 'cargo publish': {}", e)) + })?; if output.status.success() { return Ok(PublishResult::Success); } + let stdout = String::from_utf8_lossy(&output.stdout); let stderr = String::from_utf8_lossy(&output.stderr); if stderr.contains("already uploaded") || stderr.contains("already exists") { return Ok(PublishResult::Success); } Err(crate::error::Error::PublishFailed(format!( - "cargo publish failed: {}", - stderr + "cargo publish failed (exit code {}):\nstdout: {}\nstderr: {}", + output.status, + stdout.trim(), + stderr.trim(), ))) } } @@ -372,4 +377,62 @@ my-dep = { version = \"1.0.0\" }\n"; let updated = std::fs::read_to_string(&manifest).unwrap(); assert!(updated.contains("version = \"4.0.0\"")); } + + #[test] + fn publish_dry_run_returns_success() { + let dir = TempDir::new().unwrap(); + let manifest = dir.path().join("Cargo.toml"); + std::fs::write( + &manifest, + "[package]\nname = \"test\"\nversion = \"1.0.0\"\n", + ) + .unwrap(); + + let pkg = Package { + name: "test".to_string(), + version: Version::new(1, 0, 0), + path: dir.path().to_path_buf(), + manifest_path: manifest, + dependencies: vec![], + }; + + let result = RustAdapter::publish(&pkg, true, None).unwrap(); + assert_eq!(result, PublishResult::Success); + } + + #[test] + fn publish_skipped_without_token() { + let dir = TempDir::new().unwrap(); + let manifest = dir.path().join("Cargo.toml"); + std::fs::write( + &manifest, + "[package]\nname = \"test\"\nversion = \"1.0.0\"\n", + ) + .unwrap(); + + let pkg = Package { + name: "test".to_string(), + version: Version::new(1, 0, 0), + path: dir.path().to_path_buf(), + manifest_path: manifest, + dependencies: vec![], + }; + + // SAFETY: test-only, no concurrent access to this env var + unsafe { std::env::remove_var("CARGO_REGISTRY_TOKEN"); } + let result = RustAdapter::publish(&pkg, false, None).unwrap(); + assert_eq!(result, PublishResult::Skipped); + } + + #[test] + fn publish_failed_error_includes_context() { + let err = crate::error::Error::PublishFailed( + "cargo publish failed (exit code 101):\nstdout: \nstderr: error: failed to publish" + .to_string(), + ); + let msg = err.to_string(); + assert!(msg.contains("publish failed")); + assert!(msg.contains("exit code 101")); + assert!(msg.contains("failed to publish")); + } } From 7d57a506ad6a3f8e82edd6fbd65bcb886ae20352 Mon Sep 17 00:00:00 2001 From: Brendan Ryan Date: Tue, 17 Feb 2026 16:36:42 -0800 Subject: [PATCH 2/2] fmt --- src/ecosystems/python.rs | 15 ++++++++------- src/ecosystems/rust.rs | 4 +++- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/src/ecosystems/python.rs b/src/ecosystems/python.rs index aa0e30ed..3608ab00 100644 --- a/src/ecosystems/python.rs +++ b/src/ecosystems/python.rs @@ -194,9 +194,7 @@ impl EcosystemAdapter for PythonAdapter { .args(["-m", "build"]) .current_dir(&pkg_path) .output() - .map_err(|e| { - Error::PublishFailed(format!("failed to run 'python -m build': {}", e)) - })?; + .map_err(|e| Error::PublishFailed(format!("failed to run 'python -m build': {}", e)))?; if !build_output.status.success() { let stdout = String::from_utf8_lossy(&build_output.stdout); @@ -247,9 +245,9 @@ impl EcosystemAdapter for PythonAdapter { } cmd.current_dir(&pkg_path); - let upload_output = cmd.output().map_err(|e| { - Error::PublishFailed(format!("failed to run 'twine upload': {}", e)) - })?; + let upload_output = cmd + .output() + .map_err(|e| Error::PublishFailed(format!("failed to run 'twine upload': {}", e)))?; if upload_output.status.success() { return Ok(PublishResult::Success); @@ -942,7 +940,10 @@ version = "1.0.0" #[test] fn publish_failed_error_includes_context() { - let err = Error::PublishFailed("python -m build failed (exit code 1):\nstdout: \nstderr: No module named build".to_string()); + let err = Error::PublishFailed( + "python -m build failed (exit code 1):\nstdout: \nstderr: No module named build" + .to_string(), + ); let msg = err.to_string(); assert!(msg.contains("publish failed")); assert!(msg.contains("exit code 1")); diff --git a/src/ecosystems/rust.rs b/src/ecosystems/rust.rs index 10610290..09579d21 100644 --- a/src/ecosystems/rust.rs +++ b/src/ecosystems/rust.rs @@ -419,7 +419,9 @@ my-dep = { version = \"1.0.0\" }\n"; }; // SAFETY: test-only, no concurrent access to this env var - unsafe { std::env::remove_var("CARGO_REGISTRY_TOKEN"); } + unsafe { + std::env::remove_var("CARGO_REGISTRY_TOKEN"); + } let result = RustAdapter::publish(&pkg, false, None).unwrap(); assert_eq!(result, PublishResult::Skipped); }