RFR: 8257733: Move module-specific data from make to respective module [v6]

Magnus Ihse Bursie magnus.ihse.bursie at oracle.com
Wed Mar 16 08:44:51 UTC 2022


On 2022-03-16 07:31, Alan Bateman wrote:
> Magnus,
>
> This proposal does not address the previous concerns. As before, you 
> are proposing to put the data files used to generate the classes for 
> jdk.localedata and jdk.charsets into src/java.base and I don't think 
> we should do that. I really think this PR needs to be withdraw n or 
> closed until there is at least some agreement on placement. I know you 
> want to get the files moved out of the make tree but there are many 
> issues to work through before that can happen.

I'm sorry that you feel I did not properly address your concerns. You wrote:

"If you are moving them into the src tree then src/java.base (as you 
have it) is best but will still have the ugly wart that some of these 
mapping tables will be used to generate code for the jdk.charsets module."

which I interpreted as a need to file a follow-up issue to sort that 
out, not a veto on the entire PR.

If you have such strong opinions on the data files shared between 
java.base and jdk.charsets/jdk.localedata, I propose we leave them in 
make/data for the time being, clean up the associated mess, and then 
work out where they actually should be. Does that sound okay to you?

/Magnus
>
> -Alan
>
> On 15/03/2022 23:59, Magnus Ihse Bursie wrote:
>> On Tue, 15 Mar 2022 23:50:20 GMT, Magnus Ihse Bursie 
>> <ihse at openjdk.org> wrote:
>>
>>>> A lot (but not all) of the data in make/data is tied to a specific 
>>>> module. For instance, the publicsuffixlist is used by java.base, 
>>>> and fontconfig by java.desktop. (A few directories, like 
>>>> mainmanifest, is *actually* used by make for the whole build.)
>>>>
>>>> These data files should move to the module they belong to. The are, 
>>>> after all, "source code" for that module that is "compiler" into 
>>>> resulting deliverables, for that module. (But the "source code" 
>>>> language is not Java or C, but typically a highly domain specific 
>>>> language or data format, and the "compilation" is, often, a 
>>>> specialized transformation.)
>>>>
>>>> This misplacement of the data directory is most visible at code 
>>>> review time. When such data is changed, most of the time build-dev 
>>>> (or the new build label) is involved, even though this has nothing 
>>>> to do with the build. While this is annoying, a worse problem is if 
>>>> the actual team that needs to review the patch (i.e., the team 
>>>> owning the module) is missed in the review.
>>>>
>>>> ### Modules reviewed
>>>>
>>>> - [x] java.base
>>>> - [x] java.desktop
>>>> - [x] jdk.compiler
>>>> - [x] java.se
>>> Magnus Ihse Bursie has updated the pull request with a new target 
>>> base due to a merge or a rebase. The pull request now contains 12 
>>> commits:
>>>
>>>   - Merge branch 'master' into shuffle-data-reborn
>>>   - Fix merge
>>>   - Merge tag 'jdk-19+13' into shuffle-data-reborn
>>>         Added tag jdk-19+13 for changeset 5df2a057
>>>   - Move characterdata templates to share/classes/java/lang.
>>>   - Update comment refering to "make" dir
>>>   - Move new symbols to jdk.compiler
>>>   - Merge branch 'master' into shuffle-data
>>>   - Move macosxicons from share to macosx
>>>   - Move to share/data, and move jdwp.spec to java.se
>>>   - Update references in test
>>>   - ... and 2 more: 
>>> https://git.openjdk.java.net/jdk/compare/83d77186...598f740f
>> I have carefully reviewed all PR comments, and the changes I made in 
>> response to them. I believe I have resolved all requests from 
>> reviewers. What remained to do was to create an informational JEP 
>> about the new source structure, and file some follow-up issues.
>>
>> I have now created and submitted a new informational JEP ("JDK Source 
>> Structure"), available at 
>> https://bugs.openjdk.java.net/browse/JDK-8283227. When creating this 
>> JEP, it felt increasingly silly to just copy and extend the part 
>> about src/$MODULE from JEP 201, so I extended it to cover a relevant 
>> overview of the entire JDK source base structure. I actually think 
>> this JEP has a good merit on its own, notwithstanding it being a 
>> reviewer requirement for this PR.
>>
>> I have also filed follow up issues for the non-standard 
>> jdk.hotspot.agent `doc` and `test` directories 
>> (https://bugs.openjdk.java.net/browse/JDK-8283197 and 
>> https://bugs.openjdk.java.net/browse/JDK-8283198, respectively).
>>
>> I have filed a follow up issue for continued efforts to clean up 
>> charsetmapping, https://bugs.openjdk.java.net/browse/JDK-8283228.
>>
>> There were two open questions:
>>
>>   * should jdwp.spec belong to specs directory instead of data
>>   * should bin/idea.sh be changed to exclude data
>>
>> but they sounded so exploratory that I decided not to open JBS issues 
>> for them.
>>
>> @wangweij  @naotoj  @prrace  @erikj79 @jonathan-gibbons  You have all 
>> approved this PR at an older revision. Can you please reconfirm that 
>> your approval stands for the latest revision? (Sorry for the mass ping)
>>
>> -------------
>>
>> PR: https://git.openjdk.java.net/jdk/pull/1611
>




More information about the security-dev mailing list