Community
Participate
Working Groups
If there's a Java project with at least one source folder, and . isn't on the Bundle-ClassPath, things will go wrong. We should throw up an error on that case. Sometimes people will include a jar by adding 'Bundle-ClassPath: foo.jar' which of course removes . which will just break things. Some intelligence e.g. looking for 'src..' and 'bin..' in the build.properties will almost certainly be necessary here. Alex.
Alex, I think we might be able to make the algorithm even smarter. By default settings in the wizard, you Bundle-Classpath is blank and assumed to be '.'. But, if the user fills in the Classpath Text on the second page of the New Plug-in Project Wizard, the Bundle-Classpath is set to whatever you specified and the build.properties maps your source folders to the given classpath name. So the algorithm would go: - Parse the Bundle-Classpath if it exists. - If the project has source folders which are not empty, continue - Look through each classpath entry to see if the entry is mapped to a source folder in the build.properties. If none are found, then create a marker.
People can (and do) edit the Manifest.MF manually too. Would it handle that case? Also, would you like a hand in me trying to fix this?
Hey Brian, we should target this for 3.4M1 if not too late, along with the bug by Ed Merks. Let's discuss on Monday over some food ;d btw, as an OSGi purist, I hate to see anything BUT Bundle-ClassPath: . ;p
btw Alex, if you're interested, I would start with BundleErrorReporter.
oh btw, here's a context which looks at relevant methods.
Created attachment 75402 [details] mylyn/context/zip
No fair - I want to discuss it over some food too :-).
The PDE Austin team is visiting in September so we will be able to have lunch. The PDE Austin team has lunch once a month, while the Toronto team has it at least once a day :)
Created attachment 77412 [details] org.eclipse.pde.patch In PDE, we love to save our users from costly mistakes. Throws up an error if there is a mismatch between the bundle classpath and what is specified in the build.properties. Also, there is a quickfix provided to add the missing entries on the bundle classpath.
Created attachment 77413 [details] mylyn/context/zip
Mike/Brian, can you give this one a review.
Created attachment 77420 [details] org.eclipse.pde.patch Updated patch, other one was outdated.
Thanks for the work, Chris. I'd have liked to help out, but sadly, just didn't find the time. I'm sure that your fix will be much more comprehensive than mine would have been, though. In the end, I guess what matters is that there's less chance of a newbie tripping up over this. Sadly, I guess it's not important enough to get backported to 3.3.1.
Hey Chris, I found a minor flaw in the marker resolution. To reproduce do the following: 1. create a new plug-in project, but include a library in its classpath through the wizard ("test.jar"). 2. create new source folder ("src_ant") 3. Use Manifest or Build Editor to add another library ("ant.jar") with the Runtime Information section 4. highlight the new ant.jar entry, and click "Add folder" on the right table. 5. Save file (everything should be good) 6. Add another source folder ("src_text") 7. Run the quickfix. Notice source.. is created, but it duplicates content from the other two libraries. It might be handy if we have additional quickfixes to add to existing libraries. For the instance above, step 7 might generate 3 quickfixes: 1 to add src_text to test.jar, 1 to add src_text to ant.jar, and 1 to create a new source.. entry. This would definitely be an enhancement.
That quickfix in build.properties seems broken Brian and it doesn't seem we're doing enough checking. The quickfix I added is in the MANIFEST.MF which doesn't get triggered properly due to us not running the builder on the manifest. After Step 7, if you open up your manifest.mf and change say the bundle version, you should get an error about there being a mismatch of the bundle classpath and build.properties. It will ask you to add "." to the bundle-classpath. I think we need to work on build.properties validation a bit more, because something like this just looks crazy (after step 7): source.test.jar = src/ output.test.jar = bin/ bin.includes = META-INF/,\ test.jar,\ ant.jar,\ . source.ant.jar = src/,\ src_ant/ source.. = src/,\ src_ant/,\ src_text/
I think this bug is related to https://bugs.eclipse.org/bugs/show_bug.cgi?id=198465 but is representing only a particular use case of it.
After talking with Chris, the quickfix causing problems in comment #14 was not part of his patch. That is something we can look at in a different bug report. But this time I think I found a problem with the patch :). If the Bundle-Classpath header is not in the Manifest, the validation exits and does not check for mappings. Steps to reproduce: 1. Create a generic PDE project. 2. Create a second source folder (src2) 3. Add corresponding data to clear any errors in the build.properties (add entry "source.test.jar = src2/" and add "test.jar" to bin.includes) 4. Clean project (which should trigger the BundleErrorReporter) No errors are reported 5. Add implied Bundle-Classpath entry to manifest "Bundle-ClassPath: ." and save Error reported about out of sync classpath. Minor optimization, in BuildErrorReporter.getSourceEntries, when parsing an extry we get the index of '.'. We shouldn't need to do this since we already know the length (PROPERTY_SOURCE_PREFIX.length()) we need to substring. This also allows us to change the value of PROPERTY_SOURCE_PREFIX (possibly adding another '.') without breaking the code.
Created attachment 78216 [details] org.eclipse.pde.patch I addressed your concerns. Give it one last test/review Brian and we should go for it.
review away Brian, I think we're there now.
targeting for M3, not enough time for M2. sorry guys ;(
Bummer
my bad, you guys got plug-in spy instead ;p I'll check this for the first I-build of M3.
*** Bug 151171 has been marked as a duplicate of this bug. ***
next I-build is looking like a good candidate for this
You have my blessing. I did a lot of simple changes that will hopefully not break the next I-Build, but if it does this could be a good cover :)
updating patch...
Created attachment 79773 [details] org.eclipse.pde.patch
Created attachment 79774 [details] mylyn/context/zip
done, >20071004
Rumor has it this is causing an unwanted warning/error on org.eclipse.core.resources. I think it is because they have a jar contributing ant tasks included in the bundle which they do not want on the Bundle-ClassPath. The build.properties specifies ant_tasks/resources-ant.jar but that jar is should not be on the Bundle-ClassPath. Instead it is contributed to ant with an extension. PDE thinks this is an error case, I'm not sure how you will determine that this is not an error case though.
It seems like a bit of a leap to assume that any jar shipped in a bundle should be on the bundle's classpath. For the many bundles that ship ant tasks in a separate JAR file, this isn't the case.
Yes, there seems to be a problem where people build a jar for valid reasons but don't specify it on the bundle classpath. I'm not sure how to detect this case. Note, that this case probably doesn't represent the majority of the bundles out there but results in a big headache for new users. I'll investigate a bit more this morning.
The error markers should also be improved a bit. In I20071009-0800, I e.g. get 3 identical-looking errors at line 1 of org.eclipse.ant.ui/META-INF: "There is a mismatch between the bundle classpath and build.properties" The error messages should explain the individual mismatches, or there should be just one error marker with explanations of all problems. Furthermore, the Quick Fix "Add '.' to the bundle classpath" should have higher priority than the others and should somehow be marked as default.
(In reply to comment #32) > Yes, there seems to be a problem where people build a jar for valid reasons but > don't specify it on the bundle classpath. I'm not sure how to detect this case. If this setup is deemed "special", then you could add a configurable setting on the PDE > Compilers properties page, so that affected plug-ins can disable this check (disabling the check could also be another quick fix opportunity). The default severity should be "Warning", not "Error". I found no way to make by workspace compile error-free in I20071009-0800, so I think this problem is quite severe.
The next I-build will have this come up with a warning instead of an error as a quick temp fix. Now to talk about the permanent fix... I thought of a few possible solutions: A) Check for the ant extension point and library entries to catch the case of ant extensions B) Add an attribute to build.properties source entries that signify that they don't belong on the bundle classpath, ie., source.ant_tasks/resources-ant.jar=src_ant/;bundlecp=false C) Make the flag a configurable option, warning by default. This will allow ant extenders and other people to setup their project properties properly. A quickfix to disable this check should be an option also. I think A is unacceptable and too narrow. B is a possibility but complicates the problem. C feels just right as it should enable the flag to catch common mistakes and allow experienced users like John to understand that the warning doesn't apply to him. Does this sound reasonable John/Markus?
C) makes sense to me. It would also be nice if the warning message was more descriptive... "There is a mismatch" really gives me no clue what the problem is. I had to find this bug report to understand what it meant. Something like "This bundle contains <name-of-jar>, but it is not on the bundle classpath" would help users understand the problem. The preference page could also be used to indicate that it's a potential problem but not necessarily so (see for example how the JDT preference page has a "potential programming problems" category).
I believe bug 205834 is a dup of this one.
JDT/Core also has a jar that contains ant tasks. I would request a rebuild with this error being converted to a warning and configurable inside the PDE options.
*** Bug 205834 has been marked as a duplicate of this bug. ***
(In reply to comment #35) > C) Make the flag a configurable option, warning by default. Yup, (C) sounds good and is in line with other PDE compiler options that are sometimes indeed no problem in some plug-ins.
(In reply to comment #36) > C) makes sense to me. It would also be nice if the warning message was more > descriptive... "There is a mismatch" really gives me no clue what the problem > is. I had to find this bug report to understand what it meant. I'll second the request for improved text. This warning just turned up on org.eclipse.jdt.compiler.apt.tests, which has been working fine for a while now, and mystified me till I found this bug. Neither the build.properties syntax nor the Bundle-Classpath are intuitive (as evidenced by the frequent problems on eclipse.newcomers), so "there is a mismatch" is just not enough to work with.
I'm just keep you on your toes Walter :)
I was hoping to see this option configurable in this week integration build. This doesn't seem to be the case. Did I miss it ?
It will have to wait until next week. I got dragged into product work this week and leave for a small vacation in a couple of hours. You'll have to forgive me for those warnings in your projects a little bit longer ;) Let's target the next I-build.
Looks like one more week for this one, sorry for the wait.
This has to be done for next week as this is a milestone week. For 3.4M3, this warning must be optional.
OK. This is what was done. The flag is now controllable via a setting in the PDE Compilers page called "Bundle classpath synchronization" Also, the warning message was improved.
