Skip to content

Commit 1e7b99f

Browse files
committed
cli git push: allow pushing explicitly named bookmarks
By explicitly naming the bookmark to push, users already express their intent to push that particular bookmark. Requiring them to express that intent a second time by manually marking the bookmark as tracked is therefore unnecessary. There are some exceptional situations where the intent can still be unclear related to multiple remotes and glob patterns.
1 parent 8c84656 commit 1e7b99f

File tree

3 files changed

+152
-7
lines changed

3 files changed

+152
-7
lines changed

CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,9 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
2424
filesystem's behavior, but this can be overridden manually by setting
2525
`working-copy.exec-bit-change = "respect" | "ignore"`.
2626

27+
* `jj git push --bookmark <name>` will now automatically track the bookmark in
28+
cases where the user intent is unambiguous.
29+
2730
### Fixed bugs
2831

2932
* Broken symlink on Windows. [#6934](https://github.com/jj-vcs/jj/issues/6934).

cli/src/commands/git/push.rs

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ use jj_lib::revset::RevsetExpression;
4949
use jj_lib::settings::UserSettings;
5050
use jj_lib::signing::SignBehavior;
5151
use jj_lib::str_util::StringExpression;
52+
use jj_lib::str_util::StringMatcher;
5253
use jj_lib::view::View;
5354

5455
use crate::cli_util::CommandHelper;
@@ -349,6 +350,36 @@ pub fn cmd_git_push(
349350
continue;
350351
}
351352
let remote_symbol = name.to_remote_symbol(remote);
353+
// Check for some targeted allow_new overrides. These are
354+
// conservative heuristics, rather than principled rules. They
355+
// should cover most practical situations without causing harm.
356+
let allow_new = if args.bookmark.iter().any(|b| b.contains(':')) {
357+
// The user specified bookmarks with something more complicated
358+
// than exact matching. It's possible that some bookmarks which
359+
// shouldn't be pushed are matched by mistake. Therefore, do not
360+
// override allow_new.
361+
allow_new
362+
} else if args.remote.is_some() {
363+
// The user specified all bookmarks exactly and provided an
364+
// explicit remote to push to. Since the user's intent is
365+
// unambiguous, allow the bookmark to be pushed.
366+
true
367+
} else if view
368+
.remote_bookmarks_matching(&StringMatcher::Exact(name.into()), &StringMatcher::All)
369+
.any(|(_, target)| target.is_tracked())
370+
{
371+
// The bookmark is already tracked with some remote. If it
372+
// is not tracked with the default remote being pushed to,
373+
// there is a good chance the user meant to push the bookmark
374+
// to the remote it's already tracking, but forgot to supply
375+
// `--remote`. Therefore, do not override allow_new.
376+
allow_new
377+
} else {
378+
// The user specified this bookmark exactly and it is not
379+
// tracked with any other remote. The user unambiguously intends
380+
// to push this new bookmark to the default remote.
381+
true
382+
};
352383
let allow_delete = true; // named explicitly, allow delete without --delete
353384
match classify_bookmark_update(remote_symbol, targets, allow_new, allow_delete) {
354385
Ok(Some(update)) => bookmark_updates.push((name.to_owned(), update)),

cli/tests/test_git_push.rs

Lines changed: 118 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1369,7 +1369,7 @@ fn test_git_push_mixed() {
13691369
.success();
13701370
work_dir.write_file("file", "modified again");
13711371

1372-
// --allow-new is not implied for --bookmark=.. and -r=..
1372+
// --allow-new is not implied for -r=..
13731373
let output = work_dir.run_jj([
13741374
"git",
13751375
"push",
@@ -1380,10 +1380,14 @@ fn test_git_push_mixed() {
13801380
insta::assert_snapshot!(output, @r"
13811381
------- stderr -------
13821382
Creating bookmark push-yqosqzytrlsw for revision yqosqzytrlsw
1383-
Error: Refusing to create new remote bookmark bookmark-1@origin
1384-
Hint: Run `jj bookmark track bookmark-1@origin` and try again.
1383+
Warning: Refusing to create new remote bookmark bookmark-2a@origin
1384+
Hint: Run `jj bookmark track bookmark-2a@origin` and try again.
1385+
Warning: Refusing to create new remote bookmark bookmark-2b@origin
1386+
Hint: Run `jj bookmark track bookmark-2b@origin` and try again.
1387+
Changes to push to origin:
1388+
Add bookmark push-yqosqzytrlsw to 0f8164cd580b
1389+
Add bookmark bookmark-1 to e76139e55e1e
13851390
[EOF]
1386-
[exit status: 1]
13871391
");
13881392

13891393
let output = work_dir.run_jj([
@@ -1397,16 +1401,123 @@ fn test_git_push_mixed() {
13971401
insta::assert_snapshot!(output, @r"
13981402
------- stderr -------
13991403
Warning: --allow-new is deprecated, track bookmarks manually or configure remotes.<name>.auto-track-bookmarks instead.
1400-
Creating bookmark push-yqosqzytrlsw for revision yqosqzytrlsw
1404+
Bookmark push-yqosqzytrlsw@origin already matches push-yqosqzytrlsw
1405+
Bookmark bookmark-1@origin already matches bookmark-1
14011406
Changes to push to origin:
1402-
Add bookmark push-yqosqzytrlsw to 0f8164cd580b
1403-
Add bookmark bookmark-1 to e76139e55e1e
14041407
Add bookmark bookmark-2a to 57d822f901bb
14051408
Add bookmark bookmark-2b to 57d822f901bb
14061409
[EOF]
14071410
");
14081411
}
14091412

1413+
#[test]
1414+
fn test_git_push_allow_new_bookmark_heuristics() {
1415+
let test_env = TestEnvironment::default();
1416+
set_up(&test_env);
1417+
let work_dir = test_env.work_dir("local");
1418+
test_env
1419+
.run_jj_in(".", ["git", "init", "--colocate", "upstream"])
1420+
.success();
1421+
work_dir
1422+
.run_jj([
1423+
"git",
1424+
"remote",
1425+
"add",
1426+
"upstream",
1427+
test_env
1428+
.work_dir("upstream")
1429+
.root()
1430+
.join(".git")
1431+
.to_str()
1432+
.unwrap(),
1433+
])
1434+
.success();
1435+
work_dir.run_jj(["describe", "-m", "foo"]).success();
1436+
work_dir.write_file("file", "contents");
1437+
1438+
work_dir
1439+
.run_jj(["bookmark", "create", "untracked"])
1440+
.success();
1441+
1442+
let start_op = work_dir.current_operation_id();
1443+
1444+
// Pushing untracked bookmarks with --bookmark automatically trackes it.
1445+
let output = work_dir.run_jj(["git", "push", "--bookmark", "untracked"]);
1446+
insta::assert_snapshot!(output, @r"
1447+
------- stderr -------
1448+
Changes to push to origin:
1449+
Add bookmark untracked to b9124726209b
1450+
[EOF]
1451+
");
1452+
1453+
work_dir.run_jj(["op", "restore", &start_op]).success();
1454+
work_dir
1455+
.run_jj([
1456+
"git",
1457+
"push",
1458+
"--bookmark",
1459+
"untracked",
1460+
"--remote",
1461+
"upstream",
1462+
])
1463+
.success();
1464+
1465+
// ...unless the bookmark is already tracked with a different remote, in
1466+
// which case the user may have simply forgotten to specify --remote.
1467+
let output = work_dir.run_jj(["git", "push", "--bookmark", "untracked"]);
1468+
insta::assert_snapshot!(output, @r"
1469+
------- stderr -------
1470+
Error: Refusing to create new remote bookmark untracked@origin
1471+
Hint: Run `jj bookmark track untracked@origin` and try again.
1472+
[EOF]
1473+
[exit status: 1]
1474+
");
1475+
1476+
work_dir.run_jj(["op", "restore", &start_op]).success();
1477+
work_dir
1478+
.run_jj(["git", "push", "--bookmark", "untracked"])
1479+
.success();
1480+
1481+
// If the user explicitly requests to push the bookmark to a different
1482+
// remote than the one it's already tracking, that is fine.
1483+
let output = work_dir.run_jj([
1484+
"git",
1485+
"push",
1486+
"--bookmark",
1487+
"untracked",
1488+
"--remote",
1489+
"upstream",
1490+
]);
1491+
insta::assert_snapshot!(output, @r"
1492+
------- stderr -------
1493+
Changes to push to upstream:
1494+
Add bookmark untracked to b9124726209b
1495+
[EOF]
1496+
");
1497+
1498+
work_dir.run_jj(["op", "restore", &start_op]).success();
1499+
1500+
// If the user provides something other than an exact string to
1501+
// match with --bookmark, the automatic tracking is completely disabled,
1502+
// because a glob pattern could accidentally match a bookmark name
1503+
// that wasn't intended to be pushed.
1504+
let output = work_dir.run_jj([
1505+
"git",
1506+
"push",
1507+
"--bookmark",
1508+
"glob:*tracked",
1509+
"--remote",
1510+
"origin",
1511+
]);
1512+
insta::assert_snapshot!(output, @r"
1513+
------- stderr -------
1514+
Error: Refusing to create new remote bookmark untracked@origin
1515+
Hint: Run `jj bookmark track untracked@origin` and try again.
1516+
[EOF]
1517+
[exit status: 1]
1518+
");
1519+
}
1520+
14101521
#[test]
14111522
fn test_git_push_unsnapshotted_change() {
14121523
let test_env = TestEnvironment::default();

0 commit comments

Comments
 (0)