Community
Participate
Working Groups
Created attachment 128859 [details] Projects for test Build ID: I20090313-0100 Steps To Reproduce: 1. Import site, feature, and plugin project into SDK 2. Build site 3. Export to file system More information: The category testFeatureIncludes is not in content.xml
Like everything, it has to get worse before it gets better :). With the latest I-Build I can't even build your update site. However, it appears with the patches from bug 267127 not only can I build your project, I also get the categories. I'm going to leave this open since we obviously haven't resolved your problem yet. Once Andrew applies the patches in bug 267127 can you try again?
Sure
I have tried this with I20090414-0800 and it appears to work now. James, if this doesn't work for you, please re-open.
I used 0415-1348 and it didn't work for me. It goes the the uncategorized category. Here is what got built into the content.jar <properties size="3"> <property name="org.eclipse.equinox.p2.name" value="Uncategorized" /> <property name="org.eclipse.equinox.p2.description" value="Default category for otherwise uncategorized features" /> <property name="org.eclipse.equinox.p2.type.category" value="true" /> </properties>
Strange, when I build the site (using I20090416-1053) I get the Test Feature Includes. <unit id='testFeatureIncludes' version='0.0.0'> <properties size='2'> <property name='org.eclipse.equinox.p2.name' value='Test Feature Includes'/> <property name='org.eclipse.equinox.p2.type.category' value='true'/> </properties> <provides size='1'> <provided namespace='org.eclipse.equinox.p2.iu' name='testFeatureIncludes' version='0.0.0'/> </provides> To be clear, all I did was import your example, open the site.xml, select build all, and looked in the content.jar. I didn't export anything (that really should just be a filesystem copy).
Strange, ditto. I will try 0416 now.
James, Just in case it still fails for you: I'm using Linux-GTK Sun VM 1.6 Could be a platform thing.
I used 0416-2000 because I had it downloaded. I still see the same problem. I did it exactly the way you did I think. I was on a windows box using IBM JVM.
Pascal, Can you try this out? Import the project attached, go to the update site project, hit build All, then hit the resulting content.jar with the p2 UI. Do you see categories? This works each time for me, but for some reason it is failing for James.
I switched to the Sun JVM and now it is working. I have the latest IBM JVM that I will try later and see how it does.
Latest IBM JVM has the problem java version "1.6.0" Java(TM) SE Runtime Environment (build pwi3260sr4-20090219_01(SR4)) IBM J9 VM (build 2.4, J2RE 1.6.0 IBM J9 2.4 Windows XP x86-32 jvmwi3260-20090215_29883 (JIT enabled, AOT enabled) J9VM - 20090215_029883_lHdSMr JIT - r9_20090213_2028 GC - 20090213_AA) JCL - 20090218_01 Going to try the java 5 version.
I only see the problem with Java 1.6. Version 1.5 works OK here.
Updated the subject to reflect the VM issue.
Hard to tell if related, but see bug 244603 for another issue specific to IBM's 6.0 VM ... at least there was a work around. (to use -Dorg.eclipse.update.jarprocessor.pack200="@none").
See org.eclipse.pde.build.tests/PublishingTests.testPublishFeature_Bug270882 This fails for me with java version "1.6.0" Java(TM) SE Runtime Environment (build pwi3260sr1-20080416_01(SR1)) IBM J9 VM (build 2.4, J2RE 1.6.0 IBM J9 2.4 Windows XP x86-32 jvmwi3260-20080415 _18762 (JIT enabled, AOT enabled) J9VM - 20080415_018762_lHdSMr JIT - r9_20080415_1520 GC - 20080415_AA) JCL - 20080412_01 In particular, see SiteXMLAction.generateCategories and the featuresToCategories map. The debugger is actually returning false for the following statement when I evaluate it in the display view: featuresToCategories.containsKey(featuresToCategories.keySet().iterator().next())
Actually, this is not a vm problem, but rather a problem with the SiteFeature class. SiteFeature#equals is returning false when the feature is compared to itself because it has a null url. Ian or James, can one of you look at doing a proper equals and hashCode for SiteFeature?
Thanks for debugging that Andrew. I'll put together a patch.
Created attachment 132469 [details] o.e.e.p2.updatesite SiteFeature patch This patch fixes Equals and HashCode for SiteFeature. I have also added a few tests cases to demonstrate the problem.
released
Sorry I am a day late in reviewing this. A few comments on the patch. in sameURL(URL url1, URL url2) if (url1.equals(url2)) return true; According to the javadoc this will requrie name resolution which is a blocking operation. If that doesn't work then the hostnames must be case equal without regard to case. It also states that the defined behavior is inconsistent with virtual hosting in HTTP. Also since the hashcode algorithm must be same for both URLs if found to be equal and we are not using the same algorithm in hashcode I think we have a problem. Maybe the best course is to not use the URL.equals method and compare the canonical URLS
Thanks for reviewing this James, I made a test case that demonstrates the problem, however, in doing so I realized that I have made an even bigger mistake. I call return sameURL(this.url, that.url); however, unless this and that have been resolved, the URL is null. This needs to be return sameURL(this.getUrl(), that.getUrl()); But your suggestion makes more sense. In fact, we should probably just compare the field urlString (ignoring case).
reopening
Created attachment 132536 [details] Patch to change siteFeature to use getUrl instead of this.url This patch changes SiteFeature to use getUrl() instead of this.url. This is important because until getUrl() is called, this.url is null. This patch does not address the issues that James brough up so we should leave this bug open. I will address those after the IBuild spins, (Note: they should not cause a regression as we didn't even have a hashCode() method before).
(In reply to comment #20) We get in trouble in the following case: a.setURLString("http://localhost"); b.setURLString("http://127.0.0.1"); if (a.equals(b)) assertEquals("1.1", a.hashCode(), b.hashCode()); In this case we are considered equal, but the hash codes don't match. James, if we just use the canonical form, then these won't be considered equal, and I think that might be a problem too. Maybe we can do this: # Check the protocol to see if it is file: ## if so, create a File object and check equality # If it is not a file, perform .equals In the hashcode method we do the same # Check the protocol to see if it is a file: ## if so, create a File and call .hashCode # if not, get the hashCode of the URL This doesn't fix the blocking problem or the known problem with virtual hosting in HTTP. I honestly don't know if this is a concern or not.
Created attachment 132642 [details] Equals / HashCode test cases Here is a test case that checks a few more hashcode / equals cases. This test currently fails because localhost is considerd equal to 127.0.0.1, but they return different hash codes. (So we shouldn't apply this until we sort out this issue).
Another mitigation would be to check all parts of the URL (path, protocol,etc.) except for host. Then do the equals only if everything else checked out. localhost and 127.0.0.1 is such a common case it could be handled in code. A quick look on the net points out that www.site.com should not be the same as foo.site.com but will likely return true in equals. Sooo. It seems to me that we will never get this exactly right. Maybe we should go for host as a string match. We could always let the user specify the equalities (alias) for host on top of that if we wanted to. So the correct approach might be to switch to URI although I do not know if there are problems with it. Since it would appear that we can't really resolve the host part correctly anyway (in URL.equals), URI is probably the best path.
I forgot about bug 237776. So I think this indicates the the URL should be refactored. Do we want a new bug and close this one for the original problem or what?
I propose we (I) create a patch for my proposed solution in comment 25 (so we at least have consistent hashcode and equals methods), and file a new bug for moving towards URI in the site files.
Ok. I will open a bug for URL.
The URL#equals method is quite broken and should be avoided if at all possible. I suggest just comparing the string form of the URL for non file: URL's, and use java.io.File#equals for file: URLs. This will mean "localhost" and "127.0.0.1" are not equal, but this isn't a big deal. We mostly use URI elsewhere in p2, and it doesn't do domain resolution when comparing equality, and that hasn't been hurting us.
(In reply to comment #31) > The URL#equals method is quite broken and should be avoided if at all possible. > I suggest just comparing the string form of the URL for non file: URL's, and > use java.io.File#equals for file: URLs. This will mean "localhost" and > "127.0.0.1" are not equal, but this isn't a big deal. We mostly use URI > elsewhere in p2, and it doesn't do domain resolution when comparing equality, > and that hasn't been hurting us. > I will prepare a patch. Thanks John.
Do we still need a bug opened for the URL?
(In reply to comment #33) > Do we still need a bug opened for the URL? > Yes, please open one. I'm not sure if we will get around to fixing it, especially since we want to phase out the site.xml at some point. Thanks for all your help with this bug James.
Created attachment 133590 [details] Patch that doesn't use URL#Equals This patch removes the use of URL#Equals. Instead, we compare by file (if they are file URLs) otherwise we compare by string. I lower case and remove the trailing slash of the URL for both .equals and .hashCode. John, can you review this?
Yes, I will review after the test pass (later today or tomorrow).
Taking bug so I don't lose it.
Fixed in HEAD.
Verified fixed in M7 build.
