Bug 31456 - ISourceManipulation.rename() performs deep copy of resources
Summary: ISourceManipulation.rename() performs deep copy of resources
Status: RESOLVED WONTFIX
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Core (show other bugs)
Version: 2.1   Edit
Hardware: PC Windows 2000
: P3 major (vote)
Target Milestone: ---   Edit
Assignee: JDT-Core-Inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 34568 38476
  Show dependency tree
 
Reported: 2003-02-10 09:41 EST by Adam Kiezun CLA
Modified: 2009-08-30 02:34 EDT (History)
2 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Adam Kiezun CLA 2003-02-10 09:41:40 EST
20030206
ISourceManipulation should not perform deep copy of resources
but it does, which i can imagine leading to lost work.

affected ISourceManipulations are ICompilationUnit, IPackageFragment
strangle, renaming a linked package deletes the old one and creates the new one 
in the workspace while renaming a linked ICompilationUnit copies it from the 
current location to the workspace

marking critical because of the package case.
see also bug 30711
Comment 1 Adam Kiezun CLA 2003-02-10 10:20:11 EST
maybe it's just major (as a no-very common case)
but still pretty nasty
Comment 2 Philipe Mulet CLA 2003-02-13 09:02:06 EST
Feels like deleting a link shouldn't by default delete nested resources, unless 
we use a DEEP flag of some sort.
Comment 3 Adam Kiezun CLA 2003-02-13 09:06:12 EST
i couldn't agree more.
i never understood why it was the deep flag that became default - sounds like a 
dangerous idea
Comment 4 Philipe Mulet CLA 2003-02-15 05:04:42 EST
Please verify new behavior (we reject Java operations on linked folders), with 
UI workaround.
Comment 5 Adam Kiezun CLA 2003-02-17 04:59:19 EST
Philippe, if you're referring to bug 31384
then i put the workaround only for delete of packages and cus

i did not touch rename - it'd be very strange to disable rename
Comment 6 Adam Kiezun CLA 2003-02-17 08:21:36 EST
Olivier, can you annotate bug 31998
and help me decide which actions should not be allowed on linked 
ISourceManipulations?

(our code sometimes worksaround jcore limitations and calls IResource 
operations directly, so this i will have to figure out myself.
but please specify what is definitely not supported on linked resources 
ISourceManipulations)

thanks
Comment 7 Olivier Thomann CLA 2003-02-18 09:14:21 EST
All operations on linked resources should use the eclipse platform corresponding
operation. The reason is that a rename on a package fragment which points to a
linked resource will change the package declaration of the compilation units
defined in it and therefore change the linked resources. I am not sure this is
what we want.
If you copy a linked package fragment, I would say we should simply copy the
link and not the whole structure underneath. The problem is that the semantics
of a package fragment copy is not the same for a folder copy.
I am not sure we have a good story for the linked package fragment operations
right now. The best would be to redirect to platform operations for now with a
dialog helping the user to understand what is going on.
Comment 8 Adam Kiezun CLA 2003-02-18 09:18:37 EST
see bug 31998 for a description of all workarounds i put in this area for 2.1
Comment 9 Olivier Thomann CLA 2003-02-18 09:22:30 EST
That looks acceptable for now.
Comment 10 Dirk Baeumer CLA 2003-08-26 05:20:05 EDT
Olivier, what are JDT/Core's plans regarding this bug.
Comment 11 Olivier Thomann CLA 2003-08-26 10:02:15 EDT
I don't know. I need to investigate the behavior we have compare to the behavior
we want.
What is the desired behavior in case of a rename of a linked package fragment?
Should the compilation units be updated?
Comment 12 Dirk Baeumer CLA 2003-11-18 06:22:23 EST
When renaming a linked package fragement I wouldn't update the compilation 
units. It parent folder structure doesn't change. The refactoring should 
present a waring in this case.
Comment 13 Dirk Baeumer CLA 2003-11-18 06:23:04 EST
Can we do something here for M6 ?
Comment 14 Olivier Thomann CLA 2003-11-27 12:20:11 EST
Dirk, are you concerned only by the renaming operation on a linked package fragment?
Will the UI help the user to understand why the compilation unit package
declaration doesn't match anymore the package fragment name?
Comment 15 Olivier Thomann CLA 2003-11-27 13:54:02 EST
I need more clarification.
1) Are you only talking about package fragments that are in the project folder?
Using the UI I could not create a linked package fragment. I had to create a
linked folder that contained a compilation unit. A linked folder can only be
created if the parent is a project. This means that this is not an issue if the
projects is using source folder. It doesn't seem possible to create linked
folders inside a source folder.

2) If I have a link folder called link. That contains a folder p, with a
compilation unit A.java in it. The package declaration of this compilation unit
has to match the name of the linked folder.
If the folder is in D:\temp.
So we have:
D:\temp
    +---link
        +-----p
              +--- A.java

If you create a linked folder called "link2" that is pointing to link, then the
package declaration of A should be link2.p. So it means this is dependant of the
name you give for the linked folder. This is clearly not intuitive.
Is that the desired behavior?
3) In the package explorer, I selected the package "link2.p", and I called a
rename operation (renaming "link2.p" in "link3.p"). After the rename operation,
I end up with no folder or file in d:\temp\link. And a local folder "link3" has
been created in the project. There is no more linked folder in the project and
the linked folder has been modified.
I don't believe this is intended. So what do we expect in this case?
3.1) Creation of a local folder link3 and the linked folder is untouched?
3.2) Creation of a linked folder link3 in the parent of the previous linked
folder and the linked folder is untouched?

It is really unclear what we want to do in the case of linked package fragment.

I would say that we should not allowed linked package fragments. We can allow
linked class folder, but not a linked folder that contains compilation unit. The
support is really awkwards in this case.
Comment 16 Dirk Baeumer CLA 2003-12-03 13:01:50 EST
Since we can't avoid creating link folders which are interpreted by the Java 
Model as packages we have to have a reasonable support for linked packages as 
well. Additionally users can always manipulate them in the Navigator which is 
even worse for the Java model. So here is what I suggest:

Rename a compilation units:

- rename the link, but not the file to which the link points
- change the content of the CU.

This is exactly what I would expect when running a rename tool on a file 
system that supports links.

Rename a package fragement:

- we shouldn't support rename the "root" part of a package fragment if
  the root part represents a link. In the given example users can rename
  link.p to link.p2 but they can't rename link.p to link2.p.

- as well renaming link isn't supported as well.

- the rename refactoring should issue an error in those cases. Best would be if
  the ISourceManipulation could provide a method canRenameTo(String newName) 
  which returns an IStatus.

- I did some testing and it seems that the rename operation already behaves
  correctly given that we don't rename the "root" package.
Comment 17 Olivier Thomann CLA 2003-12-03 14:35:26 EST
1) I am not sure that in the case of a compilation unit rename it makes sense to
change the contents of the compilation unit. I am not sure you expect the linked
file to be changed. I would simply disable rename for a linked compilation unit.

2) For the package fragment case,

If you can rename link.p to link.p2, then you will update all CUs in the linked
folder and I don't think this is a good idea. If someones else uses the same CUs
 in a different projects => compilation errors.

I don't know if we have time for a new API regarding the canRenameTo(String
newName) on ISourceManipulation. Philippe, any comment?

My testing didn't work at all. It was possible to rename the "root" and
therefore lots of problems can happen.

If the user plays with renaming links in the navigator and then refresh the
package explorer, too bad!
My major concern right now is that we can end up losing data in the linked folder.

Philippe, any comment?
Comment 18 Dirk Baeumer CLA 2003-12-04 06:08:40 EST
The metapher eclipse uses for linked resources is excatly the same as a file 
system uses that supports links. If you build the same scenario on a file 
system with linked directories and files and ran a tool that does a 
refactoring rename you end up with the same problems. So in my opinion it is 
up to the user if he wants to perform the operation or not. What we can add is 
an warning telling the user that some outside tools will not continue to work. 
We do the same if the user renames a class having a main method.

Renaming the root: this is excatly why the PR exists. In my last post I wrote 
that renaming seems to work except if you rename the root. If we don't allow 
this I think we will not loose data.
Comment 19 Olivier Thomann CLA 2004-02-03 09:44:36 EST
Why don't we simply disable the rename of a root in case the root is a linked
resource?
Comment 20 Dirk Baeumer CLA 2004-03-18 05:18:30 EST
It feels strange to simply disable the actions. You don't have a simple work 
around in this case because renaming the package as a folder in the navigator 
will not do the same as renaming the package.

What are the plans for 3.0 regarding this bug ?
Comment 21 Philipe Mulet CLA 2004-03-25 06:40:43 EST
Will reconsider post 3.0
Comment 22 Dirk Baeumer CLA 2004-03-25 06:45:54 EST
As Adam pointed out in the first comment this can lead to loss of work and 
disabling the function in the UI isn't a good option as explained in comment 
#20.
Comment 23 Philipe Mulet CLA 2004-05-19 10:32:58 EDT
Removing milestone of deferred item.
Comment 24 Dirk Baeumer CLA 2004-09-14 09:06:54 EDT
Philippe, any action planned here for 3.1 ?
Comment 25 Dirk Baeumer CLA 2004-11-16 12:59:04 EST
ping...
Comment 26 Eclipse Webmaster CLA 2009-08-30 02:34:01 EDT
As of now 'LATER' and 'REMIND' resolutions are no longer supported.
Please reopen this bug if it is still valid for you.