Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 126575 - [Architecture] Internal API reference in plug-in
Summary: [Architecture] Internal API reference in plug-in
Status: CLOSED FIXED
Alias: None
Product: z_Archived
Classification: Eclipse Foundation
Component: TPTP (show other bugs)
Version: unspecified   Edit
Hardware: All All
: P1 blocker (vote)
Target Milestone: ---   Edit
Assignee: Jerome Gout CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on: 128262
Blocks: 126589 162891 172902
  Show dependency tree
 
Reported: 2006-02-06 10:39 EST by Ruth Lee CLA
Modified: 2016-05-05 10:31 EDT (History)
3 users (show)

See Also:


Attachments
Resolution plan (4.17 KB, text/html)
2006-10-31 05:25 EST, Julien Canches CLA
no flags Details
Fix for items 2, 4 and 15 (see resolution plan) (40.30 KB, patch)
2006-10-31 09:20 EST, Julien Canches CLA
no flags Details | Diff
Fix for item 13 (see resolution plan) (13.14 KB, patch)
2006-11-02 08:40 EST, Julien Canches CLA
no flags Details | Diff
Fix for item 14 (see resolution plan) (4.10 KB, patch)
2006-11-02 08:51 EST, Julien Canches CLA
no flags Details | Diff
Updated fix for item 13 (see resolution plan) (10.64 KB, patch)
2007-01-11 09:22 EST, Julien Canches CLA
no flags Details | Diff
Updated Resolution Plan (4.70 KB, text/html)
2007-01-11 17:36 EST, Paul Slauenwhite CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Ruth Lee CLA 2006-02-06 10:39:42 EST
The build scan found references to non-TPTP internal API that must be removed. If it is not possible to remove the API, then open a bug against the team that owns the internal API to make the internal API public, and make this bugzilla depend on that bug.

The following references were found:
plugins/org.eclipse.hyades.test.tools.core/src/org/eclipse/hyades/test/tools/core/internal/plugin/harness/WorkbenchExecutableObjectAdapter.java:
30:import org.eclipse.pde.internal.core.PDECore;

plugins/org.eclipse.hyades.test.tools.core/src/org/eclipse/hyades/test/tools/core/internal/plugin/harness/WorkbenchExecutionDeploymentAdapter.java:
80:import org.eclipse.pde.internal.core.ExternalModelManager;
81:import org.eclipse.pde.internal.core.PDECore;
82:import org.eclipse.pde.internal.core.TargetPlatform;
83:import org.eclipse.pde.internal.core.plugin.WorkspacePluginModelBase;

plugins/org.eclipse.hyades.test.tools.core/src/org/eclipse/hyades/test/tools/core/internal/plugin/codegen/PluginJUnitGenerator.java:
32:import org.eclipse.pde.internal.core.PDECore;
33:import org.eclipse.pde.internal.core.bundle.BundlePluginModelBase;
34:import org.eclipse.pde.internal.core.plugin.WorkspacePluginModelBase;

plugins/org.eclipse.hyades.test.tools.core/src/org/eclipse/hyades/test/tools/core/internal/plugin/util/PDEProjectUtil.java:
24:import org.eclipse.pde.internal.core.PDECore;

plugins/org.eclipse.hyades.test.tools.core/src/org/eclipse/hyades/test/tools/core/internal/plugin/util/ManifestUtil.java:
49:		// org.eclipse.pde.internal.core.PDEPluginConvert.modifyManifest()

plugins/org.eclipse.hyades.test.tools.core/src/org/eclipse/hyades/test/tools/core/internal/plugin/util/WorkbenchLocationUtil.java:
34:import org.eclipse.pde.internal.core.ExternalModelManager;
35:import org.eclipse.pde.internal.core.PDECore;
36:import org.eclipse.pde.internal.core.TargetPlatform;

plugins/org.eclipse.hyades.test.tools.core/src-common-runner/org/eclipse/tptp/test/common/event/provisional/AnnotatedVerdictEvent.java:
8:import org.eclipse.emf.ecore.xml.type.internal.DataValue;

plugins/org.eclipse.hyades.test.tools.core/src-common-runner/org/eclipse/tptp/test/common/event/provisional/AnnotatedMessageEvent.java:
8:import org.eclipse.emf.ecore.xml.type.internal.DataValue;

plugins/org.eclipse.hyades.test.tools.core/src-plugin-runner/org/eclipse/hyades/test/tools/plugin/runner/BundleSet.java:
21:import org.eclipse.core.internal.runtime.InternalPlatform;
Comment 1 Paul Slauenwhite CLA 2006-02-06 16:28:10 EST
I will handle the following under defect #126576:

plugins/org.eclipse.hyades.test.tools.core/src-common-runner/org/eclipse/tptp/test/common/event/provisional/AnnotatedVerdictEvent.java:
8:import org.eclipse.emf.ecore.xml.type.internal.DataValue;

plugins/org.eclipse.hyades.test.tools.core/src-common-runner/org/eclipse/tptp/test/common/event/provisional/AnnotatedMessageEvent.java:
8:import org.eclipse.emf.ecore.xml.type.internal.DataValue;
Comment 2 Julien Canches CLA 2006-02-16 12:36:02 EST
Adding a dependency on PDE bugzilla for tracking additional API request.
Comment 3 Julien Canches CLA 2006-02-16 12:59:05 EST
Fixed internal dependency for:
plugins/org.eclipse.hyades.test.tools.core/src-plugin-runner/org/eclipse/hyades/test/tools/plugin/runner/BundleSet.java:
21:import org.eclipse.core.internal.runtime.InternalPlatform;

