RFR: 8313274: Failure building java.base-jmod target

[David Holmes]

On Tue, 1 Aug 2023 10:31:13 GMT, Jaikiran Pai <jpai at openjdk.org> wrote:

>> Can I please get a review of this change which proposes to address the build failure noted in https://bugs.openjdk.org/browse/JDK-8313274?
>> 
>> The build failure is consistently reproducible with `--with-jobs=1`. Martin, in that JBS issue, has narrowed down the commit to the change in https://github.com/openjdk/jdk/pull/14561, starting which this failure is reproducible. The change in that PR, from what I understand, was meant to not require upgradable modules be a prerequisite for the `java.base-jmod` make target:
>> 
>>> ... upgradeable modules, those shouldn't be on the prerequisites list for java.base-jmod.
>> 
>> The implementation of that change uses the `FindAllUpgradeableModules` function which as commented in the make files does:
>> 
>>> #Upgradeable modules are those that are either defined as upgradeable or that
>>> #require an upradeable module.
>> 
>> The implementation of `FindAllUpgradeableModules` uses the `UPGRADEABLE_PLATFORM_MODULES` make variable which similarly comments:
>> 
>>> #Modules that directly or indirectly requiring upgradeable modules
>>> #should carefully be considered if it should be upgradeable or not.
>> 
>> However, that set currently doesn't include the "indirectly requiring upgradable modules" and thus appears to be missing some of the modules that are considered upgradable.
>> 
>> As a result, what seems to be happening is that the `java.base-jmod` make target now can (and does) end up requiring a particular target as a prerequisite, say `jdk.jdeps` (which uses `java.compiler` upgradable module), but doesn't add the `java.compiler-jmod` target as a prerequisite and thus ends up with that build failure:
>> 
>> 
>> Creating java.base.jmod
>> Error: Resolution failed: Module java.compiler not found, required by jdk.jdeps
>> 
>> 
>> The commit in this PR proposes to fix this by updating the static set of `UPGRADEABLE_PLATFORM_MODULES` to include the indirect modules that require the upgradable modules. How these additional modules were derived is explained as a separate comment in this PR.
>> 
>> The build succeeds with this change, both with `--with-jobs=1` and without the `--with-jobs` option (in which case on my setup it uses 8 jobs).
>> 
>> I have triggered tier testing in the CI to make sure this doesn't cause any unexpected regressions.
>> 
>> This change will require reviews from both the build team as well as the Java modules team - my knowledge of these areas is limited and I'm unsure if there are any additional considerations to take into...
>
>> It also opens the door to accidental mix 'n match of modules from different JDK modules. So I don't think this the right change for this issue.
> 
> Thank you Alan for that input. I'll close this PR then. I think in the PR that introduced the original change, there was a mention that the change isn't too helpful for the build but is mostly a good to have:
> 
>> Fixing this won't impact the build much, but certainly won't hurt either.
> 
> So for now I suspect we could revert that change since it currently does adversely impact the build.

@jaikiran I would concur - back out the change that caused the problem.

-------------

PR Comment: https://git.openjdk.org/jdk/pull/15102#issuecomment-1661514395