Community
Participate
Working Groups
Build Identifier: 36I20100521 After selecting the project that contains one or more logical models and run Team > Merge... between the local project (checked out from cvs HEAD) and a cvs branch in the repository (That has some changes in the sub model files), the Synchronize view always show the message "No changes in "CVS Merge 'Root_branch2 to branch2' But if I do "Compare With > Another Branch or Version..." and select the same cvs branch, this work and show some changed files in the Synchronize view. I have debug and found that the resource diff tree that returned by the org.eclipse.team.internal.ccvs.ui.mappings.MergeSubscriberContext is always empty (it always return null for any resource paths in the selected project). So, our content provider (extends SynchronizationContentProvider) could not detect the proper changes and returned empty list (in method getChildrenInContext() of SynchronizationContentProvider class). Even when I force it to provide some contents, the synchronize view still show No Changes. But if I close the project and then reopen the project, this seems to refresh the cvs synchronize states. After the project reopen, select the project and do Team > Merge and it works. So, I think there is a problem with the diff tree (ISynchronizationContext.getDiffTree()) in the MergeSubscriberContext class. Reproducible: Always Steps to Reproduce: 1. Create a project P1 with a logical model contains a couple of sub model files 2. Share the project P1 with cvs repository 3. Create a branch B1 for project P1 4. Make a small change in one sub model file and commit the changes to cvs repository. 5. Change the checkout branch for project P1 from B1 branch to HEAD branch 6. Right click on the project P1 and select Team > Merge... Expect: There are changes shown in the cvs Synchronize view. Result: The synchronize view shows No Changes between the two branches (HEAD and B1) NOTE: For non logical files such as text files (*.txt), the Team > Merge seems to work fine.
To reproduce I used the logical model available in org.eclipse.team.examples.filesystem where you can create a model object definition file (.mod) which can point to one of more model object element (.moe). So here is what I did: 1. Install the plug-in with filesystem example[1] 2. Create a project P1, create a file definition.mod containing two lines "element1.moe" and "element2.moe" 3. Create two (sub model) files element1.moe and element.moe. All files (mod and moe) are in the project root directory. 4. Share the project (ignore the NPEs), commit the files, create a branch. 5. While on the branch, make a change to element1.moe content and commit it. 6. Switch to HEAD and do Team > Merge ... and these worked fine to me. Could you please try to reproduce your problem using the suggested plug-in so we're on the same page. [1] Examples pack for 3.6 is available here: http://www.eclipse.org/downloads/download.php?file=/eclipse/downloads/drops/R-3.6-201006080911/org.eclipse.sdk.examples-3.6.zip , but from my perspective it's much easier to checkout the project from dev.eclipse.org and run a self-hosted Eclipse having the plug-in in workspace
Created attachment 174826 [details] model project in workspace Here is how the project created with my steps looks in workspace.
Created attachment 174827 [details] merging model project And this is what I get when I merge B1 branch and HEAD of the project.
I downloaded the plug-ins and launch the self hosting eclipse instance. But I don't see which actions to use to create the definition file (.mod) and the sub unit file ".moe". What is the category that these actions under in the News dialog?
You can create both of them using general New > File action. Just remember about setting the correct file extension.
It seems to me that the moe files are not group into a logical model in the cvs synchronize view. There should be two groups on the diff tree. One group is for file base merge and the second group for your logical model navigator content provider (org.eclipse.ui.navigator.navigatorContent). When I tested, there is only one group for non-logical model files. Did you bind your navigator content provider in the org.eclipse.team.examples.filesystem plug-in? (Extension point org.eclipse.ui.navigator.navigatorContent) I am not sure how to access the source at the host dev.eclipse.org. What are the repository location, username and password?
Created attachment 175214 [details] model project in sync view (In reply to comment #6) > When I tested, there is only one group for non-logical model files. The fact that the model project appears only once in the Sync view is expected. Depending on your selection in model selection drop down, the project will be presented as model project with logical elements in it (Example Model) or as regular project with files (Workspace). > Did you bind your navigator content provider in the > org.eclipse.team.examples.filesystem plug-in? (Extension point > org.eclipse.ui.navigator.navigatorContent) Yes, the ext. point you're referring to is defined in the plug-in. > I am not sure how to access the source at the host dev.eclipse.org. What are the > repository location, username and password? Here is the repository location you can paste in directly into the Sync view: :pserver:anonymous@dev.eclipse.org:/cvsroot/eclipse . You don't have to provide any password, leave it empty. The plug-in you're looking for is available in HEAD.
Thanks. I able to get the plugin source and debug it now. Although, I can see the ExampleModelProvider get called, but I still did not get the grouping in the synchronize view like yours. Anyhow, I did a Team > Merge for plain text file (txt) and I found that the problem is also happen to plain text files. The out going changes in the HEAD branch is NOT shown up in the synchronize view either. Steps to reproduce: 1. Create a project with 2 plain text file File1.txt and File2.txt 2. Share the project P1 with cvs repository 3. Create a branch B1 for project P1 4. Change the file content of File1.txt (such as Add a text line) and commit the changes to cvs repository. 5. Switch to HEAD 6. Change the file content of the File2.txt and commit the changes to cvs. 7. Right click on the project P1 and select Team > Merge... Expect: The synchronize view shows two file File1.txt (Incoming change) and File2.txt (Outgoing change). Result: The synchronize view show only one file File1.txt (Incoming change). So, the problem is the Team > Merge operation does not detect outgoing changes properly (changed files, added files in HEAD branch). But if you do Compare Wtih > Branch or Version, then it will shows both incoming and outgoing changes.
Here is the steps for showing the problem with added files. Steps to reproduce: 1. Create a project P1 with 2 plain text files File1.txt and File2.txt 2. Share the project P1 with cvs repository 3. Create a branch Branch1 for project P1 4. Change the file content of File1.txt (such as Add a text line) 5. Add a new text file File3.txt and commit all changes to cvs repository. 6. Switch to HEAD 7. Change the file content of the File2.txt (such as Add a text line) 8. Add a new text file File4.txt and commit all changes to cvs repository. 9. Right click on the project P1 and select Team > Merge... 10. Select the branch Branch1, common base version (Root_Branch1) and click Finish. Expect: The synchronize view shows four files File1.txt (Incoming change), File2.txt (Outgoing change), File3.txt (Incoming Add) and File4.txt (Outgoing Add). Result: The synchronize view shows only two incoming files File1.txt (Incoming change) and File3.txt (incoming Add)
Thanks for the precise steps Duc, now I can clearly see your point. Let me explain what is happening here: Merge is designed as a "one way" operation in which you merge changes from a selected branch to a target branch. In your case, the target of the merge is the main branch (HEAD). You can choose a different target by switching or replacing with another branch (just like you did in 5. in comment 8 and in 6. in comment 9). Next, you select a branch to be merged which in your case is B1. What you see in the sync view are changes made in B1 which are not available in HEAD. In case of any conflicts (files modified in both HEAD and B1) you would see them too. If you would like to merge the changes from HEAD into B1, you will have to switch to B1 (set it as target branch) and select HEAD as the branch to be merged. So, to sync HEAD and the branch you will have to do it in two steps (run the Merge wizard twice). For more info about branching and merging please take a look at: http://www.eclipse.org/articles/article.php?file=Article-BranchingWithEclipseAndCVS/article1.html and http://www.eclipse.org/articles/article.php?file=Article-BranchingWithEclipseAndCVS/article2.html. To sum up, from my point of view *this is not a bug*. Of course, we could consider displaying those outgoing changes when merging, and even provide an option to commit them to the ending branch, but in my opinion that would make the current flow cumbersome. "Compare With > Another Branch or Version..." you already mentioned in comment 0 does the job here. Does it make sense to you?
Hi Tomasz, You said the Merge is designed as a "one way" operation (Two-way merge?). I disagree with that. In the “Merge” dialog, it requests the user to select the “Common base version (start tag):” which is the ancestor version. When the compare editor is launched from the Synchronize view, the compare editor is also show the ancestor version (The ancestor pane may be initial hidden for text merge, you need to click the “Show Ancestor Pane” tool button). Ancestor: Root_Branch1 Remote: Branch1 Local: HEAD So, the Team Merge is a three way merge operation (NOT one way). For text merge, this workflow is acceptable because there are no inter dependency among the files. But for modeling, we need to see both incoming and outgoing changes. For example: consider the following case. Root_Branch1:(Common base version) - Ancestor version Root_Model_File | ^ v | Sub model file A (contains a Class1) | ^ v | Sub model file B Branch1: (user1 deleted the Class1 from Sub model file A) - Remote version Root_Model_File | ^ v | Sub model file A* (Class1 is deleted by user1 and then commit) | ^ v | Sub model file B HEAD: (user2 added a reference in Sub model file B that points to Class1 in Sub model file A) - Local version Root_Model_File | ^ v | Sub model file A (contains a Class1) | ^ v | Sub model file B* (Reference to Class1) This is a logical conflicting case (NOTE there is no file conflict in this case because user1 change “Sub model file A” and user2 change “Sub model file B”), so we must force the user to either pick 1) Accept the “add Reference to Class1” and reject “delete of Class1” together. OR 2) Reject the “add Reference to Class1” and accept “delete of Class1” together. But if the Team Merge operation does not show the outgoing changes (Sub model file B), then the user doing the branch merge can see only incoming delete of Class1. If user2 accepted the incoming deletion, then the reference in Sub model file B in the HEAD stream is broken after the merge. So, the outgoing changes is definitely needed for Team > Merge operation to support the Logical model Merge scenarios. Without this fix, the modelers can not do multi-branch development using CVS due to the thread of model corruption. Although, the "Compare With > Another Branch or Version..." is a work around the problem. But this is a Two-way merge (NOT three-way merge like the Team > Merge operation) and the changes report is not explicit to indicate the user actions. For example, for the test case mention above, the two way merge of "Compare With > Another Branch or Version..." will show the following changes (Branch1 is base). 1. User2 added Class1 in Sub model file A 2. User2 added Reference to Class1 But user2 did not add the Class1, the Class1 is actually already exists in the Root_Branch1. So, this change report is a misleading. Although we can guard against the model corruption by forcing the user to accept both Adds OR reject both Adds together for this two-ways merge, this is just a short term work around solution. We still need the Three-way merge solution using Team > Merge to support the multi-branch development for the CVS users.
(In reply to comment #11) > You said the Merge is designed as a "one way" operation (Two-way merge?). I > disagree with that. In the “Merge” dialog, it requests the user to select the > “Common base version (start tag):” which is the ancestor version. When the > compare editor is launched from the Synchronize view, the compare editor is also > show the ancestor version By the "one way" operation I meant the fact that you can merge the changes one way i.e. from a selected branch to HEAD. Actually, copying the changes is a two phase action as well, since you first merge the changes into your workspace (previewing them in the sync view first if you decide to) and then you commit the changes from your workspace to the working branch (e.g. HEAD). You cannot apply the outgoing changes from your workspace to the "end tag" branch during the Merge operation (while in sync view). Sorry about the confusion. Going back to your request, I think I see your point now, but I will need a day or two to get my head around it, look at the code and suggest a reasonable solution.
I must admit that I'm a little bit confused here. After reading all the comments one more time I can see that were talking about 3 different things here: 1) in comment 0 you wrote that merging "a cvs branch in the repository (That has some changes in the sub model files)" into "the project that contains one or more logical models" shows no changes. Frankly speaking, I was astonished to hear that. Merging is quite common operation and I'm sure we would have heard about that if there had been a problem like this. I tried to reproduce it anyway, but failed, so I came to a conclusion that we're talking about not seeing outgoing changes, which leads as to 2) 2) in comment 8 you noticed that changes made in a working branch (in your workspace) are not shown as outgoing changes. You provided steps to reproduce that using Resources layer (no logical model) and text files. I explained in comment 10, why this is not a bug. By the way, if you had tried that with the same files in both branches you would have seen a conflict in the sync view. 3) in comment 11 you mentioned a slightly different issue which imo boils down to missing conflict indicator when merging projects with logical model. You provided a great example of such logical conflict. You also admitted that it's not an issue for file, which I took as an agreement on comment 10. Can you confirm that this bug is actually about 3), which means that when merging projects with a logical conflict (not necessarily a conflict in the same file) the conflict is not surfaced in the UI? If so, could you please check bug 145552 and say if it's related?
1) Sorry, in comment 0, the Expected result should be “The logical model merge session in the compare editor shows no outgoing changes between the HEAD branch and the Root_Branch”. This causes confusion for customers because they don’t see their changes in the HEAD stream in this three-way compare editor. Also, when there are logical model conflicts (not file conflict – see comment 11), the logical model merge can failed to compute the logical conflicts because cvs don’t give us the outgoing changes. So, this is a bug that needs to be fixed in order for supporting logical model merge correctly. There is a case that “There are changes shown in the cvs Synchronize view”. This happen when there are changes in the HEAD stream (checked out in workspace) and no changes in the branch. This also causes confusion for the user. When doing “Compare with branch or versions”, they can see the changes in the HEAD (some changes made between HEAD and Root_Branch). But when they do Team > Merge, the cvs Synchronize view show nothing. This operation is a Three-way merge operation, so the user expects seeing changes in both directions (incoming and outgoing). But it is not. 2) In comment 8. From basic text merge support, I agree that this work flow is acceptable. This is not a bug for text merge support. But for the logical model merge that eclipse is supporting, it is a bug because we can not detect logical model conflicts correctly without having the outgoing changes. Although the comment 8 is just a plain text merge example, it is presented here to show the point that the branch merge does not giving the “outgoing changes between HEAD and the Root_Branch” that lead to the problem of fail to detect the logical model conflicts (comment 11). 3) No, this is not the problem of “missing conflict indicator” in the synchronize view. The conflict indicator is working correctly and cvs did show the conflict correctly when the same file being changed from both branches. What missing is the “outgoing non-conflicting changes” in the Synchronize view for branch merge (Team > Merge). This is needed for logical model merge! Since the logical model merge is required the outgoing changes, you may show the outgoing changes for the non-logical model files as well for consistency. The user that want to see only one directly changes can still use the filter tool buttons (incoming only, outgoing only, conflicts) to customize the diff tree in the synchronize view. NOTE: Even when there are no conflict indicators in the synchronize view, the logical model conflict can still exists (see test case in comment 11).
There is a Typo in the last paragraph of my reply for 1) It should be There is a case that “The synchronize view shows No Changes between the two branches (HEAD and B1)”. This happen when there are changes in the HEAD stream (checked out in workspace) and no changes in the branch. This also causes confusion for the user. When doing “Compare with branch or versions”, they can see the changes in the HEAD (some changes made between HEAD and Root_Branch). But when they do Team > Merge, the cvs Synchronize view show nothing. This operation is a Three-way merge operation, so the user expects to see changes in both directions (incoming and outgoing). But it is not.
(In reply to comment #14) > 2) In comment 8. > From basic text merge support, I agree that this work flow is acceptable. This > is not a bug for text merge support. > > But for the logical model merge that eclipse is supporting, it is a bug because > we can not detect logical model conflicts correctly without having the outgoing > changes. I understand that having the outgoing changes is a must for detecting potential logical conflicts. However, I'm not sure if this is actually a bug because a code that marks outgoing changes as in-sync when merging has been there for years[1]. I have no idea how did you or any other logical model provider manage to solve such conflicts so far, I guess you didn't. Of course, your request makes sense to me, but imo it's an enhancement request which would require a fundamental change in the sync'ing framework so I would be very careful about it. Which means this is *not* kind of fix we would like to backport. There is also another thing, in our conversation outside Bugzilla you sent me a project created under RSA which contained simple UML models and diagrams. This helped me observe in what exact situation you're missing the logical conflict, since we couldn't reproduce it with the model from org.eclipse.team.examples.filesystem. We also agreed that there is something wrong with merging under RSA 7.5.4[2] i.e. it results in *no changes* being shown. This scenario works fine[3] in RSA 7.0.0[4], so this is definitely a bug (a regression). I didn't disappear even when I replaced o.e.team.core and o.e.team.cvs.core with their latest versions from HEAD, which could mean it hasn't been fixed. I've managed to observe two interesting things though: * if you leave the sync view open, when the merging is done and no changes is shown, restart Eclipse and when it's up again click "Synchronize" on the view the missing change will show up * it also worked when I was debugging the code and had a breakpoint set on Subscriber#accept[5] method, so it must be some kind of a timing issue To sum up, I can see two separate issues here, which imo are not related. First one, missing the outgoing changes has been there from the beginning, and I'm not convinced it's a bug. The second one, no changes when merging, looks like a serious bug to me, but on the other hand it may occur only under some specific conditions. I haven't heard about it before. [1] org.eclipse.team.internal.ccvs.core.CVSMergeSyncInfo.calculateKind() [2] Eclipse 3.2.1 [3] By "works fine" I mean it does show incoming changes, but do not show outgoing changes like you would like it to. [4] Eclipse 3.4.x [5] org.eclipse.team.core.subscribers.Subscriber.accept(IResource, int, IDiffVisitor)
Even the missing outgoing changes have been there from the beginning, but it should have been sweep and corrected this operation when eclipse introduced the support for logical model to the public. Unfortunately, it falls through the crack and our customers are facing the difficulty of performing logical model across cvs branches. I can not fix the missing outgoing changes problem in our code because it is part of the cvs component in the eclipse platform. I believe the Team > Merge should works like the cvs update operation. Both operations are pull changes from remote into local workspace. The update operate implemented correctly because it detects all changes (incoming, outgoing and conflict). But the Team > Merge is not. If you said this is not a bug, then how do you solve the simple logical model merge case in the comment 11 without the outgoing changes?
(In reply to comment #17) > Even the missing outgoing changes have been there from the beginning, but it > should have been sweep and corrected this operation when eclipse introduced the > support for logical model to the public. Unfortunately, it falls through the > crack and our customers are facing the difficulty of performing logical model > across cvs branches. I can not fix the missing outgoing changes problem in our > code because it is part of the cvs component in the eclipse platform. I understand that and I'm with you when you're talking about fixing this. All I'm saying is that this is probably the first time someone noticed it so it can't be that critical. At the same time, a potential fix that would enable outgoing changes during merge could have serious consequences for both clients and the rest of the framework. > I believe the Team > Merge should works like the cvs update operation. Both > operations are pull changes from remote into local workspace. The update operate > implemented correctly because it detects all changes (incoming, outgoing and > conflict). I guess you're talking about updating in the Sync view (after sync'ing is done). It's true, but as I mentioned earlier there is a difference between Synchronize and Merge. In the former you can perform both commit and update operations (two way) and in the later you're supposed to update changes only (one way). Also, please correct me if I'm wrong but if you do Team > Update on a resource, for which Synchronize would indicate a logical conflict, you will get no info about the conflict and the resource will get updated. In other words, the only way to make the user aware of the conflict is to synchronize the resource and examine the result. > If you said this is not a bug, then how do you solve the simple logical model > merge case in the comment 11 without the outgoing changes? You know the answer to that is "you cannot solve that case" and this is something we would like to improve.
Created attachment 177716 [details] Merging with quick fix This is how would the case from comment 11 look like if we removed the part in org.eclipse.team.internal.ccvs.core.CVSMergeSyncInfo.calculateKind() responsible for reseting the kind of synchronization for outgoing changes during a merge. There are couple of things I don't like about it: * Using toolbar buttons you can switch between incoming and conflicting changes, but the status bar shows also outgoing changes. This is inconsistent. * The logical conflict is not treated as a conflict: it cannot be filtered out using the toolbar buttons, the status bar knows nothing about it ("<> 0") * If there are outgoing changes not involved in any logical conflict I think they will still be counted in the status bar which would be quite confusing * Some clients may assume that a merge sync never produces outgoing changes (always "> 0" in status bar) Krzysztof could you please verify that? Of course, let us know if you notice any other side effects of such fix.
> Also, please correct me if I'm wrong but if you do Team > Update on > a resource, for which Synchronize would indicate a logical conflict, you will > get no info about the conflict and the resource will get updated. In other > words, the only way to make the user aware of the conflict is to synchronize > the resource and examine the result. Eclipse Auto-Merge: Since the logical model conflict is implementation specific, eclipse can not compute the logical model conflicts for every kind of logical models. So, eclipse is invoking the merge method of the org.eclipse.team.core.mapping.IResourceMappingMerger interface (that implement by the owner of the logical model). For the case in my comment 11, we return the IStatus fail (IStorageMerger.CONFLICT). So, eclipse will not auto accept incoming change model files (even the Team > cvs > update/merge > “Update all non-conflicting changes and then preview remaining conflicts” option is set). package org.eclipse.team.core.mapping; public interface IResourceMappingMerger { public IStatus merge(IMergeContext mergeContext, IProgressMonitor monitor) throws CoreException; NOTE: we can use the eclipse’s IMergeContext to programmatically “Mark as Merge” the model files during the saving of the logical model. For Visual Merge: (see attached logicalModelUpdate.JPG) Eclipse will show the normal incoming and outgoing changes. Our logical model also shows the incoming and outgoing changes same as eclipse. But we also warm the user the possible logical conflict. So, the user can launch the logical model merge and do the proper merging.
Created attachment 177864 [details] cvs update of local model
(In reply to comment #21) > To sum up, I can see two separate issues here, which imo are not related. First > one, missing the outgoing changes has been there from the beginning, and I'm > not convinced it's a bug. The second one, no changes when merging, looks like a > serious bug to me, but on the other hand it may occur only under some specific > conditions. Duc, before we start looking at the fix, could we at least agree that we have two bugs here? I guess you want to have the first one fixed and backported. The other bug ('no changes when merging') is less important for you. Is that right?
I agree that there are two problems here. 1. Outgoing changes is missing in Team > Merge operation 2. There are no changes showing in the Synchronize view with Team > Merge operation. If there are no changes shown in the synchronize view when the user selecting the project, then the customer can not proceed with the branch merge. If there are some incoming changes shown in the synchronize view, then user can launch the compare editor. In this case, it is also bad because the changes that we report in the compare editor are incorrect (because we don't have context of the outgoing changes). Both problems are important and should be fixed. We can use one defect to fix both problems or you can create a new defect and address the second problem there. I think the first step is we should find a fix the problem #1. I can you the testing for you when you have the test fix patch available. The eclipse version that we use are eclipse 3.6 and 3.4.
Created attachment 180874 [details] patch-for testing purposes (In reply to comment #19) > Krzysztof could you please verify that? Of course, let us know if you notice > any other side effects of such fix. Tomasz, sorry for late reply (long story). The fix works also for me and fixes problem with missing changes from branch. I am attaching the patch which Tomek described in comment 19. The patch is for testing purposes only.
I have also created bug 327785 to track issue with no changes showing in the Synchronize view with Team > Merge operation.
Tomasz, we should have it fixed by M4.
Created attachment 182463 [details] patch with tests Here is valid patch with tests. tom, can you review this patch?
First thoughts: The changes are made in internal packages. Could you explain how a potential client can benefit from the fix? How he is supposed to set the flag you added?
Created attachment 183753 [details] patch v2 Hello, this is a better version of the patch. Our idea is to create additional methods in class SubscriberResourceMappingContext or create class inheriting this class. The client could then execute new method or class instead of existing one. The new methods will be executing subscriber.setOutgoingIsInSync(false) before executing subscriber.getSyncInfo(resource) method. After subscriber.getSyncInfo(resource) we could set outgoing as in sync. Tom, can you tell us if our idea is correct? Or you would like to do it in another way?
Well, this might work, but I still don't know how a potential client could use it. Who is supposed to call the setter on the Subscriber? The method is protected, so does it mean you except the client to client a subclass of Subscriber? Since the CVSMergeSubscriber is an internal class the constructor you added to cannot be called be the client either, unless we assume the client can have access to CVS internals. If so, none of the above is actually needed, but I'm sure you know that allowing to use internals is like asking for trouble.
I'm taking over this one.
It's already too late to release a fix to M4, I'll try hard to provide the patch at the very beginning of M5. btw, bug 327785 doesn't block this one, the work may take place simultaneously. These are two separate bugs, bug 327785 is just the first thing the user sees.
After having a closer look at the problem I think the best approach is to allow outgoing change to surface during merges with models. Initially, I hoped we will be able to decide for which synchronizations the outgoing changes should be shown and for which they can be omitted (as it happens now). I thought we could deduct that from a model used, so if the given mapping points to resources with both incoming and outgoing changes the merge should consider them all in order to detect a possible conflict by the model. However, since SyncInfo and Subscriber are model neutral (they don't know if models are used or not) it's not an option. The latest patch adds a switch that would allow the client to decide how outgoing changes should be treated during a merge. This is fine, but I guess that eventually all model providers would turn this option on in order to be sure they don't miss any conflict. So what I think we can do is to go a step further and create an option available for all model providers. At the same time we should make sure that: - the option is only used with models (only merging with models should be affected) - no outgoing changes are displayed in the sync view while merging (only incoming and conflicts) - status bar doesn't show any info about outgoing changes so the average user is not confused - toolbar and context menu stay intact This way model providers will have all the data in place but the way merges are presented won't change.
I haven't mentioned that imo the option could be turn on by default, making the outgoing changes available for all models, but a safety switch (a command line param) should be available to turn this new feature off in case of any troubles, like performance regression (which are unlikely to happen).
After a chat with Szymon we agreed that the safety won't be necessary as long as we have a set of performance tests showing that the fix introduced no regression.
Created attachment 185948 [details] Fix v01 First version of the patch described in comment 33. No performance tests yet.
Created attachment 185949 [details] mylyn/context/zip
Created attachment 185965 [details] Performance test v01 The perf. test verifying that Fix v01 doesn't introduce any regression. In fact, according to the test results, merging with the "isModelSync" flag set to true takes less time.
Fix v01 applied to HEAD with updated copyright dates. Available in builds >N20110102-2102.
Created attachment 186002 [details] Performance test v02 An updated version of the perf. test.
Created attachment 186004 [details] The performance test results (average over 100 samples).
(In reply to comment #40) > Created attachment 186002 [details] > Performance test v02 Applied to HEAD.
(In reply to comment #42) > (In reply to comment #40) > > Created attachment 186002 [details] > > Performance test v02 > > Applied to HEAD. Note that you need to backport this to 'perf_36x' in order to get valid results/comparisons with 3.6.
Thanks for the tip Dani, but I don't think this will be necessary. The test performed in the MergeTests class runs two groups: * NON_MODEL_MERGE which doesn't use the fix, making the results comparable (if not identical) to the state without the fix * MODEL_MERGE which uses the fix, collecting the outgoing changes After a single run, the results are presented and ready to verify whether the fix has introduced any regression. I don't think the new test should be part of the AllBenchmarkTests suite given the above and the fact that we already have SyncTests in place.
(In reply to comment #44) > Thanks for the tip Dani, but I don't think this will be necessary. The test > performed in the MergeTests class runs two groups: > * NON_MODEL_MERGE which doesn't use the fix, making the results comparable (if > not identical) to the state without the fix > * MODEL_MERGE which uses the fix, collecting the outgoing changes > > After a single run, the results are presented and ready to verify whether the > fix has introduced any regression. > I don't think the new test should be part of the AllBenchmarkTests suite given > the above and the fact that we already have SyncTests in place. Sorry, I didn't look at the code. Are you saying you didn't write a "normal" performance test which I can then look at on the performance page?
(In reply to comment #45) > Sorry, I didn't look at the code. Are you saying you didn't write a "normal" > performance test which I can then look at on the performance page? Yes, but that was my intention. Initially, I didn't even think about committing the test, but changed my mind hoping the code may be a good starting point if we decide to write a "normal" performance test in the future.
The changes are tagged with I20110104-0800. Verified by code inspection in I20110124-1800. Asked Duc to verify both backported fixes (bug 332639 and bug 332640).
