suggesting fix: cross-compilation is requiring write permissions to bootstrap jdk

[Erik Joelsson]

Fix looks good to me then.

Do you need sponsoring help? Is there a bug number for this yet?

/Erik

On 2020-06-01 01:53, Claes Redestad wrote:
> Hi Fedor,
>
> thanks for verifying!
>
> /Claes
>
> On 2020-06-01 10:39, Fedor wrote:
>> Hi Claes,
>>
>> The content of classlist files is the same in both cases (before and 
>> after change).
>>
>> And the change with updated file name $@.classlist => $@.interim as 
>> suggested before.
>>
>> diff --git a/make/GenerateLinkOptData.gmk b/make/GenerateLinkOptData.gmk
>> --- a/make/GenerateLinkOptData.gmk
>> +++ b/make/GenerateLinkOptData.gmk
>> @@ -69,10 +69,10 @@
>>              -Duser.language=en -Duser.country=US \
>>              -cp $(SUPPORT_OUTPUTDIR)/classlist.jar \
>>              build.tools.classlist.HelloClasslist $(LOG_DEBUG)
>> -       $(GREP) -v HelloClasslist $@.raw > 
>> $(INTERIM_IMAGE_DIR)/lib/classlist
>> -       $(FIXPATH) $(INTERIM_IMAGE_DIR)/bin/java -Xshare:dump \
>> +       $(GREP) -v HelloClasslist $@.raw > $@.interim
>> +       $(FIXPATH) $(INTERIM_IMAGE_DIR)/bin/java -Xshare:dump 
>> -XX:SharedClassListFile=$@.interim -XX:SharedArchiveFile=$@.jsa \
>>              -Xmx128M -Xms128M $(LOG_INFO)
>> -       $(FIXPATH) $(INTERIM_IMAGE_DIR)/bin/java 
>> -XX:DumpLoadedClassList=$@.raw \
>> +       $(FIXPATH) $(INTERIM_IMAGE_DIR)/bin/java 
>> -XX:DumpLoadedClassList=$@.raw -XX:SharedClassListFile=$@.interim 
>> -XX:SharedArchiveFile=$@.jsa \
>>              -Djava.lang.invoke.MethodHandle.TRACE_RESOLVE=true \
>>              -Duser.language=en -Duser.country=US \
>>              --module-path $(SUPPORT_OUTPUTDIR)/classlist.jar \
>>
>>
>>
>> Thanks,
>> Fedor
>>
>> On 26.05.2020 18:39, Claes Redestad wrote:
>>> Hi,
>>>
>>> I think the proposed flags should do the right thing here. It should be
>>> straightforward to verify since the contents of the generated files
>>> should ideally not change at all on a standard build.
>>>
>>> /Claes
>>>
>>> On 2020-05-26 17:25, Erik Joelsson wrote:
>>>> Hello,
>>>>
>>>> On 2020-05-26 08:00, Magnus Ihse Bursie wrote:
>>>>>> On 26/05/2020 11:14 pm, Fedor wrote:
>>>>>>> Hello all!
>>>>>>>
>>>>>>> I've tried to crossbuild jdk using current 
>>>>>>> http://hg.openjdk.java.net/jdk/jdk/ sources and noticed that 
>>>>>>> build requires write permissions to bootstrap jdk.
>>>>
>>>> Your patch seems to be doing the right thing. I would just like to 
>>>> have Claes verify that the JVM options you are setting are actually 
>>>> having the desired effect. We really shouldn't be putting any files 
>>>> in the INTERIM_IMAGE_DIR as that can possibly be located outside 
>>>> the build directory. Only thing I would want to change is the 
>>>> suffix for the file you call $@.classlist, since $@ is already 
>>>> named "classlist", I would prefer something like $@.tmp or 
>>>> $@.interim, to signal that this is the interim result between the 
>>>> two generation steps.
>>>>
>>>> Then just to clarify, it's not the BOOT_JDK, but the BUILD_JDK, 
>>>> which is a different concept. This can get tricky so it's important 
>>>> to keep the definitions correct. The BOOT_JDK, which is required to 
>>>> be of version N-1, or possibly N, is used to build a bootstrap 
>>>> version of the javac compiler, which we can then run on the 
>>>> BOOT_JDK to compiler the JDK N java code. We also use it to run 
>>>> most build tools in the build.
>>>>
>>>> The BUILD_JDK is a concept introduced for cross compiling. In a 
>>>> native build, certain build steps are required to be performed with 
>>>> the newly built JDK. This includes running jmod and jlink as well 
>>>> as generating data used to optimize the JDK image in the jlink step 
>>>> (which is where you are seeing this issue). For a native build, the 
>>>> BUILD_JDK is simply the exploded image jdk we just built. When 
>>>> cross compiling, the default behavior is to just also build enough 
>>>> of the native parts of the JDK for the build platform and use that. 
>>>> You may also supply a BUILD_JDK when configuring a cross 
>>>> compilation build. When doing so you must ensure that it's built 
>>>> from the exact same sources as the JDK you are cross compiling, 
>>>> otherwise results are unpredictable.
>>>>
>>>> In our internal build setups, we don't use the external BUILD_JDK 
>>>> feature, which is why we have not run into the problem you are 
>>>> reporting here.
>>>>
>>>> /Erik
>>>>
>>>>>>> The problem is it tries to write/rewrite class list into 
>>>>>>> bootstrap jdk directory.
>>>>>
>>>>> No, it should write to the INTERIM_IMAGE_DIR.
>>>>>
>>>>> However, I now see that we replace INTERIM_IMAGE_DIR with 
>>>>> BUILD_JDK in GenerateLinkOptData.gmk if we have an external 
>>>>> build-jdk. I assume that is how you have configured. I also assume 
>>>>> you are not cross-compiling.
>>>>>
>>>>> I'm not entirely sure about the thinking here. I can't say for 
>>>>> certain that we really should run HelloClasslist if we have an 
>>>>> external build-jdk. I think I need Erik or Claes to chime in.
>>>>>
>>>>> /Magnus
>>>>>
>>>>>
>>>>>>> I would like to suggest the fix below to solve this problem:
>>>>>>>
>>>>>>>
>>>>>>> diff --git a/make/GenerateLinkOptData.gmk 
>>>>>>> b/make/GenerateLinkOptData.gmk
>>>>>>> --- a/make/GenerateLinkOptData.gmk
>>>>>>> +++ b/make/GenerateLinkOptData.gmk
>>>>>>> @@ -69,10 +69,10 @@
>>>>>>>              -Duser.language=en -Duser.country=US \
>>>>>>>              -cp $(SUPPORT_OUTPUTDIR)/classlist.jar \
>>>>>>>              build.tools.classlist.HelloClasslist $(LOG_DEBUG)
>>>>>>> -       $(GREP) -v HelloClasslist $@.raw > 
>>>>>>> $(INTERIM_IMAGE_DIR)/lib/classlist
>>>>>>> -       $(FIXPATH) $(INTERIM_IMAGE_DIR)/bin/java -Xshare:dump \
>>>>>>> +       $(GREP) -v HelloClasslist $@.raw > $@.classlist
>>>>>>> +       $(FIXPATH) $(INTERIM_IMAGE_DIR)/bin/java -Xshare:dump 
>>>>>>> -XX:SharedClassListFile=$@.classlist -XX:SharedArchiveFile=$@.jsa \
>>>>>>>              -Xmx128M -Xms128M $(LOG_INFO)
>>>>>>> -       $(FIXPATH) $(INTERIM_IMAGE_DIR)/bin/java 
>>>>>>> -XX:DumpLoadedClassList=$@.raw \
>>>>>>> +       $(FIXPATH) $(INTERIM_IMAGE_DIR)/bin/java 
>>>>>>> -XX:DumpLoadedClassList=$@.raw 
>>>>>>> -XX:SharedClassListFile=$@.classlist -XX:SharedArchiveFile=$@.jsa \
>>>>>>> -Djava.lang.invoke.MethodHandle.TRACE_RESOLVE=true \
>>>>>>>              -Duser.language=en -Duser.country=US \
>>>>>>>              --module-path $(SUPPORT_OUTPUTDIR)/classlist.jar \
>>>>>>>
>>>>>>> Please correct me in case if it is wrong alias, or I need to 
>>>>>>> file bug first, or something else.
>>>>>>>
>>>>>>> Best Regards,
>>>>>>> Fedor
>>>>>>>
>>>>>
>>