This Bugzilla instance is deprecated, and most Eclipse projects now use GitHub or Eclipse GitLab. Please see the deprecation plan for details.
Bug 544977 - Overenthusiastic "This plug-in does not export all of its packages"
Summary: Overenthusiastic "This plug-in does not export all of its packages"
Status: RESOLVED FIXED
Alias: None
Product: PDE
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 4.10   Edit
Hardware: PC Windows 10
: P3 normal with 1 vote (vote)
Target Milestone: 4.14 RC2   Edit
Assignee: Lars Vogel CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on: 309274
Blocks:
  Show dependency tree
 
Reported: 2019-03-02 05:24 EST by Ed Willink CLA
Modified: 2020-07-07 07:48 EDT (History)
14 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Ed Willink CLA 2019-03-02 05:24:57 EST
M3. I am suddenly hit by many "This plug-in does not export all of its packages" warnings on perhaps 25% of my plugins. Has the preference changed?

I know of no guideline that all packages should be exported, indeed it is often very desirable to not-export packages. The warning therefore seems wrong.

I could just zap my severity preferences, but this seems like one that Eclipse has got wrong and is just going to cause widespread irritation / confusion.
Comment 1 Vikas Chandra CLA 2019-03-02 06:37:41 EST
See https://bugs.eclipse.org/bugs/show_bug.cgi?id=309274
Comment 2 Dani Megert CLA 2019-03-02 09:02:05 EST
See bug 309274 which deals with a real problem.

It's good practice to define the visibility and version restriction for each package.

I strongly recommend to 1. keep it as warning and 2. store in the project settings.
Comment 3 Ed Willink CLA 2019-03-03 03:12:05 EST
But bug 309274 is that there is a NEW package that is not exported from an API tooled plugin.

It should not apply

a) if there is no API tooling for a plugin

b) the package was already not exported in the baseline.

Why is API tooling infecting ordinary usage?
Comment 4 Ed Willink CLA 2019-03-03 06:12:14 EST
This is a regression wrt 4.10. A fix in 4.11 seems desirable. Else if not fixable, defer the Bug 309274 fix or leave the default severity as Ignore.
Comment 5 Vikas Chandra CLA 2019-03-03 06:37:26 EST
>>This is a regression wrt 4.10

I don't think this is a regression as user now have more information ( which was hidden earlier).

Comment#3 point a) looked fine initially to me but it has the potential to confuse users. ( A project without API tools will not give this error even if the option is set). Keeping 2 different defaults for non-api and api project is again confusing.

I think easiest way is to change project preference or workspace preference.

