Skip to content
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

git init: add option to track local bookmarks #5279

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

bryceberger
Copy link
Member

Uses the same list of bookmarks that is otherwise printed to the user as local bookmarks not associated with their remote. Essentially, just automatically runs the hinted jj bookmark track ....

I use an external tool that is not jj-aware to manage my repositories (ghq). As such, I find myself running jj git init --colocate fairly often. Almost always, I immediately follow the hint and copy and paste the given jj bookmark track ... command. Would like to just skip that step.

Checklist

If applicable:

  • I have updated CHANGELOG.md
  • I have updated the documentation (README.md, docs/, demos/)
  • I have updated the config schema (cli/src/config-schema.json)
  • I have added tests to cover my changes

@bryceberger bryceberger force-pushed the bryce/push-nykszoowpmuu branch from 767e203 to 2c74e80 Compare January 7, 2025 00:38
Copy link
Member

@martinvonz martinvonz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds like a reasonable feature, thanks

cli/src/commands/git/init.rs Outdated Show resolved Hide resolved
cli/src/commands/git/init.rs Outdated Show resolved Hide resolved
flake.nix Outdated Show resolved Hide resolved
@bryceberger bryceberger force-pushed the bryce/push-nykszoowpmuu branch 2 times, most recently from ebf10e9 to 9680ce3 Compare January 7, 2025 03:24
@bryceberger bryceberger requested a review from martinvonz January 9, 2025 03:30
CHANGELOG.md Outdated Show resolved Hide resolved
cli/src/commands/git/init.rs Outdated Show resolved Hide resolved
cli/src/commands/git/init.rs Outdated Show resolved Hide resolved
cli/src/commands/git/init.rs Show resolved Hide resolved
cli/src/commands/git/init.rs Outdated Show resolved Hide resolved
cli/tests/test_git_init.rs Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
cli/src/commands/git/init.rs Outdated Show resolved Hide resolved
Comment on lines 284 to 326
for (name, remote_name) in &pairs {
repo.track_remote_bookmark(name, remote_name);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just fyi, this can move local bookmark if remote is ahead of (or diverged from) the local bookmark, as jj bookmark track would do. It might be better to document the behavior. See also #1862

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would make more sense to only track if local and remote are the same, and print the current hint text if they're different.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or if local is ahead of remote? local == remote might be too strict.

@bryceberger bryceberger force-pushed the bryce/push-nykszoowpmuu branch from 9680ce3 to 874bf8f Compare January 9, 2025 20:58
Uses the same list of bookmarks that is otherwise printed to the user
as local bookmarks not associated with their remote. Essentially, just
automatically runs the hinted `jj bookmark track ...`.

If a bookmark is not at the same position as a given remote, will skip
tracking that remote.
@bryceberger bryceberger force-pushed the bryce/push-nykszoowpmuu branch from 874bf8f to cea7fe7 Compare January 18, 2025 06:27
Comment on lines +370 to +382
let remote_added = remote_ref.target.added_ids().cloned().collect();
let Ok(ahead_of_remote) = RevsetExpression::commits(remote_added)
.descendants()
.evaluate(repo)
.map(|r| r.containing_fn())
else {
return Some(Err(format!("{name}@{remote_name}")));
};

if local_target
.added_ids()
.any(|add| ahead_of_remote(add).unwrap_or(false))
{
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yuja would like you to look at this. My intent was:

  • get descendants of remote ref
  • check if local ref is contained in above set

Unsure if this is the best way to do that, or if that's exactly what's wanted.

Also, this triggers if any of the added_ids() are a descendant. Pretty sure this is what's wanted, as it will trigger even if local has merged in another branch.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps, you can use repo.index().is_ancestor(). It's also good to use refs::merge_ref_targets() and check the merge result, something like:

let base_target = remote_ref.tracking_target();
let merged_target = merge_ref_targets(index, local_target, base_target, &remote_ref.target);
merged_target == local_target

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants