Bug 269067 - [ccp] Copy and paste of file with same name from one pkg to another problematic
Summary: [ccp] Copy and paste of file with same name from one pkg to another problematic
Status: CLOSED WONTFIX
Alias: None
Product: JDT
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 3.4.1   Edit
Hardware: All All
: P3 major with 7 votes (vote)
Target Milestone: ---   Edit
Assignee: JDT-UI-Inbox CLA
QA Contact:
URL:
Whiteboard: stalebug
Keywords:
Depends on: 20036
Blocks: 341774
  Show dependency tree
 
Reported: 2009-03-17 15:53 EDT by John P. Petrakis CLA
Modified: 2020-03-29 05:40 EDT (History)
18 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description John P. Petrakis CLA 2009-03-17 15:53:33 EDT
Build ID: M20080911-1700

Steps To Reproduce:
1. Create Java project with pkg/class a.b.c.d0.ClassOne.java and a.b.c.d1.ClassOne.java
2. Put project into source control (ClearCase in this case)
3. Ensure all file editors are CLOSED
4. Copy file d1.ClassOne.java over and onto d0.ClassOne.java

If no file editor is open on the target file, problems occur.


More information:
This is occurring with a customer using RAD 7.0.0.x (which is eclipse 3.2.x based) and ClearCase remote client (CCRC).  While the behavior in 3.2.x is more problematic (behavior is reproduced in base Eclipse), there is still an issue with Eclipse 3.4 when a) there are no editors open and b) the file is not already checked out.  A distilled analysis follows:

I investigate the behavior of this problem using both Eclipse 3.2.2 and 3.4.1 with CCRC 7.1.  While I did not use CCRC 7.0.1 we believe that the eclipse framework was firing events that caused bad behavior under specific circumstances.  Since the problem is most severe in the package explorer, I am restricting this discussion to that use case, and to the four most relevant tries which seem to point the way to the issue.


created two files a.b.c.d0.ClassOne.java and a.b.c.d1.ClassOne.java.  Note the same file name, different packages.  

First try: target d0.ClassOne.java (the target file) with this file open in an editor, CCRC 7.1 eclipse 3.4.1.

Copied d1.ClassOne and pasted it into d0 (or onto d0.ClassOne) in the pkg explorer tree.

	==> result was a prompt to check out the file, which upon clicking ok, overwrites the contents in the editor leaving the editor dirty so that a save will be necessary.  The parent folder is NOT checked out.  This is the "most successful" scenario.  No delete file events are thrown by eclipse.


Second try: target d0.ClassOne.java (the target file) with this file open in an editor, CCRC 7.1 eclipse 3.2.2

	==> result was NO prompt to check out, file editor shows dirtied contents.  Once you hit save THEN you get a prompt to checkout.  Checking out loses updated editor contents... but no folder is checked out.  This is because the checkout events need to come before the paste is completed and that does not happen in 3.2.x, which RAD 7.0.x is based on.... no delete file events are thrown by eclipse, but the altered contents are lost on checkout!!!


Third try: NO files open in editor, copied d1.ClassOne.java and pasted it into d0 (or directly onto the d0.ClassOne entry) in the package explorer, CCRC 7.1 eclipse 3.2.2.

	==> result was no checkout of the target file, a DELETE event on the target file.  Folder d0 left checked out (default for 7.1, NOT the default for 7.0.1).  The original file is rmnamed because eclipse *fires a delete event BEFORE the checkout of the file occurs*!  Since there is no open editor you have NO chance to ever save the change.  The original file is already gone and you get a file with the same name pasted into place.  You get no prompt to add that file to source control because that file name already exists in the CCRC cache.  So the pasted file appears as a view private entity.  In CCRC 7.1 you CAN uncheckout the folder but what happens then (if you do not remove the view private file first) is the appearance of a file ClassOne.java.loading, and the original file name appears to be hijacked (because the refresh puts it in a source controlled but writable state which looks like a hijack).

Upon doing a view update of the resultant folder element, you can select replace hijacks with vob element etc. and it will clean itself up, but in 7.0.1 unless you change the default the folder will have been checked in!


Fourth try: NO files open in editor, copied d1.ClassOne.java and pasted it into d0 (or directly onto the d0.ClassOne entry) in the package explorer, CCRC 7.1 eclipse 3.4.1.

	==> result was 1) prompt to check out the target file (d0.ClassOne.java), once the checkout succeeds, move-delete hook is called to DELETE the file resulting in a folder checkout (we do rmname, not rmelem).  In this case the delete FAILS because the checkout occured first and you cannot rmname a checkedout file in CCRC.  The file is never overwritten as a result.


It is clear that the delete event thrown by eclipse is simply incorrect for a CC filesystem.  CVS won't care... but CC definitely does.  There is no way to know if the delete event was thrown by a paste or a move so we simply cannot account for it and ignore it... eclipse does not tell the move/delete hook why it's trying to delete something.

It is clear that the better behavior is to check the file out early which 3.2.2 definitely does NOT do, and the file should be overwritten once the checkout is performed and again, the delete event does not happen in eclipse 3.4.1 if the editor is open.

This seems to point to a possible issue in the paste phase.  The firing of the delete event is mostly the issue... if the file gets checked out first, then the file should be overwritten w/o calling for a deletion because in ClearCase this encourages the creation of so called "evil twins".
Comment 1 Jerome Lanneluc CLA 2009-03-25 07:06:31 EDT
I didn't read all the analysis, but the description doesn't say what the symptoms are. What do you see when you say "problems occur"? Any stack trace in the .log?
Comment 2 John P. Petrakis CLA 2009-03-25 08:07:12 EDT
There are no errors in the log per se... However, the issue is the fact that the copy/paste procedure wants to *delete* the file before re-writing it *rather* than making it writeable and overwriting it.  In 3.2.x, the delete is attempted before the file is made writeable... which was a bit of a problem in and of itself, while in 3.4.x the behavior is better in that it happens after the checkout, but if an editor on the file is NOT open, the delete is still attempted prior to paste...  The deletion is unnecessary since an overwrite would do just as well and it's problematic for ClearCase because it encourages the creation of so-called "evil twins".
Comment 3 Andrey Loskutov CLA 2010-04-05 05:09:49 EDT
The issue is Java Package Explorer specific. Also still appears in Eclipse 3.5.2/3.6 M6 on Mac/Windows.

AS IS:

1) Create (any type of) project with two folders like this:
a/test.txt
b/test.txt

2) Using java package explorer, copy a/test.txt via "Ctrl + C"
3) Using java package explorer, paste to b/test.txt via "Ctrl + V"
4) You get a breakpoint hit at MoveDeleteManager.deleteFile() coming from CopyResourceChange.perform() which is doing basically 2 things:

deleteIfAlreadyExists(...newName); 
getResource().copy(getDestinationPath(newName), ...);

this strategy is obviously wrong for files with the same name, because in terms of the SCM mamagement, the first call causes version control to mark files as deleted, and the second one creates a NEW unversioned file on top of the deleted one!!!

Interestingly, copy/paste from Navigator or Project explorer views works just file as they are internally are using (more sophisticated) WorkspaceUndoUtil.copy() method which does NOT delete files.

TO BE:

In the step 4 the order of operations it should do (similar to WorkspaceUndoUtil.copy()):

IResource resource = getResource();
if(resource instanceof IFile){
  // some extra code here to handle linked resources... and finally:
  ((IFile)resource).validateEdit()
  ((IFile)resource).setContents(...)
} else {
  do as before...
}

We are facing this issue in the MercurialEclipse plugin, see http://www.javaforge.com/issue/11192

Please increase the severity to serious and change the platform to ANY.
Comment 4 ekkehard gentz CLA 2010-04-05 07:40:24 EDT
I ran into this while using hgeclipse (Mercurial Repository Eclipse Plugin) -
would be great to get this fixed.

just tested the same with EGit - same happens:
after copy/paste using Java Package Explorer the same happens:
EGit recognizes the file to delete - then you have to add/commit again (and loosing the history)
Comment 5 Bastian Doetsch CLA 2010-04-05 07:54:33 EDT
I agree that this is bad behaviour as the team providers (MercurialEclipse/EGit/BzrEclipse) hook into the Eclipse copy/delete actions via org.eclipse.team.core.RepositoryProvider.getMoveDeleteHook() and therefore mark the files deleted before re-adding a new file.
Comment 6 ekkehard gentz CLA 2010-04-05 08:39:28 EDT
just tested using EGit (Git) and hgeclipse (Mercurial)

for me (OSX 10.6.3, cocoa-64, Helios M6 SDK)
copy/paste only worked as expected from Navigator,
but not from Java Package Explorer or Project Explorer

ekke

(In reply to comment #3)

> 
> Interestingly, copy/paste from Navigator or Project explorer views works just
> file as they are internally are using (more sophisticated)
> WorkspaceUndoUtil.copy() method which does NOT delete files.
>
Comment 7 Gunnar Wagenknecht CLA 2010-04-06 03:11:50 EDT
See also bug 37954, bug 73767 and bug 96587.
Comment 8 Aaron Digulla CLA 2010-04-06 07:52:22 EDT
The root cause is probably bug 20036. This bug is from 2002 which translates to: Fix it yourself, the team isn't at all interested.
Comment 9 derek_linttell CLA 2010-04-22 14:14:30 EDT
We ( RAD/RSA ) now have another customer reported Sev 2 attributed to this issue, we are most likely to create a patch request on 342.
Comment 10 Francis Upton IV CLA 2010-06-11 17:28:57 EDT
This makes the mercurial plugin unusable for copy/paste with the same name in the package explorer. This is a big deal.
Comment 11 Krzysztof Daniel CLA 2010-07-26 04:20:09 EDT
(In reply to comment #3)

> In the step 4 the order of operations it should do (similar to
> WorkspaceUndoUtil.copy()):


Could not WorkspaceUndoUtil be just used?
Comment 12 Szymon Ptaszkiewicz CLA 2010-07-28 08:00:44 EDT
(In reply to comment #11)
> (In reply to comment #3)
> 
> > In the step 4 the order of operations it should do (similar to
> > WorkspaceUndoUtil.copy()):
> 
> 
> Could not WorkspaceUndoUtil be just used?

I think it's not possible because of the same reason as described in bug 31883 comment 23. WorkspaceUndoUtil also requires Shell.
Comment 13 Markus Keller CLA 2010-08-10 12:36:29 EDT
This scenario should just be handled properly in the affected team providers. You already have the same situation when the user deletes a file and
then creates a new file (or copies a file) with the same name. The CVS team
provider merges the delete and create operations together and eventually
considers the file as changed.

I'd even say the current implementation in the Navigator (copy contents instead of replacing the files) is wrong. The user didn't change the contents of the original file. He really copied another file to the destination, with the side effect that the old file gets deleted before the new file can be copied to that location.

If we wanted to support an explicit "replace contents" operation, then this should be provided in the
IResource#copy(IPath, int, IProgressMonitor) API so that this is implemented in one place and all clients can use it (but that's out of scope for 3.4.2+).
Comment 14 Andrey Loskutov CLA 2010-08-10 13:18:32 EDT
(In reply to comment #13)
> This scenario should just be handled properly in the affected team providers.
> You already have the same situation when the user deletes a file and
> then creates a new file (or copies a file) with the same name. The CVS team
> provider merges the delete and create operations together and eventually
> considers the file as changed.

CVS *can* do this, as it does not track directory changes and also IMHO can not follow file renames on merge, but Clearcase and Mercurial are different, as they really track directory changes (CC) or file delete/add operations (hg) as first class citizens.

So if you say *delete + create later* (not in one shot), CC will need to checkout directory (and create a new branch for it), which will be followed by checkin (default CC plugin setting) of the directory. From now on there is NO WAY BACK for the plugin to "merge" next "create" operation with the previous removal.
 
> I'd even say the current implementation in the Navigator (copy contents instead
> of replacing the files) is wrong.

They did exactly what user wanted. You shouldn't simply say to customers that most obvious and expected result of the operation is wrong.

> The user didn't change the contents of the
> original file. He really copied another file to the destination, with the side
> effect that the old file gets deleted before the new file can be copied to that location.

If I copy file over another file with the same name, I WILL change the content of the previous file with the content of a new file in the most efficient way. Of course we can teach users to open two editors and THEN do select all + copy + paste, but isn't weird??? I'm NOT going to explain it to my customers...
 
> If we wanted to support an explicit "replace contents" operation, then this
> should be provided in the
> IResource#copy(IPath, int, IProgressMonitor) API so that this is implemented in
> one place and all clients can use it (but that's out of scope for 3.4.2+).

In the comment 3 is a proposal how to do it. There are already implementations there which CAN do this WITHOUT changing IResource API. The only "bad" implementation is of the Java Package Explorer. All other "Explorers" do the right thing.
Comment 15 ekkehard gentz CLA 2010-08-10 14:32:37 EDT
(In reply to comment #14)
...
> 
> > The user didn't change the contents of the
> > original file. He really copied another file to the destination, with the side
> > effect that the old file gets deleted before the new file can be copied to that location.
> 
> If I copy file over another file with the same name, I WILL change the content
> of the previous file with the content of a new file in the most efficient way.
> Of course we can teach users to open two editors and THEN do select all + copy
> + paste, but isn't weird??? I'm NOT going to explain it to my customers...
> 
exactly - if I replace a file with another one of same name I want to replace the content and not getting a new file and the old deleted and all history lost.

> In the comment 3 is a proposal how to do it. There are already implementations
> there which CAN do this WITHOUT changing IResource API. The only "bad"
> implementation is of the Java Package Explorer. All other "Explorers" do the
> right thing.
hopefully there will be found a solution because more and more are using Git or MercurialEclipse
Comment 16 Markus Keller CLA 2010-08-13 15:12:08 EDT
We won't change all places in jdtui, jdt.core, and ltk.refactoring to custom implementations of an operation that belongs to the core.resources layer. When the blocking bug 20036 is fixed and we can call IResource#copy(..) and #move(..) with e.g. IResource.OVERRIDE, then we can use that API instead of delete/create.

The implementation in WorkspaceUndoUtil is quite complex (see e.g. handling of links) and has bugs when files or containers have custom encodings (see also related bug 67606 in Platform/Resources). And as said before, WorkspaceUndoUtil is in a UI layer and can't be used in all necessary places.


The common usability problem of team providers that are affected by this bug is that they force users to explicitly add each resource in a project to revision control. If you use CVS on the command line, you also have to explicitly "add" files to version control. The Eclipse CVS client spares users from this step by putting all files in a shared project under version control automatically, unless they are explicitly ignored (in the .cvsignore file).

If other Eclipse team providers follow this example, then "delete/copy" should be indistinguishable from "override contents".
Comment 17 Gunnar Wagenknecht CLA 2010-08-13 15:24:31 EDT
(In reply to comment #16)
> The common usability problem of team providers that are affected by this bug is
> that they force users to explicitly add each resource in a project to revision
> control.

Markus, unfortunately it's not that easy. I could imagine that some can be actually implemented in the same way like CVS. However, I know from past experience that some are more complex.

For example, in ClearCase you need to checkout (not CVS checkout, checkout actually means "lock") the parent directory in order to delete a file. Same is true for adding files. Modifying files only requires a "lock" of the file itself. Sometimes you don't get the "lock" (for some reasons) and your workflow is broken. Another problem is that deleting a file and adding a new one with the same name does not keep the history in ClearCase. It actually considers the file a complete new object with its own history starting from zero.

Anyway, I agree that it should be fixed at a core level and not just within JDT. It would be insane otherwise.
Comment 18 Aaron Digulla CLA 2010-09-30 05:09:26 EDT
A bug which can lead to data loss is "Severity normal"???
Comment 19 Aaron Digulla CLA 2012-06-05 10:48:49 EDT
Here is another scenario which can cause data loss:

- You'll need a project in which code is generated using an Xtext DSL
- The generated code must be under version control
- Clean the project

Sometimes, the generated code isn't deleted. Sometimes, half of the generated code is marked for deletion (it seems the buggy code can't distinguish between "Clean project" deletes files and user deletes files). Sometimes, all generated files are marked for deletion.

I spent the last hour fixing a commit which deleted half of the generated files.

PS: For reasons that don't belong here, the generated code must be under version control.
Comment 20 Eclipse Genie CLA 2020-03-29 05:40:13 EDT
This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet. As such, we're closing this bug.

If you have further information on the current state of the bug, please add it and reopen this bug. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant.

--
The automated Eclipse Genie.