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

EnsureTable() should bring in relevant Standard Actions which rely on the table declared #555

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

bevanweiss
Copy link
Contributor

@bevanweiss bevanweiss commented Aug 2, 2024

The RemoveFoldersEx custom action relies upon pushing rows to the standard MSI table, and then expected the RemoveFiles standard MSI action to pick up the rows and do the removal actions. However without install files, we don't add in RemoveFiles scheduling.

This PR adds extra functionality after symbols have been resolved, where it will add in Standard Actions if they have not already been included elsewhere in the authoring. The EnsureTable symbols are checked, and the table name used as a lookup to what Standard Actions it should directly bring in.

fixes wixtoolset/issues#8199
fixes wixtoolset/issues#8632 (in the as mentioned Standard Actions case.. not for generic custom actions)

@bevanweiss bevanweiss marked this pull request as draft August 4, 2024 21:23
@bevanweiss bevanweiss force-pushed the ensurestandardaction branch 2 times, most recently from 3dc78b6 to f594bcf Compare August 5, 2024 13:48
@bevanweiss
Copy link
Contributor Author

@barnson is this direction more of what you were looking for? (obviously incomplete with table->standard action maps)

I'm still less a fan, some of them like the AppId have quite a few associated Standard Actions. It feels 'safer' to me if each one has to be explicitly brought in by any Extension that requires them (with an EnsureStandardAction symbol).

@barnson
Copy link
Member

barnson commented Aug 5, 2024

I'm still not sure what the advantage of extensions manually specifying standard actions would be.

@bevanweiss
Copy link
Contributor Author

I'm still not sure what the advantage of extensions manually specifying standard actions would be.

My revised version (PR title / leader post not yet changed) just looks at EnsureTable symbols and maps them across to Standard Actions (based on Windows online doco).
Is there an easier 'definitive' reference for the table->action(s) mapping?
Did you want it going beyond EnsureTable symbols to all tables with references in the section?

My concern is some of the tables, like AppSearch etc etc bring in a number of Standard Actions which have various slightly weird sequencing sometimes.
So if an extension has EnsureTable("AppSearch"), is it then reasonable to bring in both InstallUISequence/AppSearch and InstallExecuteSequence/AppSearch?
Or, does it make more sense that the Extension itself should know, and should specifically bring in one or the other?

@barnson
Copy link
Member

barnson commented Aug 6, 2024

My concern is some of the tables, like AppSearch etc etc bring in a number of Standard Actions which have various slightly weird sequencing sometimes.

Right---and that's why I don't want extensions to be messing around at that level. MSI is usually the one enforcing the weird sequencing requirements, so I want the core WiX tools to be keeper of that knowledge as much as possible. Ideally, it wouldn't be possible to create an MSI that violates sequencing rules.

in EnsureTables symbols (from Extensions).

This should handle dependencies from Extensions back to Standard Actions
that are coupled to Standard Tables.
@bevanweiss bevanweiss force-pushed the ensurestandardaction branch from f594bcf to 45bd53d Compare August 7, 2024 21:23
@bevanweiss bevanweiss changed the title Add EnsureStandardAction() behaviour to mimic EnsureTable() for extensions that need StandardActions to exist also.. EnsureTable() should bring in relevant Standard Actions which rely on the table declared Aug 7, 2024
@bevanweiss bevanweiss marked this pull request as ready for review August 7, 2024 21:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Schedule actions based on symbols and tables Regression of RemoveFolderEx in WiX v4
2 participants