wixtoolset / issues

WiX Toolset Issues Tracker
http://wixtoolset.org/
129 stars 36 forks source link

ExtensionValidation: Early check on valid Table Name #8645

Open bevanweiss opened 1 month ago

bevanweiss commented 1 month ago

WiX Version

5.0.1

.NET or MSBuild or Visual Studio Version

.NET 8.0.300, Visual Studio Version 17.11.0 Preview 2.0

HeatWave Version

1.0.4.5

Windows Version

Win11 23H2

Repro Repo

No response

Repro Steps

As a WiX developer, I'd like to be made aware of any violation of the Windows Installer limitations when creating WiX tooling systems (such as Core / Extensions) as early as possible, but certainly prior to end user runtime.

Current COM+ has three Custom Table Definitions which exceed the 31 character limit of the Windows Installer, and are not accessible at runtime.

Actual Result

WiX tooling was built and had no warnings / errors related to this invalid table name. However at runtime (after user authoring, and build) the extension failed to successfully read the Custom Tables referenced.

Expected Result

Validation on aspects of the WiX extensions etc that are known at tooling build / test time. Primary driver for this ticket is Custom Table Name Length, which is limited under Windows Installer 'rules' as <= 31 characters long https://learn.microsoft.com/en-us/windows/win32/msi/table-names

https://learn.microsoft.com/en-us/windows/win32/msi/naming-custom-tables-properties-and-actions

I believe this could be an opinionated validation, i.e. whilst table / custom action names starting with 'msi' are allowed by the Windows Installer, WiX should not by default allow them to be used in a new Custom Table definition (since it may result in support issues in the future), similarly with '.' characters in table names etc.

Acknowledgements

barnson commented 3 weeks ago

@bevanweiss - do you want this issue assigned to you, since you're working on the PR?

bevanweiss commented 3 weeks ago

@bevanweiss - do you want this issue assigned to you, since you're working on the PR?

Yep, that sounds ideal. This PR is 'ready for review' also.