Re: RFR: 8319457: Update jpackage to support WiX v4 and v5 on Windows [v4]
On Wed, 5 Jun 2024 14:53:09 GMT, Alexey Semenyuk wrote: >> src/jdk.jpackage/windows/classes/jdk/jpackage/internal/resources/WinResources.properties >> line 44: >> >>> 42: resource.installdirnotemptydlg-wix-file=Not empty install directory >>> dialog WiX project file >>> 43: resource.launcher-as-service-wix-file=Service installer WiX project file >>> 44: resource.wix-src-conv=XSLT stylesheet converting WiX sources from WiX >>> v3 to WiX v4 format >> >> WiX v4 -> WiX v4/v5 > > The converter does exactly as described, it converts WiX v3 sources into WiX > v4 **format** that can be used with WiX v4 and WiX v5. So I believe the > description should remain unchanged. Makes sense, but for `error.no-wix-tools` I think we should add v5, in case if users sees this message when only v5 is installed. - PR Review Comment: https://git.openjdk.org/jdk/pull/19318#discussion_r1628308316
Re: RFR: 8319457: Update jpackage to support WiX v4 and v5 on Windows [v4]
On Wed, 5 Jun 2024 03:54:46 GMT, Alexander Matveev wrote:
>> Alexey Semenyuk has updated the pull request incrementally with two
>> additional commits since the last revision:
>>
>> - WixSourceConverter#applyTo should do what
>> OverridableResource#saveToFile() does: create parent directory and replace
>> output file if it exists.
>> - Fix issue with overwriting custom l10n file in the resource directory.
>> All WiX source files from the resource directory should be copied to
>> jpackage work directory before running wxi tools. jpackage should not change
>> files in the resource directory.
>
> src/jdk.jpackage/windows/classes/jdk/jpackage/internal/WixAppImageFragmentBuilder.java
> line 157:
>
>> 155:
>> 156: @Override
>> 157: List getLoggableWixFeatures() {
>
> Maybe I am missing something, but is it used? I only see call to base class
> `WixFragmentBuilder::getLoggableWixFeatures`.
`WixFragmentBuilder::getLoggableWixFeatures` is equivalent to:
new Function>() {
public List apply(WixFragmentBuilder obj) {
return obj.getLoggableWixFeatures();
}
}
An overridden WixAppImageFragmentBuilder#getLoggableWixFeatures() method will
be called when WixAppImageFragmentBuilder instance is given.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19318#discussion_r1628020809
Re: RFR: 8319457: Update jpackage to support WiX v4 and v5 on Windows [v4]
On Tue, 4 Jun 2024 01:20:15 GMT, Alexander Matveev wrote:
> Do we need to handle default case for unknown wix type? In same cases you
> have default -> throw new IllegalArgumentException(); and in some you do not
> have.
Good catch. I'll update the code to make `switch(wixVersion) ...` expressions
consistent as follows:
switch (wixVersion) {
case Wix3 -> {}
case WiX4 -> {}
default -> throw new IllegalArgumentException();
}
-
PR Comment: https://git.openjdk.org/jdk/pull/19318#issuecomment-2150382290
Re: RFR: 8319457: Update jpackage to support WiX v4 and v5 on Windows [v4]
On Wed, 5 Jun 2024 04:20:17 GMT, Alexander Matveev wrote: >> Alexey Semenyuk has updated the pull request incrementally with two >> additional commits since the last revision: >> >> - WixSourceConverter#applyTo should do what >> OverridableResource#saveToFile() does: create parent directory and replace >> output file if it exists. >> - Fix issue with overwriting custom l10n file in the resource directory. >> All WiX source files from the resource directory should be copied to >> jpackage work directory before running wxi tools. jpackage should not change >> files in the resource directory. > > src/jdk.jpackage/windows/classes/jdk/jpackage/internal/resources/WinResources.properties > line 44: > >> 42: resource.installdirnotemptydlg-wix-file=Not empty install directory >> dialog WiX project file >> 43: resource.launcher-as-service-wix-file=Service installer WiX project file >> 44: resource.wix-src-conv=XSLT stylesheet converting WiX sources from WiX v3 >> to WiX v4 format > > WiX v4 -> WiX v4/v5 The converter does exactly as described, it converts WiX v3 sources into WiX v4 **format** that can be used with WiX v4 and WiX v5. So I believe the description should remain unchanged. - PR Review Comment: https://git.openjdk.org/jdk/pull/19318#discussion_r1627942141
