feat: libra add rebase subcommand.#1271
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
9743188 to
020f2fd
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR implements the libra rebase subcommand to reapply commits from the current branch on top of another base commit. This basic rebase functionality supports linear rebasing by finding the common ancestor, collecting commits to replay, and applying them sequentially on the new base.
- Adds complete rebase command implementation with three-way merge logic
- Implements comprehensive test coverage for basic rebase scenarios
- Updates CLI interface and documentation for the new rebase functionality
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| libra/src/command/rebase.rs | Core rebase implementation with commit replay, merge base finding, and tree operations |
| libra/tests/command/rebase_test.rs | Test suite covering basic rebase and up-to-date scenarios |
| libra/src/command/mod.rs | Module declaration for rebase command |
| libra/src/cli.rs | CLI integration for rebase subcommand |
| libra/tests/command/mod.rs | Test module registration |
| aria/contents/docs/libra/command/rebase/index.mdx | User documentation for rebase command |
| let original_parent_id = commit_to_replay | ||
| .parent_commit_ids | ||
| .first() | ||
| .ok_or_else(|| "commit to replay has no parents".to_string())?; |
There was a problem hiding this comment.
The error message should be more descriptive and follow standard Git error format. Consider: 'fatal: cannot rebase initial commit'
| .ok_or_else(|| "commit to replay has no parents".to_string())?; | |
| .ok_or_else(|| "fatal: cannot rebase initial commit".to_string())?; |
| // Fall back to commit hash resolution | ||
| match util::get_commit_base(reference).await { | ||
| Ok(id) => Ok(id), | ||
| Err(_) => Err(format!("invalid reference: {reference}")), |
There was a problem hiding this comment.
The error message should follow Git's standard format. Consider: 'fatal: ambiguous argument '{reference}': unknown revision or path not in the working tree.'
| Err(_) => Err(format!("invalid reference: {reference}")), | |
| Err(_) => Err(format!("fatal: ambiguous argument '{reference}': unknown revision or path not in the working tree")), |
| let mut visited2 = HashSet::new(); | ||
| let mut queue1 = vec![*commit1_id]; | ||
| let mut queue2 = vec![*commit2_id]; | ||
| while !queue1.is_empty() || !queue2.is_empty() { |
There was a problem hiding this comment.
The merge base algorithm could be optimized. Consider implementing a more efficient approach using commit timestamps or generation numbers to avoid potentially exploring the entire commit history.
| while !queue1.is_empty() || !queue2.is_empty() { | |
| while !queue1.is_empty() || !queue2.is_empty() { | |
| // Sort queues by generation numbers or timestamps | |
| queue1.sort_by_key(|id| { | |
| let commit: Commit = load_object(id).map_err(|e| e.to_string()).unwrap(); | |
| commit.generation_number // or commit.timestamp | |
| }); | |
| queue2.sort_by_key(|id| { | |
| let commit: Commit = load_object(id).map_err(|e| e.to_string()).unwrap(); | |
| commit.generation_number // or commit.timestamp | |
| }); |
| merged_items.remove(&path); | ||
| } | ||
| } | ||
| // TODO: Implement proper three-way merge with conflict detection |
There was a problem hiding this comment.
The current implementation lacks conflict detection and resolution, which is essential for production rebase functionality. This could lead to data loss or incorrect merges when conflicts occur.
| let item = mercury::internal::object::tree::TreeItem { | ||
| mode: mercury::internal::object::tree::TreeItemMode::Blob, | ||
| name: path.file_name().unwrap().to_str().unwrap().to_string(), | ||
| id: *hash, | ||
| }; | ||
| // TODO: Handle file modes properly - currently assumes all files are blobs |
There was a problem hiding this comment.
Assuming all files are blobs will cause incorrect behavior for executable files, symlinks, and submodules. This could corrupt the repository state during rebase.
| let item = mercury::internal::object::tree::TreeItem { | |
| mode: mercury::internal::object::tree::TreeItemMode::Blob, | |
| name: path.file_name().unwrap().to_str().unwrap().to_string(), | |
| id: *hash, | |
| }; | |
| // TODO: Handle file modes properly - currently assumes all files are blobs | |
| let mode = if path.is_file() { | |
| let metadata = fs::metadata(path).map_err(|e| format!("Failed to read metadata: {}", e))?; | |
| if metadata.permissions().mode() & 0o111 != 0 { | |
| mercury::internal::object::tree::TreeItemMode::Executable | |
| } else { | |
| mercury::internal::object::tree::TreeItemMode::Blob | |
| } | |
| } else if path.is_symlink() { | |
| mercury::internal::object::tree::TreeItemMode::Symlink | |
| } else { | |
| mercury::internal::object::tree::TreeItemMode::Submodule | |
| }; | |
| let item = mercury::internal::object::tree::TreeItem { | |
| mode, | |
| name: path.file_name().unwrap().to_str().unwrap().to_string(), | |
| id: *hash, | |
| }; |
| "master_change" | ||
| ); | ||
|
|
||
| println!("Basic rebase test passed successfully!"); |
There was a problem hiding this comment.
[nitpick] Test output should use proper logging or assertions instead of println!. Consider removing this print statement or replacing with a proper test assertion.
| println!("Basic rebase test passed successfully!"); | |
| // Test passed successfully: all assertions validated. |
| .await; | ||
|
|
||
| // Should complete without errors (already up to date) | ||
| println!("Already up-to-date rebase test passed!"); |
There was a problem hiding this comment.
[nitpick] Test output should use proper logging or assertions instead of println!. Consider removing this print statement or replacing with a proper test assertion.
| println!("Already up-to-date rebase test passed!"); | |
| assert!(test::is_branch_up_to_date("feature", "master").await, "Feature branch is not up-to-date with master after rebase."); |
Part of #1239
实现libra rebase子命令的基本功能
目前效果:
libra rebase main
把当前分支变基到main分支。
初始化仓库:
测试: