Skip to main content

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Re: [egit-dev] Logical model support in EGit

Hi,

Continuing on this effort, a number of patches that make progress in this direction have been contributed and merged. For the most part, they were patches I made as preparative work : take ancestor into account, use accurate revision instead of local data for the "left" side... I have now started to work on the real need; using the models when comparing. This requires me to modify all of the comparison action handlers ... and there are a lot.

All of the things that can launch a comparison (action handlers, history view, commit viewer...) use a different code to launch the comparison. I plan on factorizing all of this into a single entry point, removing duplicated code in the process. However, I have some trouble with the comparisons "with index" (either as left or right side of the comparison). I just pushed a draft version of this work on https://git.eclipse.org/r/11966 . This is not in a state where it can be properly reviewed (some actions are not yet refactored... and the tests need to be changed to take the new behavior into account), but I'd like some input on two points in particular :

CompareUtils (https://git.eclipse.org/r/#/c/11966/1/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/CompareUtils.java) line 554 :
I placed a TODO there... since I really don't know : is there a point in trying to find a common ancestor between "Index" and .... something? The two comparison actions we have is "compare Index with HEAD" and "Compare with Index". So the two only things we can get there with are HEAD or the working tree. IIUC, there is no real meaning in trying to find a common ancestor there, which would be either HEAD (compare index with HEAD) or the Index (compare with Index)... since the common ancestor is equal to one of the two compared versions, there are no possible conflicts, and thus no reason to use 3-way rather than 2-way comparison. Am I right in these assumptions?

GitModelSynchronize (https://git.eclipse.org/r/#/c/11966/1/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/synchronize/GitModelSynchronize.java) line 110 :
This one has been much more of a headache... How could I make it possible to launch a synchronization between "Index" and any other revision? Synchronization uses a GitSynchronizeData as input. The GitSynchronizeData uses RevCommits to describe the revisions to compare. However... there are no RevCommit or commit ID corresponding to the index version. In CompareUtils I have used GitFileRevision.INDEX as the "magic" string to use in order to compare with the index (though that in itself is a problem since the user could have a branch named "Index"... CompareTreeView.INDEX_VERSION is probably a better choice), but that isn't an option in the GitSynchronizeData. What would be the better approach? Add a "useIndex" boolean such as GitSynchronizeData.includeLocal and change every place that uses the GitSynchronizeData?  Or is there a less intrusive way? Am I even right in assuming that there is no RevCommit or special identifier that corresponds to the index and could be used?

Thanks in advance for any input on this.

Laurent Goubet
Obeo

On 15/02/2013 10:09, Laurent Goubet wrote:
Hi all,

I am currently working on the integration with EGit and the model providers from Eclipse Team. The basic need is that some actions executed on the repository content need to be aware of logical models provided by the model provider(s) and retrieve multiple files instead of a single one.

Indeed, there are times when a "physical" file is part of a bigger picture. For example, an html file depends on its associated CSS file to be properly rendered, a split zip file depends on the presence of all of its fragments to be opened, a java file requires all of its imports in order to be compiled... For most of these, it is "harmless" (sort of) to ignore the "whole picture" and consider only a single file when applying actions. Java, for one, will still allow you to open the erroneous file and edit its "import" section to remove the parts that were not properly updated. An html file with wrong links will still be openable in a browser, even though it will be missing the rendering layed out in its CSS file. For others, the "bigger picture" will be broken if we only consider a single file. The zip file will be corrupted and totally unusable if I replace one of its fragments by an older version from the git history.

The model providers API is designed to tackle such needs and allow third-party plugins to provide their own logic to resolve the logical model, from a given "starting point" resource. The starting point is usually the resource selected in the workspace for an action (such as "replace", "compare", "commit" ...). In my case, the logic I am contributing to Team is aimed at resolving EMF models. One model depends on its fragments as well as all of its referenced resources to be present, lest it be corrupted and unloadable (you may want to look at http://wiki.eclipse.org/EMF_Compare/Logical_Model for an in-depth description of the use case).

Benjamin Muskalla had worked back in the end of 2011 to make the "Replace With" action aware of model providers (through https://bugs.eclipse.org/bugs/show_bug.cgi?id=354932). The "commit" action still ignores them (trying to commit a file that is part of a model *should* prompt the user to commit the rest of the logical model). My priority though is to get all of the "comparison" actions to properly call the Team APIs (see bug https://bugs.eclipse.org/bugs/show_bug.cgi?id=354474). Some improvements towards this have been made through bug https://bugs.eclipse.org/bugs/show_bug.cgi?id=393225 and the "compare with > reference" action now properly calls the model providers for additional input. However, all of the remaining actions are still ignoring this API :
  • Compare With Menu
    • Commit
    • Git Index with HEAD
    • HEAD Revision
    • Previous Revision
    • Branch, Tag or Reference << as outlined above, this one is actually working thanks to bug 354932
    • Git Index
  • History View
    • double-click a commit
    • right-click > compare with workspace
    • double-click a file (bottom-right pane)
    • right-click a file > compare with version in ancestor
    • right-click a file > compare with version in workspace
  • Commit Viewer
    • double-click a file
    • right-click a file > compare with version in ancestor
    • right-click a file > compare with version in workspace
  • Git Staging view
    • double-click a file (staged or unstaged)

We'd like to know if the EGit team has any plans to make all of these actions support extensions to the model providers, and how we could best contribute to this effort.

We also meet some problems in other parts of EGit. We do not have an explicit dependency on EGit, JGit, CVS, Subversive .... or any other Team Provider. We target "Team" as a whole and are independent from the actual provider that's behind a given project (we don't even care whether there _is_ a Team provider in the first place). As such, we are querying the file history and revisions through the Team APIs (IFileRevision, IFileHistoryProvider, ...) and we need EGit to properly fill all information even when used through this "proxy". We've started looking at what's missing and providing patches (https://bugs.eclipse.org/bugs/show_bug.cgi?id=398982), please let us know if there are things we can do to make these contributions better.

Laurent Goubet
Obeo

_______________________________________________
egit-dev mailing list
egit-dev@xxxxxxxxxxx
https://dev.eclipse.org/mailman/listinfo/egit-dev

begin:vcard
fn:Laurent Goubet
n:Goubet;Laurent
org:<a href="http://www.obeo.fr";>Obeo</a>
email;internet:laurent.goubet@xxxxxxx
url:http://www.obeo.fr
version:2.1
end:vcard


Back to the top