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

"Cannot export conflicted branch" when updating a merged commit #463

Closed
tp-woven opened this issue Aug 8, 2022 · 1 comment
Closed

"Cannot export conflicted branch" when updating a merged commit #463

tp-woven opened this issue Aug 8, 2022 · 1 comment
Assignees

Comments

@tp-woven
Copy link
Collaborator

tp-woven commented Aug 8, 2022

Description

When jj git fetching a repo where one of my branches was just merged, I get a "Cannot export conflicted branch" error on the branch, and it shows up as conflicted despite no changes to the branch locally or remotely.

(I've seen this a few times recently, but only now got around to "documenting" it.)

Steps to Reproduce the Problem

  1. (Not sure if this is related, but) The repo is backed by a .git repo.
  2. Make some changes locally and upload them to GH (in my case, used jj git push --change=@).
  3. (Not sure if this is related, but) jj close, but make no changes to the repo.
  4. On GH, merge the PR from the pushed branch (using merge, not rebase/squash), and delete the remote branch.
  5. Run jj git fetch.

Expected Behavior

main branch should get updated from the remote, and the feature branch should get deleted.

Actual Behavior

main does get updated as expected, but the feature branch is not deleted, and instead jj reports it as being conflicted.

Including the relevant jj op log, in case I need to get more information to debug this:

❯ jj op log
@ 7367ac2fa538 XXXXX 2022-08-08 12:26:08.772 +09:00 - 2022-08-08 12:26:10.042 +09:00
| fetch from git remote origin
| args: jj git fetch
o 971afe4fe51c XXXXX 2022-08-08 12:26:07.595 +09:00 - 2022-08-08 12:26:07.646 +09:00
| import git refs
| args: jj git fetch
o 9f6e121b4fa4 XXXXX 2022-08-08 12:20:17.962 +09:00 - 2022-08-08 12:20:17.992 +09:00
| close commit 14e796c3b31b2cc643ecc9e54b8236dff21db6c5
| args: jj close
o 41c1bb7f7e43 XXXXX 2022-08-08 12:20:17.858 +09:00 - 2022-08-08 12:20:17.903 +09:00
| import git refs
| args: jj close
o 906e73e750a0 XXXXX 2022-08-08 12:20:04.457 +09:00 - 2022-08-08 12:20:07.076 +09:00
| push change e7fb6c4e410147d9a0db3b4d92b028d8 to git remote origin
| args: jj uc

Edit: Just happened again without the jj close shenanigans.

@martinvonz
Copy link
Owner

As I mentioned on Discord, I think this bug typically happens like this:

  1. Move branch foo from A to B
  2. Export to git. This updates foo to B in Git, but doesn't update our record of foo in the Git repo
  3. Move foo to C in Git
  4. Import from Git. Since we didn't update our record before, we think the branch was moved from A to C

I think what you reported here is some version of that sequence, where the branch was deleted instead of moved.

I think we want to fix two things:

  1. Make sure we update our record of the Git ref when we update it in Git
  2. If there are branches with conflicts, skip those instead of erroring out.

@martinvonz martinvonz self-assigned this Nov 2, 2022
martinvonz added a commit that referenced this issue Nov 3, 2022
This is a test case for #463. It's not exactly the same case, but I'm
confident that the root cause is the same (that the
`.jj/repo/git_export_operation_id` doesn't include the git refs we
just updated).
martinvonz added a commit that referenced this issue Nov 3, 2022
To fix #463, I think we want to skip conflicted branches when we
export instead of erroring out. It seems we didn't have test case for
the current behavior, so let's add one.
martinvonz added a commit that referenced this issue Nov 3, 2022
We had very little testing of the colocated case, so let's add a bit
more before I start working on this code in coming patches. This
includes a test for #463.
martinvonz added a commit that referenced this issue Nov 3, 2022
In order to fix #463, I'm going to make us export to Git a little
earlier, before finishing the transation. That means we won't have an
operation yet, but we don't need that anyway.
martinvonz added a commit that referenced this issue Nov 3, 2022
As I said in the previous patch, I don't know why I made the initial
export to Git a no-op. Exporting everything makes more sense to
(current-)me. It will make it slightly easier to skip exporting
conflicted branches (#463).
martinvonz added a commit that referenced this issue Nov 3, 2022
When we export branches to Git, we didn't update our own record of
Git's refs. This frequently led to spurious conflicts in these refs
(e.g. #463). This is typically what happened:

 1. Import a branch pointing to commit A from Git
 2. Modify the branch in jj to point to commit B
 3. Export the branch to Git
 4. Update the branch in Git to point to commit C
 5. Import refs from Git

In step 3, we forgot to update our record of the branch in the repo
view's `git_refs` field. That led to the import in step 5 to think
that the branch moved from A to C in Git, which conflicts with the
internal branch target of B.

This commit fixes the bug by updating the refs in the `MutableRepo`.

Closes #463.
martinvonz added a commit that referenced this issue Nov 3, 2022
This is a test case for #463. It's not exactly the same case, but I'm
confident that the root cause is the same (that the
`.jj/repo/git_export_operation_id` doesn't include the git refs we
just updated).
martinvonz added a commit that referenced this issue Nov 3, 2022
To fix #463, I think we want to skip conflicted branches when we
export instead of erroring out. It seems we didn't have test case for
the current behavior, so let's add one.
martinvonz added a commit that referenced this issue Nov 3, 2022
We had very little testing of the colocated case, so let's add a bit
more before I start working on this code in coming patches. This
includes a test for #463.
martinvonz added a commit that referenced this issue Nov 3, 2022
In order to fix #463, I'm going to make us export to Git a little
earlier, before finishing the transation. That means we won't have an
operation yet, but we don't need that anyway.
martinvonz added a commit that referenced this issue Nov 3, 2022
As I said in the previous patch, I don't know why I made the initial
export to Git a no-op. Exporting everything makes more sense to
(current-)me. It will make it slightly easier to skip exporting
conflicted branches (#463).
martinvonz added a commit that referenced this issue Nov 3, 2022
When we export branches to Git, we didn't update our own record of
Git's refs. This frequently led to spurious conflicts in these refs
(e.g. #463). This is typically what happened:

 1. Import a branch pointing to commit A from Git
 2. Modify the branch in jj to point to commit B
 3. Export the branch to Git
 4. Update the branch in Git to point to commit C
 5. Import refs from Git

In step 3, we forgot to update our record of the branch in the repo
view's `git_refs` field. That led to the import in step 5 to think
that the branch moved from A to C in Git, which conflicts with the
internal branch target of B.

This commit fixes the bug by updating the refs in the `MutableRepo`.

Closes #463.
martinvonz added a commit that referenced this issue Nov 3, 2022
This is a test case for #463. It's not exactly the same case, but I'm
confident that the root cause is the same (that the
`.jj/repo/git_export_operation_id` doesn't include the git refs we
just updated).
martinvonz added a commit that referenced this issue Nov 3, 2022
To fix #463, I think we want to skip conflicted branches when we
export instead of erroring out. It seems we didn't have test case for
the current behavior, so let's add one.
martinvonz added a commit that referenced this issue Nov 3, 2022
We had very little testing of the colocated case, so let's add a bit
more before I start working on this code in coming patches. This
includes a test for #463.
martinvonz added a commit that referenced this issue Nov 3, 2022
In order to fix #463, I'm going to make us export to Git a little
earlier, before finishing the transation. That means we won't have an
operation yet, but we don't need that anyway.
martinvonz added a commit that referenced this issue Nov 3, 2022
As I said in the previous patch, I don't know why I made the initial
export to Git a no-op. Exporting everything makes more sense to
(current-)me. It will make it slightly easier to skip exporting
conflicted branches (#463).
martinvonz added a commit that referenced this issue Nov 3, 2022
When we export branches to Git, we didn't update our own record of
Git's refs. This frequently led to spurious conflicts in these refs
(e.g. #463). This is typically what happened:

 1. Import a branch pointing to commit A from Git
 2. Modify the branch in jj to point to commit B
 3. Export the branch to Git
 4. Update the branch in Git to point to commit C
 5. Import refs from Git

In step 3, we forgot to update our record of the branch in the repo
view's `git_refs` field. That led to the import in step 5 to think
that the branch moved from A to C in Git, which conflicts with the
internal branch target of B.

This commit fixes the bug by updating the refs in the `MutableRepo`.

Closes #463.
martinvonz added a commit that referenced this issue Nov 4, 2022
We had very little testing of the colocated case, so let's add a bit
more before I start working on this code in coming patches. This
includes a test for #463.
martinvonz added a commit that referenced this issue Nov 4, 2022
In order to fix #463, I'm going to make us export to Git a little
earlier, before finishing the transation. That means we won't have an
operation yet, but we don't need that anyway.
martinvonz added a commit that referenced this issue Nov 4, 2022
As I said in the previous patch, I don't know why I made the initial
export to Git a no-op. Exporting everything makes more sense to
(current-)me. It will make it slightly easier to skip exporting
conflicted branches (#463).
martinvonz added a commit that referenced this issue Nov 4, 2022
When we export branches to Git, we didn't update our own record of
Git's refs. This frequently led to spurious conflicts in these refs
(e.g. #463). This is typically what happened:

 1. Import a branch pointing to commit A from Git
 2. Modify the branch in jj to point to commit B
 3. Export the branch to Git
 4. Update the branch in Git to point to commit C
 5. Import refs from Git

In step 3, we forgot to update our record of the branch in the repo
view's `git_refs` field. That led to the import in step 5 to think
that the branch moved from A to C in Git, which conflicts with the
internal branch target of B.

This commit fixes the bug by updating the refs in the `MutableRepo`.

Closes #463.
martinvonz added a commit that referenced this issue Nov 4, 2022
This is a test case for #463. It's not exactly the same case, but I'm
confident that the root cause is the same (that the
`.jj/repo/git_export_operation_id` doesn't include the git refs we
just updated).
martinvonz added a commit that referenced this issue Nov 4, 2022
To fix #463, I think we want to skip conflicted branches when we
export instead of erroring out. It seems we didn't have test case for
the current behavior, so let's add one.
martinvonz added a commit that referenced this issue Nov 4, 2022
We had very little testing of the colocated case, so let's add a bit
more before I start working on this code in coming patches. This
includes a test for #463.
martinvonz added a commit that referenced this issue Nov 9, 2022
To fix #463, I think we want to skip conflicted branches when we
export instead of erroring out. It seems we didn't have test case for
the current behavior, so let's add one.
martinvonz added a commit that referenced this issue Nov 9, 2022
We had very little testing of the colocated case, so let's add a bit
more before I start working on this code in coming patches. This
includes a test for #463.
martinvonz added a commit that referenced this issue Nov 9, 2022
In order to fix #463, I'm going to make us export to Git a little
earlier, before finishing the transation. That means we won't have an
operation yet, but we don't need that anyway.
martinvonz added a commit that referenced this issue Nov 9, 2022
As I said in the previous patch, I don't know why I made the initial
export to Git a no-op. Exporting everything makes more sense to
(current-)me. It will make it slightly easier to skip exporting
conflicted branches (#463).
martinvonz added a commit that referenced this issue Nov 9, 2022
When we export branches to Git, we didn't update our own record of
Git's refs. This frequently led to spurious conflicts in these refs
(e.g. #463). This is typically what happened:

 1. Import a branch pointing to commit A from Git
 2. Modify the branch in jj to point to commit B
 3. Export the branch to Git
 4. Update the branch in Git to point to commit C
 5. Import refs from Git

In step 3, we forgot to update our record of the branch in the repo
view's `git_refs` field. That led to the import in step 5 to think
that the branch moved from A to C in Git, which conflicts with the
internal branch target of B.

This commit fixes the bug by updating the refs in the `MutableRepo`.

Closes #463.
martinvonz added a commit that referenced this issue Nov 10, 2022
This is a test case for #463. It's not exactly the same case, but I'm
confident that the root cause is the same (that the
`.jj/repo/git_export_operation_id` doesn't include the git refs we
just updated).
martinvonz added a commit that referenced this issue Nov 10, 2022
To fix #463, I think we want to skip conflicted branches when we
export instead of erroring out. It seems we didn't have test case for
the current behavior, so let's add one.
martinvonz added a commit that referenced this issue Nov 10, 2022
We had very little testing of the colocated case, so let's add a bit
more before I start working on this code in coming patches. This
includes a test for #463.
martinvonz added a commit that referenced this issue Nov 10, 2022
In order to fix #463, I'm going to make us export to Git a little
earlier, before finishing the transation. That means we won't have an
operation yet, but we don't need that anyway.
martinvonz added a commit that referenced this issue Nov 10, 2022
As I said in the previous patch, I don't know why I made the initial
export to Git a no-op. Exporting everything makes more sense to
(current-)me. It will make it slightly easier to skip exporting
conflicted branches (#463). It also lets us remove a `jj export` call
from `test_templater.rs`.
martinvonz added a commit that referenced this issue Nov 10, 2022
When we export branches to Git, we didn't update our own record of
Git's refs. This frequently led to spurious conflicts in these refs
(e.g. #463). This is typically what happened:

 1. Import a branch pointing to commit A from Git
 2. Modify the branch in jj to point to commit B
 3. Export the branch to Git
 4. Update the branch in Git to point to commit C
 5. Import refs from Git

In step 3, we forgot to update our record of the branch in the repo
view's `git_refs` field. That led to the import in step 5 to think
that the branch moved from A to C in Git, which conflicts with the
internal branch target of B.

This commit fixes the bug by updating the refs in the `MutableRepo`.

Closes #463.
martinvonz added a commit that referenced this issue Nov 13, 2022
This is a test case for #463. It's not exactly the same case, but I'm
confident that the root cause is the same (that the
`.jj/repo/git_export_operation_id` doesn't include the git refs we
just updated).
martinvonz added a commit that referenced this issue Nov 13, 2022
To fix #463, I think we want to skip conflicted branches when we
export instead of erroring out. It seems we didn't have test case for
the current behavior, so let's add one.
martinvonz added a commit that referenced this issue Nov 13, 2022
We had very little testing of the colocated case, so let's add a bit
more before I start working on this code in coming patches. This
includes a test for #463.
martinvonz added a commit that referenced this issue Nov 13, 2022
In order to fix #463, I'm going to make us export to Git a little
earlier, before finishing the transation. That means we won't have an
operation yet, but we don't need that anyway.
martinvonz added a commit that referenced this issue Nov 13, 2022
As I said in the previous patch, I don't know why I made the initial
export to Git a no-op. Exporting everything makes more sense to
(current-)me. It will make it slightly easier to skip exporting
conflicted branches (#463). It also lets us remove a `jj export` call
from `test_templater.rs`.
martinvonz added a commit that referenced this issue Nov 13, 2022
When we export branches to Git, we didn't update our own record of
Git's refs. This frequently led to spurious conflicts in these refs
(e.g. #463). This is typically what happened:

 1. Import a branch pointing to commit A from Git
 2. Modify the branch in jj to point to commit B
 3. Export the branch to Git
 4. Update the branch in Git to point to commit C
 5. Import refs from Git

In step 3, we forgot to update our record of the branch in the repo
view's `git_refs` field. That led to the import in step 5 to think
that the branch moved from A to C in Git, which conflicts with the
internal branch target of B.

This commit fixes the bug by updating the refs in the `MutableRepo`.

Closes #463.
martinvonz added a commit that referenced this issue Nov 13, 2022
When we export branches to Git, we didn't update our own record of
Git's refs. This frequently led to spurious conflicts in these refs
(e.g. #463). This is typically what happened:

 1. Import a branch pointing to commit A from Git
 2. Modify the branch in jj to point to commit B
 3. Export the branch to Git
 4. Update the branch in Git to point to commit C
 5. Import refs from Git

In step 3, we forgot to update our record of the branch in the repo
view's `git_refs` field. That led to the import in step 5 to think
that the branch moved from A to C in Git, which conflicts with the
internal branch target of B.

This commit fixes the bug by updating the refs in the `MutableRepo`.

Closes #463.
martinvonz added a commit that referenced this issue Nov 13, 2022
This is a test case for #463. It's not exactly the same case, but I'm
confident that the root cause is the same (that the
`.jj/repo/git_export_operation_id` doesn't include the git refs we
just updated).
martinvonz added a commit that referenced this issue Nov 13, 2022
To fix #463, I think we want to skip conflicted branches when we
export instead of erroring out. It seems we didn't have test case for
the current behavior, so let's add one.
martinvonz added a commit that referenced this issue Nov 13, 2022
We had very little testing of the colocated case, so let's add a bit
more before I start working on this code in coming patches. This
includes a test for #463.
martinvonz added a commit that referenced this issue Nov 13, 2022
In order to fix #463, I'm going to make us export to Git a little
earlier, before finishing the transation. That means we won't have an
operation yet, but we don't need that anyway.
martinvonz added a commit that referenced this issue Nov 13, 2022
As I said in the previous patch, I don't know why I made the initial
export to Git a no-op. Exporting everything makes more sense to
(current-)me. It will make it slightly easier to skip exporting
conflicted branches (#463). It also lets us remove a `jj export` call
from `test_templater.rs`.
martinvonz added a commit that referenced this issue Nov 13, 2022
When we export branches to Git, we didn't update our own record of
Git's refs. This frequently led to spurious conflicts in these refs
(e.g. #463). This is typically what happened:

 1. Import a branch pointing to commit A from Git
 2. Modify the branch in jj to point to commit B
 3. Export the branch to Git
 4. Update the branch in Git to point to commit C
 5. Import refs from Git

In step 3, we forgot to update our record of the branch in the repo
view's `git_refs` field. That led to the import in step 5 to think
that the branch moved from A to C in Git, which conflicts with the
internal branch target of B.

This commit fixes the bug by updating the refs in the `MutableRepo`.

Closes #463.
martinvonz added a commit that referenced this issue Nov 13, 2022
This is a test case for #463. It's not exactly the same case, but I'm
confident that the root cause is the same (that the
`.jj/repo/git_export_operation_id` doesn't include the git refs we
just updated).
martinvonz added a commit that referenced this issue Nov 13, 2022
We had very little testing of the colocated case, so let's add a bit
more before I start working on this code in coming patches. This
includes a test for #463.
martinvonz added a commit that referenced this issue Nov 13, 2022
In order to fix #463, I'm going to make us export to Git a little
earlier, before finishing the transation. That means we won't have an
operation yet, but we don't need that anyway.
martinvonz added a commit that referenced this issue Nov 13, 2022
As I said in the previous patch, I don't know why I made the initial
export to Git a no-op. Exporting everything makes more sense to
(current-)me. It will make it slightly easier to skip exporting
conflicted branches (#463). It also lets us remove a `jj export` call
from `test_templater.rs`.
martinvonz added a commit that referenced this issue Nov 13, 2022
When we export branches to Git, we didn't update our own record of
Git's refs. This frequently led to spurious conflicts in these refs
(e.g. #463). This is typically what happened:

 1. Import a branch pointing to commit A from Git
 2. Modify the branch in jj to point to commit B
 3. Export the branch to Git
 4. Update the branch in Git to point to commit C
 5. Import refs from Git

In step 3, we forgot to update our record of the branch in the repo
view's `git_refs` field. That led to the import in step 5 to think
that the branch moved from A to C in Git, which conflicts with the
internal branch target of B.

This commit fixes the bug by updating the refs in the `MutableRepo`.

Closes #463.
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

2 participants