bndtools
Parameter headers and merging
Working on a feature I noticed that we skip duplicate Require-Capability and Provide-Capability clauses. Looking into the code I could not but observe that we handle the merging purely string based. We use a Parameters in CDIAnnotations & DSAnnotations but then turn it into a string representing a clause in a sorted. Then we join all the requirements and capabilities with a ",", and write it back to the property. This is done for each component & CDI thingy. The code between the two looks copied.
Later in AnnotationHeaders.getHeader(), we get the property. This is quite suspect code. AnnotationHeaders keeps the headers in a MultiMap. The values (the clauses) for the key are then sorted and it is checked if the header already contains the existing value. However, the annotation headers add per clause but the existing property is highly likely a string containing multiple clauses. This means that the resulting set is not properly sorted per clause.
A bit like:
bnd: Require-Capability bar,foo
java: @Header(name="Require-Capability", "cat")
manifest: RC bar,foo,cat
I'd like to propose an addClause(key,attrs) to Processor. Processor will keep a Map<String,Parameters> for any clause that has been used by addClause. The versions of getProperty(key) for a key that has been used this way will the string version
of this Parameters. CDIAnnotations and DSAnnotations must then use this. They should not merge the requirements and capabilities since this should be a global policy.
AnnotationHeaders will then properly use these Parameters. Before the Parameters is printed, we should apply a header specific merge & order function. I.e. requirements will be merged differently then capabilities. Currently we duplicate this policy in CDIAnnotations and DSAnnotations and likely ignore it in other places. This will probably make MergedRequirement superfluous, at least it would now work on the final Parameters instead of a sequence of small subsets.
I think an addClause(key,attrs) & getParameters(key) will clean up the handling of requirements and capabilities and will likely be useful in other places. As far as I can see, it would significantly clean up DS- and CDIAnnotations.
The reason for this is that I need to be able to know how many service capabilities are becoming merged, I'd like to represent this as an attribute, or at least do not deduplicate the capabilities in the service namespace. And I do not want to hack this functionality into the current code due to the aforementioned issues.
Looking for feedback before I clean up this code.
for me the proposal sounds reasonable. I was always worried that we may not be handling merge operations correctly. thx for looking into this.
I don't understand the root problem here. Is it that you don't want deduplication?
I am not sure having Processor manage 2 maps (normal properties and "clauses") is a great idea either. If one does setProperty and addClause for the same key, what should happen? Are they to be merged somehow?
I don't understand the root problem here. Is it that you don't want deduplication?
I failed to be clear again it seems. What I tried to say:
- very redundant code
- no single policy for merging headers like Require-Capability & Provide-Capability
- lots of inefficient string operations
- using properties (= user input from the bnd files) as temporary scratch memory for the code
- non sorted ordering of the output is possible with the current implementation
- hard to implement a feature I want and knowing it is applied reliably
I am not sure having Processor manage 2 maps (normal properties and "clauses") is a great idea either. If one does setProperty and addClause for the same key, what should happen? Are they to be merged somehow?
Good issue. Clearly we need to keep the same semantics as today. Therefore, merging seems the proper solution since we already share these types of properties by many different parties. However, in our own code we should look where this happens and go to the more efficient route for any header we will support this way.
As an alternative we could provide a requirements and a capability Parameters in Analyzer initialized by the bnd headers and change the rest of the code to use these. However, I suspect we have this problem on many places other than RC and PC; having a general solution might help. However, if we want to be less ambitious I could live with this solution as well :-)
As an alternative we could provide a requirements and a capability Parameters in Analyzer initialized by the bnd headers and change the rest of the code to use these. However, I suspect we have this problem on many places other than RC and PC; having a general solution might help. However, if we want to be less ambitious I could live with this solution as well :-)
This sounds more promising to me. RC and PC are the main issue here.
This sounds more promising to me. RC and PC are the main issue here.
It is a trade off. If there is a common pattern I think software remains more healthy if you find a generic common solution that is useful for a specific problem but might come in handy later. This code tends to be simpler and easier to test. Much of bnd was initially designed, or at least attempted, with that philosophy in mind. Redundant code, or very similar code that is just irritatingly different enough so you can't reuse it, is imho the biggest friend of complexity.
So I rather strongly prefer to take the generic route unless it becomes too hairy.