Bug 274695

Comment 1 Thomas Hallgren 2009-05-01 16:48:43 EDT

Created attachment 134117 [details]
Patch that removes the redundancy

This patch removes the generation of the license and copyright info in the features.jar.

Comment 2 Thomas Hallgren 2009-05-01 16:53:02 EDT

The bug manifests itself in the Galileo Capabilities feature since the publisher is used when that feature is generated into an interim repository during build.

Comment 3 Ian Bull 2009-05-01 19:26:55 EDT

Thomas,

Should the properties (license) be set on the feature jar or the feature group? Simon was moving things to the feature jar (I think).

Comment 4 Thomas Hallgren 2009-05-02 01:46:28 EDT

Isn't the jar file of an old relic that we want to get rid of? Seems to me the meta-data should be enough and that the jar file doesn't bring anything useful into the equation. If we're moving in that direction (which I though we were), then surely, we need to keep all meta-data in the group, not in the IU that represents the jar artifact.

Comment 5 Ian Bull 2009-05-02 14:17:07 EDT

Sorry, it was DJ that moved things to the feature jar instead of the feature group. (Bug 273626).  DJ should the license be set on the feature jar or the feature group?

Once we have this sorted out, I think we should put together a regression test with this patch.

Comment 6 Thomas Hallgren 2009-05-02 19:27:37 EDT

I think the regression test can be written regardless of the decision. It should use the same logic as the UI "Review licenses" page and assert that only one entry show up. I can put something together.

Comment 7 Thomas Hallgren 2009-05-03 11:52:48 EDT

Created attachment 134178 [details]
Patch for the FeaturesActionTest

The patch adds assertions that at exactly one license and copyright has been generated. It doesn't currently care what IU it ended up in.

Comment 8 Thomas Hallgren 2009-05-03 11:56:50 EDT

(In reply to comment #5)
> DJ should the license be set on the feature jar or the feature group?
> 
Another fact that speaks for having the license and copyright in the feature.group IU is that the feature.jar IU shows up untranslated on the 'Review license' page. Not sure if that's a bug or if it's just not intended to be displayed that way.

Comment 9 DJ Houghton 2009-05-04 08:59:04 EDT

(In reply to comment #5)
> Sorry, it was DJ that moved things to the feature jar instead of the feature
> group. (Bug 273626).  DJ should the license be set on the feature jar or the
> feature group?
> 
When you pointed out that the license info and other things were already on the group, I backed out of that part of the patch. Check out the diff between FeaturesAction 1.45 and 1.46, I only added the update manager properties to aid with feature installation related to branding.

It looks like the license info was put in both places in the old code (MetadataGeneratorHelper #createFeatureJarIU and #createGroupIU). I'm not sure which is right. 

Comment 10 Pascal Rapicault 2009-05-04 12:07:14 EDT

Going forward every IU (plug-in, feature, exe, etc.) should have a license and a copyright (bug #255075). The presentation is unfortunate however removing the license from the jar IU is a band aid aiming in the wrong direction. 
The real fix should be to change the UI (see bug 217944 which implies fixing bug 212218)

Comment 11 Pascal Rapicault 2009-05-04 12:07:30 EDT

*** Bug 267919 has been marked as a duplicate of this bug. ***

Comment 12 John Arthorne 2009-05-05 11:46:03 EDT

For what it's worth, I had 3.4.0 loaded today, and noticed that this is actually a regression from 3.4. We didn't show the licenses separately for the feature JARs in 3.4.

Looking at the UI code, I see we had a workaround in there that we omitted showing duplicates that have the same name. I think the bug here is that we are not setting the name property on the feature jar IU's.


From AcceptLicensesWizardPage:

// Current metadata generation can result with a feature group IU and the feature jar IU
// having the same name and license.  We will weed out duplicates if the license and name are both
// the same.  

Comment 13 John Arthorne 2009-05-05 11:56:39 EDT

Created attachment 134456 [details]
Fix

Comparing MetadataGeneratorHelper.createFeatureJarIU to FeatureAction.createFeatureJARIU, I noticed the publisher is simply missing several of the properties that we were setting in 3.4 (name, description, provider). If we set the name property then the "duplicate entries" problem should go away.

Comment 14 Ian Bull 2009-05-05 12:10:44 EDT

As I mentioned on Bug 273626, these are all set on the feature.group.  Does everything need to be duplicated on both the jar and the group?  What about translation?  (I don't think we create a translation fragment for the jar, so these properties likely won't be translated).

Comment 15 John Arthorne 2009-05-05 13:37:28 EDT

> Does everything need to be duplicated on both the jar and the group?

Yes, things like the name and description should always be set, otherwise they won't show up correctly for end users in the UI.

Comment 16 Pascal Rapicault 2009-05-05 14:00:57 EDT

Created attachment 134476 [details]
New patch adding translation

Comment 17 Pascal Rapicault 2009-05-05 14:33:57 EDT

Patch reviewed and released.

Comment 18 Ian Bull 2009-05-05 14:38:15 EDT

I think we should add some feature.jar translation tests then.  i opened bug 275040 to track.  Feel free to push it on me.

Comment 19 John Arthorne 2009-05-05 14:56:39 EDT

Created attachment 134493 [details]
Regression test in publisher tests

Comment 20 John Arthorne 2009-05-05 14:57:15 EDT

I have also released the regression test to HEAD.