Bug 316269

Comment 1 Markus Keller 2010-06-14 10:37:27 EDT

In 3.4.2, it worked as expected for binary-imported plug-ins, but it already failed when using importAsBinaryWithLinks.

I guess the problem is that PluginImportOperation#importPlugin(IPluginModelBase, IProgressMonitor) used to call RepositoryProvider.unmap(project); and later only called
project.setPersistentProperty(PDECore.EXTERNAL_PROJECT_PROPERTY, PDECore.BINARY_PROJECT_VALUE); but left the session property alone.

But in HEAD (and in the broken case in 3.4.2), the session and persistent properties are both set and thus RepositoryProvider.isShared(IProject) returns true.

Moving to PDE.

Comment 2 Ankur Sharma 2011-03-21 15:05:32 EDT

Created attachment 191636 [details]
Patch

Patch removes the setting of repository provider as BinaryRepositoryProvider. Will commit it once am sure  of the real purpose of BinaryRepositoryProvider.

Comment 3 Ankur Sharma 2011-03-28 04:56:00 EDT

Dani, plz confirm if the fix is good.

Comment 4 Dani Megert 2011-03-28 12:27:06 EDT

>Will commit it once am sure  of the real purpose of BinaryRepositoryProvider.
OK, what is its real purpose?

Comment 5 Dani Megert 2011-03-29 06:02:36 EDT

Comment on attachment 191636 [details]
Patch

The patch fixes the importer but it won't fix it for existing workspaces with imported binary projects. However, I think we have to live with that.

Comment 6 Ankur Sharma 2011-03-29 07:16:47 EDT

(In reply to comment #4)
> >Will commit it once am sure  of the real purpose of BinaryRepositoryProvider.
> OK, what is its real purpose?

It helps identify a project as binary. WorkspaceModelManager.isBinary uses it. If project has PDECore.EXTERNAL_PROJECT_PROPERTY persistent property attached to it, it is consider to be binary. The BinaryRepositoryProvider will then provide the resource operations to delete/move file/folders in the binary project.

Comment 7 Dani Megert 2011-03-29 08:10:57 EDT

(In reply to comment #6)
> (In reply to comment #4)
> > >Will commit it once am sure  of the real purpose of BinaryRepositoryProvider.
> > OK, what is its real purpose?
> 
> It helps identify a project as binary. WorkspaceModelManager.isBinary uses it.
> If project has PDECore.EXTERNAL_PROJECT_PROPERTY persistent property attached
> to it, it is consider to be binary. The BinaryRepositoryProvider will then
> provide the resource operations to delete/move file/folders in the binary
> project.
Wouldn't that imply that we can't remove the repository information?

Comment 8 Ankur Sharma 2011-03-30 10:37:25 EDT

(In reply to comment #7)
> Wouldn't that imply that we can't remove the repository information?

That is true. The way Team decides a project to be isShared by checking if the project has a provider which extends RepositoryProvider. Thus, if we attach BinaryRepositoryProvider, then Team will consider binary projects as shared.

Like I mentioned before, the purpose of it is to get callbacks for the resource operations. This hook is done by contributing it to extension point 'org.eclipse.team.core.repository' (which needs it to be extending RepositoryProvider). Thus, any contributor which asks for callback for repository resource operations will be considered as shared.

So the only real way to fis it would be have Team add a new distinction between the two. My fix is a compromise.

Comment 9 Dani Megert 2011-03-30 11:05:52 EDT

(In reply to comment #8)
> (In reply to comment #7)
> > Wouldn't that imply that we can't remove the repository information?
> 
> That is true. The way Team decides a project to be isShared by checking if the
> project has a provider which extends RepositoryProvider. Thus, if we attach
> BinaryRepositoryProvider, then Team will consider binary projects as shared.
> 
> Like I mentioned before, the purpose of it is to get callbacks for the resource
> operations. This hook is done by contributing it to extension point
> 'org.eclipse.team.core.repository' (which needs it to be extending
> RepositoryProvider). Thus, any contributor which asks for callback for
> repository resource operations will be considered as shared.
> 
> So the only real way to fis it would be have Team add a new distinction between
> the two. My fix is a compromise.
Sorry to be dense but isn't your "compromise" breaking the resource operations for operations for every imported project?

Comment 10 Ankur Sharma 2011-03-31 01:14:48 EDT

(In reply to comment #9)
> Sorry to be dense but isn't your "compromise" breaking the resource operations
> for operations for every imported project?

I wasn't happy with the approach either thats the very reason I requested for code review. What you suggest would be right approach here?

Comment 11 Dani Megert 2011-03-31 11:50:39 EDT

We can't remove the repository provider as this would break the checks when moving or editing linked files or folders in the imported project.

In addition, the fix is not complete as indicated in comment 5.

The best compromise is to had a little layer breaker in JDT UI and test for the PDE's persisted "imported" property.

Comment 12 Dani Megert 2011-03-31 11:55:07 EDT

Fixed in HEAD (NonSharedProjectFilter.java, rev. 1.12).

Comment 13 Raksha Vasisht 2011-04-27 07:09:41 EDT

Verified for 3.7 M7 with I20110425-1800.