[stbischof]

Felix applied the type of a typed attribute (name:Long=42) only for Provide-Capability, Require-Capability and Bundle-SymbolicName. Export-Package kept the value as a String, which fails ExportAnnotationsTestCase#testAttributes of the OSGi bundle annotations TCK. Import-Package, DynamicImport-Package, Require-Bundle and Fragment-Host dropped the type as well, so a list attribute there could never match a typed capability.

The conversion now runs for all of these headers. A list value in a requirement header becomes one filter per element, so it matches a capability list that contains all of them. The version and specification-version check compares versions instead of strings, since version can now be typed.

osgi/org.osgi.annotation.bundle#24

[paulrutter]

Review summary

What it does: Typed attributes (name:Long=42, List<Version>, etc.) are now converted for Export-Package, Import-Package, DynamicImport-Package, Require-Bundle and Fragment-Host, not only the capability headers. List values in requirements become one filter per element. The version/specification-version check now compares versions instead of strings.

Verdict: The approach is sound and small, and it reuses the existing conversion code. The tests check both the parsed values and that the generated requirement matches the typed capability. CI is green on JDK 17/21/25/27. I reviewed by reading the code and did not run the build locally. No blocking issues, only the points below.

Points to consider

1. Error handling (ManifestParser.convertTypedAttributes)
   A malformed value such as Long=abc throws a raw NumberFormatException/IllegalArgumentException with no attribute name. This is now reachable from five more headers. Consider rethrowing as BundleException (MANIFEST_ERROR) with the attribute key and type.

2. Running twice (ManifestParser.convertTypedAttributes)
   m_types is not cleared after conversion, so a second run on the same clause would corrupt list values ("[a, b]" gets re-split). Nothing does this today, but clearing the entry or adding a comment would protect against future changes.

3. Empty list (SimpleFilter.convert)
   colors:List<String>="" adds no filter, so the requirement silently matches anything. Please reject it, warn, or at least test and document the behavior.

4. List semantics (SimpleFilter.convert)
   "A list attribute matches if the capability contains all values" is a design choice. I could not find it defined for Import-Package in the spec. Please document it in the Javadoc, which still says equality is assumed for everything except version ranges. Please also confirm it matches what the TCK in tck: a second run against Apache Felix osgi/org.osgi.annotation.bundle#24 expects.

5. Good catch, needs a test (SimpleFilter.convert)
   Changing attrs.size() > 1 to filters.size() > 1 fixes an existing bug: a single bounded VersionRange produced two filters but attrs.size() == 1, so the filter ended up null. Please add a SimpleFilterTest case for it (single bounded VersionRange, and an empty list next to another attribute).

6. Version-equality check (import, dynamic import, export)

   • The same block now exists three times; a small helper would remove the duplication.
   • Accepting 1.0 vs 1.0.0 is a behavior change worth a line in the commit message.
   • Error messages are less clear for invalid input, e.g. a List-typed version shows up as "[1.0.0, 2.0.0]" in a parse error.

7. Test gaps

   • All new tests are positive. Please add a negative case (a non-matching list element or typed value must not match) and a malformed-value case.
   • Require-Bundle and Fragment-Host have no test with a typed bundle-version.
   • Nit: move the TYPED_* constants and helpers to the bottom of the test class.

---

Generated by Claude Code

[stbischof]

I will do some adjustments related to the review

[stbischof]

@paulrutter
Thanks for the review. All points are addressed in 6268090:

1. Error handling: a malformed typed value now fails with a BundleException (MANIFEST_ERROR) that names the attribute, its type and the value, e.g. Invalid value for attribute 'longAttr' of type Long : abc. Unknown types (x:Foo=1, List<Foo>) and a version/specification-version mismatch are reported as MANIFEST_ERROR too.
2. Running twice: m_types is cleared after the conversion, so converting a clause again does nothing.
3. Empty list: an empty list in a requirement now becomes (attr=*): the capability must have the attribute, and any value matches. Before, it matched every capability, also one without the attribute.
4. List semantics: documented in the SimpleFilter.convert Javadoc, including that the spec does not define list matching for these headers. The TCK in tck: a second run against Apache Felix osgi/org.osgi.annotation.bundle#24 does not exercise list attributes in requirement headers (bnd generates Import-Package with version ranges only), and it passes 23/23 against this branch. For comparison, Equinox builds (l=[a, b]) here, which never matches.
5. filters.size() fix: SimpleFilterTest now covers a single bounded VersionRange and an empty list next to another attribute. Both tests fail on the code before this PR.
6. Version check: the helpers getPackageVersion and convertVersionRanges replace the duplicated blocks. version, specification-version and bundle-version must be of type String or Version, otherwise there is a clear error instead of a parse error on "[1.0.0, 2.0.0]". The 1.0 vs 1.0.0 change is described in the commit message.
7. Test gaps: added negative cases (typed values, list elements, bundle-version), malformed values, unknown types, a typed bundle-version for Require-Bundle and Fragment-Host, and moved the constants and helpers to the bottom of the class.

A second review pass found one more issue, also fixed in this commit: typed attributes are now only converted for R4 bundles in Import-Package, DynamicImport-Package and Export-Package. R3 bundles keep dropping unknown attributes with a warning instead of failing on a malformed one.

[paulrutter]

Looks good to me!