-
Notifications
You must be signed in to change notification settings - Fork 1
feat: add --abort flag to dgr sync #21
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,6 +18,7 @@ use crate::core::{adopt, commit, git, merge, orphan, reparent}; | |
| #[derive(Debug, Clone, Default, PartialEq, Eq)] | ||
| pub struct SyncOptions { | ||
| pub continue_operation: bool, | ||
| pub abort_operation: bool, | ||
| } | ||
|
|
||
| #[derive(Debug, Clone, PartialEq, Eq)] | ||
|
|
@@ -185,6 +186,10 @@ pub fn run_with_reporter<F>(options: &SyncOptions, reporter: &mut F) -> io::Resu | |
| where | ||
| F: FnMut(SyncEvent) -> io::Result<()>, | ||
| { | ||
| if options.abort_operation { | ||
| return abort_sync(); | ||
| } | ||
|
|
||
| if !options.continue_operation { | ||
| return run_full_sync_with_reporter(reporter); | ||
| } | ||
|
|
@@ -307,6 +312,35 @@ where | |
| } | ||
| } | ||
|
|
||
| fn abort_sync() -> io::Result<SyncOutcome> { | ||
| let session = open_initialized("dagger is not initialized; run 'dgr init' first")?; | ||
|
|
||
| if load_operation(&session.paths)?.is_none() { | ||
| return Err(io::Error::other("no paused dgr operation to abort")); | ||
| } | ||
|
|
||
| if git::is_rebase_in_progress(&session.repo) { | ||
| let abort_output = git::abort_rebase()?; | ||
| if !abort_output.status.success() { | ||
| return Ok(SyncOutcome { | ||
|
Comment on lines
+315
to
+325
|
||
| status: abort_output.status, | ||
| completion: None, | ||
| failure_output: Some(abort_output.combined_output()), | ||
| paused: true, | ||
| }); | ||
| } | ||
| } | ||
|
|
||
| clear_operation(&session.paths)?; | ||
|
|
||
| Ok(SyncOutcome { | ||
| status: git::success_status()?, | ||
| completion: None, | ||
| failure_output: None, | ||
| paused: false, | ||
| }) | ||
| } | ||
|
|
||
| fn run_full_sync_with_reporter<F>(reporter: &mut F) -> io::Result<SyncOutcome> | ||
| where | ||
| F: FnMut(SyncEvent) -> io::Result<()>, | ||
|
|
@@ -1346,6 +1380,7 @@ mod tests { | |
| pull_request_needs_repair, run, run_with_reporter, | ||
| }; | ||
| use crate::core::gh::{PullRequestState, PullRequestStatus}; | ||
| use crate::core::store::load_operation; | ||
| use crate::core::test_support::{ | ||
| append_file, commit_file, create_tracked_branch, git_ok, initialize_main_repo, | ||
| with_temp_repo, | ||
|
|
@@ -1623,6 +1658,7 @@ mod tests { | |
| let outcome = run_with_reporter( | ||
| &SyncOptions { | ||
| continue_operation: true, | ||
| abort_operation: false, | ||
| }, | ||
| &mut |event| { | ||
| events.push(event.clone()); | ||
|
|
@@ -1646,4 +1682,63 @@ mod tests { | |
| )); | ||
| }); | ||
| } | ||
|
|
||
| #[test] | ||
| fn abort_cancels_paused_sync_and_clears_pending_operation() { | ||
| with_temp_repo("dgr-sync-abort", |repo| { | ||
| initialize_main_repo(repo); | ||
| crate::core::init::run(&crate::core::init::InitOptions::default()).unwrap(); | ||
| create_tracked_branch("feat/auth"); | ||
| commit_file(repo, "auth.txt", "auth\n", "feat: auth"); | ||
| create_tracked_branch("feat/auth-ui"); | ||
| commit_file(repo, "shared.txt", "child\n", "feat: ui"); | ||
| git_ok(repo, &["checkout", "main"]); | ||
| commit_file(repo, "shared.txt", "main\n", "feat: trunk"); | ||
| git_ok(repo, &["checkout", "feat/auth"]); | ||
|
|
||
| // Trigger a conflict so sync pauses | ||
| let paused = run(&SyncOptions::default()).unwrap(); | ||
| assert!(!paused.status.success()); | ||
| assert!(paused.paused); | ||
|
|
||
| // Verify a pending operation exists | ||
| let session = | ||
| crate::core::store::open_initialized("should be initialized").unwrap(); | ||
| assert!(load_operation(&session.paths).unwrap().is_some()); | ||
|
|
||
| // Abort the paused sync | ||
| let outcome = run(&SyncOptions { | ||
| continue_operation: false, | ||
| abort_operation: true, | ||
| }) | ||
| .unwrap(); | ||
|
|
||
| assert!(outcome.status.success()); | ||
| assert!(!outcome.paused); | ||
|
|
||
| // Verify the pending operation has been cleared | ||
| assert!(load_operation(&session.paths).unwrap().is_none()); | ||
| }); | ||
| } | ||
|
|
||
| #[test] | ||
| fn abort_returns_error_when_no_pending_operation() { | ||
| with_temp_repo("dgr-sync-abort-no-op", |repo| { | ||
| initialize_main_repo(repo); | ||
| crate::core::init::run(&crate::core::init::InitOptions::default()).unwrap(); | ||
|
|
||
| let result = run(&SyncOptions { | ||
| continue_operation: false, | ||
| abort_operation: true, | ||
| }); | ||
|
|
||
| assert!(result.is_err()); | ||
| assert!( | ||
| result | ||
| .unwrap_err() | ||
| .to_string() | ||
| .contains("no paused dgr operation to abort") | ||
| ); | ||
| }); | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding
abort_operationtoSyncOptionsrequires updating allSyncOptions { ... }struct literals. There is at least one existing literal later in this file (in the tests) that now omitsabort_operation, which will not compile. Consider either adding the field at that call site(s) or switching those literals to..Default::default()to make future option additions non-breaking.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed — verified no other SyncOptions literals exist in the codebase besides the test, which already has the field. Added the missing
abort_operation: falseto the test struct literal. The other usages all go throughSyncOptions::default()which handles it via the derivedDefaultimpl.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in b5801c8. All
SyncOptionsstruct literals in production and test code now include theabort_operationfield. The struct derivesDefault, so the three call sites usingSyncOptions::default()correctly getabort_operation: false. The one explicit struct literal (in the resume test) already had the field from the prior commit. The new abort tests also use explicit struct literals with both fields.