RFR: Move jlink plugin resources into their own files (was: Re: Help with build changes for: 8214796: Create a jlink plugin for stripping debug info symbols from native libraries)

[Severin Gehwolf]

Hi Mandy, Alan,

On Mon, 2019-01-28 at 13:02 -0800, Mandy Chung wrote:
> 
> On 1/28/19 1:26 AM, Severin Gehwolf wrote:
> > Hi Alan, Mandy,
> > 
> > On Sun, 2019-01-27 at 08:14 +0000, Alan Bateman wrote:
> > > On 26/01/2019 00:06, Mandy Chung wrote:
> > > > Hi Severin,
> > > > 
> > > > Another alternative would be to support per-jlink-plugin resource
> > > > bundle to avoid merging .properties files at build time.  The
> > > > plugin-specific messages should only be used by the plugin itself
> > > > and it would be cleaner for each plugin to manage its resource bundle.
> > > 
> > > That seems a cleaner idea than the suggestion we had on jigsaw-dev to 
> > > merge the properties. If Severin does this for this plugin then I assume 
> > > the resources for the other plugins could be moved to their own resource 
> > > files in their own time.
> > 
> > Alright. I'll try this then.
>  
> One simplest way is to have StripNativeDebugSymbolsPlugin loads its own ResourceBundle.
> 
> I would add a new PluginsResourceBundle::getMessage method taking a ResourceBundle object
> that StripNativeDebugSymbolsPlugin can call.
> 
> This is a patch for PluginsResourceBundle that you can use.
> 
> diff --git a/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/PluginsResourceBundle.java b/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/PluginsResourceBundle.java
> --- a/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/PluginsResourceBundle.java
> +++ b/src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/PluginsResourceBundle.java
> @@ -62,7 +62,11 @@
>      }
>  
>      public static String getMessage(String key, Object... args) throws MissingResourceException {
> -        String val = pluginsBundle.getString(key);
> +        return getMessage(pluginsBundle, key, args);
> +    }
> +
> +    public static String getMessage(ResourceBundle rb, String key, Object... args) throws MissingResourceException {
> +        String val = rb.getString(key);
>          return MessageFormat.format(val, args);
>      }
>  }

Right, thanks! In fact, I've come up with something similar. Here is a
patch which moves resource files for jlink plugins into their own
properties files:

http://cr.openjdk.java.net/~sgehwolf/webrevs/jlink-plugins-resources/01/webrev/

It's not super clean as there is still this weird plugin.opt.<name>
artifact which has to do with TaskHelper and it's use of PluginOption,
but I wanted to keep the patch as minimal as possible.

Following this model, having platform specific plugins should be
easier. Would this be something acceptable upstream? If so, I'll file
an enhancement bug and an official RFR.

I've tested this with tools/jlink/ and jdk/modules tests.

Then the patch for the native debug strip plugin would become this:
http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8214796-wip/02/webrev/index.html

Is this going into the right direction?

Thanks,
Severin