Bug 577605 - [target] allow project classpath entries to contribute to the resolved target content
Summary: [target] allow project classpath entries to contribute to the resolved target...
Status: NEW
Alias: None
Product: PDE
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 4.22   Edit
Hardware: All All
: P3 enhancement (vote)
Target Milestone: ---   Edit
Assignee: PDE-UI-Inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-12-02 23:22 EST by Christoph Laeubrich CLA
Modified: 2021-12-04 14:02 EST (History)
4 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Christoph Laeubrich CLA 2021-12-02 23:22:30 EST
This is related to a discussion at m2e [1] about further integration of maven<->PDE

To summarize, there is a demand to manage dependencies (what are bundles) inside maven (aka pom.xml) to allow automatic version management through standard maven tools (especially dependabot).

While making such a project to compile is relative straight forward via either maven classpath-container [2] or a classpath variable[3] it leaves the problem that PDE is not able to understand such entries to be used in runtime-configurations (e.g. products, launches, features, require-bundle, ...) as this would require additional add these as target content.

As for sure it would be possible to invent new target locations, configuration options and alike, a more generic approach would be if PDE allows to include project classpathentries (assuming they are regular bundles) in the current target platform state.

The proposal is that PDE includes all classpath items that have an attribute of
<attribute name="pde.target" value="include"/> in the current target automatically, a full example might looks like this:

<classpathentry kind="var" path="M2_REPO/commons-io/commons-io/2.8.0/commons-io-2.8.0.jar">
	<attributes>
		<attribute name="pde.target" value="include"/>
	</attributes>
</classpathentry>


Given that I have an empty target platform set, it will still include commons-io.

Beside always including such items no further actions or considerations are necessary (e.g. no need to worry about finding dependencies or alike), duplicate items are simply filtered by there path name and this will behave like a Directory-Location pointing directly to the jar file as its only component.

For the UI-Part I would think it will be useful to show a new root-item in the target named "project managed bundles" that appears if there was more than one item discovered. Maybe there could be an attribute in the target-file to opt-out for this behavior to allow people to still disable this if desired.

If it could bee agreed that this is useful I could offer to contribute some code towards this feature via gerrits.

[1] https://github.com/eclipse-m2e/m2e-core/issues/418
[2] <classpathentry kind="con" path="org.eclipse.m2e.MAVEN2_CLASSPATH_CONTAINER" />
[3] <classpathentry kind="var" path="M2_REPO/commons-io/commons-io/2.8.0/commons-io-2.8.0.jar" />
Comment 1 Andrey Loskutov CLA 2021-12-03 01:35:35 EST
So given an empty target file, it will mean nothing in an empty workspace, it will mean something different after project import and again something different if a project will be closed? Such target would be not self-contained, and sharing it will not make any sense? Also target that works on one workspace would not work on another one, with little chance to understand the difference.

What about existing explicit target files - would they automatically include such classpath entries, even if not desired? E.g. targets that represent production environment would include all possible libraries from the workspace that can never be seen in the real product? Would be a constant pain point trying to sort out problems only visible in production or on concrete workspace etc.

