On Fri, 15 Dec 2023 10:02:56 GMT, Severin Gehwolf <sgehw...@openjdk.org> wrote:
>> Severin Gehwolf has updated the pull request incrementally with two
>> additional commits since the last revision:
>>
>> - Disallow packaged modules and run-time image link
>> - Only check for existing path when not a scratch task
>>
>> When using a run-time image link and the initial build was produced with
>> the --keep-packaged-modules option, we don't need to check existing
>> paths to the location where packaged modules need to be copied. This
>> breaks the --verbose output validation.
>
> Thanks. I'm not super happy with it either (yet). I'll do some pondering
> myself to try to clean this up. Basically it would be good to know if the
> functionality so far would be complete as far as you're concerned as I could
> then focus on a cleaner patch without needing to add more bits.
Thanks for the update @jerboaa.
`--unlock-run-image` is changed to be an internal option. It is ok while we
should
revisit this per the customer feedback. If not needed, we should take this out
in
another release. The help output for this option in `jlink.properties` is no
longer
needed and should be removed. Maybe this option should be
`--ignore-modified-runtime`
or something explicit.
If I read the code correctly, the image created with this option will enable
multi-hops unconditionally?
i.e. no timestamp file created and disable the check completely. I think the
.stamp file should be present in
any image created without packaged modules.
I agree that there is no difference to the plugins of SORTER, PROCESSOR,
COMPRESSOR
categories if the input to linking is packaged modules vs runtime image.
I also agree that the transformation done by filtering plugins will persist in
the new
image if linking from the runtime image. Currently, filtering plugins can be
of
FILTER and TRANSFORMER category.
For the remaining plugins (ADDER and TRANSFORMER category) such as
`--add-options`,
`--system-modules` etc, it sounds right not to persist the transformed data.
But
a few exceptions:
--vendor-bug-url
--vendor-version
--vendor-vm-bug-url
These plugins change the value of `java.vendor.*` system properties defined in
`VersionProps.class`. This behavioral difference when linking with packaged
modules and runtime image is hard to notice as those would need the internal
knowledge of these plugins (note consider the future plugins too).
I also agree with Alan that --verbose output is a bit confusing (my initial
feedback). The image
has already been created. The users can't tell which plugins are implicitly
run. To list the plugin options, it has to delete the already-created image
and re-do the jlink command with `--verbose`.
In addition, this change would consider that the default module path now
becomes:
<paths-given-by-add-modules>:$JAVA_HOME/jmods:<runtime-image>
I wonder if the warning about "$JAVA_HOME/jmods" not present is strictly needed.
I am inclined to consider that non-filtering plugins are responsible to restore
the data if transformed to the original form. It seems to be simpler model
for users to understand. They can expect that the image produced is the same
when linking with packaged modules or from the runtime image except filtered
data.
The use of `--exclude-resources` plugin to exclude transformed data to restore
the data back to original form is clever and works for plugins that add
new resources. But it does not work for plugins that modifying existing data
(`--vendor-bug-url` etc plugins). The change in `TaskHelper::getPluginsConfig`
raises the question that it isn't the right way to support this feature. I
think we need
to explore a better way to add this support. One possibility I consider is to
run
non-filtering plugins of ADDER and TRANSFORMER categories always (similar
to auto-enabled). It has to determine if it's requested by the user or
to restore the data to original form via `Plugin::configure` time.
`Plugin::transform` will handle if the data is from packaged-modules and
from runtime image which may contain the data transformed by this plugin.
I haven't explored this fully yet.
A side note: some filtering plugins are of TRANSFORMER plugin. Maybe those
plugins should be FILTER category so that plugins not of FILTER category are
non-filtering plugins and expected to restore the data to original form if
linking
from the runtime image.
-------------
PR Comment: https://git.openjdk.org/jdk/pull/14787#issuecomment-1863312923
