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.

Sign up for GitHub

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

Jump to bottom

jj close (and some others) sometimes crashes on first attempt with Git backend #27

Closed
martinvonz opened this issue Sep 13, 2021 · 3 comments · Fixed by #1049
Closed

jj close (and some others) sometimes crashes on first attempt with Git backend #27

martinvonz opened this issue Sep 13, 2021 · 3 comments · Fixed by #1049
Labels
🐛bug Something isn't working

Comments

@martinvonz
Copy link
Owner

jj prune, like most commands, first commit the working copy. When using the Git backend, that produces a Git commit with some hash. The next step for jj prune is to prune the commit. That's done by creating a successor commit with a is_pruned flag set. The problem is that both the predecessors field and the is_pruned field are stored outside of the Git commit and the timestamps are typically the same as well (it happens in much less than a second), so the resulting Git commit remains the same. The Git backend then fails the write (it doesn't allow two Git commits with different non-Git metadata).

I've known about this bug for a long time, but I figured it can be useful to track it. I'm working on removing support for evolution. The bug should be resolved by that work.
@martinvonz martinvonz added the 🐛bug Something isn't working label Sep 13, 2021
@martinvonz martinvonz mentioned this issue Sep 16, 2021
Closed
@martinvonz
Copy link
Owner Author

With the recent work on #32, the specific case of jj prune (now called jj abandon) shouldn't crash anymore. That was the most common way to run into the bug. However, jj close still suffers from the same problem: jj describe -m test; echo a >> a; jj close will almost always crash. At least it's pretty harmless and you can simply run jj close again after waiting a second.

@martinvonz martinvonz changed the title jj prune (and some others) often crashes on first attempt with Git backend jj prune (and some others) somtimes crashes on first attempt with Git backend Jan 21, 2022
@martinvonz martinvonz changed the title jj prune (and some others) somtimes crashes on first attempt with Git backend jj prune (and some others) sometimes crashes on first attempt with Git backend Jan 21, 2022
@martinvonz martinvonz mentioned this issue Feb 26, 2022
Closed
@martinvonz
Copy link
Owner Author

I'm not sure what the best fix for this is. One option is to catch the error and have some commands wait a bit and then retry so the user doesn't have to do it, but that's pretty ugly. A better solution is probably to give each commit created from jj its own native commit ID that's unrelated to the git commit ID. Then those commits would have the extra metadata and it would be fine for two of them to point to the same git commit. Then we'd need to display both commit IDs in e.g. log output, although the native commit ID would only be needed for the few cases where multiple native commits pointed to the same git commit.

@martinvonz martinvonz changed the title jj prune (and some others) sometimes crashes on first attempt with Git backend jj close (and some others) sometimes crashes on first attempt with Git backend Feb 26, 2022
@martinvonz martinvonz mentioned this issue Mar 28, 2022
Closed
martinvonz added a commit that referenced this issue Mar 29, 2022
@martinvonz
cli: fix crash when initializing workspace colocated with Git repo
36a1aa5 
When initializing a workspace that shares its working copy with a Git
repo (i.e. `jj init --git-repo=.`), we import refs and HEAD when
creating the `WorkspaceCommandHelper` (as we do for all commands when
the working copy is shared). That makes the explicit import we do in
`cmd_init()` unnecessary. It also makes the checkout of HEAD I added
for the fix of #102 unnecessary. More importantly, as @yuja reported
in #177, it makes the command crash (at least if the repo is small
enough that the two checkouts happen within a second). I think the
problem is that the second checkout tries to create the same commit
except that the Change ID is different (the problem is not the
predecessors as I speculated in the issue tracker). The fix is to
simply avoid doing the redundant work. We still need a proper fix for
#27 eventually.

Closes #177.
martinvonz added a commit that referenced this issue Mar 29, 2022
@martinvonz
cli: fix crash when initializing workspace colocated with Git repo
5a4f9ad 
When initializing a workspace that shares its working copy with a Git
repo (i.e. `jj init --git-repo=.`), we import refs and HEAD when
creating the `WorkspaceCommandHelper` (as we do for all commands when
the working copy is shared). That makes the explicit import we do in
`cmd_init()` unnecessary. It also makes the checkout of HEAD I added
for the fix of #102 unnecessary. More importantly, as @yuja reported
in #177, it makes the command crash (at least if the repo is small
enough that the two checkouts happen within a second). I think the
problem is that the second checkout tries to create the same commit
except that the Change ID is different (the problem is not the
predecessors as I speculated in the issue tracker). The fix is to
simply avoid doing the redundant work. We still need a proper fix for
#27 eventually.

