RFR: 8287971: Throw exception for missing values in .jpackage.xml [v2]

[Alexander Matveev]

On Tue, 14 Jun 2022 18:07:12 GMT, Alexey Semenyuk <asemenyuk at openjdk.org> wrote:

>> Alexander Matveev has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   8287971: Throw exception for missing values in .jpackage.xml [v2]
>
> src/jdk.jpackage/share/classes/jdk/jpackage/internal/AppImageFile.java line 68:
> 
>> 66:     private final List<LauncherInfo> addLauncherInfos;
>> 67:     private final String signedStr;
>> 68:     private final String appStoreStr;
> 
> What is the idea of this change?

To store string values, so it can be validated to make sure it is true or false only. I changed it back, since I moved validation to ctor.

> src/jdk.jpackage/share/classes/jdk/jpackage/internal/AppImageFile.java line 339:
> 
>> 337:                 !("true".equals(appStoreStr) || "false".equals(appStoreStr))) {
>> 338:             return false;
>> 339:         }
> 
> It makes sense to unfold this function in the ctor as we don't allow empty AppImageFile instances.

Done.

> src/jdk.jpackage/share/classes/jdk/jpackage/internal/resources/MainResources_de.properties line 79:
> 
>> 77: warning.no.jdk.modules.found=Warnung: Keine JDK-Module gefunden
>> 78: 
>> 79: error.foreign-app-image=Error: app-image dir ({0}) not generated by jpackage. Missing .jpackage.xml.
> 
> This error message will be misleading in case app image was generated by jpackage and adjusted after.
> I'd put it as "Missing .jpackage.xml file in app-image dir ({0})."

Done.

> test/jdk/tools/jpackage/helpers/jdk/jpackage/test/TKit.java line 272:
> 
>> 270:             ));
>> 271:     }
>> 272: 
> 
> 1. This is jpackage specific, so it should belong to JPackageCommand class.
> 2. Please use proper XML API to create XML output.
> 3. Having `<app-version>1.0</app-version>` in .jpackage.xml file will make `AppImageFile.isValid()` return `false` and `AppImageFile.load()` to throw exception. This seems to be equivalent to not having .jpackage.xml file in app image at all raising a question if this new function is needed at all.

1 and 2 done. I think you mistaken `<app-version>1.0</app-version>` with `<jpackage-state version="%s"`. We never read <app-version> and I do put correct value for `<jpackage-state version="%s"`. It is required for several tests which uses empty/dummy app images and since we started required .jpackage.xml file we need it for empty app images as well.

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

PR: https://git.openjdk.org/jdk19/pull/9