Bug 546455 - branch node label not updated after rebase
Summary: branch node label not updated after rebase
Status: RESOLVED FIXED
Alias: None
Product: EGit
Classification: Technology
Component: UI (show other bugs)
Version: 5.4   Edit
Hardware: PC Windows 10
: P3 normal (vote)
Target Milestone: 5.7   Edit
Assignee: Thomas Wolf CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-04-15 17:35 EDT by Michael Keppler CLA
Modified: 2019-12-15 09:32 EST (History)
1 user (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Keppler CLA 2019-04-15 17:35:06 EDT
I've meanwhile noticed the following behaviour at least 2 times: I've rebased a local branch onto origin/master and afterwards it is not shown in history, when clicking on that branch (I have link with selection active, so this works always, just not for the rebased branch). There are no workspace builds running.

Now that I noticed this the second time I also checked the labels in the repo view. Normally a rebased branch should not have the same SHA1 as the ORIG_HEAD (before the rebase), right? I suspect the label provider does not recognize the "changed commit" from the rebase for some reason, and therefore the branch node is not updated, and history view sync fails.

Side note: Now that I have written the bug report, the node label _has_ refreshed (after roughly 5 minutes) and everything works as before. What should I investigate next time this happens?
Comment 1 Thomas Wolf CLA 2019-04-16 05:37:18 EDT
Could it be that the rebase triggered a long-running build, which may delay the IndexDiff update until after that build?
Comment 2 Thomas Wolf CLA 2019-12-01 10:49:11 EST
See that too, now. In one particular setup only, though, and only if certain views and editors are open. But then it is at least 100% reproducible for me, so I can actually debug it and if I come up with a fix check that it works. Though I have so far been unsuccessful in trying to write an SWTBot test that would fail for the case I'm seeing.

The problem is not with the various caching that we do; we clear and reset those caches correctly, and the new Ref object pointing to the new commit is also added correctly to the Tree.

This is a problem with the decorator in the RepositoriesView. The DecoratorManager has its own cache of already produced decorations, and because we compare Refs for equality only by name, it finds the old decoration and never schedules a new decoration request for the new Ref, and thus we end up with the wrong decoration being shown.

Changing the RefNode equals/hashcode to also consider the target SHA1 fixes this problem, but leads to flickering - the decoration briefly disappears before the new decoration re-appears.

The fix is not going to be simple; there's something wrong in the whole setup of that label provider/lightweight decorator for RepositoryTreeNodes. We'll have to revisit this again and try to figure out how to set this up correctly such that this doesn't occur and we don't get flickering.

Probably the label provider in the view should not be a decorating label provider at all.
Comment 3 Eclipse Genie CLA 2019-12-14 06:12:43 EST
New Gerrit change created: https://git.eclipse.org/r/154526
Comment 4 Thomas Wolf CLA 2019-12-14 06:24:20 EST
Now that the label provider and decorator setup in the repositories view has been simplified and fixed (see bug 553841) it's much easier to see what is happening here.

In my case I can reproduce the problem by simply resetting the current branch (reset --hard) between two commits. What happens is quite simple. On a hard reset, we get two events: a RefsChangedEvent, followed by an IndexDiffChangedEvent.

On the RefsChangedEvent, the content provider clears its ref cache and the repositories view schedules a refresh after 300ms.

Then the IndexDiffChangedEvent occurs. The GitDecorator schedules a job that fires a LabelProviderChangedEvent after 100ms.

The LabelProviderChangedEvent occurs, the DecoratorManager clears the cached labels for this decorator, and propagates the event to the CommonViewer. The CommonViewer refreshes all labels (but only the labels, it doesn't call the content provider to get structural changes, so the nodes remain the same and in particular ref nodes still have their old target ObjectId). The decorator is invoked, and computes a new decoration based on the ObjectId stored in the node. This decoration is shown.

Finally, the refresh job that was scheduled for the RefsChangedEvent runs and does a full refresh of the CommonViewer. The content provider re-loads its ref cache and produces a new ref node with the correct new target ObjectId. The label provider calls the decorator manager, the decorator manager finds the (now outdated) decoration computed in response to the LabelProviderChangedEvent and returns that.

End result: we have the wrong decoration in the view.

The above change fixes this by making the decorator not use the Ref stored in the RefNode but the one in the ref cache. Content provider and decorator use the same ref cache instance.

Another way to fix this would have been to make equals() for Ref nodes always also consider the ObjectId being pointed to, but that gives a perceptible delay between the old decoration disappearing and the new one appearing.
Comment 5 Eclipse Genie CLA 2019-12-15 09:15:15 EST
Gerrit change https://git.eclipse.org/r/154526 was merged to [master].
Commit: http://git.eclipse.org/c/egit/egit.git/commit/?id=9244bc84355894bc353ba374978f1f92d49d81de
Comment 6 Michael Keppler CLA 2019-12-15 09:32:00 EST
I'm closing as resolved. Will take some time to verify in daily work, so I would rather re-open than leave it open for long time.