Closes #177.
martinvonz added a commit that referenced this issue Mar 30, 2022
@martinvonz
cli: fix crash when initializing workspace colocated with Git repo
4072a1e 
When initializing a workspace that shares its working copy with a Git
repo (i.e. `jj init --git-repo=.`), we import refs and HEAD when
creating the `WorkspaceCommandHelper` (as we do for all commands when
the working copy is shared). That makes the explicit import we do in
`cmd_init()` unnecessary. It also makes the checkout of HEAD I added
for the fix of #102 unnecessary. More importantly, as @yuja reported
in #177, it makes the command crash (at least if the repo is small
enough that the two checkouts happen within a second). I think the
problem is that the second checkout tries to create the same commit
except that the Change ID is different (the problem is not the
predecessors as I speculated in the issue tracker). The fix is to
simply avoid doing the redundant work. We still need a proper fix for
#27 eventually.

Closes #177.
martinvonz added a commit that referenced this issue Mar 30, 2022
@martinvonz
cli: fix crash when initializing workspace colocated with Git repo
16c25c5 
When initializing a workspace that shares its working copy with a Git
repo (i.e. `jj init --git-repo=.`), we import refs and HEAD when
creating the `WorkspaceCommandHelper` (as we do for all commands when
the working copy is shared). That makes the explicit import we do in
`cmd_init()` unnecessary. It also makes the checkout of HEAD I added
for the fix of #102 unnecessary. More importantly, as @yuja reported
in #177, it makes the command crash (at least if the repo is small
enough that the two checkouts happen within a second). I think the
problem is that the second checkout tries to create the same commit
except that the Change ID is different (the problem is not the
predecessors as I speculated in the issue tracker). The fix is to
simply avoid doing the redundant work. We still need a proper fix for
#27 eventually.

Closes #177.
martinvonz added a commit that referenced this issue Mar 30, 2022
@martinvonz
cli: fix crash when initializing workspace colocated with Git repo
3a91bf1 
When initializing a workspace that shares its working copy with a Git
repo (i.e. `jj init --git-repo=.`), we import refs and HEAD when
creating the `WorkspaceCommandHelper` (as we do for all commands when
the working copy is shared). That makes the explicit import we do in
`cmd_init()` unnecessary. It also makes the checkout of HEAD I added
for the fix of #102 unnecessary. More importantly, as @yuja reported
in #177, it makes the command crash (at least if the repo is small
enough that the two checkouts happen within a second). I think the
problem is that the second checkout tries to create the same commit
except that the Change ID is different (the problem is not the
predecessors as I speculated in the issue tracker). The fix is to
simply avoid doing the redundant work. We still need a proper fix for
#27 eventually.

Closes #177.
martinvonz added a commit that referenced this issue Mar 30, 2022
@martinvonz
cli: fix crash when initializing workspace colocated with Git repo
729868d 
When initializing a workspace that shares its working copy with a Git
repo (i.e. `jj init --git-repo=.`), we import refs and HEAD when
creating the `WorkspaceCommandHelper` (as we do for all commands when
the working copy is shared). That makes the explicit import we do in
`cmd_init()` unnecessary. It also makes the checkout of HEAD I added
for the fix of #102 unnecessary. More importantly, as @yuja reported
in #177, it makes the command crash (at least if the repo is small
enough that the two checkouts happen within a second). I think the
problem is that the second checkout tries to create the same commit
except that the Change ID is different (the problem is not the
predecessors as I speculated in the issue tracker). The fix is to
simply avoid doing the redundant work. We still need a proper fix for
#27 eventually.

Closes #177.
martinvonz added a commit that referenced this issue Mar 31, 2022
@martinvonz
cli: fix crash when initializing workspace colocated with Git repo
c91aff2 
When initializing a workspace that shares its working copy with a Git
repo (i.e. `jj init --git-repo=.`), we import refs and HEAD when
creating the `WorkspaceCommandHelper` (as we do for all commands when
the working copy is shared). That makes the explicit import we do in
`cmd_init()` unnecessary. It also makes the checkout of HEAD I added
for the fix of #102 unnecessary. More importantly, as @yuja reported
in #177, it makes the command crash (at least if the repo is small
enough that the two checkouts happen within a second). I think the
problem is that the second checkout tries to create the same commit
except that the Change ID is different (the problem is not the
predecessors as I speculated in the issue tracker). The fix is to
simply avoid doing the redundant work. We still need a proper fix for
#27 eventually.

