RFR(XS): 8066589: Make importing sa-jdi.jar optional on its existance

[Erik Joelsson]

On 2014-12-09 18:12, Volker Simonis wrote:
> On Fri, Dec 5, 2014 at 10:11 AM, Erik Joelsson <erik.joelsson at oracle.com> wrote:
>> Hello Volker,
>>
>> Are these the only conditions for when sa-jdi.jar is not built? If so, then
>> I suppose this is fine.
> Yes. But with my proposed solution any new platform may easily add
> itself to the list of platforms which don't have the SA-agent.
>
>> The old Import.gmk would only copy sa-jdi.jar if it existed, and I think we
>> can keep that behavior, so just an existence check on sa-jdi.jar is good
>> enough in Import.gmk. In Gensrc-jdk.jdi.gmk, checking if
>> $(SUPPORT_OUTPUTDIR)/gensrc/jdk.hotspot.agent/_the.sa.services exists should
>> be fine with me. We lose a bit of error checking in the build doing it that
>> way as we won't fail if that file is missing for other reasons.
>>
> I don't quite understand. If a platform doesn't support the SA-agent
> there's no need for any error checking. This fix doesn't change the
> behaviour on any other platform except for aix-ppc64 and ZERO. Any
> other platforms which don't support the SA just add themselves to the
> lisst in the if-statement without affecting the other platforms.
>
What I meant with the "old Import.gmk" is the version before modular 
images change. In that version, there was no error checking, sa-jdi.jar 
would just get copied if it existed and otherwise not. In the current 
version of Import.gmk, the build fails if it's not there. What I tried 
to say was, it would be ok if your change made it so that simply the 
existence of sa-jdi.jar was controlling this logic. But I'm fine with 
this solution too.
>
>> Note that this hacking of the service provider files is a temporary hack
>> until service providers are properly handled in the modular world, so no
>> need for fancy solutions.
> OK fine. I've added one more tiny fix which was needed to build on
> AIX. It's in an if-def AIX anyway, so it won't impact other platforms.
> It just fixes the location of the static version of libjli:
>
> http://cr.openjdk.java.net/~simonis/webrevs/8066589.v2/
>
> OK to push now?
Please do. Looks ok to me.

/Erik
> Thanks,
> Volker
>
>> /Erik
>>
>>
>> On 2014-12-04 18:49, Volker Simonis wrote:
>>> Hi,
>>>
>>> could you please review this tiny change which fixes the build on
>>> platforms which don't built the SA agent after the recent modualrity
>>> integrations:
>>>
>>> http://cr.openjdk.java.net/~simonis/webrevs/8066589
>>> https://bugs.openjdk.java.net/browse/JDK-8066589
>>>
>>> I've tested that the fix works on AIX but I havn't had a chance to build
>>> Zero.
>>>
>>> @Xerxes: maybe you could check if my suggested fix also solves your
>>> build problems. I'm also no sure if the "ifneq ($(JVM_VARIANT_ZERO),
>>> true)" clause also catches the ZEROSHARK case (altough I think it
>>> should). If not we would need yet another "ifneq
>>> ($(JVM_VARIANT_ZEROSHARK), true)"
>>>
>>> Thanks,
>>> Volker
>>