On Wed, 18 Oct 2023 15:13:14 GMT, Magnus Ihse Bursie <i...@openjdk.org> wrote:
>> Naoto Sato has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Delay populating GMT format at runtime. Reflecting review comments. > > make/modules/jdk.localedata/Gensrc.gmk line 39: > >> 37: GENSRC_DIR := $(SUPPORT_OUTPUTDIR)/gensrc/jdk.localedata >> 38: CLDR_GEN_DONE := $(GENSRC_DIR)/_cldr-gensrc.marker >> 39: TZ_DATA_DIR := $(MODULE_SRC)/../java.base/share/data/tzdata > > This seems like a bit of abuse of `$(MODULE_SRC)`. I thought we had a macro > to get the proper source path for another module, but if we do, I cannot find > it now. > > If you feel up for it, I'd encourage you to change this to: > > Suggestion: > > TZ_DATA_DIR := $(call FindModuleSrc, java.base)/share/data/tzdata > > > and add > > ################################################################################ > # Find source dir for module > # Param 1 - module name > FindModuleSrc = \ > $(TOPDIR)/src/$(strip $1) > > at a suitable place in `Utils.gmk`. > > Otherwise, just replace it with: > > > TZ_DATA_DIR := $(TOPDIR)/src/java.base/share/data/tzdata Thanks, Magnus. In fact, I was wondering if I should move back tzdata into make as it is shared like cldr, but it would revert the work you did before, so I ended up traversing the directory structure. Fixed with the latter suggestion. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/16206#discussion_r1364102062
