Test Unicode support for "SplashScreen-Image" manifest attribute and potential clean up of some c code
Hi, When I tried to improve Unicode support in JAR manifests in [1], independent of what will happen with that, I found that there are not only Manifest and Attributes classes parsing manifests but also some c code which parses "SplashScreen-Image" attribute and also used to parse some other attributes such as "Main-Class" and others. There already are tests for splash screen images but those existing ones work with the "-splash" command line option and not with the "SplashScreen-Image" manifest attribute. I found "SplashScreen-Image" manifest attribute not yet covered with a test and extended the existing UnicodeTest accordingly, see attached patch which confirmed that the "SplashScreen-Image" manifest attribute already fully supports Unicode. Support for "JRE-Version" manifest attribute and "-jre-restrict-search" and "-jre-no-restrict-search" command line attributes has already been removed earlier already and the relevant lines of code determining the main class from the manifest when launching have already been moved to or near LauncherHelper::getMainClassFromJar in earlier versions, apparently leaving them with no use any longer in java.c, java.h, manifest_info.h, and parse_manifest.c, I figure. Hence, I propose to remove those parts as in the attached patch. This leaves manifest_info.h and parse_manifest.c with "SplashScreen- Image" as the only attribute parsed there. Certainly it would be a different change but anyway it might be worth a consideration to move the code opening the splash screen image to LauncherHelper or a similar appropriate place in Java which would allow to remove quite a number of some lines of c code, provided it could be acceptable to show the splash screen image slightly later. There is no existing related bug and I didn't find a new one. It would be nice to have "SplashScreen-Image" manifest attribute covered with a test and there is some potential for cleaning up unused code which certainly is not urgent at all and I would not know how desirable this really is. Also I'm not sure whether it's better or not to add SPLASHSCREEN_IMAGE to Attributes.Name.KNOWN_NAMES. Any opinion about to how to proceed with this, if at all or would someone sponsor this patch? Regards, Philipp [1] https://mail.openjdk.java.net/pipermail/core-libs-dev/2019-December/064149.h...
Hi, Essentially the same patch as last time [1] but this time with a bug number [2].The patch does not include the updated UnicodeTest.jar which I created with ../jtreg-5.0-b01/bin/jtreg -va -nr -jdk:build/linux-x86_64-server- release/images/jdk/ -Xshare:off test/jdk/tools/launcher/UnicodeTest.javacp JTwork/scratch/UnicodeTest.jar test/jdk/tools/launcher/ Regards,Philipp [1] https://mail.openjdk.java.net/pipermail/core-libs-dev/2020-January/064190.html[2 ] https://bugs.openjdk.java.net/browse/JDK-8243454 On Sat, 2020-01-04 at 11:00 +0100, Philipp Kunz wrote:
Hi, When I tried to improve Unicode support in JAR manifests in [1], independent of what will happen with that, I found that there are not only Manifest and Attributes classes parsing manifests but also some c code which parses "SplashScreen-Image" attribute and also used to parse some other attributes such as "Main-Class" and others. There already are tests for splash screen images but those existing ones work with the "-splash" command line option and not with the "SplashScreen-Image" manifest attribute. I found "SplashScreen-Image" manifest attribute not yet covered with a test and extended the existing UnicodeTest accordingly, see attached patch which confirmed that the "SplashScreen-Image" manifest attribute already fully supports Unicode. Support for "JRE-Version" manifest attribute and "-jre-restrict- search" and "-jre-no-restrict-search" command line attributes has already been removed earlier already and the relevant lines of code determining the main class from the manifest when launching have already been moved to or near LauncherHelper::getMainClassFromJar in earlier versions, apparently leaving them with no use any longer in java.c, java.h, manifest_info.h, and parse_manifest.c, I figure. Hence, I propose to remove those parts as in the attached patch. This leaves manifest_info.h and parse_manifest.c with "SplashScreen- Image" as the only attribute parsed there. Certainly it would be a different change but anyway it might be worth a consideration to move the code opening the splash screen image to LauncherHelper or a similar appropriate place in Java which would allow to remove quite a number of some lines of c code, provided it could be acceptable to show the splash screen image slightly later. There is no existing related bug and I didn't find a new one. It would be nice to have "SplashScreen-Image" manifest attribute covered with a test and there is some potential for cleaning up unused code which certainly is not urgent at all and I would not know how desirable this really is. Also I'm not sure whether it's better or not to add SPLASHSCREEN_IMAGE to Attributes.Name.KNOWN_NAMES. Any opinion about to how to proceed with this, if at all or would someone sponsor this patch? Regards,Philipp
[1] https://mail.openjdk.java.net/pipermail/core-libs-dev/2019-December/064149.h...
Hi Philipp, I have not looked at this in detail yet, but the changes you are proposing would require a CSR and probably an update to the JAR specification I am not sure of the history as to why this was done, perhaps others might know. I think this would be good to know as part of considering this change/ Best, Lance
On May 17, 2020, at 5:53 AM, Philipp Kunz <philipp.kunz@paratix.ch> wrote:
Hi, Essentially the same patch as last time [1] but this time with a bug number [2].The patch does not include the updated UnicodeTest.jar which I created with ../jtreg-5.0-b01/bin/jtreg -va -nr -jdk:build/linux-x86_64-server- release/images/jdk/ -Xshare:off test/jdk/tools/launcher/UnicodeTest.javacp JTwork/scratch/UnicodeTest.jar test/jdk/tools/launcher/ Regards,Philipp [1] https://mail.openjdk.java.net/pipermail/core-libs-dev/2020-January/064190.html[2 ] https://bugs.openjdk.java.net/browse/JDK-8243454
On Sat, 2020-01-04 at 11:00 +0100, Philipp Kunz wrote:
Hi, When I tried to improve Unicode support in JAR manifests in [1], independent of what will happen with that, I found that there are not only Manifest and Attributes classes parsing manifests but also some c code which parses "SplashScreen-Image" attribute and also used to parse some other attributes such as "Main-Class" and others. There already are tests for splash screen images but those existing ones work with the "-splash" command line option and not with the "SplashScreen-Image" manifest attribute. I found "SplashScreen-Image" manifest attribute not yet covered with a test and extended the existing UnicodeTest accordingly, see attached patch which confirmed that the "SplashScreen-Image" manifest attribute already fully supports Unicode. Support for "JRE-Version" manifest attribute and "-jre-restrict- search" and "-jre-no-restrict-search" command line attributes has already been removed earlier already and the relevant lines of code determining the main class from the manifest when launching have already been moved to or near LauncherHelper::getMainClassFromJar in earlier versions, apparently leaving them with no use any longer in java.c, java.h, manifest_info.h, and parse_manifest.c, I figure. Hence, I propose to remove those parts as in the attached patch. This leaves manifest_info.h and parse_manifest.c with "SplashScreen- Image" as the only attribute parsed there. Certainly it would be a different change but anyway it might be worth a consideration to move the code opening the splash screen image to LauncherHelper or a similar appropriate place in Java which would allow to remove quite a number of some lines of c code, provided it could be acceptable to show the splash screen image slightly later. There is no existing related bug and I didn't find a new one. It would be nice to have "SplashScreen-Image" manifest attribute covered with a test and there is some potential for cleaning up unused code which certainly is not urgent at all and I would not know how desirable this really is. Also I'm not sure whether it's better or not to add SPLASHSCREEN_IMAGE to Attributes.Name.KNOWN_NAMES. Any opinion about to how to proceed with this, if at all or would someone sponsor this patch? Regards,Philipp
[1] https://mail.openjdk.java.net/pipermail/core-libs-dev/2019-December/064149.h...
<20200517-splashscreenimageunicodetestwithoutunicodetestjar.patch>
<http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 Lance.Andersen@oracle.com <mailto:Lance.Andersen@oracle.com>
participants (2)
-
Lance Andersen
-
Philipp Kunz