search

eclipse-embed-cdt / eclipse-plugins

The Eclipse Embedded CDT plug-ins for Arm & RISC-V C/C++ developers (formerly known as the GNU MCU Eclipse plug-ins). Includes the archive of previous plug-ins versions, as Releases.
http://eclipse-embed-cdt.github.io/
Eclipse Public License 2.0
555 stars 130 forks source link

Activator illegally extends AbstractUIActivator #456

Closed ilg-ul closed 3 years ago

ilg-ul commented 3 years ago

After the recent changes, I see this warning:

Activator illegally extends AbstractUIActivator

Can we do something to fix this?

jonahgraham commented 3 years ago

See commit message in https://github.com/eclipse-embed-cdt/eclipse-plugins/commit/fe3ff6c839f419fc75a8ef11e900789ea8985162

ilg-ul commented 3 years ago

I removed the 6.0.0 milestone. If this cannot be fixed, please close it.

jonahgraham commented 3 years ago

I can move the abstract activators to internal package and mark it as friend with the rest of embedcdt. That will clear the warnings of illegally extends. All the activators can be moved to internal packages.

But none of that needs to be done urgently, so I agree with removal of v6.0.0 milestone.

ilg-ul commented 3 years ago

All the activators can be moved to internal packages.

I'll give it a try tomorrow morning, now it is almost 3 am and I don't want to mess anything.

ilg-ul commented 3 years ago

I finally moved the Activator classes to the internal name space, and I exported all internal packages as x-internal, although I did mechanically, since I don't really understand the consequences.

The result still builds locally and runs (more test after CI & Jenkins are ready).

However the number of warnings increased significantly:

Description Resource    Path    Location    Type
Discouraged access: The constructor 'AbstractActivator()' is not API (restriction on required project 'org.eclipse.embedcdt.core')  Activator.java  /org.eclipse.embedcdt.debug.core/src/org/eclipse/embedcdt/internal/debug/core   line 50 Java Problem
...
Discouraged access: The constructor 'AbstractUIActivator()' is not API (restriction on required project 'org.eclipse.embedcdt.ui')  Activator.java  /org.eclipse.embedcdt.templates.freescale.pe.ui/src/org/eclipse/embedcdt/internal/templates/freescale/pe/ui line 50 Java Problem
...
Discouraged access: The method 'AbstractActivator.isDebugging()' is not API (restriction on required project 'org.eclipse.embedcdt.core')   CProjectExtraDataManager.java   /org.eclipse.embedcdt.managedbuild.cross.core/src/org/eclipse/embedcdt/managedbuild/cross/core/xpi  line 43 Java Problem

I'm not sure I'm doing the right thing.

In my use cases, Activator classes are used extensively, not only for debugging purposes, but also to share preferences between .core and .ui plugins.

Jonah @jonahgraham, the code is in develop-internal, for review and further improvements.

jonahgraham commented 3 years ago

I will commit something in a moment that marks the internal packages as friends with what it is friends with to resolve the warnings.

ilg-ul commented 3 years ago

I tried something, but defining packages both x-internal and x-friend did not work.

Since I don't know exactly how this works, I thought that the x-internal definition is more important.

jonahgraham commented 3 years ago

x-internal was most important, the x-friends solves the warnings. See 23b0549ca8 for the fix.

ilg-ul commented 3 years ago

x-internal was most important

I thought so too.

the x-friends solves the warnings

But aren't changes like this:

-  org.eclipse.embedcdt.internal.debug.gdbjtag.core;x-internal:=true
+  org.eclipse.embedcdt.internal.debug.gdbjtag.core;x-friends:="org.eclipse.embedcdt.debug.gdbjtag.ui"

invalidating x-internal in favour of x-friends?

jonahgraham commented 3 years ago

x-friends means that the package is internal, except to the named bundles listed in the friends list.

jonahgraham commented 3 years ago

"The use of x-friends is an indication of tightly coupled bundles because its use is for when one bundle needs "approved" access to the internals of another bundle."

ilg-ul commented 3 years ago

Ah, ok, so x-friends includes x-internal.

Then it should be no problem.

ilg-ul commented 3 years ago

If no further work is required here, I would merge develop-internal into develop.

jonahgraham commented 3 years ago

Please go ahead with the merge.

ilg-ul commented 3 years ago

Done. Tomorrow I'll merge into master and update SimRel.