Community
Participate
Working Groups
Created attachment 285847 [details] Video showing the problem, using reproduction steps from the description. We have a convoluted case in our product, where a user has submodules and doesn't see their changes in the Git Staging view. See attached video (reproduction steps are simple once everything is set up, but lengthy): "git_submodule_changed_file_not_staged.mp4" Steps to reproduce: 1. Extract repository from attached archive "SubmoduleRepository.zip". 2. Import plug-in from attached archive "ExampleTestGitSubmodules.zip". 3. Debug Eclipse with git auto-share enabled, for workspace use the root of the repository extracted in 1.. 4. Several projects are opened by the plug-in from step 2. (this "simulates" a part of our product). 5. Navigate to file in Package Explorer and open it: "submodulepath2/somefolder/somepackage/somefile.txt" 6. Switch to the Git perspective. 6.1. In the Git Repositories view, select the submodule: "submodulepath2" 6.2. Open/Show the Git Staging view. 7. Switch back to the Java perspective. 8. Modify the file: "somefile.txt". 8.1. Observe that the modified/dirty symbol ">" is not shown in the Package Explorer. 9. Switch back to the Git perspective. 9.1. Observe that the submodule "submodulepath2" is not shown as modified/dirty in the Git Repositories view. 9.2. Observe that the changed file "somefile.txt." is not shown in the Git Staging view. 9.3. Use Compare With -> HEAD revision in the editor for "somefile.txt", observe that the index pane is empty and shows no file history. Seen in our product based on Eclipse 4.15 and EGit 4.7, but also seen with latest JGit and EGit master branches on Eclipse 4.19 (I20210210-1800).
Created attachment 285848 [details] Plug-in that opens workspace projects, necessary for reproduction of the problem. See reproduction steps.
Created attachment 285849 [details] Repository with submodules, necessary for reproduction of the problem. See reproduction steps.
Note that if the projects in the repository are imported, as opposed to created+opened (see plug-in code), the problem is not seen. From what I've debugged, during org.eclipse.jgit.lib.IndexDiff.diff(ProgressMonitor, int, int, String, RepositoryBuilderFactory), the IndexDiff.modified set gets an entry with the top of the submodule "submodulepath2/somefolder" as opposed to the file that actually changed. I assume the unwanted behaviour from the description comes directly from that. I'll try to understand what IndexDiff.diff() is doing, any tips or help are welcome.
Created attachment 285851 [details] Video showing the problem, using reproduction steps from the description. Attached a better video, in the original I was selecting the wrong submodule.
> From what I've debugged, during > org.eclipse.jgit.lib.IndexDiff.diff(ProgressMonitor, int, int, String, > RepositoryBuilderFactory), the IndexDiff.modified set gets an entry with the > top of the submodule "submodulepath2/somefolder" as opposed to the file that > actually changed. I assume the unwanted behaviour from the description comes > directly from that. To be more specific, I see two calls. The first call adds the expected path to the "modified" set: "Worker-10: Updating Git status for repository ..." #57 prio=5 os_prio=0 cpu=91.14ms elapsed=74.19s tid=0x00007ffe20005800 nid=0x45e6 at breakpoint [0x00007ffe4a5f4000] java.lang.Thread.State: RUNNABLE at org.eclipse.jgit.lib.IndexDiff.diff(IndexDiff.java:554) at org.eclipse.jgit.lib.IndexDiff.diff(IndexDiff.java:379) at org.eclipse.jgit.lib.IndexDiff.diff(IndexDiff.java:603) at org.eclipse.egit.core.internal.indexdiff.IndexDiffCacheEntry.lambda$3(IndexDiffCacheEntry.java:629) at org.eclipse.egit.core.internal.indexdiff.IndexDiffCacheEntry$$Lambda$707/0x00000008408f6840.get(Unknown Source) at org.eclipse.egit.core.UnitOfWork.run(UnitOfWork.java:115) at org.eclipse.egit.core.internal.indexdiff.IndexDiffCacheEntry.calcIndexDiffDataIncremental(IndexDiffCacheEntry.java:620) at org.eclipse.egit.core.internal.indexdiff.IndexDiffCacheEntry.access$13(IndexDiffCacheEntry.java:606) at org.eclipse.egit.core.internal.indexdiff.IndexDiffCacheEntry$5.updateIndexDiff(IndexDiffCacheEntry.java:552) at org.eclipse.egit.core.internal.indexdiff.IndexDiffUpdateJob.run(IndexDiffUpdateJob.java:77) at org.eclipse.core.internal.jobs.Worker.run(Worker.java:63) Then another call is made, which adds the submodule root path: "Worker-10: Updating Git status for repository ..." #57 prio=5 os_prio=0 cpu=91.43ms elapsed=189.24s tid=0x00007ffe20005800 nid=0x45e6 at breakpoint [0x00007ffe4a5f4000] java.lang.Thread.State: RUNNABLE at org.eclipse.jgit.lib.IndexDiff.diff(IndexDiff.java:613) at org.eclipse.egit.core.internal.indexdiff.IndexDiffCacheEntry.lambda$3(IndexDiffCacheEntry.java:629) at org.eclipse.egit.core.internal.indexdiff.IndexDiffCacheEntry$$Lambda$707/0x00000008408f6840.get(Unknown Source) at org.eclipse.egit.core.UnitOfWork.run(UnitOfWork.java:115) at org.eclipse.egit.core.internal.indexdiff.IndexDiffCacheEntry.calcIndexDiffDataIncremental(IndexDiffCacheEntry.java:620) at org.eclipse.egit.core.internal.indexdiff.IndexDiffCacheEntry.access$13(IndexDiffCacheEntry.java:606) at org.eclipse.egit.core.internal.indexdiff.IndexDiffCacheEntry$5.updateIndexDiff(IndexDiffCacheEntry.java:552) at org.eclipse.egit.core.internal.indexdiff.IndexDiffUpdateJob.run(IndexDiffUpdateJob.java:77) at org.eclipse.core.internal.jobs.Worker.run(Worker.java:63) E.g. when the decorations are being computed, the IndexDiff of the 2nd call is being used (that has wrong contents in "modified").
This change seems to fix the missing modified/dirty indicator in the Package Explorer, but not the other problems: diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/IndexDiff.java b/org.eclipse.jgit/src/org/eclipse/jgit/lib/IndexDiff.java index 28ea927b1..e3034bdd6 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/IndexDiff.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/IndexDiff.java @@ -611,6 +611,12 @@ public boolean diff(ProgressMonitor monitor, int estWorkTreeSize, continue; } modified.add(subRepoPath); + for (String submoduleModified : smid.getModified()) { + String modifiedPath = subRepoPath + + File.separatorChar + + submoduleModified; + modified.add(modifiedPath); + } recordFileMode(subRepoPath, FileMode.GITLINK); } Since the other problems remain, I'm guessing this is not the right place for the fix. Next, I'll check why the index version of the changed file is missing.
OK it seems its not necessary to do weird work on start-up. Its enough to import the submodule project from the Git Repositories view, after adding the parent repository to the view (then right-click and import projects). The problem also persists after a restart, so its not only in a fresh workspace (as I assumed so far). In the bad case for comparing with HEAD, CompareUtils.compareWorkspaceWithRef() computes the path of the resource relative to the parent repository and not relative to the submodule repository. Andrey can you add an EGit maintainer to the CC list? This might be trivial for them to understand (e.g. after looking at the .gitmodules file). I'll keep looking into the EGit/JGit code meanwhile.
(In reply to Simeon Andreev from comment #7) > Andrey can you add an EGit maintainer to the CC list? I was one, but you probably meant *active* one - adding Thomas :-).
(In reply to Andrey Loskutov from comment #8) > I was one, but you probably meant *active* one - adding Thomas :-). You still are one. Would be nice if found time to be more active here again ;-)
(In reply to Simeon Andreev from comment #6) > This change seems to fix the missing modified/dirty indicator in the Package > Explorer, but not the other problems: > > diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/lib/IndexDiff.java > b/org.eclipse.jgit/src/org/eclipse/jgit/lib/IndexDiff.java > index 28ea927b1..e3034bdd6 100644 > --- a/org.eclipse.jgit/src/org/eclipse/jgit/lib/IndexDiff.java > +++ b/org.eclipse.jgit/src/org/eclipse/jgit/lib/IndexDiff.java > @@ -611,6 +611,12 @@ public boolean diff(ProgressMonitor monitor, int > estWorkTreeSize, > > continue; > } > > modified.add(subRepoPath); > + for > (String submoduleModified : smid.getModified()) { > + > String modifiedPath = subRepoPath > + > + File.separatorChar > + > + submoduleModified; > + > modified.add(modifiedPath); > + } > > recordFileMode(subRepoPath, > > FileMode.GITLINK); > } > > Since the other problems remain, I'm guessing this is not the right place > for the fix. > > Next, I'll check why the index version of the changed file is missing. No, that's definitely not the right place. Besides, note that in git the file separator is always '/', not File.separatorChar. Don't know when I'll have time to dig into this. It's been a while since I had to deal with submodules. Just from the looks of it I wonder what RepositoryMappings there are. Looks like there's one missing. In the project explorer, submodulepath2/somefolder should have a submodule decoration "[somefolder master]". But I'll have to download the zips and try for real to see what's going on.
You have the submodules physically nested in the main repo. That is a rather strange setup. (Or perhaps it's just old? IIRC old git did it that way...) Your main repo index only records the git links, so if it were a bare repo hosted on some server, I think already cloning it (recursively) might be a challenge. Or are the "./..." submodule URLs only for demo purposes? That said, I have a fix for your woes. The real problem is twofold: 1. IResource.touch() in ConnectProviderOperation.touchGitResources() does not have the desired effect and does not include FOLDERs in the subsequent resource delta. 2. ConnectProviderOperation.touchGitResources() is not called always. As a result, there's no resource change event including these .git directories, and thus GitDataProvider never picks up the nested repositories.
New Gerrit change created: https://git.eclipse.org/r/c/egit/egit/+/177864
(In reply to Thomas Wolf from comment #11) > You have the submodules physically nested in the main repo. That is a rather > strange setup. (Or perhaps it's just old? IIRC old git did it that way...) > Your main repo index only records the git links, so if it were a bare repo > hosted on some server, I think already cloning it (recursively) might be a > challenge. Or are the "./..." submodule URLs only for demo purposes? The submodules URLs in the attached example are for showcasing only. The original git repository, with which the problem in out product was reported, has contents in .gitmodules that are more or less: [submodule "path1/path12"] path = path1/path12 url = ssh://... [submodule "path2"] path = path2 url = ssh://... [submodule "path3/path31"] path = path3/path31 url = ssh://... > That said, I have a fix for your woes. Many thanks! With patch set 3 of https://git.eclipse.org/r/c/egit/egit/+/177864, the problem is gone in our product.
Gerrit change https://git.eclipse.org/r/c/egit/egit/+/177864 was merged to [master]. Commit: http://git.eclipse.org/c/egit/egit.git/commit/?id=ac9321680de575e3e98f5e232bd0f5a02b20ec5b
Thanks for the fix!
