Re: Framework changes

[Richard S. Hall]

Felix Meschberger wrote:
Hi Richard,
Just from looking at the code, it seems ok. Also keeping the trailing dot
for the prefix check seems reasonable. But leaving the dot would fail the
equality check.

Given dynPkgName="com.package.*" and pkgName="com.package". In this case,
the dynPkgName would be shortened to "com.package.", which would neither
evaluate to true for pkgName.equals(dynPkgName) nor
pkgName.startsWith(dynPkgName).

True. D'oh! Funny thing is, I had the same issue at one time with this 
code in its previous incarnation...

I will look at it one more time taking into consideration your proposal 
from below.

Thanks for looking at it closely.

-> richard


[ Given that "com.package.*" should match "com.package". ]

How about this solution :

              // Ignore any dynamic requirements whose packages don't
match.
              String dynPkgName = ((Requirement)
dynamics[i]).getPackageName();
              boolean wildcard = (dynPkgName.lastIndexOf(".*") >= 0);
              if (wildcard)
              {
                   dynPkgName = dynPkgName.substring(0, 
dynPkgName.length()
- 1):
                   pkgName += ".";
              {
              if (dynPkgName.equals("*") ||
                  pkgName.equals(dynPkgName) ||
                  (wildcard && pkgName.startsWith(dynPkgName)))
              {
                  // Find candidate...
              }

Regards
Felix


On 1/24/07, Richard S. Hall <[EMAIL PROTECTED]> wrote:

Richard S. Hall wrote:
> Felix,
>
> Your patch seems ok for now, although I would like to eventually
> optimize this away if possible.
>
> I have modified your patch slightly to make it a positive test, rather
> than a negative test, and have also extended it to catch more cases.
> Here is the gist of the patch now, let me know if it looks like it
> will work for you:
>
>                // Ignore any dynamic requirements whose packages don't
> match.
>                String dynPkgName = ((Requirement)
> dynamics[i]).getPackageName();
>                boolean wildcard = (dynPkgName.lastIndexOf(".*") >= 0);
>                dynPkgName = (wildcard)
>                    ? dynPkgName.substring(0, dynPkgName.length() - 2)
> : dynPkgName;
>                if (dynPkgName.equals("*") ||
>                    pkgName.equals(dynPkgName) ||
>                    (wildcard && pkgName.startsWith(dynPkgName)))
>                {
>                    // Find candidate...
>                }

Actually, I think that should be "- 1" instead of "- 2", because we will
want to keep the "." so when we do "startsWith()" it will correctly
match package names.

-> richard
>
> -> richard
>
> Felix Meschberger wrote:
>> HI Richard,
>>
>> I updated my local framework copy from SVN today and noticed, that
>> performance degraded remarkably for bundles using
DynamicImport-Package.
>> After looking around a while, I saw, that
>> R4SearchPolicyCore.attemptDynamicImport looks after dynamic
>> Requirements and
>> builds a filter from the requirement filter and the requested
>> package. This
>> filter is then used to scan all bundles ... Using this over and over
>> is very
>> expensive.
>>
>> I wonder, whether this small (somewhat hacky) fix is legal [ at least
it
>> does seem to work for me, yet I am not sure, whether this is
acceptable)
>>
>> --------------------------------
>> Index:
>>
src/main/java/org/apache/felix/framework/searchpolicy/R4SearchPolicyCore.java 

>>
>> ===================================================================
>> ---
>>
src/main/java/org/apache/felix/framework/searchpolicy/R4SearchPolicyCore.java 

>>
>> (revision 499005)
>> +++
>>
src/main/java/org/apache/felix/framework/searchpolicy/R4SearchPolicyCore.java 

>>
>> (working copy)
>> @@ -510,6 +510,17 @@
>>                 // is necessary because we cannot easily determine
which
>>                 // package name a given dynamic requirement matches,
>> since
>>                 // it is only a filter.
>> +
>> +                // do not consider dynamic packags not matching the
>> requested
>> +                if ( ICapability.PACKAGE_NAMESPACE.equals(
>> dynamics[i].getNamespace() ) )
>> +                {
>> +                    String dynPack = ( ( Requirement ) dynamics[i]
>> ).getPackageName();
>> +                    if ( !pkgName.equals( dynPack ) && !( pkgName +
>> ".*"
>> ).equals( dynPack ) )
>> +                    {
>> +                        continue;
>> +                    }
>> +                }
>> +
>>                 IRequirement req = null;
>>                 try
>>                 {
>> -------------------------------
>>
>>
>> What do you think ?
>>
>> Regards and Thanks
>> Felix
>>
>> PS: Yes I know, that this is work in progress :-)
>>
>> On 1/22/07, Richard S. Hall <[EMAIL PROTECTED]> wrote:
>>>
>>> Steven E. Harris wrote:
>>> > "Richard S. Hall" <[EMAIL PROTECTED]> writes:
>>> >
>>> >
>>> >> In short, the module loader abstraction previously worked in 
terms
>>> >> of exports/imports, but now it has been generalized to work in
terms
>>> >> of capabilities/requirements. A capability is simply a set of
>>> >> properties, while a requirement is a filter over a set of
>>> >> properties.
>>> >>
>>> >
>>> > Does this mean that some of the capabilities and requirements 
types
>>> > from the OBR bundle will be moved (or copied) into Felix proper?
>>> >
>>>
>>> Essentially, yes. I have had to make some modifications to their
>>> implementation for the time being, but I hope to continue to work on
>>> them to make them more generic like the original OBR types.
>>>
>>> -> richard
>>>
>>