Re: RFR: 8319457: Update jpackage to support WiX v4 and v5 on Windows
On Tue, 4 Jun 2024 12:37:03 GMT, Alexey Semenyuk wrote: > > I assume WiX5 will just work if installed instead of WiX4? > > Correct. > > > Do you think it will make sense to introduce Wix5 ToolsetType in case if we > > need to do something different between 4 and 5? > > They claim WiX5 is backward compatible with WiX4, i.e. WiX4 code should work > with WiX5 toolkit. So I don't see a reason to introduce Wix5 ToolsetType now. jpackage allows override of main WiX source file (main.wxs), do you know what will happen if user will add main.wxs with format features available only in WiX 5 and will have WiX 5 toolkit installed? Do you know if there any benefits to use any features available in WiX5 if WiX5 toolkit is installed, instead of using only WiX4 features with WiX5 toolkit? - PR Comment: https://git.openjdk.org/jdk/pull/19318#issuecomment-2150809380
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
