On Thu, 28 May 2026 00:34:49 GMT, Mikael Vidstedt <[email protected]> wrote:
> ### Background (from JBS) > > The devkit creation makefiles under make/devkit have been more or less > untouched since they were first introduced. While they have served us well > there are some key challenges which are slowly becoming more urgent to > address. > > One such issue relates to the logic for creating the sysroot. Currently, it > is implemented using wget with a set of pattern to download packages directly > from the upstream yum repo. One problem with this is that it will download > all packages which happen to match the pattern, no matter if those packages > are actually needed or not. More problematically though, using wget like this > does not (necessarily) work for more recent linux distribution > versions/repositories. > > The dnf tool is the native solution for downloading packages, including > dependencies etc. > > The sysroot logic could also benefit from being split out into its own file, > making the fairly involved Tools.gmk slightly less complex. > > A secondary goal of this cleanup is to make it possible to reuse the logic > (especially the sysroot logic) for creating devkit equivalents for building > project Detroit. > > ### About the change > > The goal of this change was, initially, to change the sysroot logic to use > dnf. In making the required changes I relatively quickly I realized that the > Tools.gmk file was long overdue for cleanup. I chose to split out the sysroot > logic to a separate file but couldn't resist making some fairly significant, > (_mostly_) cosmetic changes to Tools.gmk while at it, moving it closer to the > general style of the JDK build system. That said, a lot of the logic in > Tools.gmk remains "old-style" even after this change. > > Some specific things worth highlighting: > > - I've tried to adjust formatting to closer match the JDK build system style, > but I'm sure there are places/cases where it's off. > - I updated versions and URLs to reflect the latest reality, and adjusted the > documentation in Makefile accordingly. > - To avoid having to call the Sysroot.gmk file twice (first to download > packages and then to create the actual sysroot) I updated the logic to not > depend on parse time globing. Specifically, what used to be `RPM_FILE_LIST` > is now handled in the recipe instead. > - I fixed some missing dependencies which were most likely not actual > problems, unless running with parallelism (which is not needed - Tools.gmk > sets BUILDPAR where relevant). Even with these fixes parallelism is unlikely > to work well. > - I dropped some packages which no longer exist (assuming ... See https://openjdk.org/groups/build/doc/code-conventions.html for reference. make/devkit/Sysroot.gmk line 137: > 135: endif > 136: ls $(DOWNLOAD_RPMS)/*.rpm > /dev/null 2>&1 || \ > 137: { echo "Failed to download rpms"; exit 1; } Curious about this construct. What situation are you trying to handle? If any rpms are already present, you silently don't download anything but still update the touch file. I would think deleting the touch file should trigger a full new download, or am I missing something? make/devkit/Sysroot.gmk line 152: > 150: # > 151: > 152: $(RPMS_UNPACKED_MARKER): $(DOWNLOAD_RPMS_MARKER) This recipe will happily write into a dirty target dir, which is fine most of the time, but could also cause surprises. Is it worth printing a warning if the target is dirty? make/devkit/Tools.gmk line 147: > 145: $$(foreach p,$$(abspath $$(wildcard patches/$$(ARCH)-$$(notdir > $$($(1)_DIR)).patch)), \ > 146: echo PATCHING $$(p) ; \ > 147: patch -d $$($(1)_DIR) -p1 -i $$(p) ; \ Stylewise, we would use 2 space indent here because this is logically the foreach block. These rules become muddy, but the idea is that if you can think of it as a logic block, then use 2 spaces, but if it's a non logic block driven continuation, then 4 spaces. make/devkit/Tools.gmk line 176: > 174: --target=$(TARGET) \ > 175: --host=$(HOST) --build=$(BUILD) \ > 176: --prefix=$(PREFIX) If you really want to adapt style, terminate the list with `#`. make/devkit/Tools.gmk line 270: > 268: ( \ > 269: cd $(@D) ; \ > 270: $(PATHPRE) $(ENVS) CFLAGS="-O2 $(CFLAGS)" \ I don't have a strong opinion on these, but I think applying the same block logic, these should stay 2 spaces. make/devkit/Tools.gmk line 431: > 429: cd $(PREFIX)/bin && \ > 430: ln -fs $$f $(TARGET)-$$f ; \ > 431: fi \ This is definitely logic blocks. ------------- PR Review: https://git.openjdk.org/jdk/pull/31303#pullrequestreview-4381156756 PR Review Comment: https://git.openjdk.org/jdk/pull/31303#discussion_r3317943406 PR Review Comment: https://git.openjdk.org/jdk/pull/31303#discussion_r3317959535 PR Review Comment: https://git.openjdk.org/jdk/pull/31303#discussion_r3317986312 PR Review Comment: https://git.openjdk.org/jdk/pull/31303#discussion_r3317993175 PR Review Comment: https://git.openjdk.org/jdk/pull/31303#discussion_r3318010633 PR Review Comment: https://git.openjdk.org/jdk/pull/31303#discussion_r3318014961
