Community
Participate
Working Groups
I20130430-0031 and previous builds, including 4.3 M6. Works in 4.3 M5a and 4.2.2. The source features of RCP and Help appear in the About dialog. They should be removed again.
Created attachment 230288 [details] Picture showing the bug
Status and a question: I've looked in repo at definitions and POMs and could find no substantial difference between "those that work" and "those that don't". I think the next step is to slog though the 300 Megabyte log file and find out if they are "inheriting" something a little differently that makes a difference. Question, I thought elsewhere you have said that source features are _supposed_ to have branding bundles, such as RCP hand Help do (which is why they show up in about box) ... am I remembering that wrong, or do you know of some "trick" that even if a feature has a branding bundle it won't show up? Or, is the issue that source features should not have branding bundles to begin with?
I tried to figure out a difference and it turns out, that *all* source features are completely broken: - they lack the license and epl files, - they no longer start with: <?xml version="1.0" encoding="UTF-8"?> - the properties files are much smaller, missing stuff - they are reformatted and look unreadable So, at this point, I assume it is just pure luck that some source bundles appear in the dialog and some don't. NOTE: I verified that the problem is not caused by code changes i.e. the bug is somewhere in the shape of the source features.
Adding Paul and Thanh for any insights ... Paul, I have a vague memory of you saying that Igor recommended we define explicit *.source features ... but, as far as I can tell, we do not. Does that sound familiar? Or am I simply mis-remembering?
(In reply to comment #4) > Adding Paul and Thanh for any insights ... > > Paul, I have a vague memory of you saying that Igor recommended we define > explicit *.source features ... but, as far as I can tell, we do not. > > Does that sound familiar? Or am I simply mis-remembering? I believe we went that route due to a problem with Tycho which was eventually resolved so we moved back to using the auto-generated source features because the manual source features had some issues too if I recall correctly.
Some history regarding the manual source features can be found in Bug 389983
I have more data: the bug is that for whatever reason, the RCP and the Help source feature set a branding plugin in the generated feature.xml: <feature id="org.eclipse.help.source" version="2.0.0.v20130314-1330" plugin="org.eclipse.help.base" .... ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ This is not the case for all other source features and was not the case for Help and RCP in M5a. Does that ring any bell?
(In reply to comment #7) > I have more data: the bug is that for whatever reason, the RCP and the Help > source feature set a branding plugin in the generated feature.xml: > > <feature id="org.eclipse.help.source" version="2.0.0.v20130314-1330" > plugin="org.eclipse.help.base" .... > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > > This is not the case for all other source features and was not the case for > Help and RCP in M5a. Does that ring any bell? Recall we were still using PDE build for M5a. http://wiki.eclipse.org/Platform-releng/Releng_Plan_for_CBI_adoption I have a theory of why this effects (only) 'help' and 'rcp': because they (and their code bundles) are "included" by other, "higher" features ... so, however ever the sequence is determined, those branding bundles exist by the time 'help' and 'rcp' are created. That, combined with the fact that it appears it is a hard-coded assumption in Tycho's SourceFeatureMojo that is always assumes that a source features branding bundle should be the exact same one as the code's branding bundle ... (if that branding bundle exists, which it would due to the hierarchy of features). The code looks like this: if (feature.getBrandingPluginId() != null) { sourceFeature.setBrandingPluginId(feature.getBrandingPluginId()); } Where as, in PDE build, the sourceFeature's branding bundle was dynamically created from a special folder named 'sourceTemplatePlugin'. That is apparently not used at all, by Tycho. Looking at several of our 'sourceTemplatePlugin' folders, and comparing them to the code's branding bundle ... I notice our 'sourceTemplatePlugin' do not contain "about.properties" or icon images, and suspect it is their absence that causes them to NOT be included in the about box (when PDE build used). Tycho, though, does make use of 'sourceTemplateFeature' folder. I mention this since we could probably improve the situation some, by adding src.includes=feature.properties to the build.properties files in those folders. I say this just from looking at the code in SourceFeatureMojo and it appears it will use the feature.properties if it finds it on disk ... and not sure it's finding it in the right place, if we just have bin.includes, as we do now. But, that just effects things like name, description, copyright, provider ... maybe license except we don't actually have that info in there now (since we always "built it in" at build time) ... but, wouldn't effect about.properties, etc. in branding bundle. A larger concern may be what's said in bug 389983 comment 4. If I'm reading that right, it is saying included source features simply don't work. It refers to bug which is listed as a 'minor' bug?! But, not sure ... org.eclipse.rcp.source feature, as generated by Tycho, does have the element <includes id="org.eclipse.e4.rcp.source" version="1.2.0.v20130429-1813"/> so maybe that part fixed? Also found bug 389762 for some history. Last complaint ... it'd sure be nice if they formated the generated XML! :) I had to use xmllint to make any sense of what the generated feature said.
I don't see anything we can do for M7 ... we'll see about RC1.
David, do you need a separate bug regarding the completely broken source feature (missing stuff, incomplete and/or wrong feature.xml and properties files), or do we use this bug here?
(In reply to comment #8) > if (feature.getBrandingPluginId() != null) { > sourceFeature.setBrandingPluginId(feature.getBrandingPluginId()); > } > > Where as, in PDE build, the sourceFeature's branding bundle was dynamically > created from a special folder named 'sourceTemplatePlugin'. That is > apparently not used at all, by Tycho. I think this is a bug in Tycho. It doesn't make sense to me that a source feature would automatically get the same branding plugin as the feature it provides source for. The consequence is that you will always end up with two entries that look identical in the about dialog and the source feature may have the wrong title and description. For example org.eclipse.help.source the correct name is "Eclipse Help Developer Resources", but because it is inheriting the wrong branding information it appears in About dialog as "Eclipse Help Base". A better "default" behaviour in my opinion is that source features don't have any branding plugin at all unless there is one explicitly specified. Branding plugins are not required, and I think defaulting to no branding is more reasonable that wrong branding. I propose moving this bug to Tycho, unless a bug exists already and we use this one to track a work-around on our side.
(In reply to comment #10) > David, do you need a separate bug regarding the completely broken source > feature (missing stuff, incomplete and/or wrong feature.xml and properties > files), or do we use this bug here? Separate bug I guess ... especially since I'm not sure which ones are right and which ones are wrong ... would be good to give an idea of that so I could see what was different about them.
(In reply to comment #11) > (In reply to comment #8) > > if (feature.getBrandingPluginId() != null) { > > sourceFeature.setBrandingPluginId(feature.getBrandingPluginId()); > > } > > > > Where as, in PDE build, the sourceFeature's branding bundle was dynamically > > created from a special folder named 'sourceTemplatePlugin'. That is > > apparently not used at all, by Tycho. > > I think this is a bug in Tycho. It doesn't make sense to me that a source > feature would automatically get the same branding plugin as the feature it > provides source for. The consequence is that you will always end up with two > entries that look identical in the about dialog and the source feature may > have the wrong title and description. For example org.eclipse.help.source > the correct name is "Eclipse Help Developer Resources", but because it is > inheriting the wrong branding information it appears in About dialog as > "Eclipse Help Base". > > A better "default" behaviour in my opinion is that source features don't > have any branding plugin at all unless there is one explicitly specified. > Branding plugins are not required, and I think defaulting to no branding is > more reasonable that wrong branding. I propose moving this bug to Tycho, > unless a bug exists already and we use this one to track a work-around on > our side. I found and re-opened bug 378424 where this behavior was introduced in 0.17 4 months ago.
> A better "default" behaviour in my opinion is that source features don't have > any branding plugin at all unless there is one explicitly specified. Correct, and all our source features don't have a branding plug-in (OK, same in 3.x). I also think the bug is in Tycho, which just copies and reuses the branding plug-in (id) of the real feature. Since most of the (real) features do not specify a specific branding plug-in (id), the branding plug-in has the same id as the feature. Exceptions are RCP and Help, which set a custom branding bundle id. Since the source bundle ids end with ".source", and there is no corresponding plug-in, those source features don't show up in the dialog. Exceptions are the RCP and Help source bundles where the custom branding bundle id was copied from the real bundle. NOTE: the plug-ins that end with ".source" are not real bundles, but eclipse source bundles and hence they are not considered real bundles.
(In reply to comment #12) > (In reply to comment #10) > > David, do you need a separate bug regarding the completely broken source > > feature (missing stuff, incomplete and/or wrong feature.xml and properties > > files), or do we use this bug here? > > Separate bug I guess ... Here you go: bug 407389.
I've opened bug 407706 to see if Tycho can provide a configuration setting for the source-feature-plugin so that we could at least, for Kepler, set source feature branding bundles to 'none', so that they do not clutter up the about box display, etc. (It is understandable that they would not want to change existing released behavior that was introduced by bug 378424).
As a (very) last resort we could add a hack to the About dialog and exclude the two source features.
No hacks please. :) ... plus, apparently, some people like their source features to show up there, so I believe this should be deferred. Does not seem like a "stop ship" problem ... and if/when we want to exclude source features from about box, that may take some care, since there's lots of cases ... possibilities to effect others. (I had hoped to work on a patch for the "configuration" item in Tycho ... but, the way things are going, seems like there are many more important things to do.
(In reply to comment #18) > No hacks please. :) ... plus, apparently, some people like their source > features to show up there, so I believe this should be deferred. Does not > seem like a "stop ship" problem ... and if/when we want to exclude source > features from about box, that may take some care, since there's lots of > cases ... possibilities to effect others. I would of course only exclude the two concrete features. Having them in the dialog for the final release is wrong and misleading for users, because - the list of plug-ins is empty - it has the same name as the original feature ==> without the knowledge we have, a user has no clue what those entries mean. For Kepler I see 3 possible solutions: 1) fix Tycho and exclude them 2) fix the name and make sure the source plug-ins are listed when clicking on the 'Plug-in Details' button 3) exclude them for now (this would be quick and safe fix for now) I'd prefer 1), but if all fails, we have to go with 3).
If I'm reading bug 407706 correctly, this should be in 0.18.0-SNAPSHOT, so we should give it a test run. From the "doc" in http://git.eclipse.org/c/tycho/org.eclipse.tycho.extras.git/commit/?id=073211b6671a0d95db3998e5dc3fdea45a726c49 We need to 'configure' our source-feature-plugins with something like <plugin> <groupId>org.eclipse.tycho.extras</groupId> <artifactId>tycho-source-feature-plugin</artifactId> <version>${tycho-extras-version}</version> <configuration> <reuseBrandingPlugin>false</reuseBrandingPlugin> </configuration> </plugin>
Created attachment 231343 [details] patch to configure in parent pom If we are set up correctly, we should just have to modify he configuration we already have in the parent pom and it's apply to all source features. (That's what we want, right? Or, target just the two or three that show up about box?)
Commit in master, to make use of new feature. http://git.eclipse.org/c/platform/eclipse.platform.releng.aggregator.git/commit/?id=fba28bfbbd8be391e8a35cf5d494cb8b549811a4 I've committed to master so it can be part of a test build this morning, assuming no problems, I propose we respin RC2 to get this in.
+1 for RC2. I verified that the change looks good, but I can't tell whether it will really work (did not look at the Tycho fix).
+1 for RC2. It looks like what we need to do, but I'm waiting for the test build to prove it works. PW
Test build is at http://build.eclipse.org/eclipse/builds/4I/siteDir/eclipse/downloads/drops4/I20130523-0810/ And appears to me a) the change is in 0.18 snapshot (whew) and that it does what we intended it to. Please re-open if anyone sees side effect issues or anything.
Looks good!
