Bug 302152

Comment 1 Serge Beauchamp 2010-02-08 14:51:36 EST

Hum, you point is that it should ask a confirmation before doing it?

That was the meaning of the 'Automatic' selection, i.e. select the best path variable available, or create one if necessary.

The thing is that the transparent creation of path variables is necessary not only in that context.

For example, when you move or copy linked resource between projects, all the path variables are automatically transferred, or created, so that the linked resource works properly on the destination project.

If the user doesn't want path variables to be created automatically, he can choose 'absolute path', and it will do that (keeping absolute path locations) - although the project would not be portable.

But I don't see that the case where the user wants his project to be portable, but the import to either fail or revert to absolute paths if there's no path variable that match at the moment.

With the new 'Linked Resource' editor in the project properties, it's really easy to organize linked resources and path variables.

For example, if the user doesn't want a set of linked resources to be defined using a given path variable, he can convert them to absolute path - then change tab, delete the path variable they were using - then convert them to relative again, and they would be using the now most appropriate path variable.

What do you think?

Comment 2 Szymon Brandys 2010-02-09 04:40:57 EST

(In reply to comment #1)
> That was the meaning of the 'Automatic' selection, i.e. select the best path
> variable available, or create one if necessary.

If I choose 'Default', I would expect that only existing variables will be used. I'm not expecting that any variable will be added.
If later I want to convert locations to use path variables, I can do this in the properties dialog, Linked Resources section.

> For example, when you move or copy linked resource between projects, all the
> path variables are automatically transferred, or created, so that the linked
> resource works properly on the destination project.

This may be confusing for pre-defined variables. We can copy manually added variables.
However I find it confusing that copying a linked resource (using PROJECT_LOC) adds PROJECT_LOC1 variable.
I would rather expect that the linked resource at the destination will use an absolute location.
And again we can convert it in the properties dialog later.

> What do you think?

IMO 'Default' should try to reuse existing variables or use absolute locations if no path variable can be applied. 'Absolute' should force absolute locations.
If Eclipse is to add anything for me I should be aware of it and be able to say 'No'.

For copying folders, IMO user-defined variables may be copied. However I'm not sure if we should create user-defined variables that match pre-defined variables at source of a copy operation.

Comment 3 Serge Beauchamp 2010-02-09 07:55:31 EST

(In reply to comment #2)
> (In reply to comment #1)
> > That was the meaning of the 'Automatic' selection, i.e. select the best path
> > variable available, or create one if necessary.
> 
> If I choose 'Default', I would expect that only existing variables will be
> used. I'm not expecting that any variable will be added.
> If later I want to convert locations to use path variables, I can do this in
> the properties dialog, Linked Resources section.
> 

Well, let me put things in perspective.

First, from a user's point of view, a project that contains linked resources with absolute paths is a broken project, that is non-portable.  The user can't put it as-in in a source control repository, he has to fix it (and first and foremost, be aware that the project needs to be fixed).

For the overwhelming majority of users, a project that is non-portable (that works only on a given computer, with a given set of hard-coded paths) is a major problem.

This was one significant reason why linked resources weren't used before, and a obstacle that the flexible resource effort aim at overcoming.

Now, from first hand experience with our own customers using our IDE, and using the Eclipse CDT toots, the lessons that we learn, is that in order for the users to have portable projects, the following principles must be followed:

1) The IDE operations don't create non-portable projects by default  (creates projects, files, linked resources, copy, move, etc...), so non-portable projects are not the result of an operation other than the explicit creation of a non-portable setting.

2) 'Cleaning-up' a project that is portable is quick and easy.

The solution for 1), is that all operations (import files, copy, move, etc..) by default maintain the portability of the project.

The solution for 2), is the new linked resource editor (the second tab of the linked resource property page).

Now, what you are suggesting, is that if there's no readily available linked resource, it will 'break' the project without the user being aware of it.  

I strongly think this is wrong, because 
1) I can't conceive of any significant amount of users that desire to have absolute path linked resource locations for its own sake in the project, apart from a temporary state.

2) This makes the project non-portable without the user being aware of it, so he can commit the project to a SVC, and break the build for someone else without being aware of it. 

3) if the core.resource algorithm picked a variable the user didn't want, it's easy for the user to correct it in the linked resource editor (and especially easy converting the resources to absolute paths).

> > For example, when you move or copy linked resource between projects, all the
> > path variables are automatically transferred, or created, so that the linked
> > resource works properly on the destination project.
> 
> This may be confusing for pre-defined variables. We can copy manually added
> variables.
> However I find it confusing that copying a linked resource (using PROJECT_LOC)
> adds PROJECT_LOC1 variable.
> I would rather expect that the linked resource at the destination will use an
> absolute location.
> And again we can convert it in the properties dialog later.
> 

How it works currently, is that if the destination "PROJECT_LOC" variable is compatible with the linked resource location path, it will be reused, so that PROJECT_LOC1 won't be created needlessly.

