Skip to content

Revisions cannot contains the character @ #802

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

Closed
1 task done
lthms opened this issue Mar 31, 2023 · 1 comment
Closed
1 task done

Revisions cannot contains the character @ #802

lthms opened this issue Mar 31, 2023 · 1 comment
Labels
acknowledged an issue is accepted as shortcoming to be fixed

Comments

@lthms
Copy link
Contributor

lthms commented Mar 31, 2023

Duplicates

  • I have searched the existing issues

Current behavior 😯

The function gix_revision::spec::parse::function::parse (in gix-revision/src/spec/parse/function.rs does not support revision which contains the character @.

For instance, at work, we have the guidelines to prefix our branches names by our handle, so my branch looks like lthms@fix-something.

I’ve run into the issue because I am using Stacked Git, which has moved from its previous git library to gitoxide (see stacked-git/stgit#306). Since I’ve upgraded, I get errors related to branch names with @, and after some digging, I was able to isolate the parse function.

Expected behavior 🤔

git branch and git rev-parse handle branches with @ in their name gracefully.

I would have expected that gix would do so too. But maybe it is not a goal? I’ve seen that there are tests explicitely checking that gix fails in these cases (in gix-revision-tests, in refnames.rs).

I am trying to get my head around the code, but so far it’s a bit hard to follow. Any help is welcome.

Steps to reproduce 🕹

  1. git branch a@b
  2. cargo run rev resolve a@b

Result is

Error: The ref partially named "a" could not be found
@Byron Byron added the acknowledged an issue is accepted as shortcoming to be fixed label Apr 1, 2023
@Byron
Copy link
Member

Byron commented Apr 1, 2023

Thanks a lot for making me aware!

I would have expected that gix would do so too. But maybe it is not a goal?

gix desperately wants to be fully compatible to git, and this is clearly a shortcoming to be fixed. I didn't think of it, which explains why the code can't handle it.

Note that for testing gix rev parse -e revspec is most helpful as it visualizes the parser callbacks which ultimately drives the resolver.

The fix was made available here.

@Byron Byron closed this as completed in 1c27e7a Apr 1, 2023
Byron added a commit that referenced this issue Apr 1, 2023
Previously these would cause a parse error due to confusing `@` with
the short form of `HEAD`.

Merge branch 'fix-rev-parse-with-at'
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
acknowledged an issue is accepted as shortcoming to be fixed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants