Skip to content

Commit 26aed7e

Browse files
authored
fix(core/enrichment): credit changeset original creator (#174)
1 parent 1dcc188 commit 26aed7e

8 files changed

Lines changed: 225 additions & 15 deletions

File tree

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
---
2+
cargo/sampo: patch
3+
cargo/sampo-core: patch
4+
cargo/sampo-github-action: patch
5+
---
6+
7+
Changelog entries now correctly credit the original changeset author, even if the file was later edited by someone else.

crates/sampo-core/src/adapters/hex.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
use crate::errors::{Result, SampoError, WorkspaceError};
22
use crate::types::PackageInfo;
3-
use reqwest::blocking::Client;
43
use reqwest::StatusCode;
4+
use reqwest::blocking::Client;
55
use std::collections::BTreeMap;
66
use std::path::{Path, PathBuf};
77
use std::sync::{Mutex, OnceLock};

crates/sampo-core/src/adapters/npm.rs

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,8 @@ use crate::errors::{Result, SampoError, WorkspaceError};
22
use crate::types::{PackageInfo, PackageKind};
33
use reqwest::StatusCode;
44
use serde::Deserialize;
5-
use serde_json::value::RawValue;
65
use serde_json::Value as JsonValue;
6+
use serde_json::value::RawValue;
77
use std::collections::{BTreeMap, BTreeSet, HashMap};
88
use std::fs;
99
use std::path::{Component, Path, PathBuf};
@@ -63,11 +63,7 @@ impl NpmAdapter {
6363
pub(super) fn is_publishable(&self, manifest_path: &Path) -> Result<bool> {
6464
let manifest = load_package_json(manifest_path)?;
6565
let info = parse_manifest_info(manifest_path, &manifest)?;
66-
if info.private {
67-
Ok(false)
68-
} else {
69-
Ok(true)
70-
}
66+
if info.private { Ok(false) } else { Ok(true) }
7167
}
7268

7369
pub(super) fn version_exists(

crates/sampo-core/src/adapters/pypi.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
use crate::errors::{Result, SampoError, WorkspaceError};
22
use crate::types::PackageInfo;
3-
use reqwest::blocking::Client;
43
use reqwest::StatusCode;
4+
use reqwest::blocking::Client;
55
use serde_json::Value as JsonValue;
66
use std::collections::BTreeMap;
77
use std::path::{Path, PathBuf};

crates/sampo-core/src/adapters/pypi/pip/pip_tests.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,10 @@ dependencies = []
7676
let packages = discover(root).unwrap();
7777
assert_eq!(packages.len(), 3);
7878

79-
let root_pkg = packages.iter().find(|p| p.name == "workspace-root").unwrap();
79+
let root_pkg = packages
80+
.iter()
81+
.find(|p| p.name == "workspace-root")
82+
.unwrap();
8083
assert!(root_pkg.internal_deps.contains("pypi/pkg-a"));
8184

8285
let pkg_a = packages.iter().find(|p| p.name == "pkg-a").unwrap();

crates/sampo-core/src/enrichment.rs

Lines changed: 201 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ pub fn get_commit_hash_for_path(repo_root: &Path, file_path: &Path) -> Option<St
4848
.current_dir(repo_root)
4949
.args([
5050
"log",
51+
"--diff-filter=A",
5152
"-1",
5253
"--format=%H",
5354
"--",
@@ -546,6 +547,206 @@ mod tests {
546547
);
547548
}
548549

550+
#[test]
551+
fn get_commit_hash_returns_creation_commit_not_modification() {
552+
use std::fs;
553+
use tempfile::TempDir;
554+
555+
let temp_dir = TempDir::new().unwrap();
556+
let repo_path = temp_dir.path();
557+
558+
// Initialize a git repo
559+
std::process::Command::new("git")
560+
.arg("init")
561+
.current_dir(repo_path)
562+
.output()
563+
.unwrap();
564+
565+
// Configure git user for initial commit
566+
std::process::Command::new("git")
567+
.args(["config", "user.name", "Original Author"])
568+
.current_dir(repo_path)
569+
.output()
570+
.unwrap();
571+
572+
std::process::Command::new("git")
573+
.args(["config", "user.email", "[email protected]"])
574+
.current_dir(repo_path)
575+
.output()
576+
.unwrap();
577+
578+
// Create and commit file
579+
let test_file = repo_path.join("changeset.md");
580+
fs::write(&test_file, "initial content").unwrap();
581+
582+
std::process::Command::new("git")
583+
.args(["add", "changeset.md"])
584+
.current_dir(repo_path)
585+
.output()
586+
.unwrap();
587+
588+
std::process::Command::new("git")
589+
.args(["commit", "-m", "create changeset"])
590+
.current_dir(repo_path)
591+
.output()
592+
.unwrap();
593+
594+
// Record the creation commit hash
595+
let creation_commit =
596+
get_commit_hash_for_path(repo_path, &test_file).expect("Should find creation commit");
597+
598+
// Now modify the file with a different author
599+
std::process::Command::new("git")
600+
.args(["config", "user.name", "Editor"])
601+
.current_dir(repo_path)
602+
.output()
603+
.unwrap();
604+
605+
std::process::Command::new("git")
606+
.args(["config", "user.email", "[email protected]"])
607+
.current_dir(repo_path)
608+
.output()
609+
.unwrap();
610+
611+
fs::write(&test_file, "modified content").unwrap();
612+
613+
std::process::Command::new("git")
614+
.args(["add", "changeset.md"])
615+
.current_dir(repo_path)
616+
.output()
617+
.unwrap();
618+
619+
std::process::Command::new("git")
620+
.args(["commit", "-m", "edit changeset"])
621+
.current_dir(repo_path)
622+
.output()
623+
.unwrap();
624+
625+
// After modification, get_commit_hash_for_path should still return the creation commit
626+
let hash_after_modification = get_commit_hash_for_path(repo_path, &test_file)
627+
.expect("Should find commit after modification");
628+
629+
assert_eq!(
630+
hash_after_modification, creation_commit,
631+
"Should return creation commit, not modification commit"
632+
);
633+
634+
// Verify we can get the correct author from the creation commit
635+
let commit_info =
636+
get_commit_info_for_hash(repo_path, &creation_commit).expect("Should get commit info");
637+
assert_eq!(
638+
commit_info.author_name, "Original Author",
639+
"Should credit original author, not editor"
640+
);
641+
}
642+
643+
#[test]
644+
fn get_commit_hash_returns_latest_creation_after_delete_recreate() {
645+
use std::fs;
646+
use tempfile::TempDir;
647+
648+
let temp_dir = TempDir::new().unwrap();
649+
let repo_path = temp_dir.path();
650+
651+
// Initialize a git repo
652+
std::process::Command::new("git")
653+
.arg("init")
654+
.current_dir(repo_path)
655+
.output()
656+
.unwrap();
657+
658+
// Configure git user for first creation
659+
std::process::Command::new("git")
660+
.args(["config", "user.name", "First Author"])
661+
.current_dir(repo_path)
662+
.output()
663+
.unwrap();
664+
665+
std::process::Command::new("git")
666+
.args(["config", "user.email", "[email protected]"])
667+
.current_dir(repo_path)
668+
.output()
669+
.unwrap();
670+
671+
// Create and commit file
672+
let test_file = repo_path.join("changeset.md");
673+
fs::write(&test_file, "first version").unwrap();
674+
675+
std::process::Command::new("git")
676+
.args(["add", "changeset.md"])
677+
.current_dir(repo_path)
678+
.output()
679+
.unwrap();
680+
681+
std::process::Command::new("git")
682+
.args(["commit", "-m", "first creation"])
683+
.current_dir(repo_path)
684+
.output()
685+
.unwrap();
686+
687+
let first_creation_commit = get_commit_hash_for_path(repo_path, &test_file)
688+
.expect("Should find first creation commit");
689+
690+
// Delete the file
691+
fs::remove_file(&test_file).unwrap();
692+
693+
std::process::Command::new("git")
694+
.args(["add", "changeset.md"])
695+
.current_dir(repo_path)
696+
.output()
697+
.unwrap();
698+
699+
std::process::Command::new("git")
700+
.args(["commit", "-m", "delete changeset"])
701+
.current_dir(repo_path)
702+
.output()
703+
.unwrap();
704+
705+
// Re-create the file with a different author
706+
std::process::Command::new("git")
707+
.args(["config", "user.name", "Second Author"])
708+
.current_dir(repo_path)
709+
.output()
710+
.unwrap();
711+
712+
std::process::Command::new("git")
713+
.args(["config", "user.email", "[email protected]"])
714+
.current_dir(repo_path)
715+
.output()
716+
.unwrap();
717+
718+
fs::write(&test_file, "second version").unwrap();
719+
720+
std::process::Command::new("git")
721+
.args(["add", "changeset.md"])
722+
.current_dir(repo_path)
723+
.output()
724+
.unwrap();
725+
726+
std::process::Command::new("git")
727+
.args(["commit", "-m", "recreate changeset"])
728+
.current_dir(repo_path)
729+
.output()
730+
.unwrap();
731+
732+
// Should return the latest creation commit (second author's)
733+
let latest_creation_commit = get_commit_hash_for_path(repo_path, &test_file)
734+
.expect("Should find latest creation commit");
735+
736+
assert_ne!(
737+
latest_creation_commit, first_creation_commit,
738+
"Should return different commit after delete/recreate"
739+
);
740+
741+
// Verify we credit the second author (current file creator)
742+
let commit_info = get_commit_info_for_hash(repo_path, &latest_creation_commit)
743+
.expect("Should get commit info");
744+
assert_eq!(
745+
commit_info.author_name, "Second Author",
746+
"Should credit the author who re-created the file"
747+
);
748+
}
749+
549750
#[tokio::test]
550751
async fn test_github_api_get_with_invalid_token() {
551752
// Test with invalid token should return None (graceful failure)

crates/sampo-core/src/lib.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,20 +18,20 @@ pub const USER_AGENT: &str = concat!("sampo-core/", env!("CARGO_PKG_VERSION"));
1818
// Re-export commonly used items
1919
pub use adapters::ManifestMetadata;
2020
pub use changeset::{
21-
load_changesets, parse_changeset, render_changeset_markdown,
22-
render_changeset_markdown_with_tags, ChangesetInfo,
21+
ChangesetInfo, load_changesets, parse_changeset, render_changeset_markdown,
22+
render_changeset_markdown_with_tags,
2323
};
2424
pub use config::Config;
2525
pub use enrichment::{
26-
detect_github_repo_slug, detect_github_repo_slug_with_config, enrich_changeset_message,
27-
get_commit_hash_for_path, CommitInfo, GitHubUserInfo,
26+
CommitInfo, GitHubUserInfo, detect_github_repo_slug, detect_github_repo_slug_with_config,
27+
enrich_changeset_message, get_commit_hash_for_path,
2828
};
2929
pub use errors::{Result, SampoError, WorkspaceError};
3030
pub use filters::{filter_members, list_visible_packages, should_ignore_package, wildcard_match};
3131
pub use git::current_branch;
3232
pub use markdown::format_markdown_list_item;
3333
pub use prerelease::{
34-
enter_prerelease, exit_prerelease, restore_preserved_changesets, VersionChange,
34+
VersionChange, enter_prerelease, exit_prerelease, restore_preserved_changesets,
3535
};
3636
pub use publish::{run_publish, tag_published_crate, topo_order};
3737
pub use release::{

crates/sampo/README.md

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ At the root of each published package, a human-readable file listing all changes
8080
... previous entries ...
8181
```
8282

83-
Sampo generates changelog entries from consumed changesets and enriches them with commit hash links and author acknowledgments (can be disabled in config).
83+
Sampo generates changelog entries from consumed changesets and enriches them with commit hash links and author acknowledgments (can be disabled in config).
8484

8585
Any intro content or custom main header before the first `##` section is preserved. You can also manually edit the previously released entries, and Sampo will keep them intact.
8686

@@ -164,6 +164,9 @@ linked = [["cargo/pkg-e", "cargo/pkg-f"], ["cargo/pkg-g", "cargo/pkg-h"]]
164164

165165
### `[changelog]` section
166166

167+
> [!WARNING]
168+
> Commit hash links and author acknowledgments won't work in shallow clones or CI environments where the full git history is not available.
169+
167170
`show_commit_hash`: Whether to include commit hash links in changelog entries (default: `true`). When enabled, changelog entries include clickable commit hash links that point to the commit on GitHub.
168171

169172
`show_acknowledgments`: Whether to include author acknowledgments in changelog entries (default: `true`). When enabled, changelog entries include author acknowledgments with special messages for first-time contributors.

0 commit comments

Comments
 (0)