Fix bug where a forgotten branch couldn't be fetched (v1) #1714

ilyagr · 2023-06-19T23:50:35Z

The fix is slightly messy, but I don't know a better way without reworking
the logic of jj git fetch and jj git push in-depth.

This bug was originally reported by @arxanas in a Discord discussion.

Summary:

How do I proceed here to check out the remote branch that I know exists?

$ jj git fetch --branch 'arxanas/fsmonitor' --remote origin
Nothing changed.
$ jj checkout arxanas/fsmonitor                            
Error: Revision "arxanas/fsmonitor" doesn't exist
$ jj checkout arxanas/fsmonitor@origin
Error: Revision "arxanas/fsmonitor@origin" doesn't exist
$ git show origin/arxanas/fsmonitor                        
commit https://github.com/ilyagr/jj/commit/bac43af9fbaade36a2374f4b2a7457ca4caa05ee (origin/arxanas/fsmonitor, refs/jj/keep/bac43af9fbaade36a2374f4b2a7457ca4caa05ee)
Author: Waleed Khan <[email protected]>
Date:   Fri Jun 10 18:58:21 2022 -0700

    fsmonitor: add `.watchmanconfig` to repo
    
    This identifies the directory as Watchman-enabled. Additional config settings can go in this file.

diff --git a/.watchmanconfig b/.watchmanconfig
new file mode 100644
index 00000000..0967ef42
--- /dev/null
+++ b/.watchmanconfig
@@ -0,0 +1 @@
+{}

Checklist

If applicable:

I have updated CHANGELOG.md
(n/a) I have updated the documentation (README.md, docs/, demos/)
(n/a) I have updated the config schema (src/config-schema.json)
I have added tests to cover my changes

yuja

