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] jj cannot export refs when remote-tracking branch is path-like prefix to a branch #493

Closed
chooglen opened this issue Aug 26, 2022 · 7 comments
Assignees

Comments

@chooglen
Copy link
Collaborator

My repo has a remote-tracking branch, e.g. origin/test, and I have a local branch test/something. When trying to export refs to Git, jj complains with

Internal error: Failed to export refs to underlying Git repo: cannot lock ref 'refs/heads/test', there are refs beneath that folder; class=Reference (4)

Note that because Git uses file paths to store refs, it doesn't allow both test and test/something to be branches at the same time.

@martinvonz
Copy link
Owner

Hmm, I'm not sure what to do about that other than to skip one of the refs in such cases. That would at least be better than failing.

I think I've heard that the reftable format allows that case but even if that's true, it's a long time until we can start depending on that...

@chooglen
Copy link
Collaborator Author

A band-aid might be to add args to jj git export to control which branches get exported. e.g. in this particular instance, I only care about the branch I was working on, so I could make do with exporting just that branch.

@tp-woven
Copy link
Collaborator

tp-woven commented Sep 27, 2022

This just happened to me when trying to jj git fetch - The GH remote has a foo branch and 3 foo/bar branches (all of which are other people's, so i can't delete/rename them), so I can't fetch any more... 😓

Is there any "local" workaround for this?
Following a short Discord discussion with Martin, using git fetch directly and only fetching specific branches "solved" the problem.

@martinvonz martinvonz self-assigned this Nov 25, 2022
@martinvonz
Copy link
Owner

I'm almost done with a fix for this. The only thing I'm wondering is how to report it to the user. I've made git export report the names of branches that failed to export, but I'm not sure if we should do the same for the automatic export in colocated workspaces. Since @tp-woven and @chooglen have run into this in practice, do you think it would be too spammy to see a message about it after every command?

FYI, the message I added to git export looks like this:

Failed to export some branches:
  
  main/sub

That indicates that a branch with the empty string as name and the main/sub branch failed. We can of course use a different format for automatic exports (and for explicit git export if you think we should).

martinvonz added a commit that referenced this issue Nov 25, 2022
This adds a test for attempting to export both a branch called `main`
and one called `main/sub` (#493), as well as for exporting a branch
with an empty string as name (reported directly to me by @lkorinth).
martinvonz added a commit that referenced this issue Nov 25, 2022
Since we now write a (partial) view object of the exported branches to
disk (since 7904474), we can safely skip exporting some
branches. We already skip conflicted branches. This commit makes us
also skip branches that we fail to write to the backing Git repo,
instead of failing the whole operation (after possibly updating some
Git refs).

I made the `export_refs()` function return the branches that
failed. We should probably make that a struct later and have a
separate field for branches that we skipped due to conflicts.

Closes #493.
martinvonz added a commit that referenced this issue Nov 26, 2022
This adds a test for attempting to export both a branch called `main`
and one called `main/sub` (#493), as well as for exporting a branch
with an empty string as name (reported directly to me by @lkorinth).
@tp-woven
Copy link
Collaborator

Thanks Martin!

I think showing this for every command is OK, as it would encourage me to actually resolve the issue. Another thing that would be useful (for error messages in general, TBH) would be suggestions on how to resolve these situations.

@martinvonz
Copy link
Owner

Another thing that would be useful (for error messages in general, TBH) would be suggestions on how to resolve these situations.

Good point. I sent #812 to add a hint.

@tp-woven
Copy link
Collaborator

Thanks!

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

No branches or pull requests

3 participants