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

jj bookmark create allows creating unreferenceable bookmarks #5705

Open
zx8 opened this issue Feb 14, 2025 · 1 comment · May be fixed by #5725
Open

jj bookmark create allows creating unreferenceable bookmarks #5705

zx8 opened this issue Feb 14, 2025 · 1 comment · May be fixed by #5725

Comments

@zx8
Copy link

zx8 commented Feb 14, 2025

Using 514a009


Repro:

$ jj bookmark create foo:bar
Warning: Failed to export some bookmarks:
  foo:bar: Failed to set: A reference must be a valid tag name as well: A ref must not
  contain invalid bytes or ascii control characters: ":"

At this point, the bookmark is still created, despite the error:

○  yxlrwnxw [email protected] 2025-02-14 15:54:52 foo:bar 7d98d8bc
│  (empty) (no description set)

If I try to reference that bookmark, it does not work:

$ jj show foo:bar
Error: Failed to parse revset: Modifier `foo` doesn't exist
Caused by:  --> 1:1
  |
1 | foo:bar
  | ^-^
  |
  = Modifier `foo` doesn't exist

If I try to delete the bookmark:

$ jj bookmark delete foo:bar
error: invalid value 'foo:bar' for '<NAMES>...': Invalid string pattern kind `foo:`

If I use exact:foo:bar, it does work.

@PhilipMetzger
Copy link
Contributor

this will be fixed by #5423 where we make bookmark creation stricter.

yuja added a commit to yuja/jj that referenced this issue Feb 17, 2025
This should help prevent misuse of "jj bookmark set" with e.g. remote bookmark
name. A remote-looking bookmark can be created by quoting the name if needed.

This patch doesn't change the parsing of name patterns. For patterns, I think
it's better to add support for compound expressions (e.g. `glob:foo* & ~bar`)
rather than adding revset::parse_string_pattern(text) -> StringPattern.

Closes jj-vcs#5705
@yuja yuja linked a pull request Feb 17, 2025 that will close this issue
4 tasks
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 a pull request may close this issue.

2 participants