Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rename firewall extension table name to Wix5FirewallException #450

Closed

Conversation

chrisbednarski
Copy link
Contributor

merge after #449

@chrisbednarski
Copy link
Contributor Author

I see that I haven't taken CUSTOM_ACTION_DECORATION into account, so custom actions are still defined as V4 and not merging as required.

Any guidance on how to deal with this just for the Firewall extension?
Or should it be changed for all of V5 ?

@chrisbednarski
Copy link
Contributor Author

chrisbednarski commented Sep 1, 2023

I have Wix4 and Wix5 firewall extension tables, but only a single set of custom actions (after suppressing WIX1055 and WIX1056) - missing Wix5 CAs

image

@barnson
Copy link
Member

barnson commented Sep 1, 2023

Because the firewall CAs suppress modularization GUIDs, they'll definitely need to be renamed. (Annoyingly, the CA DLL does not have its modularization GUID suppressed. But it's been that way for about ever.) I lean toward changing the prefix only to match a table name change -- it's seems almost cruel to mix Wix4 table names with Wix5 custom action ids. I'll add this question to the issue for triage.

@chrisbednarski
Copy link
Contributor Author

chrisbednarski commented Sep 1, 2023

I've updated the PR so all three names are changed:

  • table name
  • CA name
  • CA binary/DLL

image

The CAs of the v5 module (that matches the version of the MSI) is stomped over (expected)
This was done by suppressing warnings 1055 and 1056
image

Wix5 exception records are being merged as expected
image

Firewall records from the 4.0.1 module are safely in their own table
image

@chrisbednarski
Copy link
Contributor Author

Thanks for your reply. I'm sure this triage discussion will be interesting.

@chrisbednarski
Copy link
Contributor Author

I think I missed this one

Will this have to be changed to 5.0.0?

@barnson
Copy link
Member

barnson commented Sep 11, 2023

I don't think so: #455

But let's see what @robmen says.

@chrisbednarski
Copy link
Contributor Author

Test project files are included in #431, but tests run using binaries.
So this PR is redundant I think.

@github-actions github-actions bot locked and limited conversation to collaborators Oct 22, 2023
@chrisbednarski chrisbednarski deleted the feat/change-table-name branch November 19, 2023 21:21
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants