Bug 547001 - quickAccess providers extension should only participate when contributing bundle is active
Summary: quickAccess providers extension should only participate when contributing bun...
Status: RESOLVED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 4.12   Edit
Hardware: All All
: P3 enhancement (vote)
Target Milestone: 4.12 M3   Edit
Assignee: Mickael Istria CLA
QA Contact:
URL:
Whiteboard:
Keywords: api, noteworthy
Depends on:
Blocks: 545544
  Show dependency tree
 
Reported: 2019-05-06 06:31 EDT by Mickael Istria CLA
Modified: 2020-03-19 10:21 EDT (History)
2 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Mickael Istria CLA 2019-05-06 06:31:42 EDT
Extension point org.eclipse.ui.quickAccess allows to add QuickAccessProviders.
However, this will activate the host bundle, which -in some cases- may not be desired. When the bundle is already activated, then it's all fine invoking the provider; in cases it's not activated, extenders should be able to provide some "activeWhen" condition or similar to declare whether the provider should be loaded.
Comment 1 Andrey Loskutov CLA 2019-05-06 06:47:01 EDT
I believe we had something similar in common navigator.
Was just an attribute in xml AFAIR, not a condition.
Comment 2 Andrey Loskutov CLA 2019-05-06 07:02:49 EDT
Do we really need a flag? Or we simply will not load extensions from not yet activated bundles?

Because let assume 3rd party bundle wants to contribute to QA and only to that. It would be activated only if it contribution is loaded, so it will always set the flag to activate.

So flag alone will not be enough. But which condition should such a bundle provide? Always activate me, because I'm important one?
Comment 3 Mickael Istria CLA 2019-05-06 07:14:44 EDT
(In reply to Andrey Loskutov from comment #2)
> Do we really need a flag? Or we simply will not load extensions from not yet
> activated bundles?

Right, we don't need a flag at the moment.
Especially since early startup can be triggered in many different ways (like an IStartup) so adding one extra control doesn't seem relevant.
Comment 4 Mickael Istria CLA 2019-05-06 07:17:29 EDT
That said, I've spent an hour looking at the code, and this won't be trivial as it relies a lot on the assumption that the QuickAccessProvider is available early enough. I think we'll need to change a bit the extension point.
Comment 5 Eclipse Genie CLA 2019-05-06 10:23:34 EDT
New Gerrit change created: https://git.eclipse.org/r/141667
Comment 6 Mickael Istria CLA 2019-05-07 04:05:12 EDT
@Dani: this proposal heavily modifies the extension point and API that were introduced in previous milestone, for good value IMO. Is it fine to merge it since it was not in a release?
Comment 7 Dani Megert CLA 2019-05-07 09:00:35 EDT
(In reply to Mickael Istria from comment #6)
> @Dani: this proposal heavily modifies the extension point and API that were
> introduced in previous milestone, for good value IMO. Is it fine to merge it
> since it was not in a release?
Yes, that's fine with me.
Comment 9 Andrey Loskutov CLA 2019-05-08 11:40:17 EDT
(In reply to Eclipse Genie from comment #8)
> Gerrit change https://git.eclipse.org/r/141667 was merged to [master].
> Commit:
> http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/
> ?id=b185256c735d863079e384ceeb0b771960dc6c65

This caused lot of API errors in org.eclipse.ui.workbench, because minor segment on org.eclipse.ui.workbench version was increased *second time* in the 4.12 release.

@Mickael: please install API tools if you change API and make sure you have right baseline. Please fix new API errors.
Comment 10 Dani Megert CLA 2019-05-08 11:51:39 EDT
(In reply to Andrey Loskutov from comment #9)
> (In reply to Eclipse Genie from comment #8)
> > Gerrit change https://git.eclipse.org/r/141667 was merged to [master].
> > Commit:
> > http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/
> > ?id=b185256c735d863079e384ceeb0b771960dc6c65
> 
> This caused lot of API errors in org.eclipse.ui.workbench, because minor
> segment on org.eclipse.ui.workbench version was increased *second time* in
> the 4.12 release.
> 
> @Mickael: please install API tools if you change API and make sure you have
> right baseline. Please fix new API errors.
How many times do we have to tell this?
Comment 11 Mickael Istria CLA 2019-05-08 12:24:55 EDT
The reason is that some other patch did erroneously bump the version by +0.0.100, and to ensure now APIs can be referenced properly, it was necessary to bump it a second time.

(In reply to Dani Megert from comment #10)
> (In reply to Andrey Loskutov from comment #9)
> > @Mickael: please install API tools if you change API and make sure you have
> > right baseline. Please fix new API errors.
> How many times do we have to tell this?

Whenever I start API Tools, I have to disable it after a few hours because they make my workspace too slow to build and not usable. I cannot work efficiently with them installed.
IMO, since API checks are a quality gate, they have to be automated as part of the build and make build fail in case of error. That would make things simpler for everyone.
Comment 12 Andrey Loskutov CLA 2019-05-08 12:35:54 EDT
(In reply to Mickael Istria from comment #11)
> The reason is that some other patch did erroneously bump the version by
> +0.0.100, and to ensure now APIs can be referenced properly, it was
> necessary to bump it a second time.

OMG I see.

> Whenever I start API Tools, I have to disable it after a few hours because
> they make my workspace too slow to build and not usable. I cannot work
> efficiently with them installed.
> IMO, since API checks are a quality gate, they have to be automated as part
> of the build and make build fail in case of error. That would make things
> simpler for everyone.

As long as this is not implemented you *have to* install and use API tools. I have them always on and I can work without disturbing others. Please either contribute your automated check or do the same. I can't always do the dirty work for others.
Comment 13 Dani Megert CLA 2019-05-08 13:03:17 EDT
(In reply to Andrey Loskutov from comment #12)
> As long as this is not implemented you *have to* install and use API tools.
> I have them always on and I can work without disturbing others. Please
> either contribute your automated check or do the same. I can't always do the
> dirty work for others.
+1!
Comment 14 Mickael Istria CLA 2019-05-08 13:40:09 EDT
So in this case, is the best approach to create API Filters, or is there something better we can do about versioning?
Comment 15 Eclipse Genie CLA 2019-05-08 13:47:35 EDT
New Gerrit change created: https://git.eclipse.org/r/141806
Comment 16 Andrey Loskutov CLA 2019-05-08 13:48:08 EDT
(In reply to Mickael Istria from comment #14)
> So in this case, is the best approach to create API Filters, or is there
> something better we can do about versioning?

(In reply to Eclipse Genie from comment #15)
> New Gerrit change created: https://git.eclipse.org/r/141806

That is what I would do.
Comment 18 Mickael Istria CLA 2019-05-08 14:32:03 EDT
Thanks Andrey.