So classpath that is supposed to contribute to a target would need to have filter to which target it can contribute, or targets would have filters from which projects they want see contributions, or both?
Comment 2 Christoph Laeubrich CLA 2021-12-03 01:46:05 EST
(In reply to Andrey Loskutov from comment #1)
> So given an empty target file, it will mean nothing in an empty workspace,
> it will mean something different after project import and again something
> different if a project will be closed? 

I would expect that one needs to open the target and set it again to take these changes into effect.

> Such target would be not self-contained, and sharing it will not
> make any sense? Also target that
> works on one workspace would not work on another one, with little chance to
> understand the difference.

In fact currently targets can already reference variables, directories, relative or absolute locations and so on. So if sharing the target across different setups is the main goal this feature should most probably not used.

But given that a target is in most case not that stand-alone but belongs to a project-set (e.g. github project with target file and projects) this could work quite nice.

> What about existing explicit target files - would they automatically include
> such classpath entries, even if not desired? E.g. targets that represent
> production environment would include all possible libraries from the
> workspace that can never be seen in the real product? Would be a constant
> pain point trying to sort out problems only visible in production or on
> concrete workspace etc.

As mentioned above, there could be a flag to either enable or disable such behavior, weather or not it should be enabled by default could be decided on.

Nerveless I don't expect much confusion here as if one e.g. uses this to import dependencies from maven its very likely people do so because the also build the project with maven so everything will be available there as well, so it is an active choice for them.

> So classpath that is supposed to contribute to a target would need to have
> filter to which target it can contribute, or targets would have filters from
> which projects they want see contributions, or both?

I won't complicate this too much, a simple on/off attribute in target.xml would be enough. One might even use the existing "content" tab to control what is actually included.
Comment 3 Christoph Laeubrich CLA 2021-12-03 02:13:37 EST
(In reply to Andrey Loskutov from comment #1)
> So given an empty target file, it will mean nothing in an empty workspace,
> it will mean something different after project import and again something
> different if a project will be closed?

After thinking a bit more about this, I must confess that I find it working that way very interesting.

That would finally allow to develop osgi-plugins in PDE without having a target at all.

Assume you have a plain maven project that has all its 'target content' defined as maven dependencies, then importing it (with a plugin setting the necessary classpath attributes) would instantly allow you to create a run-config as all requirements become eligible for bundle-selection.

Thinking a bit further, this would even allow to right-click that project and choose 'Run As > OSGi Application' as it is clear from the beginning what to include as the project already defines all items as 'pde.target=include' ...
Comment 4 Christoph Laeubrich CLA 2021-12-03 02:24:15 EST
(In reply to Andrey Loskutov from comment #1)
> it will mean something different after project import and again
> something different if a project will be closed?

Sorry for tripple-post but I want to add another aspect:
Maybe my wording/initial idea to bind this to the target is not necessary at all.

Thinking one more time about your concerns here, one could rethink the idea a bit different:

Lets say we have a traditional target and a bundle project initially closed, now we open that project (name it test.bundle) the test.bundle becomes visible to the workspace and I can use it the usual way.

Lets assume this bundle has the proposed 'pde.target=include' in its classpath container, opening this project could behave as if I have opened test.bundle and a (invisible) project commons-io, so both bundles are available as if I have imported them as separate projects.

This should nicely integrate in the current workflow model (would then be a bit like 'Close unrelated projects' but the other way round).
Comment 5 Andrey Loskutov CLA 2021-12-03 02:25:27 EST
(In reply to Christoph Laeubrich from comment #2)
> (In reply to Andrey Loskutov from comment #1)
> > So given an empty target file, it will mean nothing in an empty workspace,
> > it will mean something different after project import and again something
> > different if a project will be closed? 
> 
> I would expect that one needs to open the target and set it again to take
> these changes into effect.

Why that? This would render the new proposal ad absurdum - I have a project that is supposed to *automatically* contribute to target, but it is not contributing because it was opened later as other projects, or vice versa.

> > Such target would be not self-contained, and sharing it will not
> > make any sense? Also target that
> > works on one workspace would not work on another one, with little chance to
> > understand the difference.
> 
> In fact currently targets can already reference variables, directories,
> relative or absolute locations and so on. So if sharing the target across
> different setups is the main goal this feature should most probably not used.

Well, typically these "variable" locations aren't variable but fixed, user *typically* can't change them by creating/closing projects in the workspace.

So by giving a well defined target typically one wants a *reproducible* result. Otherwise one could always use "current platform".

> But given that a target is in most case not that stand-alone but belongs to
> a project-set (e.g. github project with target file and projects) this could
> work quite nice.
> 
> > What about existing explicit target files - would they automatically include
> > such classpath entries, even if not desired? E.g. targets that represent
> > production environment would include all possible libraries from the
> > workspace that can never be seen in the real product? Would be a constant
> > pain point trying to sort out problems only visible in production or on
> > concrete workspace etc.
> 
> As mentioned above, there could be a flag to either enable or disable such
> behavior, weather or not it should be enabled by default could be decided on.

If we go this way, it must be explicit flag added to allow such content to be there, existing targets shouldn't need any adoption and should have no additional classpath entries included.
Comment 6 Christoph Laeubrich CLA 2021-12-03 02:55:20 EST
(In reply to Andrey Loskutov from comment #5)
> (In reply to Christoph Laeubrich from comment #2)
> > (In reply to Andrey Loskutov from comment #1)
> > > So given an empty target file, it will mean nothing in an empty workspace,
> > > it will mean something different after project import and again something
> > > different if a project will be closed? 
> > 
> > I would expect that one needs to open the target and set it again to take
> > these changes into effect.
> 
> Why that? This would render the new proposal ad absurdum - I have a project
> that is supposed to *automatically* contribute to target, but it is not
> contributing because it was opened later as other projects, or vice versa.

You are right, see my last comment for a more consistent proposal.
Comment 7 Mickael Istria CLA 2021-12-03 03:21:03 EST
That's a very interesting proposal. And I support that both goals are equally important: better work with Maven or other standard project description formats, and provide a way to clearly and strictly define a set of allowed dependencies. The later is already well covered by .target which can be shared (unless using properties or other stuff that's not portable, but it's not no usual), and we should not "break" it. Support for .target files and what it means for the project should remain as it since many RCP apps heavily rely on it.
So IMO, any strategy that is about adding Maven or other .classpath deps into the .target definitely has to be an opt-in. However, I don't think it's an opt-in from IDE perspective, but according to the project user is working on. And those projects usually define... a .target file to contributors to use.
So the best place where to put the switch seems to be the .target; because some projects will want it and some others won'. And since it's an opt-in in the .target, I think the simplest way to express it would be a new target location.
Comment 8 Christoph Laeubrich CLA 2021-12-03 03:28:09 EST
(In reply to Mickael Istria from comment #7)
> I think the simplest way to express it would be a new target
> location.

I think we don't need a target location or switch at all see:

https://bugs.eclipse.org/bugs/show_bug.cgi?id=577605#c4

If you think its easier to discuss if I open a new ticket with the improved idea/suggestion.
Comment 9 Mickael Istria CLA 2021-12-03 03:37:06 EST
(In reply to Christoph Laeubrich from comment #8)
> https://bugs.eclipse.org/bugs/show_bug.cgi?id=577605#c4
> [...]
> Lets assume this bundle has the proposed 'pde.target=include' in its classpath container, opening this project could behave as if I have opened test.bundle and a (invisible) project commons-io, so both bundles are available as if I have imported them as separate projects.

Whether to "pollute" the .target or not is to be decided on the individual bundle level; it's really a project decision to decide whether to accept such "pollution" or not, and projects typically provide the .target so it's where the decision can be placed.

I personally work with a few dozen distinct projects in the workspace, rarely close bundles (so I have ~200 of them constantly open in the worksapce), each project provides one or more .target and when I switch between projects, I switch the .target and get the dependencies constraints expected by the project.
If I'm on eg. Platform or TM4E and Platform or TM4E recommends that we use Maven deps, I use their target files that would be open to Maven/classpath deps; if I'm working on an older version of jbosstools, then I want to have the exact target content that this version is supposed to work with and don't want this .target polluted by other unrelated (eg TM4E) bundles I've left open on the workspace.
Comment 10 Christoph Laeubrich CLA 2021-12-03 03:41:28 EST
(In reply to Mickael Istria from comment #9)
> then I want to have
> the exact target content that this version is supposed to work with and
> don't want this .target polluted by other unrelated (eg TM4E) bundles I've
> left open on the workspace.

You are already "polluting" your "bundle-space" with items not relevant if you don't close unrelated bundles (or simply use different workspace) so I don't see much of a difference here.

Also say you have two targets and both would opt-in to consume project-bundle-dependencies the situation won't be much different.
Comment 11 Mickael Istria CLA 2021-12-03 04:01:06 EST
(In reply to Christoph Laeubrich from comment #10)
> You are already "polluting" your "bundle-space" with items not relevant if
> you don't close unrelated bundles (or simply use different workspace) so I
> don't see much of a difference here.

It's similar in theory, however the workspace is a simpler concept to map by user than the "transitive deps of all workspace projects".
The difference is that it's easy for me to notice and close the project I don't want in my bundle space as it's visible in my IDE and I already manipulated it at least once (to open/import it). While with an extensible target, it would be some libs that I'm not much aware of that would be suggested in eg Require-Bundle or Import-Package, it's much harder to realize those are *not* expected to be there in the scope of the project and where they do come from and what to close. So the problem of "I get a proposal that's not valid for my project" is much more frequent and much harder to resolve.

What's the actual issue you foresee with this "add to TP all transitive deps in classpath of workspace folders" being defined in the .target file? The conceptual match is perfect (after all a Target Location is about adding some more bundles to TP and that's what we want to do here), and it covers the initial need + Andey's/my concerns about predictability/portability of dev environments.
Comment 12 Christoph Laeubrich CLA 2021-12-03 04:12:21 EST
(In reply to Mickael Istria from comment #11)
> What's the actual issue you foresee with this "add to TP all transitive deps
> in classpath of workspace folders" being defined in the .target file?

Actually I don't want to define a target file to use PDE at all, second adding more an more location types is confusing, third its hard to do right (as Andrey pointed out) because targets only "change" when you resolve, so if I open/close a project I have to reopen the target and reload it, that sounds confusing.

Also I don't think it helps much for the "where is it from", because currently a workspace-project can already override from the target.

If we think users should be made more aware of this, we can better show a new "Project Scoped Plugin Dependencies" Classpath container and list items defined in the project there, it should then also be possible to jump there if one is uncertain where something originates from.

As there is only "one-workspace-one-target" right now, I would always recommend to use different workspace for different targets anyways, everything else might work but requires special attention by the user (with or without this change).
Comment 13 Mickael Istria CLA 2021-12-03 04:49:16 EST
(In reply to Christoph Laeubrich from comment #12)
> Actually I don't want to define a target file to use PDE at all

If it seems desired in the majority of cases, this "Workspace project deps" Target Location could be added to the default target (usually IDE installation) so it would be on by default.

> second adding more an more location types is confusing

IMO adding things into TP definition to define what does the TP contain is less confusing that adding unpredictable behavior or new flags here and there; to actually control the TP resolution...

> third its hard to do right
> (as Andrey pointed out) because targets only "change" when you resolve, so
> if I open/close a project I have to reopen the target and reload it, that
> sounds confusing.

Updating TP on project change can be relatively easily implemented by PDE adding a resource listener and triggering TP update when the current TP contains the modified project.

> If we think users should be made more aware of this, we can better show a
> new "Project Scoped Plugin Dependencies" Classpath container and list items
> defined in the project there, it should then also be possible to jump there
> if one is uncertain where something originates from.

I'm a bit confused here; where would such container be and what would it contain? I'm not sure what "items defined in the project" means.

> As there is only "one-workspace-one-target" right now, I would always
> recommend to use different workspace for different targets anyways,
> everything else might work but requires special attention by the user (with
> or without this change).

It's you recommendation, but in my experience it's not what works best to me and I don't plan to revert to the 1 workspace per project workflow. And I know many other people who would keep their single-workspace-for-everything workflow. So we can't tell people "we'll break your workflow, please change it", because usually, their workflow is more important for their productivity than their tool; it's often easier to change tool than to change workflow.
Comment 14 Christoph Laeubrich CLA 2021-12-03 04:58:31 EST
(In reply to Mickael Istria from comment #13)
> > second adding more an more location types is confusing
> 
> IMO adding things into TP definition to define what does the TP contain is
> less confusing that adding unpredictable behavior or new flags here and
> there; to actually control the TP resolution...

But if target content is different when different projects are there it is not well-defined anymore as Andrey pointed out.

While on the other hand a project I import currently already contributes one bundle/feature/... or multiple items (eg. products or sites), that would just enhance this concept to allow one project to contribute more than one bundle.

> 
> > third its hard to do right
> > (as Andrey pointed out) because targets only "change" when you resolve, so
> > if I open/close a project I have to reopen the target and reload it, that
> > sounds confusing.
> 
> Updating TP on project change can be relatively easily implemented by PDE
> adding a resource listener and triggering TP update when the current TP
> contains the modified project.

As this could be the case, automatic updates are currently not supported and often undesired as they require network-io and CPU resources to resolve it.

> > If we think users should be made more aware of this, we can better show a
> > new "Project Scoped Plugin Dependencies" Classpath container and list items
> > defined in the project there, it should then also be possible to jump there
> > if one is uncertain where something originates from.
> 
> I'm a bit confused here; where would such container be and what would it
> contain? I'm not sure what "items defined in the project" means.

If you expand a PDE project there is a child element that reads "Plugin Dependencies" if you expand that it shows everything is bound by PDE to that project (m2e has "Maven Dependencies" container), if we wan't to express that there is something happen "special" pde could add "Project Scoped Plugin Dependencies" as these are effectively are derived from the projects (e.g. "Maven Dependencies" container) in contrast to the target and workspace where "Plugin Dependencies" are currently sourced from.

> > As there is only "one-workspace-one-target" right now, I would always
> > recommend to use different workspace for different targets anyways,
> > everything else might work but requires special attention by the user (with
> > or without this change).
> 
> It's you recommendation, but in my experience it's not what works best to me
> and I don't plan to revert to the 1 workspace per project workflow. And I
> know many other people who would keep their single-workspace-for-everything
> workflow. So we can't tell people "we'll break your workflow, please change
> it", because usually, their workflow is more important for their
> productivity than their tool; it's often easier to change tool than to
> change workflow.

This won't break anything compared to the current state. Andrey claimed it might confuse if things diverge between build+workspace but that's a different story and your workspace already diverges from the setup that is actually build on the ci-server.
Comment 15 Mickael Istria CLA 2021-12-03 05:09:38 EST
(In reply to Christoph Laeubrich from comment #14)
> But if target content is different when different projects are there it is
> not well-defined anymore as Andrey pointed out.

I don't think there is anything in .target that requires the resolution result is reproducible. Think about a .target using some composite p2 repo, remote content can be changing, resolution results as well...
So to me, the argument of "well-defined" as in "static and predictable" is not valid. Particularly for a new and opt-in location type.

> > Updating TP on project change can be relatively easily implemented by PDE
> > adding a resource listener and triggering TP update when the current TP
> > contains the modified project.
> 
> As this could be the case, automatic updates are currently not supported and
> often undesired as they require network-io and CPU resources to resolve it.

OK, so we can just emit some warning or notification, or find another form of interaction for that. I think this part is not so critical.
And again, same is true if one uses p2 repo that are mutable: user has to refresh to leverage the change.

> If you expand a PDE project there is a child element that reads "Plugin
> Dependencies" if you expand that it shows everything is bound by PDE to that
> project (m2e has "Maven Dependencies" container), if we wan't to express
> that there is something happen "special" pde could add "Project Scoped
> Plugin Dependencies" as these are effectively are derived from the projects
> (e.g. "Maven Dependencies" container) in contrast to the target and
> workspace where "Plugin Dependencies" are currently sourced from.

OK. I don't really see a clear gain with such an extra container to be honest.

> This won't break anything compared to the current state.

I explained in comment #9 how this would break my workflow at least. So some things, expectations, workflows would be broken.
Comment 16 Andrey Loskutov CLA 2021-12-03 05:11:44 EST
To clarify my main concern and explain where it is coming from: I have a lab with ~100 developers working on different products / releases. If something breaks, they call me for help.

Each workspace has a clearly defined target platform, that no one except platform / product owners should change.

I honestly don't want to see 99 people running into troubles and asking me for help if they start see "unexpected" errors / target platform conflicts / debugging troubles just because they had some projects imported / opened / closed.

My naive expectation (and requirement to the proposal here) is that existing target platform definitions can simply stay "as is" without changes and remain stable independently of the possible workspace projects after this enhancement is implemented.
Comment 17 Hannes Wellmann CLA 2021-12-04 14:02:38 EST
(In reply to Christoph Laeubrich from comment #14)
> (In reply to Mickael Istria from comment #13)
> > 
> > > third its hard to do right
> > > (as Andrey pointed out) because targets only "change" when you resolve, so
> > > if I open/close a project I have to reopen the target and reload it, that
> > > sounds confusing.
> > 
> > Updating TP on project change can be relatively easily implemented by PDE
> > adding a resource listener and triggering TP update when the current TP
> > contains the modified project.
> 
> As this could be the case, automatic updates are currently not supported and
> often undesired as they require network-io and CPU resources to resolve it.
> 

I have not yet looked into the details, but I think it should be possible to implement a partial 'refresh/update' of of a target location (as long it does not interfere with other locations like the default 'InstallableUnit'-type does by sharing its repositories with all other IULoctions in the target).
That partial 'refresh' would only re-resolve the changed location and then do what's done in LoadTargetDefinition.resetPlatform(). Respectively it would be a LoadTargetDefinitionJob that skips resolving all locations but the one to be refreshed.

It could/should be a new generic mechanism that a ITargetLocation can notify the workspace that it would like to be refreshed. Maybe a new sub-Interface of (or a new default method for) ITargetLocation that allows to set a notifier to a location, that the location can use to notify the workspace and request a partial reload.

Then resource-listener listeners could be used by the corresponding location to detect the demand for a refresh as Mickael suggested and then request a reload from the TargetPlatformService (or so). The (partial) reload could happen automatically or only with user-permission.

If m2e's Maven location then facilitates that new mechanism, in combination with your just proposed resolution of workspace-Maven-projects (https://github.com/eclipse-m2e/m2e-core/issues/419) and m2e ability to resolve pom-projects it should do the job for the initial problem, shouldn't it?