However, I think we can discuss further on this bug for 4.12 if there is a solution that works best for all scenarios.
Comment 6 Ed Willink CLA 2019-03-03 06:55:05 EST
(In reply to Vikas Chandra from comment #5)
> ( A project without API tools will not give this error even
> if the option is set).

Well perhaps this is the regression then. I have numerous, particularly test, plugins that do not have API tooling but which now report this warning => regression.
Comment 7 Vikas Chandra CLA 2019-03-03 07:11:10 EST
>>Well perhaps this is the regression then.

I was telling that if we make such a change ( comment#3 point a), then this will happen. 

This doesn't happen as of now.
Comment 8 Ed Willink CLA 2019-03-03 08:18:18 EST
You seem to misunderstand. '4.11M2' or rather its equivalent I-build was fine; no stupid missing package warnings.

For 4.11M3 you fixed Bug 309274 and introduced this regression; numerous missing package warnings.

Therefore the fix @M3 was bad and should be reworked for RC2 or retracted for more consideration in 4.12..
Comment 9 Dani Megert CLA 2019-03-03 10:51:15 EST
By all means, just change your preference(s). We won't change this. You can of course reopen this again and we will just ignore this bug.
Comment 10 Ed Willink CLA 2019-03-08 07:27:05 EST
It should of course be possible to mark the 'missing' packages as x-internal, but it isn't. Bug 545211.
Comment 11 Vince Molnár CLA 2019-04-23 13:59:46 EDT
Even though I see the decision and the arguments, I have to back Ed here. It is very annoying to have the problems view littered with false positive warnings. The warning would make sense if we could explicitly mark packages as internal. Turning it off, however, will hide the true positives as well, which will occur during development.
Comment 12 Ed Willink CLA 2019-04-24 03:33:52 EDT
(In reply to Vince Molnár from comment #11)
> I have to back Ed here.

My opinion is now less decided. I have discovered that there is a quick fix that does an export all packages as internal and that a bug in it has been fixed; Bug 545776. It is a nuisance but perhaps it is necessary.

I applied the quick fix and found a few packages to think about, but as soon as I made some of them non-internal I got PDE @since violations, so even those packages that might be exported are now explicitly not-exported. And because they are now explicitly not exported, there will be no prompts to encourage export at the next version change. I think the code is now worse as a result.

Many inconvenient changes seem to be driven by Java changes, particularly modules and so I wonder if the real motivation here is to accommodate some problem with Java modules. If so, I guess we have to live with it since the platform teams clearly do not have the cycles to provide all the new Java support without impacting Java 8.
Comment 13 Axel Uhl CLA 2019-05-17 06:17:03 EDT
We had a warning-free workspace covering close to 1M LOC. Then we upgraded to 2019-03 and now see 103 warnings "This plug-in does not export all of its packages".

It is irritating. We don't develop Eclipse plug-ins but straight OSGi bundles that we run in an Equinox container. As such, the message ("plug-in") is confusing.

Our default policy is that packages added to a bundle are not exported. But with this new warning we have to explicitly state that a new package is not to be exported.

I find this very inconvenient and unexpected. We now have to extend our set-up instructions to contain a hint to turn off this warning. I find this the wrong default.
Comment 14 Dani Megert CLA 2019-05-27 12:27:16 EDT
(In reply to Axel Uhl from comment #13)
> We now have to extend our
> set-up instructions to contain a hint to turn off this warning. I find this
> the wrong default.
No you don't just store it as project specific setting.
Comment 15 Dani Megert CLA 2019-05-27 12:30:40 EDT
(In reply to Axel Uhl from comment #13)
> It is irritating. We don't develop Eclipse plug-ins but straight OSGi
> bundles 
OK. But did you maybe then wrongly declare it as "plug-in" and enable the PDE builder? That would be wrong and of course the result expected.
Comment 16 Axel Uhl CLA 2019-05-27 16:48:41 EDT
(In reply to Dani Megert from comment #15)
> (In reply to Axel Uhl from comment #13)
> > It is irritating. We don't develop Eclipse plug-ins but straight OSGi
> > bundles 
> OK. But did you maybe then wrongly declare it as "plug-in" and enable the
> PDE builder? That would be wrong and of course the result expected.

Dani, we obviously need the ManifestBuilder to validate OSGi dependencies and such. I've played with removing the org.eclipse.pde.PluginNature from the .project file, but that doesn't make a difference. I'm still getting the warning even after removing the PluginNature. I guess, the warning is produced by the ManifestBuilder, and I currently don't see how we could build an OSGi project properly in Eclipse without it.
Comment 17 Axel Uhl CLA 2019-05-27 16:50:23 EDT
(In reply to Dani Megert from comment #14)
> (In reply to Axel Uhl from comment #13)
> > We now have to extend our
> > set-up instructions to contain a hint to turn off this warning. I find this
> > the wrong default.
> No you don't just store it as project specific setting.

Sorry, I don't understand this. We didn't store it as a project-specific setting. What helped was turning the warning off in Preferences -- Plug-in Development -- Compilers for the entire workspace. Of course we don't want to have to repeat this for each new OSGi bundle project that we add.
Comment 18 Dani Megert CLA 2019-05-28 06:20:37 EDT
(In reply to Axel Uhl from comment #16)
> (In reply to Dani Megert from comment #15)
> > (In reply to Axel Uhl from comment #13)
> > > It is irritating. We don't develop Eclipse plug-ins but straight OSGi
> > > bundles 
> > OK. But did you maybe then wrongly declare it as "plug-in" and enable the
> > PDE builder? That would be wrong and of course the result expected.
> 
> Dani, we obviously need the ManifestBuilder to validate OSGi dependencies
> and such. I've played with removing the org.eclipse.pde.PluginNature from
> the .project file, but that doesn't make a difference. I'm still getting the
> warning even after removing the PluginNature. I guess, the warning is
> produced by the ManifestBuilder, and I currently don't see how we could
> build an OSGi project properly in Eclipse without it.
OK, I see. You use the PDE builder.
Comment 19 Dani Megert CLA 2019-05-28 06:24:59 EDT
(In reply to Axel Uhl from comment #17)
> > No you don't just store it as project specific setting.
> 
> Sorry, I don't understand this. We didn't store it as a project-specific
> setting. What helped was turning the warning off in Preferences -- Plug-in
> Development -- Compilers for the entire workspace. Of course we don't want
> to have to repeat this for each new OSGi bundle project that we add.
What a/no comma can do ;-)

What I wanted to say is: store it in the projects as project specific setting. That way, no one has to adjust the preference. If you have many projects, you can search and replace (or add) the setting in all projects in one go and then commit the changes.
Comment 20 Axel Uhl CLA 2019-05-28 07:02:52 EDT
(In reply to Dani Megert from comment #19)
> > Of course we don't want
> > to have to repeat this for each new OSGi bundle project that we add.
> What a/no comma can do ;-)
> 
> What I wanted to say is: store it in the projects as project specific
> setting. That way, no one has to adjust the preference. If you have many
> projects, you can search and replace (or add) the setting in all projects in
> one go and then commit the changes.

See above. We would have to make sure this happens for each bundle added to the workspace. I claim that the defaults you chose now ignore our case of developing OSGi bundles that are not plugins. In my opinion this is not the best choice. Letting this depend on the PluginNature may be a better choice IMHO.
Comment 21 Dani Megert CLA 2019-05-28 07:09:22 EDT
(In reply to Axel Uhl from comment #20)
> See above. We would have to make sure this happens for each bundle added to
> the workspace.
If with "add", you mean "create a new bundle", then "yes". But since you will get the (unwanted) warning, you could just use the quick fix to set the severity and you're done.


> Letting this depend on the PluginNature may be a better choice IMHO.
I doubt that everything works fine if you manually remove the PDE nature but keep the builder. That's a completely unsupported and untested scenario.
Comment 22 Benjamin Brandl CLA 2019-06-21 02:36:20 EDT
After migrating an Eclipse 4.10 workspace to 4.12, I also have many plug-ins with warning "This plug-in does not export all of its packages".

Within a plug-in a package is declared API by adding a corresponding export package directive. By adding all packages to the exported packages list, all plug-in classes become visible to the environment. The access to internal structures is only restricted by convention and can be overriden. There's good reason to not make internal API structures available.

So exporting all packages and declaring internal packages "x-internal" seems dangerous and counterintuitive to me. I vote for the problem severity being changed back from warning to ignore.
Comment 23 Phil Beauvoir CLA 2019-07-16 05:45:24 EDT
> I vote for the problem severity being changed back from warning to ignore.

+!

Just encountered this regression.

The default should be to NOT export packages, many are not API.
Comment 24 Lars Vogel CLA 2019-12-02 09:45:45 EST
The default should not be a warning. If platform wants this, it should use the project settings.
Comment 25 Lars Vogel CLA 2019-12-02 09:47:10 EST
*** Bug 545211 has been marked as a duplicate of this bug. ***
Comment 26 Dani Megert CLA 2019-12-02 09:52:23 EST
Most project benefit from the warning. Those who do not want this can use project specific settings.
Comment 27 Will Collins CLA 2019-12-02 10:02:16 EST
I'm not really understanding why this is useful for "most users"? The original ticket only referenced one person who happened to forget to export the package before trying to use it. There are any number of solutions for that rather than changing the existing functionality of the warnings to something that, as Benjamin said, is quite dangerous.

I agree with Lars that if you need or want this, you should enable it in your workspace or projects. It should not be a default though.

I "fixed" this by changing my local workspace settings but that needs to be done for every workspace. The structure of our plugins and source control makes changing this on a per-project basis untenable.
Comment 28 Lars Vogel CLA 2019-12-02 10:21:25 EST
(In reply to Dani Megert from comment #26)
> Most project benefit from the warning. Those who do not want this can use
> project specific settings.

No, why should all projects exports all their packages? The benefit of OSGi is that you can and should hide your internal packages.
Comment 29 Lars Vogel CLA 2019-12-02 10:22:20 EST
(In reply to Lars Vogel from comment #28)
> (In reply to Dani Megert from comment #26)
> > Most project benefit from the warning. Those who do not want this can use
> > project specific settings.
> 
> No, why should all projects exports all their packages? The benefit of OSGi
> is that you can and should hide your internal packages.

Dani, if you disagree, please check if Thomas Watson if OSGi recommends to always export all packages. I would be surprised.
Comment 30 Dani Megert CLA 2019-12-02 10:24:27 EST
(In reply to Lars Vogel from comment #29)
> (In reply to Lars Vogel from comment #28)
> > (In reply to Dani Megert from comment #26)
> > > Most project benefit from the warning. Those who do not want this can use
> > > project specific settings.
> > 
> > No, why should all projects exports all their packages? The benefit of OSGi
> > is that you can and should hide your internal packages.
> 
> Dani, if you disagree, please check if Thomas Watson if OSGi recommends to
> always export all packages. I would be surprised.
Well, you can do this too, right?
Comment 31 Lars Vogel CLA 2019-12-02 10:26:01 EST
(In reply to Dani Megert from comment #30)
> Well, you can do this too, right?

Ok, fine for me to check with Thomas or Neil. Please stop closing the bug in this case, until we got confirmation from an OSGi expert.
Comment 32 Neil Bartlett CLA 2019-12-02 10:46:22 EST
(In reply to Lars Vogel from comment #31)
> (In reply to Dani Megert from comment #30)
> > Well, you can do this too, right?
> 
> Ok, fine for me to check with Thomas or Neil. Please stop closing the bug in
> this case, until we got confirmation from an OSGi expert.

With the caveat that I was not present at the meetings where the initial design of OSGi was discussed, it seems clear to me that exporting is not intended to be the default. The fact that you must explicitly enumerate each exported package using the Export-Package header strongly implies this.

If the intention was the opposite then we would not have to write an Export-Package header at all. Instead we would have a "Hide-Package" header or similar, to enumerate the few packages that are NOT meant to be exported.

PDE is not my preferred OSGi tooling for many reasons so I may be missing some subtleties, but this warning doesn't make a lot of sense to me.
Comment 33 Dani Megert CLA 2019-12-02 10:57:21 EST
(In reply to Neil Bartlett from comment #32)
OK.

How about setting it to 'Info' as a compromise?
Comment 34 Phil Beauvoir CLA 2019-12-02 11:04:34 EST
How about just putting it back to how it was so that 99% of Eclipse developers can be happy again.
Comment 35 Dani Megert CLA 2019-12-02 11:10:37 EST
(In reply to Phil Beauvoir from comment #34)
> How about just putting it back to how it was so that 99% of Eclipse
> developers can be happy again.
It's easy to claim this. Do you have any data that backs your "99%"?
Comment 36 Raymond Auge CLA 2019-12-02 11:30:56 EST
At no time should any tool suggest exporting all packages by default.

In fact, the default should actually be to export _nothing_.

The logic being that you're working on implementation until you _expressly_ declare parts which are API.

This is even true of JPMS where the exports are written into your module-info AFTER you know what the package name is and what its intention is.
Comment 37 Thomas Watson CLA 2019-12-02 11:55:55 EST
IIRC the Eclipse project itself had a policy long ago to export ALL packages and mark the ones that are not API as x-internal.  The reasoning for the Eclipse project (right or wrong) was this allowed extenders to reach into internals when there was no other alternative for the current release.  The extender could then experiment with a solution using internals.  Then bugs could be opened to get official API added to support the usecase that required internal usage.

I personally stopped following this policy for bundles in Equinox for any new internal packages.  Although I still have plenty of old internal packages that are still exported (I really just should remove all of them also).  The reason is that it really makes no sense in the OSGi world to export things that are not meant to be API.  Only PDE has any knowledge of x-internal (or x-friends).  The Equinox framework has no knowledge of these directives and the packages are treated as normal exports.  Other build tools for OSGi will give no warning at all if internals are used.

In my experience, I have seen no benefit for exporting internal packages for experimental usage.  I do not recall a single instance, with Equinox bundles, where some internal was found and used by a developer and then they opened a bug and we negotiated a proper API to be used in the future.  In the worse case scenario where some product or extender must use internals they can maintain a fragment to the bundle that has the package and export it themselves.

My vote is this setting should be set to ignore by default.
Comment 38 Lars Vogel CLA 2019-12-02 13:29:13 EST
Thanks Neil, Raymond and Thomas for the detailed input. I suggest we follow the advice of the OSGi experts and set the setting back to ignore in 4.15.
Comment 39 Ed Willink CLA 2019-12-02 15:17:30 EST
(In reply to Lars Vogel from comment #38)
> Thanks Neil, Raymond and Thomas for the detailed input. I suggest we follow
> the advice of the OSGi experts and set the setting back to ignore in 4.15.

This is a very annoying regression and only requires a default preference to be changed.

Surely it can be done for 4.14?
Comment 40 Eclipse Genie CLA 2019-12-02 16:36:05 EST
New Gerrit change created: https://git.eclipse.org/r/153670
Comment 41 Lars Vogel CLA 2019-12-02 16:37:25 EST
(In reply to Eclipse Genie from comment #40)
> New Gerrit change created: https://git.eclipse.org/r/153670

Thomas Watson, please review. IIRC I need another committer for RC2.
Comment 42 Ed Merks CLA 2019-12-03 00:30:30 EST
(In reply to Thomas Watson from comment #37)
> IIRC the Eclipse project itself had a policy long ago to export ALL packages
> and mark the ones that are not API as x-internal.  The reasoning for the
> Eclipse project (right or wrong) was this allowed extenders to reach into
> internals when there was no other alternative for the current release.  The
> extender could then experiment with a solution using internals.  Then bugs
> could be opened to get official API added to support the usecase that
> required internal usage.
> 

Yes, this is exactly what I recall as the historical advice.

> I personally stopped following this policy for bundles in Equinox for any
> new internal packages.  Although I still have plenty of old internal
> packages that are still exported (I really just should remove all of them
> also). 

That would assume that no one is really using them which is a kind of bad assumption when there exists no data to verify it.
 
> The reason is that it really makes no sense in the OSGi world to
> export things that are not meant to be API. 

Yes, that is true. If you export it, someone can and likely will use it.

> Only PDE has any knowledge of
> x-internal (or x-friends).  The Equinox framework has no knowledge of these
> directives and the packages are treated as normal exports.  Other build
> tools for OSGi will give no warning at all if internals are used.
> 

This is also an important fact.

> In my experience, I have seen no benefit for exporting internal packages for
> experimental usage.  

As a counter point, I can't imagine having implemented much of Oomph's p2-derived infrastructure without this policy having been in place though, so my experience says the opposite it true.

> I do not recall a single instance, with Equinox
> bundles, where some internal was found and used by a developer and then they
> opened a bug and we negotiated a proper API to be used in the future. 

At the other end of the producer-consumer chain, certainly we (Oomph guys) did not open any bug(s) to negotiate new p2 API.  In principle we should have, but in practice negotiating APIs is only possible in principle yet highly unlikely to pan out in practice for a number of reasons: 

a) No one exists with whom to negotiate.
b) If someone exists, they don't have time to negotiate and review the changes.
c) Because no one has time, insurmountable hurdles are created (to disguise the underlying we-don't-want-to-do-this). This is fully understandable because new APIs must be documented and maintained forever, which is a new workload that will continue forever, so the person who doesn't have the time currently must agree to take on a new permanent workload/burden.

I would draw your attention at the following as a case in point:

https://bugs.eclipse.org/bugs/show_bug.cgi?id=538077

 
> In the worse case scenario where some product or extender must use internals
> they can maintain a fragment to the bundle that has the package and export
> it themselves.
> 

That's an interesting point.  I hadn't thought of that.

> My vote is this setting should be set to ignore by default.

I don't disagree with the final conclusion.  Why?  Because the Platform UI team itself is also no longer following the approach of exporting everything and using x-internal.  For example, this file has precisely the warning that is the topic of this Bugzilla:

/org.eclipse.urischeme/META-INF/MANIFEST.MF

The projects that comprise the Eclipse SDK are so rife with warnings that no developer will ever notice any new warnings, let alone try to reduce them.  My Platform SDK workspace has 21,737 warnings and 2,196 informational messages.  Certainly in the case of org.eclipse.urischeme, there was no attempt to eliminate the warning via a project-specific preference.

For those of us who maintain projects that are 100% free of all warnings and informational messages, a new warning or informal message is a "big deal".  Reducing warning to info does not make this less of a concern.  In all cases we must take action to get back into a pristine state. So I hate info messages such as this:

At least one of the problems in category 'foo-bar' is not analysed due to a compiler option being ignored
Comment 43 Lars Vogel CLA 2019-12-03 08:11:36 EST
Thanks Ed for the additional review.
Comment 45 Thomas Watson CLA 2019-12-03 08:27:35 EST
(In reply to Ed Merks from comment #42)
> > I personally stopped following this policy for bundles in Equinox for any
> > new internal packages.  Although I still have plenty of old internal
> > packages that are still exported (I really just should remove all of them
> > also). 
> 
> That would assume that no one is really using them which is a kind of bad
> assumption when there exists no data to verify it.
>  

These are internals, projects must have freedom to change internals.  In the past I have renamed x-internal packages with no warning to the community and little to no memory of bug reports being opened asking where the internal went.  Perhaps I got lucky?  There are exceptions to this rule where if an internal proves to be widely used and disruptive to change then a more careful plan can be made to change/remove it (e.g. stuff like the Unsafe class in the base JVM).

But the fact remains that internals are fluid, have no version and are very precarious to depend on.  Any developer potentially broken by use of internals must be prepared to adapt their usage of them each and every release.
Comment 46 Lars Vogel CLA 2019-12-03 08:31:03 EST
(In reply to Thomas Watson from comment #45)
> But the fact remains that internals are fluid, have no version and are very
> precarious to depend on.  Any developer potentially broken by use of
> internals must be prepared to adapt their usage of them each and every
> release.

+1, freezing the internals would kill any option to improve our code base.
Comment 47 Dani Megert CLA 2019-12-03 08:58:07 EST
OK, got it, but note that https://wiki.eclipse.org/Export-Package is still active for the Eclipse top-level project. If someone wants to have it changed, please open a bug report where this can be discussed and post it here.
Comment 48 Thomas Watson CLA 2019-12-03 11:27:02 EST
(In reply to Dani Megert from comment #47)
> OK, got it, but note that https://wiki.eclipse.org/Export-Package is still
> active for the Eclipse top-level project. If someone wants to have it
> changed, please open a bug report where this can be discussed and post it
> here.

I opened PMC bug 553709 to discuss at the PMC level for the project.
Comment 49 Ed Merks CLA 2019-12-03 11:42:30 EST
(In reply to Lars Vogel from comment #46)
> (In reply to Thomas Watson from comment #45)
> > But the fact remains that internals are fluid, have no version and are very
> > precarious to depend on.  Any developer potentially broken by use of
> > internals must be prepared to adapt their usage of them each and every
> > release.
> 
> +1, freezing the internals would kill any option to improve our code base.

Lars, while you +1 that statement.  You don't +1 the statement which precedes it.
 
> There are exceptions to this rule where if an
> internal proves to be widely used and disruptive to change then a more
> careful plan can be made to change/remove it (e.g. stuff like the Unsafe
> class in the base JVM).
> 

+1

I've never argued that internals should frozen. I've merely suggested that careful planning is often in order, e.g., as ought to be the case with the broadly extended Viewer classes that are currently marked @noextends, which also highlights how realistic the option to negotiate is in actual fact.