Skip to content

fix(git): scope commit staging to dataset #207

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
presempathy-awb wants to merge 1 commit into from
Closed

fix(git): scope commit staging to dataset #207

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 34 additions & 6 deletions src/git/metadata.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,12 @@ pub trait MetadataBackend: Send {

// ── Commit creation ──────────────────────────────────────────────

/// Stage all files under `git_root` and create a tree object.
fn stage_and_write_tree(&self, git_root: &Path) -> Result<gix::ObjectId, GitKvError>;
/// Stage all files under `dataset_dir` and create a tree object.
fn stage_and_write_tree(
&self,
git_root: &Path,
dataset_dir: &Path,
) -> Result<gix::ObjectId, GitKvError>;

/// Create a commit on top of the current HEAD (or as a root commit).
fn write_commit(
Expand Down Expand Up @@ -221,10 +225,34 @@ impl MetadataBackend for GitMetadataBackend {

// ── Commit creation ──────────────────────────────────────────────

fn stage_and_write_tree(&self, git_root: &Path) -> Result<gix::ObjectId, GitKvError> {
fn stage_and_write_tree(
&self,
git_root: &Path,
dataset_dir: &Path,
) -> Result<gix::ObjectId, GitKvError> {
let git_root = git_root
.canonicalize()
.map_err(|e| GitKvError::GitObjectError(format!("Failed to resolve git root: {e}")))?;
let dataset_dir = dataset_dir.canonicalize().map_err(|e| {
GitKvError::GitObjectError(format!("Failed to resolve dataset dir: {e}"))
})?;
let dataset_rel = dataset_dir.strip_prefix(&git_root).map_err(|_| {
GitKvError::GitObjectError(format!(
"Dataset dir '{}' is not under git root '{}'",
dataset_dir.display(),
git_root.display()
))
})?;
if dataset_rel.as_os_str().is_empty() {
return Err(GitKvError::GitObjectError(
"Refusing to stage the repository root as a dataset".to_string(),
));
}

let add_cmd = std::process::Command::new("git")
.args(["add", "-A", "."])
.current_dir(git_root)
.args(["add", "-A", "--"])
.arg(dataset_rel)
.current_dir(&git_root)
.output()
.map_err(|e| GitKvError::GitObjectError(format!("Failed to run git add: {e}")))?;

Expand All @@ -237,7 +265,7 @@ impl MetadataBackend for GitMetadataBackend {

let write_tree_cmd = std::process::Command::new("git")
.args(["write-tree"])
.current_dir(git_root)
.current_dir(&git_root)
.output()
.map_err(|e| {
GitKvError::GitObjectError(format!("Failed to run git write-tree: {e}"))
Expand Down
16 changes: 8 additions & 8 deletions src/git/versioned_store/backends.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -439,7 +439,7 @@ impl<const N: usize> VersionedKvStore<N, GitNodeStorage<N>, GitMetadataBackend>
.or_else(|| Self::find_git_root(dataset_dir))
.ok_or_else(|| GitKvError::GitObjectError("Could not find git root".into()))?;

let tree_id = self.metadata.stage_and_write_tree(&git_root)?;
let tree_id = self.metadata.stage_and_write_tree(&git_root, dataset_dir)?;

// Create merge commit with two parents using gix (bypasses shell hooks)
let now = std::time::SystemTime::now()
Expand Down Expand Up @@ -541,7 +541,7 @@ impl<const N: usize> VersionedKvStore<N, GitNodeStorage<N>, GitMetadataBackend>
) -> Result<Self, GitKvError> {
let path = path.as_ref();

// Refuse to init at git root — `git add -A .` would stage everything.
// Refuse to init at git root: scoped staging would still target the whole repo.
if Self::is_in_git_root(path)? {
return Err(GitKvError::GitObjectError(
"Cannot initialize git-prolly in git root directory. \
Expand Down Expand Up @@ -606,7 +606,7 @@ impl<const N: usize> VersionedKvStore<N, GitNodeStorage<N>, GitMetadataBackend>
) -> Result<Self, GitKvError> {
let path = path.as_ref();

// Refuse to open at git root — `git add -A .` would stage everything.
// Refuse to open at git root: scoped staging would still target the whole repo.
if Self::is_in_git_root(path)? {
return Err(GitKvError::GitObjectError(
"Cannot open git-prolly in git root directory. \
Expand Down Expand Up @@ -876,7 +876,7 @@ impl<const N: usize> VersionedKvStore<N, InMemoryNodeStorage<N>, GitMetadataBack
pub fn init<P: AsRef<Path>>(path: P) -> Result<Self, GitKvError> {
let path = path.as_ref();

// Safety check: prevent initializing at git root to avoid `git add -A .` staging all files
// Safety check: prevent initializing at git root, where scoped staging targets all files.
if Self::is_in_git_root(path)? {
return Err(GitKvError::GitObjectError(
"Cannot initialize in-memory store in git root directory. \
Expand Down Expand Up @@ -971,7 +971,7 @@ impl<const N: usize> VersionedKvStore<N, FileNodeStorage<N>, GitMetadataBackend>
pub fn init<P: AsRef<Path>>(path: P) -> Result<Self, GitKvError> {
let path = path.as_ref();

// Safety check: prevent initializing at git root to avoid `git add -A .` staging all files
// Safety check: prevent initializing at git root, where scoped staging targets all files.
if Self::is_in_git_root(path)? {
return Err(GitKvError::GitObjectError(
"Cannot initialize file store in git root directory. \
Expand Down Expand Up @@ -1037,7 +1037,7 @@ impl<const N: usize> VersionedKvStore<N, FileNodeStorage<N>, GitMetadataBackend>
let path = path.as_ref();
let dataset_dir = path.to_path_buf();

// Safety check: prevent opening at git root to avoid `git add -A .` staging all files
// Safety check: prevent opening at git root, where scoped staging targets all files.
if Self::is_in_git_root(path)? {
return Err(GitKvError::GitObjectError(
"Cannot open file store in git root directory. \
Expand Down Expand Up @@ -1170,7 +1170,7 @@ impl<const N: usize> VersionedKvStore<N, RocksDBNodeStorage<N>, GitMetadataBacke
pub fn init<P: AsRef<Path>>(path: P) -> Result<Self, GitKvError> {
let path = path.as_ref();

// Safety check: prevent initializing at git root to avoid `git add -A .` staging all files
// Safety check: prevent initializing at git root, where scoped staging targets all files.
if Self::is_in_git_root(path)? {
return Err(GitKvError::GitObjectError(
"Cannot initialize RocksDB store in git root directory. \
Expand Down Expand Up @@ -1235,7 +1235,7 @@ impl<const N: usize> VersionedKvStore<N, RocksDBNodeStorage<N>, GitMetadataBacke
let path = path.as_ref();
let dataset_dir = path.to_path_buf();

// Safety check: prevent opening at git root to avoid `git add -A .` staging all files
// Safety check: prevent opening at git root, where scoped staging targets all files.
if Self::is_in_git_root(path)? {
return Err(GitKvError::GitObjectError(
"Cannot open RocksDB store in git root directory. \
Expand Down
4 changes: 2 additions & 2 deletions src/git/versioned_store/core.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ where

/// Check if the given path is the git repository root directory
/// This is used to prevent initializing a dataset at the git root,
/// which could cause `git add -A .` to stage unrelated files.
/// where scoped staging would still target the whole repository.
pub(super) fn is_in_git_root<P: AsRef<Path>>(path: P) -> Result<bool, GitKvError> {
let path = path.as_ref();

Expand Down Expand Up @@ -301,7 +301,7 @@ where
.ok_or_else(|| GitKvError::GitObjectError("Could not find git root".into()))?;

// Stage and write tree via metadata backend
let tree_id = self.metadata.stage_and_write_tree(&git_root)?;
let tree_id = self.metadata.stage_and_write_tree(&git_root, dataset_dir)?;

// Create commit via metadata backend
let commit_id = self.metadata.write_commit(tree_id, message)?;
Expand Down
39 changes: 39 additions & 0 deletions tests/git_versioning_lifecycle.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,45 @@ fn test_history_tracks_key_across_commits() {
std::mem::forget(_temp);
}

// ---------------------------------------------------------------------------
// Commits stage only the dataset
// ---------------------------------------------------------------------------

#[test]
fn test_commit_does_not_stage_unrelated_repo_root_file() {
let (temp, dataset) = common::setup_repo_and_dataset();
let mut store = GitVersionedKvStore::<32>::init(&dataset).unwrap();

std::fs::write(temp.path().join("unrelated.txt"), "keep me out").unwrap();
store
.insert(b"dataset_k".to_vec(), b"dataset_v".to_vec())
.unwrap();
let commit_id = store.commit("dataset-only commit").unwrap();

let output = std::process::Command::new("git")
.args(["ls-tree", "-r", "--name-only", &commit_id.to_string()])
.current_dir(temp.path())
.output()
.expect("git ls-tree");
assert!(
output.status.success(),
"git ls-tree failed: {}",
String::from_utf8_lossy(&output.stderr)
);

let tree_entries = String::from_utf8_lossy(&output.stdout);
assert!(
tree_entries
.lines()
.any(|line| line.starts_with("dataset/")),
"commit should still include dataset entries, got:\n{tree_entries}"
);
assert!(
!tree_entries.lines().any(|line| line == "unrelated.txt"),
"commit tree should not include unrelated repo-root file, got:\n{tree_entries}"
);
}

// ---------------------------------------------------------------------------
// Get keys at historical ref
// ---------------------------------------------------------------------------
Expand Down