Skip to content

Commit

Permalink
revset: drop support for HEAD@git symbol resolution
Browse files Browse the repository at this point in the history
This was added at f5f61f6 "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.
  • Loading branch information
yuja committed Oct 21, 2024
1 parent 0092847 commit 3d31928
Show file tree
Hide file tree
Showing 8 changed files with 29 additions and 78 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
2 changes: 1 addition & 1 deletion cli/src/commands/git/import.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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())?;
Expand Down
2 changes: 1 addition & 1 deletion cli/tests/test_git_init.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 1 addition & 2 deletions docs/revsets.md
Original file line number Diff line number Diff line change
Expand Up @@ -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())`).

Expand Down
10 changes: 5 additions & 5 deletions lib/src/git.rs
Original file line number Diff line number Diff line change
Expand Up @@ -541,13 +541,13 @@ fn remotely_pinned_commit_ids(view: &View) -> Vec<CommitId> {
.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)?;
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down
39 changes: 16 additions & 23 deletions lib/src/revset.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1582,12 +1582,7 @@ fn reload_repo_at_operation(
}

fn resolve_remote_bookmark(repo: &dyn Repo, name: &str, remote: &str) -> Option<Vec<CommitId>> {
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())
Expand All @@ -1598,23 +1593,21 @@ fn all_bookmark_symbols(
include_synced_remotes: bool,
) -> impl Iterator<Item = String> + '_ {
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<String>) -> RevsetResolutionError {
Expand Down
4 changes: 2 additions & 2 deletions lib/src/view.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
44 changes: 0 additions & 44 deletions lib/tests/test_revset.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down

0 comments on commit 3d31928

Please sign in to comment.