This is only relevant if the location is relative to a parent of PROJECT_LOC (${PROJECT_LOC}/../something), otherwise, if the location is a child of the source project location, obviously, unless the destination project is actually a child of the source project, the destination location can't be relative to the destination project's variable PROJECT_LOC, since they don't have the same value, hence another variable will be created.

The example I can give that works well with PROJECT_LOC, is say you have the following projects and file in the file system:

dir/ 
   project1/
   project2/
   file.txt

Inside project1, you have a linked resource 'file.txt' that has the location "FOO/file.txt", and "FOO" has the value "${PROJECT_LOC}/../"

You copy the linked resource 'file.txt' in the project2, then the variable 'FOO' will be created automatically, and will also have the same value "${PROJECT_LOC}/../", since that value is the same on both projects.

I changed the code to ensure WORKSPACE_PARENT_LOC won't be used unexpectedly too (see attached path).

> > What do you think?
> 
> IMO 'Default' should try to reuse existing variables or use absolute locations
> if no path variable can be applied. 'Absolute' should force absolute locations.
> If Eclipse is to add anything for me I should be aware of it and be able to say
> 'No'.
> 
> For copying folders, IMO user-defined variables may be copied. However I'm not
> sure if we should create user-defined variables that match pre-defined
> variables at source of a copy operation.

For the reasons I mentioned above, I believe it should works as follows:

Default:  existing variables are used, otherwise automatically created
Absolute Path :  only absolute paths are used

Comment 4 Serge Beauchamp 2010-02-09 07:56:52 EST

Created attachment 158583 [details]
Path to prevent WORKSPACE_PARENT_LOC to be used consistently when creating relative paths.

Comment 5 Serge Beauchamp 2010-02-09 08:25:02 EST

Another point:

In your example, where PROJECT_LOC1 is created, I'm guessing that this is because your linked resource points to another file inside the same source project, this is why the variable create looks strange.

But this is a synthetic case, because in the vast majority of cases, the user has linked resources to point *outside* of the project directory (otherwise there's little point into having linked resources), hence the locations are relative to the PROJECT_LOC/../ variables, and are converted exactly as the user expects them to (see my example above with FOO that points to ${PROJECT_LOC}/../).

Yet another point:

Even if in some cases, the code automatically generates variables the user might want generated differently, it's much easier to change the generated variables, than to create new ones from scratch, especially since the user can't see two project property page for two different projects at the same time.

Yet yet another point:

This is how other IDEs work.  In Visual Studio, you add files to a project, and they are encoded as project relative paths, not as absolute paths.

You drag them in Eclipse (and choose link), they should be encoded as project relative too.

The only reason why we have to create a path variable, is because we can't have a linked resource location encoded as follows: "PROJECT_LOC/../foo.txt" (because of the IPath limitation).  

If it weren't for that internal limitation, we wouldn't have to create variables to encode new file locations as  project relative paths, and the question wouldn't even come up.

But the bottom line, is that we shouldn't restrict the user workflow and usability, simply because of internal core.resources API legacy.

At best, maybe something could be done into hiding from the UI those automatically generated variables for API limitation purposes, but I think it would be more troublesome than it's worth.

Comment 6 Szymon Brandys 2010-02-09 09:05:06 EST

Sorry, but I'm not convinced that creating path variables silently is the right way to go.

What you are saying in 'Yet yet another point' is actually what I think is fine. We should resolve against existing variables. If the issue is an IPath limitation, maybe we should fix this limitation instead of making workaround in the Resources layer. 

And going back to linked resources with absolute locations. I would rather like to see an action that converts absolute locations to path variable related ones, when it is necessary, e.g. during sharing a project. This action could create new variables, however there should be some interaction with users IMO.

I would like to see to what others think.

Comment 7 Serge Beauchamp 2010-02-09 09:31:49 EST

Actually, there's one way to avoid creating a path variable when dragging and dropping files in the project that are located up the parent location.

That way, which the code doesn't leverage at the moment, is to use the PARENT variable directly in the linked resource locations instead of in a variable.

So instead of creating a variable "FOO" which contains "${PROJECT_LOC}/../" (really ${PARENT-1-PROJECT_LOC} internally), and the linked resource location to be:

FOO/file.txt

The linked resource location would itself be:

PARENT-1-PROJECT_LOC/file.txt

Which we can manage to display as "PROJECT_LOC/../file.txt" in the UI.

I tested it and it works fine (besides right now, the user has to type the PARENT-COUNT-VAR manually).

Thinking about it, there's no really draw back to this method, besides the linked resource location containing more text.

(In reply to comment #6)
> Sorry, but I'm not convinced that creating path variables silently is the right
> way to go.
> 

Ok, lets take this example:

I have project1.

It has 12 path variables, that I carefully built.  

Project1 has 4 thousand files, that use a mix of those 12 path variables.

Now, I drag 1500 of those files in a new project2, that I just created.

Your opinion, is that all those 1500 files should revert in project2 to have absolute paths locations?  

And that the user is just expected to a) re-create all 12 path variable, and b) go through all the files, figure out which one was relative to which of the 12, and change their locations?