(Not finished reviewing yet. I'll take a look later today or tomorrow.)

yuja · 2023-06-20T02:55:28Z

lib/src/git.rs

-        import_refs(mut_repo, git_repo, git_settings)
+        import_some_refs(mut_repo, git_repo, git_settings, |_| {
+            ImportMethod::MergeOrResurrect
+        })


I think it's equally annoying if forgotten branches are resurrected by jj git fetch.

It might be nice if we had a set of "tracked" remote branches. No local branch will be re-created for forgotten (or "untracked") remote branch. And to re-track such branch, user will run jj branch track.

I think it's equally annoying if forgotten branches are resurrected by jj git fetch.

I'm going to rework this a bit so that only remote-tracking branches are imported at this point.

I'm not sure whether that addresses your question at all; the question could be in regard to several things, but for most of the others I wouldn't expect it to show up at this line of code.

It might be nice if we had a set of "tracked" remote branches. No local branch will be re-created for forgotten (or "untracked") remote branch. And to re-track such branch, user will run jj branch track.

This is a very interesting idea. One possible conclusion from this PR and all the jj branch forget tests I'm adding is that jj branch forget is a bad command to have, and some other abstraction could be better. I haven't thought your specific idea through entirely, but it seems to match what I usually want to do better.

But until we do that, I think jj git fetch should behave as I described. The difference between it and jj git import is that jj git fetch takes glob arguments and is not run automatically in colocated repos.

I'm going to rework this a bit so that only remote-tracking branches are imported at this point.

This is now more or less done. I hoped to go further and make fetch actually not touch other branches, but I had a test failure with ilyagr@tmp_onlyremotes. I couldn't immediately decide whether it's safe to change the corresponding test. Here is the portion until the failing assertion:
https://github.com/martinvonz/jj/blob/8a8b96a448a949c50e40966ead8eac0c730ce158/lib/tests/test_git.rs#L1561-L1579

I mean I want jj branch forget persist as much as possible. The current behavior isn't great, but it doesn't resurrect forgotten (i.e. uninteresting) branch so long as the remote is unchanged.

Perhaps, I would probably want jj git fetch that doesn't import most of the remote branches. I'd like to get remote changes, but doesn't want to maintain branches they have.

jj branch forget is a bad command to have, and some other abstraction could be better.

I agree.

It might be nice if we had a set of "tracked" remote branches. No local branch will be re-created for forgotten (or "untracked") remote branch. And to re-track such branch, user will run jj branch track.

[...]

Perhaps, I would probably want jj git fetch that doesn't import most of the remote branches. I'd like to get remote changes, but doesn't want to maintain branches they have.

I've also felt this way: I don't want to logically import a given remote branch without explicitly asking for it.

Here's my write-up: #1734. It is very much a draft, it's quite possible that I missed something important, and I'm happy to change it, or to throw it out if we decide on a different approach (or switch to more git-like branch model).

I mainly used it when I accidentally fetched too many branches

Isn't undo a better option in that case? I guess undo wouldn't work at least for colocated repo, but I think it's the same kind of problem we've discussed in the other jj undo thread.

As for what gets fixed/merged first, I'm unsure what the best option is.

Does this forget + fetch issue need an immediate fix? I feel it's not serious bug, but I might be wrong.

Here's my write-up: #1734

Thanks!

Isn't undo a better option in that case? I guess undo wouldn't work at least for colocated repo, but I think it's the same kind of problem we've discussed in the other jj undo thread.

Sometimes a bit of time has passed and it's hard to find in the op-log.

Sometimes it might be easier to fetch all branches and then forget some globs than to fetch some globs.

More generally, I think that, as far as possible, anything undo can do should be doable by regular commands for similar reasons (at least, that I can think of now): it may be hard to search the oplog or you might want to partially undo something. I'm not sure how important this principle is to me, but this situation feels like it should apply.

I guess undo wouldn't work at least for colocated repo

Aargh. At the moment, I have more emotions than words for this.

I actually think it might work and we might even have a test for it, but I need to double-check. If not, I should add a test to #1700 to see what happens, at least.

Does this forget + fetch issue need an immediate fix? I feel it's not serious bug, but I might be wrong.

I felt like the situation from Discord that I put in the summary is quite a confusing situation for a user to end up in. I find it hard to weigh that against you (and potentially other people) who may be relying on the current behavior.

Another use-case for branch forget I just came upon: you did a jj branch delete and realized that you don't want to destroy the branch on push. I just remembered it after looking at #1537.

I think that, as far as possible, anything undo can do should be doable by regular commands

Yes, I agree about that. It might be a maintenance command like jj git forget-refs or option to ignore known git_refs state, but I don't know.

lib/src/git.rs

ilyagr · 2023-06-20T04:13:22Z

Not sure where to mention this, but another thing I was considering is having
jj git import --resurrect-branches-from-remote that could resurrect branches from the git repo's remote-tracking branches without running the actual fetch. I didn't do it because the UI seems confusing, but OTOH that would more directly address the use-case.

ilyagr · 2023-06-21T00:45:14Z

@arxanas, @yuja, @martinvonz I'm not sure whether I'll have time for this in the near future (especially since I'm itching to be done with #1700), but here are some thoughts (an RFC?) about a redesign the forget command into something closer to @yuja's idea.

@yuja suggested track and untrack commands, but I think we could also stick with forget and a new remember command. At least, that what I do for the purposes of this message. jj branch forget would act similarly to what it does now, but it would also put the branch on a new list of forgotten branches. The list would be stored in a new field in jj's View proto. jj git fetch and jj git import would know not to fetch those. jj branch remember would simply remove the branch from the list.

It's not clear to me whether the list of forgotten branches should list branches or globs. E.g., should jj branch forget --branch 'dontcare/*' forget all the branches that currently exist and match the glob, or all branches matching the glob now or in the future. In the latter case, we might need to introduce commands to check if there are any unexpected branches in the list of forgotten branches.

A technical point: in colocated repos, I'd want jj git fetch to ignore forgotten branches entirely. Is there a way to prevent it from updating the git repo's remote-tracking branches in the forgotten list? Can libgit2 take a glob or list of branches not to fetch?

I'm not sure how long it'd take it to settle on a design and implement it, and whether it's worth waiting for that or whether it's better to call that a long-term plan.

ilyagr · 2023-06-22T04:04:39Z

I tested out what happens when fetching a forgotten branch that moved on the remote. It indeed creates a conflict. This is part of the tests in this PR now.

It also turns out that my fix doesn't actually fix that part of the problem. I need to think about how big of a problem that is. It wouldn't be a frequently encountered bug (especially since I don't think our UI shows the difference between a normal branch and a move-deletion branch conflict), but it would be confusing to those who notice it.

lib/src/git.rs

@arxanas

The fix is slightly messy, but I don't know a better way without reworking the logic of `jj git fetch` and `jj git push` in-depth. This bug was originally reported by @arxanas in a [Discord discussion]. [Discord discussion]: https://discord.com/channels/968932220549103686/969829516539228222/1114997445949136936 Summary: > How do I proceed here to check out the remote branch that I know exists? > > ``` > $ jj git fetch --branch 'arxanas/fsmonitor' --remote origin > Nothing changed. > $ jj checkout arxanas/fsmonitor > Error: Revision "arxanas/fsmonitor" doesn't exist > $ jj checkout arxanas/fsmonitor@origin > Error: Revision "arxanas/fsmonitor@origin" doesn't exist > $ git show origin/arxanas/fsmonitor > commit bac43af (origin/arxanas/fsmonitor, refs/jj/keep/bac43af9fbaade36a2374f4b2a7457ca4caa05ee) > Author: Waleed Khan <[email protected]> > Date: Fri Jun 10 18:58:21 2022 -0700 > > fsmonitor: add `.watchmanconfig` to repo > > This identifies the directory as Watchman-enabled. Additional config settings can go in this file. > > diff --git a/.watchmanconfig b/.watchmanconfig > new file mode 100644 > index 00000000..0967ef42 > --- /dev/null > +++ b/.watchmanconfig > @@ -0,0 +1 @@ > +{} > ```

ilyagr · 2023-06-26T04:44:42Z

I'll reopen this if it seems more urgent, but otherwise I'm hoping we'll either have a minimal version of this that doesn't mess up Yuya's usecase or we'll have a full-blown redesign as discussed in #1734.

This is a bit unwieldy; see a comment in the code for a detailed explanation. We'll have to decide whether we prefer this, or we prefer the approach of jj-vcs#1714, or we have a better idea.

ilyagr · 2023-07-03T04:51:01Z

This PR is obsoleted by #1771.

ilyagr mentioned this pull request Jun 19, 2023

FR: Allow users to see and address git repo refs as @git branches #1666

Closed

4 tasks

ilyagr force-pushed the forget-fetch branch 2 times, most recently from 7ec4905 to e0ddf77 Compare June 19, 2023 23:55

ilyagr marked this pull request as ready for review June 19, 2023 23:57

ilyagr force-pushed the forget-fetch branch from e0ddf77 to 19ca8a5 Compare June 20, 2023 00:01

ilyagr changed the title ~~Forget fetch~~ Fix bug where a forgotten branch couldn't be fetched Jun 20, 2023

ilyagr force-pushed the forget-fetch branch 7 times, most recently from 56125fc to fc9a791 Compare June 20, 2023 01:17

yuja reviewed Jun 20, 2023

View reviewed changes

ilyagr force-pushed the forget-fetch branch from fc9a791 to 0c1c5ac Compare June 20, 2023 03:54

ilyagr force-pushed the forget-fetch branch from 0c1c5ac to 32c6643 Compare June 22, 2023 04:02

ilyagr force-pushed the forget-fetch branch from 32c6643 to 4557aa0 Compare June 22, 2023 04:21

ilyagr commented Jun 22, 2023

View reviewed changes

lib/src/git.rs Outdated Show resolved Hide resolved

ilyagr marked this pull request as draft June 23, 2023 05:23

ilyagr force-pushed the forget-fetch branch from 4557aa0 to 22c1641 Compare June 24, 2023 00:10

ilyagr marked this pull request as ready for review June 24, 2023 00:14

This was referenced Jun 24, 2023

FR: better design for ignoring branches #1734

Closed

branch cli: additional tests and allow defusing deleted branches #1735

Merged

ilyagr force-pushed the forget-fetch branch 2 times, most recently from 5584286 to de95b6d Compare June 24, 2023 16:28

ilyagr force-pushed the forget-fetch branch from de95b6d to db8df67 Compare June 24, 2023 20:43

ilyagr marked this pull request as draft June 26, 2023 04:44

ilyagr mentioned this pull request Jun 29, 2023

Fix bug where a forgotten branch couldn't be fetched (v2) #1771

Merged

2 tasks

ilyagr changed the title ~~Fix bug where a forgotten branch couldn't be fetched~~ Fix bug where a forgotten branch couldn't be fetched (v1) Jul 1, 2023

ilyagr closed this Jul 3, 2023

chenrui333 mentioned this pull request Jul 16, 2023

jj 0.8.0 Homebrew/homebrew-core#136764

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix bug where a forgotten branch couldn't be fetched (v1) #1714

Fix bug where a forgotten branch couldn't be fetched (v1) #1714

ilyagr commented Jun 19, 2023 •

edited

Loading

yuja left a comment

yuja Jun 20, 2023

ilyagr Jun 20, 2023 •

edited

Loading

ilyagr Jun 20, 2023 •

edited

Loading

yuja Jun 20, 2023

arxanas Jun 20, 2023

ilyagr Jun 24, 2023 •

edited

Loading

yuja Jun 24, 2023

ilyagr Jun 24, 2023

ilyagr Jun 24, 2023

yuja Jun 24, 2023

ilyagr commented Jun 20, 2023

ilyagr commented Jun 21, 2023 •

edited

Loading

ilyagr commented Jun 22, 2023 •

edited

Loading

ilyagr commented Jun 26, 2023

ilyagr commented Jul 3, 2023

Fix bug where a forgotten branch couldn't be fetched (v1) #1714

Fix bug where a forgotten branch couldn't be fetched (v1) #1714

Conversation

ilyagr commented Jun 19, 2023 • edited Loading

Checklist

yuja left a comment

Choose a reason for hiding this comment

yuja Jun 20, 2023

Choose a reason for hiding this comment

ilyagr Jun 20, 2023 • edited Loading

Choose a reason for hiding this comment

ilyagr Jun 20, 2023 • edited Loading

Choose a reason for hiding this comment

yuja Jun 20, 2023

Choose a reason for hiding this comment

arxanas Jun 20, 2023

Choose a reason for hiding this comment

ilyagr Jun 24, 2023 • edited Loading

Choose a reason for hiding this comment

yuja Jun 24, 2023

Choose a reason for hiding this comment

ilyagr Jun 24, 2023

Choose a reason for hiding this comment

ilyagr Jun 24, 2023

Choose a reason for hiding this comment

yuja Jun 24, 2023

Choose a reason for hiding this comment

ilyagr commented Jun 20, 2023

ilyagr commented Jun 21, 2023 • edited Loading

ilyagr commented Jun 22, 2023 • edited Loading

ilyagr commented Jun 26, 2023

ilyagr commented Jul 3, 2023

ilyagr commented Jun 19, 2023 •

edited

Loading

ilyagr Jun 20, 2023 •

edited

Loading

ilyagr Jun 20, 2023 •

edited

Loading

ilyagr Jun 24, 2023 •

edited

Loading

ilyagr commented Jun 21, 2023 •

edited

Loading

ilyagr commented Jun 22, 2023 •

edited

Loading