Closes #177.
martinvonz added a commit that referenced this issue Mar 31, 2022
@martinvonz
cli: fix crash when initializing workspace colocated with Git repo
9e3867a 
When initializing a workspace that shares its working copy with a Git
repo (i.e. `jj init --git-repo=.`), we import refs and HEAD when
creating the `WorkspaceCommandHelper` (as we do for all commands when
the working copy is shared). That makes the explicit import we do in
`cmd_init()` unnecessary. It also makes the checkout of HEAD I added
for the fix of #102 unnecessary. More importantly, as @yuja reported
in #177, it makes the command crash (at least if the repo is small
enough that the two checkouts happen within a second). I think the
problem is that the second checkout tries to create the same commit
except that the Change ID is different (the problem is not the
predecessors as I speculated in the issue tracker). The fix is to
simply avoid doing the redundant work. We still need a proper fix for
#27 eventually.

Closes #177.
martinvonz added a commit that referenced this issue Mar 31, 2022
@martinvonz
cli: fix crash when initializing workspace colocated with Git repo
d9273cf 
When initializing a workspace that shares its working copy with a Git
repo (i.e. `jj init --git-repo=.`), we import refs and HEAD when
creating the `WorkspaceCommandHelper` (as we do for all commands when
the working copy is shared). That makes the explicit import we do in
`cmd_init()` unnecessary. It also makes the checkout of HEAD I added
for the fix of #102 unnecessary. More importantly, as @yuja reported
in #177, it makes the command crash (at least if the repo is small
enough that the two checkouts happen within a second). I think the
problem is that the second checkout tries to create the same commit
except that the Change ID is different (the problem is not the
predecessors as I speculated in the issue tracker). The fix is to
simply avoid doing the redundant work. We still need a proper fix for
#27 eventually.

Closes #177.
martinvonz added a commit that referenced this issue Mar 31, 2022
@martinvonz
cli: fix crash when initializing workspace colocated with Git repo
034fbb4 
When initializing a workspace that shares its working copy with a Git
repo (i.e. `jj init --git-repo=.`), we import refs and HEAD when
creating the `WorkspaceCommandHelper` (as we do for all commands when
the working copy is shared). That makes the explicit import we do in
`cmd_init()` unnecessary. It also makes the checkout of HEAD I added
for the fix of #102 unnecessary. More importantly, as @yuja reported
in #177, it makes the command crash (at least if the repo is small
enough that the two checkouts happen within a second). I think the
problem is that the second checkout tries to create the same commit
except that the Change ID is different (the problem is not the
predecessors as I speculated in the issue tracker). The fix is to
simply avoid doing the redundant work. We still need a proper fix for
#27 eventually.

Closes #177.
@martinvonz martinvonz mentioned this issue Apr 23, 2022
Merged
@martinvonz martinvonz mentioned this issue Oct 17, 2022
Closed
@martinvonz
Copy link
Owner Author

In #635, we saw that e.g. jj duplicate; jj describe -m 'new description'-r @- is another easy way to trigger this bug.

@martinvonz martinvonz mentioned this issue Nov 1, 2022
Closed
martinvonz added a commit that referenced this issue Nov 14, 2022
@martinvonz
demos: allow running without ui.allow-init-native = true
e9504e9 
We need to use the native backend here because of #27, but we
shouldn't require the user running the script to have it in their
config.
@martinvonz martinvonz mentioned this issue Nov 14, 2022
Merged
2 tasks
martinvonz added a commit that referenced this issue Nov 14, 2022
@martinvonz
demos: allow running without ui.allow-init-native = true
f86455a 
We need to use the native backend here because of #27, but we
shouldn't require the user running the script to have it in their
config.
@martinvonz martinvonz mentioned this issue Jan 16, 2023
Merged
3 tasks
ilyagr added a commit that referenced this issue Jan 17, 2023
@ilyagr
Git backend: Allow simultaneous rebasing of duplicate commits
9bdd545 
Fixes #27
Fixes #694
ilyagr added a commit that referenced this issue Jan 18, 2023
@ilyagr
Git backend: Allow simultaneous rebasing of duplicate commits
3729fe0 
Fixes #27
Fixes #694
ilyagr added a commit that referenced this issue Jan 18, 2023
@ilyagr
Git backend: Allow simultaneous rebasing of duplicate commits
12ee2b1 
Fixes #27
Fixes #694
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
🐛bug Something isn't working
Projects
None yet
Milestone
No milestone
1 participant
@martinvonz