From 3d31928dac2318ba2075eb8bf590fe1d6012c12b Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Sun, 13 Oct 2024 19:48:30 +0900 Subject: [PATCH] revset: drop support for HEAD@git symbol resolution This was added at f5f61f6bfe36 "revset: resolve 'HEAD@git' just like other pseudo @git branches." As I said in this patch, there was no practical use case of the HEAD@git symbol. Suppose we implement colocated workspaces/worktrees #4436, there may be multiple Git HEAD revisions. This means HEAD can no longer be abstracted as a symbol of the "git" remote. --- CHANGELOG.md | 3 +++ cli/src/commands/git/import.rs | 2 +- cli/tests/test_git_init.rs | 2 +- docs/revsets.md | 3 +-- lib/src/git.rs | 10 ++++---- lib/src/revset.rs | 39 +++++++++++++----------------- lib/src/view.rs | 4 ++-- lib/tests/test_revset.rs | 44 ---------------------------------- 8 files changed, 29 insertions(+), 78 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 70678aff8f..9f75a530eb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,9 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). * Evaluation error of `revsets.short-prefixes` configuration is now reported. +* The `HEAD@git` symbol no longer resolves to the Git HEAD revision. Use + `git_head()` or `@-` revset expression instead. + * Help command doesn't work recursively anymore, i.e. `jj workspace help root` doesn't work anymore. diff --git a/cli/src/commands/git/import.rs b/cli/src/commands/git/import.rs index b1e7357e41..062cf54920 100644 --- a/cli/src/commands/git/import.rs +++ b/cli/src/commands/git/import.rs @@ -33,7 +33,7 @@ pub fn cmd_git_import( ) -> Result<(), CommandError> { let mut workspace_command = command.workspace_helper(ui)?; let mut tx = workspace_command.start_transaction(); - // In non-colocated repo, HEAD@git will never be moved internally by jj. + // In non-colocated repo, Git HEAD will never be moved internally by jj. // That's why cmd_git_export() doesn't export the HEAD ref. git::import_head(tx.repo_mut())?; let stats = git::import_refs(tx.repo_mut(), &command.settings().git_settings())?; diff --git a/cli/tests/test_git_init.rs b/cli/tests/test_git_init.rs index 0ed6d7e94a..9b09f6abe5 100644 --- a/cli/tests/test_git_init.rs +++ b/cli/tests/test_git_init.rs @@ -743,7 +743,7 @@ fn test_git_init_colocated_via_flag_git_dir_not_exists() { insta::assert_snapshot!(stderr, @r###" Initialized repo in "repo" "###); - // No HEAD@git ref is available yet + // No HEAD ref is available yet insta::assert_snapshot!(get_log_output(&test_env, &workspace_root), @r###" @ 230dd059e1b0 ◆ 000000000000 diff --git a/docs/revsets.md b/docs/revsets.md index 9d42df006b..dc67938775 100644 --- a/docs/revsets.md +++ b/docs/revsets.md @@ -234,8 +234,7 @@ revsets (expressions) as arguments. * `git_refs()`: All Git ref targets as of the last import. If a Git ref is in a conflicted state, all its possible targets are included. -* `git_head()`: The Git `HEAD` target as of the last import. Equivalent to - `present(HEAD@git)`. +* `git_head()`: The Git `HEAD` target as of the last import. * `visible_heads()`: All visible heads (same as `heads(all())`). diff --git a/lib/src/git.rs b/lib/src/git.rs index bd30b1ec95..83e619fe4e 100644 --- a/lib/src/git.rs +++ b/lib/src/git.rs @@ -541,13 +541,13 @@ fn remotely_pinned_commit_ids(view: &View) -> Vec { .collect() } -/// Imports `HEAD@git` from the underlying Git repo. +/// Imports HEAD from the underlying Git repo. /// /// Unlike `import_refs()`, the old HEAD branch is not abandoned because HEAD /// move doesn't always mean the old HEAD branch has been rewritten. /// /// Unlike `reset_head()`, this function doesn't move the working-copy commit to -/// the child of the new `HEAD@git` revision. +/// the child of the new HEAD revision. pub fn import_head(mut_repo: &mut MutableRepo) -> Result<(), GitImportError> { let store = mut_repo.store(); let git_backend = get_git_backend(store).ok_or(GitImportError::UnexpectedBackend)?; @@ -938,7 +938,7 @@ fn update_git_ref( Ok(()) } -/// Ensures `HEAD@git` is detached and pointing to the `new_oid`. If `new_oid` +/// Ensures Git HEAD is detached and pointing to the `new_oid`. If `new_oid` /// is `None` (meaning absent), dummy placeholder ref will be set. fn update_git_head( git_repo: &gix::Repository, @@ -981,7 +981,7 @@ fn update_git_head( Ok(()) } -/// Sets `HEAD@git` to the parent of the given working-copy commit and resets +/// Sets Git HEAD to the parent of the given working-copy commit and resets /// the Git index. pub fn reset_head( mut_repo: &mut MutableRepo, @@ -1012,7 +1012,7 @@ pub fn reset_head( false }; let skip_reset = if is_same_tree { - // `HEAD@git` already points to a commit with the correct tree contents, + // HEAD already points to a commit with the correct tree contents, // so we only need to reset the Git index. We can skip the reset if // the Git index is empty (i.e. `git add` was never used). // In large repositories, this is around 2x faster if the Git index is empty diff --git a/lib/src/revset.rs b/lib/src/revset.rs index ce045d590a..4b3598e99f 100644 --- a/lib/src/revset.rs +++ b/lib/src/revset.rs @@ -1582,12 +1582,7 @@ fn reload_repo_at_operation( } fn resolve_remote_bookmark(repo: &dyn Repo, name: &str, remote: &str) -> Option> { - let view = repo.view(); - let target = match (name, remote) { - #[cfg(feature = "git")] - ("HEAD", crate::git::REMOTE_NAME_FOR_LOCAL_GIT_REPO) => view.git_head(), - (name, remote) => &view.get_remote_bookmark(name, remote).target, - }; + let target = &repo.view().get_remote_bookmark(name, remote).target; target .is_present() .then(|| target.added_ids().cloned().collect()) @@ -1598,23 +1593,21 @@ fn all_bookmark_symbols( include_synced_remotes: bool, ) -> impl Iterator + '_ { let view = repo.view(); - view.bookmarks() - .flat_map(move |(name, bookmark_target)| { - // Remote bookmark "x"@"y" may conflict with local "x@y" in unquoted form. - let local_target = bookmark_target.local_target; - let local_symbol = local_target.is_present().then(|| name.to_owned()); - let remote_symbols = bookmark_target - .remote_refs - .into_iter() - .filter(move |&(_, remote_ref)| { - include_synced_remotes - || !remote_ref.is_tracking() - || remote_ref.target != *local_target - }) - .map(move |(remote_name, _)| format!("{name}@{remote_name}")); - local_symbol.into_iter().chain(remote_symbols) - }) - .chain(view.git_head().is_present().then(|| "HEAD@git".to_owned())) + view.bookmarks().flat_map(move |(name, bookmark_target)| { + // Remote bookmark "x"@"y" may conflict with local "x@y" in unquoted form. + let local_target = bookmark_target.local_target; + let local_symbol = local_target.is_present().then(|| name.to_owned()); + let remote_symbols = bookmark_target + .remote_refs + .into_iter() + .filter(move |&(_, remote_ref)| { + include_synced_remotes + || !remote_ref.is_tracking() + || remote_ref.target != *local_target + }) + .map(move |(remote_name, _)| format!("{name}@{remote_name}")); + local_symbol.into_iter().chain(remote_symbols) + }) } fn make_no_such_symbol_error(repo: &dyn Repo, name: impl Into) -> RevsetResolutionError { diff --git a/lib/src/view.rs b/lib/src/view.rs index 63b29cbdbd..64b677bcdb 100644 --- a/lib/src/view.rs +++ b/lib/src/view.rs @@ -333,8 +333,8 @@ impl View { } } - /// Sets `HEAD@git` to point to the given target. If the target is absent, - /// the reference will be cleared. + /// Sets Git HEAD to point to the given target. If the target is absent, the + /// reference will be cleared. pub fn set_git_head_target(&mut self, target: RefTarget) { self.data.git_head = target; } diff --git a/lib/tests/test_revset.rs b/lib/tests/test_revset.rs index af259af7bb..61e2eb7141 100644 --- a/lib/tests/test_revset.rs +++ b/lib/tests/test_revset.rs @@ -806,50 +806,6 @@ fn test_resolve_symbol_tags() { ); } -#[test] -fn test_resolve_symbol_git_head() { - let settings = testutils::user_settings(); - let test_repo = TestRepo::init(); - let repo = &test_repo.repo; - - let mut tx = repo.start_transaction(&settings); - let mut_repo = tx.repo_mut(); - - let commit1 = write_random_commit(mut_repo, &settings); - - // Without HEAD@git - insta::assert_debug_snapshot!( - resolve_symbol(mut_repo, "HEAD").unwrap_err(), @r###" - NoSuchRevision { - name: "HEAD", - candidates: [], - } - "###); - insta::assert_debug_snapshot!( - resolve_symbol(mut_repo, "HEAD@git").unwrap_err(), @r###" - NoSuchRevision { - name: "HEAD@git", - candidates: [], - } - "###); - - // With HEAD@git - mut_repo.set_git_head_target(RefTarget::normal(commit1.id().clone())); - insta::assert_debug_snapshot!( - resolve_symbol(mut_repo, "HEAD").unwrap_err(), @r###" - NoSuchRevision { - name: "HEAD", - candidates: [ - "HEAD@git", - ], - } - "###); - assert_eq!( - resolve_symbol(mut_repo, "HEAD@git").unwrap(), - vec![commit1.id().clone()], - ); -} - #[test] fn test_resolve_symbol_git_refs() { let settings = testutils::user_settings();