Any remaining internal API use is depending on bugzilla_128262 resolution.
Comment 4 Harm Sluiman CLA 2006-02-17 08:34:37 EST
128262 is targeted as later, so we can not rely on this to solve the api usage problem.
This dependancy is a violation of the api rules in the project and needs resolution in the current iteration.
Comment 5 Julien Canches CLA 2006-02-17 09:04:37 EST
Access to PDE model is absolutely necessary for the "JUnit Plugin Test" feature. The only way to remove the internal dependency is to remove the feature.
Comment 6 Harm Sluiman CLA 2006-02-17 09:33:54 EST
(In reply to comment #5)
> Access to PDE model is absolutely necessary for the "JUnit Plugin Test"
> feature. The only way to remove the internal dependency is to remove the
> feature.

Then it seems we need a very agressive plan to resolve this with the PDE team. let me know how I can help. If the API is not already frozen it will be soon.
Comment 7 Paul Slauenwhite CLA 2006-02-27 10:20:12 EST
As of the TPTP-4.2.0-200602262249 build, the following internal references are still outstanding for the Test project:

plugins/org.eclipse.hyades.test.tools.core/src/org/eclipse/hyades/test/tools/core/internal/plugin/harness/WorkbenchExecutableObjectAdapter.java:
30:import org.eclipse.pde.internal.core.PDECore;

plugins/org.eclipse.hyades.test.tools.core/src/org/eclipse/hyades/test/tools/core/internal/plugin/harness/WorkbenchExecutionDeploymentAdapter.java:
80:import org.eclipse.pde.internal.core.ExternalModelManager;
81:import org.eclipse.pde.internal.core.PDECore;
82:import org.eclipse.pde.internal.core.TargetPlatform;
83:import org.eclipse.pde.internal.core.plugin.WorkspacePluginModelBase;

plugins/org.eclipse.hyades.test.tools.core/src/org/eclipse/hyades/test/tools/core/internal/plugin/codegen/PluginJUnitGenerator.java:
32:import org.eclipse.pde.internal.core.PDECore;
33:import org.eclipse.pde.internal.core.bundle.BundlePluginModelBase;
34:import org.eclipse.pde.internal.core.plugin.WorkspacePluginModelBase;

plugins/org.eclipse.hyades.test.tools.core/src/org/eclipse/hyades/test/tools/core/internal/plugin/util/PDEProjectUtil.java:
24:import org.eclipse.pde.internal.core.PDECore;

plugins/org.eclipse.hyades.test.tools.core/src/org/eclipse/hyades/test/tools/core/internal/plugin/util/WorkbenchLocationUtil.java:
34:import org.eclipse.pde.internal.core.ExternalModelManager;
35:import org.eclipse.pde.internal.core.PDECore;
36:import org.eclipse.pde.internal.core.TargetPlatform;

plugins/org.eclipse.hyades.test.ui/src/org/eclipse/hyades/test/ui/internal/navigator/action/RenameAction.java:
109:if (System.getProperty("org.eclipse.swt.internal.carbon.noFocusRing") == null || c.getShell().getParent() != null) //$NON-NLS-1$

plugins/org.eclipse.hyades.test.core/src/org/eclipse/hyades/test/core/util/ArtifactUtil.java:
55:private static final String PDE_CLASSPATH_CONTAINER_ID = "org.eclipse.pde.core.requiredPlugins"; // see org.eclipse.pde.internal.core.PDECore.CLASSPATH_CONTAINER_ID

plugins/org.eclipse.hyades.test.tools.ui/src/org/eclipse/hyades/test/tools/ui/plugin/internal/junit/editor/WorkbenchPropertyGroupForm.java:
40:import org.eclipse.pde.internal.ui.PDEPlugin;

plugins/org.eclipse.hyades.test.tools.ui/src/org/eclipse/hyades/test/tools/ui/common/internal/wizard/RunDestinationPage.java:
29:import org.eclipse.ui.internal.ide.misc.ResourceAndContainerGroup;

plugins/org.eclipse.ui.testing/src/org/eclipse/ui/testing/internal/WorkbenchPartTestable.java:
11:package org.eclipse.ui.testing.internal;

plugins/org.eclipse.ui.testing/src/org/eclipse/ui/testing/internal/TestableAdapterFactory.java:
11:package org.eclipse.ui.testing.internal;
16:import org.eclipse.ui.internal.PartSite;


Comment 8 Scott E. Schneider CLA 2006-02-28 14:26:16 EST
Julien, please update status on this blocker.
Comment 9 Julien Canches CLA 2006-03-01 04:52:32 EST
As mentionned by the PDE team, their 3.2 API was already frozen at the time the request was submitted, so we are stucked for 4.2. The only solution I can imagine is to send a request to the PDE team to allow, as an exception to the API rules, the 4.2 release of TPTP to be granted access to some of PDE internal packages (this would be done in their MANIFEST file using the "x-friend" directive).
Comment 10 Julien Canches CLA 2006-03-31 04:14:34 EST
Targetting to 4.3 due to dependency on an external "RESOLVED FUTURE" enhancement.
Comment 11 Paul Slauenwhite CLA 2006-09-06 10:36:22 EDT
Ruth, can we reduce the severity of this defect to critical since we have to ship 4.2.1 without this defect?
Comment 12 Ruth Lee CLA 2006-09-06 14:36:19 EDT
Hi Paul,

You need to ask Harm because I opened all of these defects (see top-level defect 126589) at his request and at the severity that he indicated was necessary.

Harm? Reduce to critical or leave as-is?

Thanks,
Ruth.
Comment 13 Harm Sluiman CLA 2006-09-06 14:48:17 EDT
I guess I took my eye of the ball. 
We are suppsoed ot ebe API clean. The severity and priority should stand.
The question is why did this P1 not get addressed?
Given it is too late for 4.2.1, please ensure it is addressed early in 4.3
Comment 14 Julien Canches CLA 2006-09-12 11:41:36 EDT
(Copy of emails on TPTP Test Dev mailing list):

JC> The actual date when this defect can be fixed will be known when PDE gives us an ETA for enhancement 128262. I don't know how the PDE team is processing enhancements requests, but it looks like this enhancement has been sleeping in the "RESOLVED FUTURE" for months. I have re-opened it to try getting more information from them. 

PS>
Thanks Julien.  Is there any way to work around some if not all these
internal PDE dependancies?

JC> The only work around I can imagine is to copy PDE code (and this code includes, for instance, plugin.xml and MANIFEST.MF parsing, plugin model loading and saving, configuration files generation). All of this code has regularly changed in every version of eclipse since 3.0, although the internal API we rely on is pretty stable. If I had copied PDE code instead of using "internal APIs", the maintenance work would have been significantely greater, so I don't think this would be a good option for us. 
I have asked PDE if they can see other work arounds and am waiting for their response. 
Comment 15 Harm Sluiman CLA 2006-09-12 16:44:28 EDT
If there is no fix in sight, no agreement that can be reached, and no alternate path, then we need to remove the code that violates the basic laws.

It is mandatory that we be api clean.
Comment 16 Paul Slauenwhite CLA 2006-09-13 06:59:18 EDT
Julien, we need to resolve this defect in 4.3  Can you please target to 4.3i3?
Comment 17 Julien Canches CLA 2006-09-13 08:05:44 EDT
I have a reflection-based work-around that removes the internal dependencies at the API level. Harm, Paul, would you accept this solution until PDE provides official APIs?
Comment 18 Paul Slauenwhite CLA 2006-09-13 08:43:57 EDT
(In reply to comment #17)
> I have a reflection-based work-around that removes the internal dependencies at
> the API level. Harm, Paul, would you accept this solution until PDE provides
> official APIs?

I am not sure how reflection solves the problem since the requirement on the internal API will still exist.  Can you investigate providing similar as the internal PDE function in TPTP?
Comment 19 Harm Sluiman CLA 2006-09-13 09:31:29 EDT
This might remove a build dependancy but still has a runtime dependancy, which is the precise problem we need to fix. So this is not really a workaround.
Comment 20 Julien Canches CLA 2006-09-14 10:34:06 EDT
Paul, the work required for coding something equivalent to the internal APIs we use goes way beyond the duration of i3 (although it would enable TPTP to explore platforms located on a remote machine).
I have no short-term solution (i.e. for 4.3i3) to this situation. If you think someone else can fix the problem for i3, feel free to reassign this defect.

Harm, if we keep these internals, then we're not API clean, but if we remove them, we introduce a feature regression. Without these fuctions, the user cannot select the plug-ins to launch, and more critical, TPTP is not able to generate a configuration area to launch eclipse. In other terms, TPTP cannot run a JUnit Plug-in test. This looks to me a major regression, even worse than not being API clean. Not that I don't care not being API clean, but I think we need to be pragmatic and retain the least worst solution.
Comment 21 Harm Sluiman CLA 2006-09-14 11:18:24 EDT
You are absolutely right. Removing the support is a feature regression. The root problem is that the function should not have gone in with the internal API dependancy.

If this can not be resolved with either API or agreed apii stability form the provider to get us through 4.3. We will continue to need this exception and we need a plan fo rclosure in 4.4. If that means removing the function then sobeit.  I have to believe that with full discussion we can get api or at least an api freeze.

Valentina needs to be brought into this if it requires a cross project escalation. 
Comment 22 Julien Canches CLA 2006-09-15 09:12:35 EDT
(In reply to comment #21)
> Valentina needs to be brought into this if it requires a cross project
> escalation. 

Well, it does :) Valentina, could you esclate this issue at the cross project level? We need to keep this API exception for 4.3. We would also need at least a status from the PDE team about https://bugs.eclipse.org/bugs/show_bug.cgi?id=128262, so we can anticipate what we'll need to do in 4.4. Thanks.
Comment 23 Valentina Popescu CLA 2006-09-15 09:24:00 EDT
Absolutely, it's on my list
Comment 24 Julien Canches CLA 2006-10-20 04:53:07 EDT
Based on priority and available resources, this defect is considered for deferral from 4.3.  If you object to this deferral, please provide a comment
ASAP explaining the reason that you need this in 4.3 and we will take that
into consideration.
Comment 25 Harm Sluiman CLA 2006-10-20 05:32:29 EDT
I don't know how a priority higher than blocking P1 can be indicated, so I don't know why priority is an issue.

The reaosn for this defect is simple. There are not supposed ot be any violations of api usage rules in a GA release of TPTP or any other Eclipse project. This is a contract we have with consuming products and is an exit criteria in each release cycle.

When api can not be had either the dependancy and function has to be removed, or an agreement with the internal api provider must be reached.

Without clean api, release to release compatibilty can not be achieved in the products.

This defect and others like it were opened a long time ago, and there appears to be no progress toward resolution.
Comment 26 Julien Canches CLA 2006-10-20 06:03:07 EDT
This is the standard message for all candidates to deferral. Obviously, the reason here is not the priority, but the available resources.

Maybe I have misunderstood your comment #21. Reading this comment, I understand that we need to keep the API exception for 4.3, and we need a plan for closure for 4.4, this plan being either 1) an agreement for an API with PDE, 2) our own solution (whose cost is about 8 weeks), 3) withdrawing the JUnit Plug-in Test feature. If you meant that the feature should be withdrawn from 4.3, please confirm.
Comment 27 Harm Sluiman CLA 2006-10-20 08:24:48 EDT
You did not miss understand. My key point is that the internal "discussion" has gone on for many months, and the resource could have been applied. 
There is no mention here of any progress on driving to closure on the API. 

Just deferring out of 4.3 is not acceptable without a concrete alternate proposal and commitment that will be addressed in 4.4. If none can be made, then the function should be removed now.

If it is 8 weeks of work, in hind sight there was plenty of time to do this since hte defect was opened, and the priority was simply not being respected. This is a comment about process execution.
Comment 28 Julien Canches CLA 2006-10-31 05:25:47 EST
Created attachment 52976 [details]
Resolution plan
Comment 29 Julien Canches CLA 2006-10-31 09:20:27 EST
Created attachment 52989 [details]
Fix for items 2, 4 and 15 (see resolution plan)

Here's a manually tested patch for items planned for 4.3i3. In the test, all changed lines of code have been exercised to make sure that new calls return the same result as the internal PDE APIs we were using.
Comment 30 Julien Canches CLA 2006-11-02 08:40:13 EST
Created attachment 53130 [details]
Fix for item 13 (see resolution plan)
Comment 31 Julien Canches CLA 2006-11-02 08:51:04 EST
Created attachment 53134 [details]
Fix for item 14 (see resolution plan)
Comment 32 Harm Sluiman CLA 2006-11-02 08:52:03 EST
This is excellent progress.

For the API that is going to be provided, we need to push for a timely delivery
for Europa which will include our 4.4 release. This typically means the API
should show up in a PDE driver in Jan. 07
Comment 33 Paul Slauenwhite CLA 2006-11-06 10:48:25 EST
(In reply to comment #29)

This is great progress!  

We will need approval to use the PDE icons.  You will need to contact Janet Campbell (Eclipse Legal Counsel & Manager, Intellectual Property) and request approval.  
Comment 34 Paul Slauenwhite CLA 2006-11-06 10:50:19 EST
Status update:

-Julien has checked in the patches for items 2, 4, 14 and 15 to CVS (EHAD).
-Julien has sent a request for deferring the defect to the test dev mailing list.
-Paul has approved the request to defer the defect.
-Paul has sent a request for deferring the defect to the PMC/PG via their mailing list.
Comment 35 Paul Slauenwhite CLA 2006-11-08 10:05:03 EST
PMC approval has been given to defer the remaining work to 4.4.
Comment 36 Harm Sluiman CLA 2006-12-25 09:15:06 EST
(In reply to comment #35)
> PMC approval has been given to defer the remaining work to 4.4.

Given we now have a 4.4 stream open and there is growing concern about our API problems We need ot get this work completed in the immeadiate future (aka before late January)
What is the target for completion?
Comment 37 Julien Canches CLA 2007-01-11 09:22:30 EST
Created attachment 56762 [details]
Updated fix for item 13 (see resolution plan)
Comment 38 Paul Slauenwhite CLA 2007-01-11 10:59:06 EST
(In reply to comment #36)

We are attempting to complete this work by the end of 4.4 i1 (e.g. end of Jan.).  However, this target is contingent on the PDE Team providing the remaining public APIs (e.g. line items 1 and 5 - 12 in the Resolution Plan) which we are meeting/discussing with the PDE Team today.
Comment 39 Paul Slauenwhite CLA 2007-01-11 17:36:28 EST
Created attachment 56805 [details]
Updated Resolution Plan
Comment 40 Paul Slauenwhite CLA 2007-01-15 11:08:57 EST
Reassigning to Jerome to track for 4.4.
Comment 41 Paul Slauenwhite CLA 2007-01-15 11:10:37 EST
On Thursday, January 11, 2006, the PDE Team has confirmed that the agreed public PDE APIs will be provided in M5.
Comment 42 Paul Slauenwhite CLA 2007-01-16 09:52:30 EST
(In reply to comment #40)
> Reassigning to Jerome to track for 4.4.

Jerome has been assigned the remaining work required for this defect in TPTP.  I will work with the PDE Team to ensure the public PDE APIs will be provided in M5.
Comment 43 Paul Slauenwhite CLA 2007-02-05 15:05:33 EST
Jerome, the fixes mentioned under https://bugs.eclipse.org/bugs/show_bug.cgi?id=172902#c3 are tagged by the following comment:

//TODO: Replace with fix for https://bugs.eclipse.org/bugs/show_bug.cgi?id=126575:

Please review and remove the comment when completing this defect.
Comment 44 Jerome Gout CLA 2007-02-06 12:26:14 EST
Fixed
No longer internal API used by plugins test.
Comment 45 Paul Slauenwhite CLA 2007-06-02 14:51:08 EDT
Reporter: Please verify and close in preparation for shutting down the TPTP 4.4 release.  Thanks.
Comment 46 Paul Slauenwhite CLA 2007-06-28 11:27:10 EDT
Verfieid (http://download.eclipse.org/tptp/4.4.0/TPTP-4.4.0-200706140100C/internal-pkg-use.txt) and closing for Ruth.