Bug 265103

Summary:

Manifest Class-Path is not read correctly with ECJ

Product:

[Eclipse Project] JDT

Reporter:

Fabrice Matrat <fmatrat>

Component:

Core

Assignee:

Olivier Thomann <Olivier_Thomann>

Status:

VERIFIED FIXED

QA Contact:

Severity:

normal

Priority:

CC:

fmatrat, Olivier_Thomann, philippe_mulet

Version:

3.4.1

Target Milestone:

3.5 M6

Hardware:

All

OS:

All

Whiteboard:

Attachments:

Description	Flags
Modified version of ClasspathJar.java and its unit test associated	none
Proposed fix	none
Proposed fix for HEAD	none
Proposed fix	none
Proposed fix for 3.4.x	none

Description Fabrice Matrat

2009-02-17 04:09:38 EST

Created attachment 125864 [details]
Modified version of ClasspathJar.java and its unit test associated

The maximum length of a line in the Manifest is 72 characters. If a jar need to be split over two lines the next line should have a white space. If a jar finish at the end of a line the next line should start with two whitespaces.
If the next line start with a character it means, it's the next item and CLASS-Path is finished.

A full explanation of the way to read the Class-Path can be found for example at 
http://forums.sun.com/thread.jspa?threadID=364938

I attached a Unit test with some values and the fix I did in ClassPathJar.jar.
I already integrated the bug 251079 for the \r.

I don't know what we should do with an empty declaration of Class-Path.

See my changes attached and a unit test with it.

Comment 1 Fabrice Matrat

2009-02-17 04:50:09 EST

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

Comment 2 Olivier Thomann

2009-02-23 11:47:29 EST

Two comments:
1) Why org.eclipse.jdt.internal.compiler.util.ManifestAnalyzer was moved as a static member class of org.eclipse.jdt.internal.compiler.batch.ClasspathJar?
It is also used in org.eclipse.jdt.internal.core.ClasspathEntry.
2) Why using the manifest analyser as a static field in ClasspathJar? This would not make it thread-safe.

Once I get answers to these concerns, I'll work on releasing the fix.

Comment 3 Fabrice Matrat

2009-02-24 03:29:27 EST

1) Why org.eclipse.jdt.internal.compiler.util.ManifestAnalyzer was moved as a
static member class of org.eclipse.jdt.internal.compiler.batch.ClasspathJar?
It is also used in org.eclipse.jdt.internal.core.ClasspathEntry.

I took the code from 3.4.1 where the ManifestAnalyser was a static class inside ClassPathJar. I think it was moved in 3.5 beta version.

2) Why using the manifest analyser as a static field in ClasspathJar? This
would not make it thread-safe.

You are right but I didn't change that. The only changes I made where marked with // >>>>>>>>>>>>>>>>>> before the lines I changed.


We are using 3.4 here so that's why I used the code from 3.4 but apparently the manifest analyser class has moved to its own file in 3.5

Comment 4 Olivier Thomann

2009-02-24 08:36:34 EST

Ok, I'll make the required changes to get a 3.4.x patch as well as 3.5 patch.
Thanks for your answers.

Comment 5 Olivier Thomann

2009-03-02 12:34:34 EST

In 3.4 maintenance, the manifest analyzer is not longer a static field.
So I'll make appropriate changes to the proposed fix and will attach new patches for 3.4.x and HEAD for review.

Comment 6 Olivier Thomann

2009-03-02 12:48:53 EST

Created attachment 127195 [details]
Proposed fix

Patch for 3.4.x including regression tests.

Comment 7 Olivier Thomann

2009-03-02 13:05:59 EST

Created attachment 127196 [details]
Proposed fix for HEAD

Patch for HEAD including regression tests

Comment 8 Olivier Thomann

2009-03-04 09:08:33 EST

Some of the org.eclipse.jdt.core.tests.compiler.regression.BatchCompilerTest tests are failing with this patch.
Needs more investigation.

Comment 9 Olivier Thomann

2009-03-04 10:54:26 EST

Created attachment 127487 [details]
Proposed fix

New patch for HEAD that passes all batch compiler tests and the new test for the ManifestAnalyzer.
When the last line doesn't end with a line break, the corresponding jar entry should not be reported.

Fabrice, could you please review it?
Thanks.

Comment 10 Olivier Thomann

2009-03-04 11:11:02 EST

Created attachment 127491 [details]
Proposed fix for 3.4.x

New fix for 3.4.x that is passing all BatchCompilerTests + new manifest analyzer tests.

Comment 11 Olivier Thomann

2009-03-04 11:27:02 EST

Released for 3.5M6.
Released into 3.4.x branch (post 3.4.2)

Comment 12 Kent Johnson

2009-03-10 11:40:43 EDT

Verified for 3.5M6 using I20090310-0100