RFR: 8311302: Allow for jlinking a custom runtime without packaged modules being present [v3]

[Severin Gehwolf]

On Tue, 19 Sep 2023 17:39:04 GMT, Alan Bateman <alanb at openjdk.org> wrote:

> I did a pass over this to see where this proposal is currently at. At a high-level I think good progress since the discussion on leyden-dev some time ago. A few comments this from this pass:

Thanks, Alan!

> If I read it correctly, the default behavior is now to fail if jlink is executed from a run-time image that has been modified from the original and the packaged modules are not observable on the module path (this includes the default module path effectively appended by jlink). That seems the right policy as it avoids modifications by plugins or conf changes silently propagating.

Correct.

> If I read the code correctly, the error when a hash doesn't match is a very terse "Run image links only allow single-hop" so that probably needs to be looked to make sure the message says that the run-time image has been modified and maybe include one of the files/resources that has changed so there is something real to go with the error.

Not quite. See the example I've added in the [previous comment](https://github.com/openjdk/jdk/pull/14787#issuecomment-1673592876). It already includes that info. It looks like this:


Error: java.lang.IllegalArgumentException: /disk/openjdk/upstream-sources/git/jdk-jdk/build/jmodless-copy-jmods-removed/conf/net.properties has been modified. Please double check!


The message can of course be modified what we think is best.

> The command line options to override and change the error to a warning or silently ignore seems to be "-run-time-ignore-single-hop" and "--run-image-only-warnings". Maybe it should be reduced to just one option and maybe it should contain the word "clone" to something that makes it more obvious that it's just copying or cloning the contents of the modules in the run-time image (I suspect "single hop" won't be understood).

I believe keeping them both makes sense. Or at least make it so that it takes an argument to be able to distinguish between the two. The reason why I think this is useful is in the container case, where say - the JDK got installed by an installer - which didn't include packaged modules, but is otherwise a full JDK (with all modules). The installer already does integrity checking of the files and it might be desirable to do multi-hop evolutions, while still wanting to get warnings/errors on modified *configuration* files, for example. Happy to change the name of that flag/flags to `--clone-run-image=ignore-multi,warn-only` or something similar.

> Adding a new jdk.tools.jlink.internal.Archive implementation where the resources come from the run-time image seems a sensible approach. I don't particularly like the name "JmodLessArchive" because the original module may not have been packaged as a JMOD file, it may have been a modular JAR. Same comment on the BOM file "jmod_resources" added to the top-level directory of each module.

OK. How about `RunimageArchive` and  `runimage_resources` or `clone_resources`?

> I think it's clever to add this to each module in the container image but part of me wonders if it should be hidden in some way to avoid tools depending on it. I don't have a concrete suggestion except to wonder if jrtfs should filter it out, same thing in the SystemModuleReader code as ModuleReader.find("jmod_resources") will find the resource, could be an attractive nuisance.

That seems fine. I can add code to filter those resources out. On the other hand, no special handling is being made for `jdk/internal/vm/options`. Perhaps that should be handled uniformly?

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

PR Comment: https://git.openjdk.org/jdk/pull/14787#issuecomment-1741195090