On Mon, 26 Feb 2024 16:33:02 GMT, Magnus Ihse Bursie <i...@openjdk.org> wrote:
>> make/modules/java.base/gensrc/GensrcLocaleData.gmk line 58: >> >>> 56: ifeq ($(MODULE), jdk.localedata) >>> 57: $(shell $(RM) >>> $(SUPPORT_OUTPUTDIR)/gensrc/jdk.localedata/sun/util/resources/cldr/provider/CLDRLocaleDataMetaInfo_jdk_localedata.java) >>> 58: endif >> >> The remainder of this file doesn't seem to be doing anything useful. Do we >> really need to keep it around? If I understand this correctly, the files >> referenced and deleted here are generated by the cldrconverter. Is that >> build tool relying on make removing the file first? Reading the java source, >> my impression is that it will generate the file unconditionally regardless >> of the files current existence as long as the tool is run. > > Actually, as the file currently in the PR, it does not seem like it does > anything at all, since we no longer create `_the.locale_resources`. Or... hm, > without that file, `PREV_LOCALE_RESOURCES` will always be empty. I wonder > what `$(filter-out $(PREV_LOCALE_RESOURCES), $(LOCALE_RESOURCES))` returns in > that case. If it is empty, then this file does nothing. If it is > `$(LOCALE_RESOURCES)`, then we will unconditionally rm the two files. > > I believe the original intention was to spot added or removed locale > resources, and use that to trigger a re-generation. This does not seem to > work at all anymore. Do we still need that kind of behavior? Oh, this is a rabbit hole. :-( I tried to figure out where `CLDRBaseLocaleDataMetaInfo.java` comes from; I could not see where we generated that file. Turns out it is created in `make/jdk/src/classes/build/tools/cldrconverter/ResourceBundleGenerator.java`, in the function `generateMetaInfo()`. I assume this is called as part of calling the CLDRConverter buildtool. But this is done in `make/modules/[java.base|jdk.localedata]/Gensrc.gmk`. So if we need to invalidate that build tool result, it should be done in these files. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/17991#discussion_r1502964932
