Skip to content

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

Closed
presempathy-awb wants to merge 1 commit into
zhangfengcdt:mainfrom
presempathy-awb:pr-clean/git-scoped-staging
Closed

fix(git): scope commit staging to dataset#207
presempathy-awb wants to merge 1 commit into
zhangfengcdt:mainfrom
presempathy-awb:pr-clean/git-scoped-staging

Conversation

@presempathy-awb

Copy link
Copy Markdown
Contributor

Git Commits Stage Only the Dataset

Bug

GitVersionedKvStore::commit staged the whole repository with a repo-root git add -A . before writing the tree. A dataset commit could accidentally include unrelated files from the parent Git repository.

Impact

git-prolly datasets are intended to live in subdirectories. In that layout, a normal data commit could capture unrelated source files, scratch files, credentials, or other sibling project state. The bug also made tests and local worktree hygiene misleading because committing a key-value change mutated the broader Git index.

Root Cause

Commit creation knew the Git root but did not pass the dataset directory into the staging helper. The helper therefore staged from the repository root instead of the dataset-relative path.

Regression Proof

test_commit_does_not_stage_unrelated_repo_root_file writes unrelated.txt at the repository root, commits a key under the dataset, then inspects the commit tree. On main, the unrelated root file is present in the commit tree. The correct behavior is that only dataset/... entries are included.

Fix

The branch threads dataset_dir into stage_and_write_tree, canonicalizes both paths, refuses to stage the repository root as a dataset, and runs:

git add -A -- <dataset-relative-path>

The same scoped staging path is used by the backend commit flow, so normal commits still include all dataset changes without crossing the dataset boundary.

Verification

Run from a temporary target directory:

CARGO_TARGET_DIR=/tmp/prollytree-target cargo test --features git test_commit_does_not_stage_unrelated_repo_root_file -- --nocapture
CARGO_TARGET_DIR=/tmp/prollytree-target cargo test --features "git sql" --test git_versioning_lifecycle

@zhangfengcdt zhangfengcdt left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed: implementation and tests pass; no public API surface change detected.

@zhangfengcdt

Copy link
Copy Markdown
Owner

Resolved and integrated via #246 after the full CI matrix passed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants