Skip to content

Commit

Permalink
cli_util: consolidate update_stale command fully into cli_util
Browse files Browse the repository at this point in the history
  • Loading branch information
@torquestomp
torquestomp committed Nov 14, 2024
1 parent 49890fa commit e141698
Show file tree
Hide file tree
Showing 3 changed files with 64 additions and 95 deletions.
106 changes: 56 additions & 50 deletions cli/src/cli_util.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -283,11 +283,6 @@ struct CommandHelperData {
working_copy_factories: WorkingCopyFactories,
}

pub enum StaleWorkingCopy {
Recovered(WorkspaceCommandHelper),
Snapshotted((Arc<ReadonlyRepo>, Commit)),
}

impl CommandHelper {
pub fn app(&self) -> &Command {
&self.data.app
Expand Down Expand Up @@ -389,29 +384,7 @@ impl CommandHelper {
// auto-update-stale, so let's do that now. We need to do it up here, not at a
// lower level (e.g. inside snapshot_working_copy()) to avoid recursive locking
// of the working copy.
match self.load_stale_working_copy_commit(ui)? {
StaleWorkingCopy::Recovered(workspace_command) => workspace_command,
StaleWorkingCopy::Snapshotted((repo, stale_commit)) => {
let mut workspace_command = self.workspace_helper_no_snapshot(ui)?;
let (locked_ws, new_commit) =
workspace_command.unchecked_start_working_copy_mutation()?;
let stats = update_stale_working_copy(
locked_ws,
repo.op_id().clone(),
&stale_commit,
&new_commit,
)?;
writeln!(
ui.warning_default(),
"Automatically updated to fresh commit {}",
short_commit_hash(stale_commit.id())
)?;
workspace_command.write_stale_commit_stats(ui, &new_commit, stats)?;

workspace_command.user_repo = ReadonlyUserRepo::new(repo);
workspace_command
}
}
self.recover_stale_working_copy(ui)?
}
};

Expand Down Expand Up @@ -460,23 +433,72 @@ impl CommandHelper {
})
}

pub fn load_stale_working_copy_commit(
pub fn recover_stale_working_copy(
&self,
ui: &Ui,
) -> Result<StaleWorkingCopy, CommandError> {
) -> Result<WorkspaceCommandHelper, CommandError> {
let workspace = self.load_workspace()?;
let op_id = workspace.working_copy().operation_id();

match workspace.repo_loader().load_operation(op_id) {
Ok(op) => {
let repo = workspace.repo_loader().load_at(&op)?;
let mut workspace_command = self.for_workable_repo(ui, workspace, repo)?;

// Snapshot the current working copy on top of the last known working-copy
// operation, then merge the divergent operations. The wc_commit_id of the
// merged repo wouldn't change because the old one wins, but it's probably
// fine if we picked the new wc_commit_id.
workspace_command.maybe_snapshot(ui)?;

let wc_commit_id = workspace_command.get_wc_commit_id().unwrap();
let repo = workspace_command.repo().clone();
let wc_commit = repo.store().get_commit(wc_commit_id)?;
Ok(StaleWorkingCopy::Snapshotted((repo, wc_commit)))
let stale_wc_commit = repo.store().get_commit(wc_commit_id)?;

let mut workspace_command = self.workspace_helper_no_snapshot(ui)?;

let repo = workspace_command.repo().clone();
let (mut locked_ws, desired_wc_commit) =
workspace_command.unchecked_start_working_copy_mutation()?;
match WorkingCopyFreshness::check_stale(
locked_ws.locked_wc(),
&desired_wc_commit,
&repo,
)? {
WorkingCopyFreshness::Fresh | WorkingCopyFreshness::Updated(_) => {
writeln!(
ui.status(),
"Attempted recovery, but the working copy is not stale"
)?;
}
WorkingCopyFreshness::WorkingCopyStale
| WorkingCopyFreshness::SiblingOperation => {
let stats = update_stale_working_copy(
locked_ws,
repo.op_id().clone(),
&stale_wc_commit,
&desired_wc_commit,
)?;

// TODO: Share this code with new/checkout somehow.
if let Some(mut formatter) = ui.status_formatter() {
write!(formatter, "Working copy now at: ")?;
formatter.with_label("working_copy", |fmt| {
workspace_command.write_commit_summary(fmt, &desired_wc_commit)
})?;
writeln!(formatter)?;
}
print_checkout_stats(ui, stats, &desired_wc_commit)?;

writeln!(
ui.status(),
"Updated working copy to fresh commit {}",
short_commit_hash(desired_wc_commit.id())
)?;
}
};

Ok(workspace_command)
}
Err(e @ OpStoreError::ObjectNotFound { .. }) => {
writeln!(
Expand All @@ -487,7 +509,7 @@ impl CommandHelper {

let mut workspace_command = self.workspace_helper_no_snapshot(ui)?;
workspace_command.create_and_check_out_recovery_commit(ui)?;
Ok(StaleWorkingCopy::Recovered(workspace_command))
Ok(workspace_command)
}
Err(e) => Err(e.into()),
}
Expand Down Expand Up @@ -1643,22 +1665,6 @@ to the current parents may contain changes from multiple commits.
self.commit_summary_template().format(commit, formatter)
}

pub fn write_stale_commit_stats(
&self,
ui: &Ui,
commit: &Commit,
stats: CheckoutStats,
) -> std::io::Result<()> {
if let Some(mut formatter) = ui.status_formatter() {
write!(formatter, "Working copy now at: ")?;
formatter.with_label("working_copy", |fmt| self.write_commit_summary(fmt, commit))?;
writeln!(formatter)?;
}
print_checkout_stats(ui, stats, commit)?;

Ok(())
}

pub fn check_rewritable<'a>(
&self,
commits: impl IntoIterator<Item = &'a CommitId>,
Expand Down Expand Up @@ -2404,7 +2410,7 @@ pub fn start_repo_transaction(
tx
}

pub fn update_stale_working_copy(
fn update_stale_working_copy(
mut locked_ws: LockedWorkspace,
op_id: OperationId,
stale_commit: &Commit,
Expand Down
39 changes: 1 addition & 38 deletions cli/src/commands/workspace/update_stale.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,9 @@
// See the License for the specific language governing permissions and
// limitations under the License.

use jj_lib::working_copy::WorkingCopyFreshness;
use tracing::instrument;

use crate::cli_util::update_stale_working_copy;
use crate::cli_util::CommandHelper;
use crate::cli_util::StaleWorkingCopy;
use crate::command_error::CommandError;
use crate::ui::Ui;

Expand All @@ -34,41 +31,7 @@ pub fn cmd_workspace_update_stale(
command: &CommandHelper,
_args: &WorkspaceUpdateStaleArgs,
) -> Result<(), CommandError> {
// Snapshot the current working copy on top of the last known working-copy
// operation, then merge the divergent operations. The wc_commit_id of the
// merged repo wouldn't change because the old one wins, but it's probably
// fine if we picked the new wc_commit_id.
let known_wc_commit = match command.load_stale_working_copy_commit(ui)? {
StaleWorkingCopy::Recovered(_) => {
// We have already recovered from the situation that prompted the user to run
// this command, and it is known that the workspace is not stale
// (since we just updated it), so we can return early.
return Ok(());
}
StaleWorkingCopy::Snapshotted((_repo, commit)) => commit,
};
let mut workspace_command = command.workspace_helper_no_snapshot(ui)?;
command.recover_stale_working_copy(ui)?;

let repo = workspace_command.repo().clone();
let (mut locked_ws, desired_wc_commit) =
workspace_command.unchecked_start_working_copy_mutation()?;
match WorkingCopyFreshness::check_stale(locked_ws.locked_wc(), &desired_wc_commit, &repo)? {
WorkingCopyFreshness::Fresh | WorkingCopyFreshness::Updated(_) => {
writeln!(
ui.status(),
"Nothing to do (the working copy is not stale)."
)?;
}
WorkingCopyFreshness::WorkingCopyStale | WorkingCopyFreshness::SiblingOperation => {
let stats = update_stale_working_copy(
locked_ws,
repo.op_id().clone(),
&known_wc_commit,
&desired_wc_commit,
)?;

workspace_command.write_stale_commit_stats(ui, &desired_wc_commit, stats)?;
}
}
Ok(())
}
14 changes: 7 additions & 7 deletions cli/tests/test_workspaces.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -523,6 +523,7 @@ fn test_workspaces_conflicting_edits() {
Rebased 1 descendant commits onto commits rewritten by other operation
Working copy now at: pmmvwywv?? e82cd4ee (empty) (no description set)
Added 0 files, modified 1 files, removed 0 files
Updated working copy to fresh commit e82cd4ee8faa
"###);
insta::assert_snapshot!(get_log_output(&test_env, &secondary_path),
@r###"
Expand Down Expand Up @@ -600,6 +601,7 @@ fn test_workspaces_updated_by_other() {
insta::assert_snapshot!(stderr, @r###"
Working copy now at: pmmvwywv e82cd4ee (empty) (no description set)
Added 0 files, modified 1 files, removed 0 files
Updated working copy to fresh commit e82cd4ee8faa
"###);
insta::assert_snapshot!(get_log_output(&test_env, &secondary_path),
@r###"
Expand Down Expand Up @@ -657,13 +659,13 @@ fn test_workspaces_updated_by_other_automatic() {
let (stdout, stderr) = test_env.jj_cmd_ok(&secondary_path, &["st"]);
insta::assert_snapshot!(stdout, @r###"
The working copy is clean
Working copy : pmmvwywv 3224de8a (empty) (no description set)
Parent commit: qpvuntsm 506f4ec3 (no description set)
Working copy : pmmvwywv e82cd4ee (empty) (no description set)
Parent commit: qpvuntsm d4124476 (no description set)
"###);
insta::assert_snapshot!(stderr, @r###"
Warning: Automatically updated to fresh commit 3224de8ae048
Working copy now at: pmmvwywv e82cd4ee (empty) (no description set)
Added 0 files, modified 1 files, removed 0 files
Updated working copy to fresh commit e82cd4ee8faa
"###);

insta::assert_snapshot!(get_log_output(&test_env, &secondary_path),
Expand Down Expand Up @@ -848,9 +850,7 @@ fn test_workspaces_update_stale_noop() {

let (stdout, stderr) = test_env.jj_cmd_ok(&main_path, &["workspace", "update-stale"]);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(stderr, @r###"
Nothing to do (the working copy is not stale).
"###);
insta::assert_snapshot!(stderr, @"Attempted recovery, but the working copy is not stale");

let stderr = test_env.jj_cmd_failure(
&main_path,
Expand Down Expand Up @@ -890,7 +890,7 @@ fn test_workspaces_update_stale_snapshot() {
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(stderr, @r###"
Concurrent modification detected, resolving automatically.
Nothing to do (the working copy is not stale).
Attempted recovery, but the working copy is not stale
"###);

insta::assert_snapshot!(get_log_output(&test_env, &secondary_path), @r###"
Expand Down

0 comments on commit e141698

Please sign in to comment.