On Thu, 28 May 2026 13:03:08 GMT, Erik Joelsson <[email protected]> wrote:
>> Mikael Vidstedt has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Review feedback and formatting
>
> 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?
This is a remnant from before I switched to dnf for downloading the rpms and
was getting tired of running into wget not downloading anything but the build
still proceeded. dnf does the checking so it's no longer needed.
Removed.
> 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?
Good idea, I added a warning.
It should be noted that I don't think the devkit build has ever handled
incremental builds well. These changes hopefully move us one step closer to
supporting that, but I'm sure there are still cases where it will fail.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/31303#discussion_r3326849475
PR Review Comment: https://git.openjdk.org/jdk/pull/31303#discussion_r3326860168
