From 755b7b7b517070cdd36c2ea930dc6ccb2eb1366d Mon Sep 17 00:00:00 2001 From: rishiad Date: Fri, 1 May 2026 23:43:51 +0930 Subject: [PATCH] gerrit: check default upload revisions are rewritable The implicit revision selection path for `jj gerrit upload` bypassed the usual rewritability check. This leads to a panic if the selected revision was immutable. Run the check before continuing so the command fails with a user error instead. Fixes: #9398 --- CHANGELOG.md | 3 +++ cli/src/commands/gerrit/upload.rs | 11 +++++++-- cli/tests/test_gerrit_upload.rs | 37 +++++++++++++++++++++++++++++++ 3 files changed, 49 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4c0908c35..fab6f2985 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -67,6 +67,9 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). * Pre-existing Git submodule directories are no longer considered conflicts in checkouts. [#8065](https://github.com/jj-vcs/jj/issues/8065). +* Fixed a panic in `jj gerrit upload` when run without `-r` and the + inferred revision was immutable. [#9398](https://github.com/jj-vcs/jj/issues/9398) + ## [0.40.0] - 2026-04-01 ### Release highlights diff --git a/cli/src/commands/gerrit/upload.rs b/cli/src/commands/gerrit/upload.rs index 870d329ef..282dd3c80 100644 --- a/cli/src/commands/gerrit/upload.rs +++ b/cli/src/commands/gerrit/upload.rs @@ -430,7 +430,7 @@ pub async fn cmd_gerrit_upload( } // This distinguishes between the "squash workflow" and "edit workflow". Some(commit) => { - if commit.description().is_empty() { + let revisions = if commit.description().is_empty() { let parents = commit.parent_ids(); if parents.len() != 1 { return Err(user_error( @@ -447,7 +447,14 @@ pub async fn cmd_gerrit_upload( } else { writeln!(ui.status(), "No revision provided. Defaulting to @")?; vec![commit.id().clone()] - } + }; + + let revisions_expr = RevsetExpression::commits(revisions.clone()); + workspace_command + .check_rewritable_expr(&revisions_expr) + .await?; + + revisions } } } else { diff --git a/cli/tests/test_gerrit_upload.rs b/cli/tests/test_gerrit_upload.rs index 869e46f0e..4618e806a 100644 --- a/cli/tests/test_gerrit_upload.rs +++ b/cli/tests/test_gerrit_upload.rs @@ -149,6 +149,43 @@ fn test_gerrit_upload_default_revision() { "); } +#[test] +fn test_gerrit_upload_default_revision_already_in_trunk() { + let test_env = TestEnvironment::default(); + test_env + .run_jj_in(".", ["git", "init", "--colocate", "remote"]) + .success(); + let remote_dir = test_env.work_dir("remote"); + create_commit(&remote_dir, "main", &[]); + + test_env + .run_jj_in(".", ["git", "clone", "remote", "local"]) + .success(); + let local_dir = test_env.work_dir("local"); + test_env.add_config(r#"gerrit.default-remote="origin""#); + test_env.add_config(r#"gerrit.default-remote-branch="main""#); + test_env.add_config(r#"revset-aliases."trunk()" = "main@origin""#); + + // Make @ an empty working-copy commit over immutable main@origin. + local_dir.run_jj(["new", "main@origin"]).success(); + local_dir.run_jj(["describe", "-m="]).success(); + + let output = local_dir.run_jj(["gerrit", "upload", "--dry-run"]); + insta::assert_snapshot!(output, @" + ------- stderr ------- + No revision provided and @ has no description. Defaulting to @- + Error: Commit 57df4838fd85 is immutable + Hint: Could not modify commit: rlvkpnrz 57df4838 main@origin | main + Hint: Immutable commits are used to protect shared history. + Hint: For more information, see: + - https://docs.jj-vcs.dev/latest/config/#set-of-immutable-commits + - `jj help -k config`, \"Set of immutable commits\" + Hint: This operation would rewrite 1 immutable commits. + [EOF] + [exit status: 1] + "); +} + #[test] fn test_gerrit_upload_option_failure() { let test_env = TestEnvironment::default();