I don't understand how we could impose that to the user, while right now, that it does, is automatically recreate the same path variables so that the 1500 files are still variable relative,and pointing to the proper variables.  

In the rare case where such conversion is not possible (such as in your synthetic example, or if another variable exist of the same name in the destination project, but point somewhere else), then another appropriate name is chosen.

It would such a major waste of time, and something that there's no reason not to be automated.

> What you are saying in 'Yet yet another point' is actually what I think is
> fine. We should resolve against existing variables. If the issue is an IPath
> limitation, maybe we should fix this limitation instead of making workaround in
> the Resources layer. 

I'm not sure if you remember, but the fundamental limitation, if that the linked resource location can't contain "..", because an IPath is used to pass its value around, and the IPath implementation available to clients of the core.resources API is Path.java, which coalesces the ".." elements.

So if the client puts "PROJECT_LOC/../foo.txt" as a location, it will be automatically coalesced into "foo.txt".

I don't see anyway of working around that basic limitation while preserving API compatibility than the one is implemented:  Having the "PARENT" variable that holds the indirection itself.

> And going back to linked resources with absolute locations. I would rather like
> to see an action that converts absolute locations to path variable related
> ones, when it is necessary, e.g. during sharing a project. This action could
> create new variables, however there should be some interaction with users IMO.
> 

There's one action that does that:  Selecting the 'Convert..." button in the linked resource editor property page.

Comment 8 Szymon Brandys 2010-02-09 10:52:55 EST

(In reply to comment #7)
> The linked resource location would itself be:
> 
> PARENT-1-PROJECT_LOC/file.txt

Makes sense. I think this is a reasonable workaround for the IPath limitation. And seems to be consistent with other IDEs, right?

> (In reply to comment #6)
> > Sorry, but I'm not convinced that creating path variables silently is the
> right
> > way to go.
> Ok, lets take this example:
> 
> I have project1.

I see two cases here. 

First when we drop folders to Eclipse from Desktop. In this case 'Default' should resolve locations against existing variables. We should not create new variables.
And this report describes this case.

Second. Copying projects or folders inside Eclipse. If path variables are used at the copy source, it's fine to copy them (see my comment 2).
I may be confused about new variables, like PROJECT_LOC1. But it seems reasonable to postfix with a number conflicting variables.

> > What you are saying in 'Yet yet another point' is actually what I think is
> > fine. We should resolve against existing variables. If the issue is an IPath
> > limitation, maybe we should fix this limitation instead of making workaround
> in
> > the Resources layer.

PARENT-COUNT-PROJECT_LOC sounds like a good compromise.

Comment 9 Serge Beauchamp 2010-02-09 11:00:59 EST

(In reply to comment #8)
> (In reply to comment #7)
> > The linked resource location would itself be:
> > 
> > PARENT-1-PROJECT_LOC/file.txt
> 
> Makes sense. I think this is a reasonable workaround for the IPath limitation.
> And seems to be consistent with other IDEs, right?
> 
That's right, yes.

Visual Studio, for example, always encode files as (equivalent of) PROJECT_LOC relative when added to the project.

That would be the same in our case.

> > (In reply to comment #6)
> > > Sorry, but I'm not convinced that creating path variables silently is the
> > right
> > > way to go.
> > Ok, lets take this example:
> > 
> > I have project1.
> 
> I see two cases here. 
> 
> First when we drop folders to Eclipse from Desktop. In this case 'Default'
> should resolve locations against existing variables. We should not create new
> variables.
> And this report describes this case.
> 
> Second. Copying projects or folders inside Eclipse. If path variables are used
> at the copy source, it's fine to copy them (see my comment 2).
> I may be confused about new variables, like PROJECT_LOC1. But it seems
> reasonable to postfix with a number conflicting variables.
> 
> > > What you are saying in 'Yet yet another point' is actually what I think is
> > > fine. We should resolve against existing variables. If the issue is an IPath
> > > limitation, maybe we should fix this limitation instead of making workaround
> > in
> > > the Resources layer.
> 
> PARENT-COUNT-PROJECT_LOC sounds like a good compromise.

Ok fine, so I think we've reached an agreement:

1) The code will be changed so that no variables will be automatically created when dragging files unto a project, but instead the location will be recorded as a PARENT-COUNT-VAR directly in the location (if required).

2) Path variables are still created automatically when moving and copying resources between projects.

Does that make sense?

Comment 10 Szymon Brandys 2010-02-09 11:07:32 EST

Yes. Thanks Serge.

Comment 11 Serge Beauchamp 2010-02-10 14:42:51 EST

Created attachment 158766 [details]
Implementation Patch

Comment 12 Serge Beauchamp 2010-02-10 15:29:12 EST

Created attachment 158773 [details]
Implementation Patch

correct a junit test case

Comment 13 Serge Beauchamp 2010-02-12 07:28:30 EST

This is now fixed on head.

Comment 14 Serge Beauchamp 2010-02-22 06:45:07 EST

*** Bug 302447 has been marked as a duplicate of this bug. ***

Comment 15 Serge Beauchamp 2010-03-12 08:46:40 EST

fixed in I20100311-1616