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

ComPlus CUSTOM_ACTION_DECORATION #550

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

bevanweiss
Copy link
Contributor

@bevanweiss bevanweiss commented Jul 15, 2024

Whilst looking into the MSMQ, I noticed a similar issue for the ComPlus items.

I haven't yet added a set of tests for this, I've never use the ComPlus functionality before, so unsure how it fits together.
It would be great if someone would have an example for a very basic wxs that would test basic 'install/uninstall' of the ComPlus range.

fixes wixtoolset/issues#2916
and I think it also
fixes wixtoolset/issues#3073 since this bug mentions .NET assembly registration as the problem.

@bevanweiss bevanweiss marked this pull request as draft July 15, 2024 13:19
@bevanweiss
Copy link
Contributor Author

bevanweiss commented Jul 19, 2024

@barnson / @robmen how fixed are you fellas on this Wix prefix?
I'm testing the ComPlus... and unfortunately it's throwing an error on

WcaTableExists(L"Wix4ComPlusApplicationRoleProperty")

Any guesses what the issue might be.... if someone said "that's a silly long Windows Installer table name" you'd be spot on.
It comes in at 34 characters, Microsoft say the limit is 31...

So.. options are:

  1. Drop Wix4 prefix again (not preferred)
  2. Hack up the name with short forms e.g. Wix4ComPlusAppRoleProperty (preferred.. but would change Compiler, Decompiler, Table Definitions, Converter.. )
  3. Drop ComPlus entirely, no one has complained about it not working since v4 (not preferred at this stage.. I've sunk some time into it..)

Full List of names (diff highlighting for good vs bad)

+ Wix4ComPlusPartition (20)
+ Wix4ComPlusPartitionProperty (28)
+ Wix4ComPlusPartitionRole (24)
+ Wix4ComPlusUserInPartitionRole (30)
+ Wix4ComPlusGroupInPartitionRole (31)
+ Wix4ComPlusPartitionUser (24)
+ Wix4ComPlusApplication (22)
+ Wix4ComPlusApplicationProperty (30)
+ Wix4ComPlusApplicationRole (26)
- Wix4ComPlusApplicationRoleProperty (34)
- Wix4ComPlusUserInApplicationRole (32)
- Wix4ComPlusGroupInApplicationRole (33)
+ Wix4ComPlusAssembly (19)
+ Wix4ComPlusAssemblyDependency (29)
+ Wix4ComPlusComponent (20)
+ Wix4ComPlusComponentProperty (28)
+ Wix4ComPlusRoleForComponent (27)
+ Wix4ComPlusInterface (20)
+ Wix4ComPlusInterfaceProperty (28)
+ Wix4ComPlusRoleForInterface (27)
+ Wix4ComPlusMethod (17)
+ Wix4ComPlusMethodProperty (25)
+ Wix4ComPlusRoleForMethod (24)
+ Wix4ComPlusSubscription (23)
+ Wix4ComPlusSubscriptionProperty (31)

Current thoughts are:
Application => App
Which would deal with the current problem children.

But open to other suggestions.

EDIT: Missed one (Wix4ComPlusUserInApplicationRole) in my original list..

@robmen
Copy link
Member

robmen commented Jul 19, 2024

Hah, hah. Bit by lack of validation of Extension provided tables because we "trust" to remember not to make them too long. :)

I think 2 is the right choice. "App" for "Application" is completely reasonable. Given it's been broken since the name change, I think the prefix can stay Wix4 so only shortening the too long table names is necessary.

@bevanweiss
Copy link
Contributor Author

Hah, hah. Bit by lack of validation of Extension provided tables because we "trust" to remember not to make them too long. :)

I think 2 is the right choice. "App" for "Application" is completely reasonable. Given it's been broken since the name change, I think the prefix can stay Wix4 so only shortening the too long table names is necessary.

Any concerns with me putting in an Exception on attempting to create a TableDefinition with a name that is 'too long'?
It does look like there's already some messages in there around this, but nothing that I could see would capture the compiled-in definitions.. it seemed more intended to capture XML authored table names..

        public TableDefinition(string name, IntermediateSymbolDefinition symbolDefinition, IEnumerable<ColumnDefinition> columns, bool unreal = false, bool symbolIdIsPrimaryKey = false, Type strongRowType = null)
        {
            if (name.Length > MaxTableNameLength)
            {
                throw new ArgumentOutOfRangeException(nameof(name), $"Windows Installer table names must be <= {MaxTableNameLength} characters long, '{name}' is {name.Length} characters");
            }
            ...

@robmen
Copy link
Member

robmen commented Jul 20, 2024

Exception on attempting to create a TableDefinition with a name that is 'too long'?

Yeah, that's not the correct place. The limitation only applies to MSI. Instead, open an issue, and we can think about the most accurate place.

@bevanweiss
Copy link
Contributor Author

bevanweiss commented Jul 20, 2024

hmm, so I keep finding more things to trip over on this COM+ stuff
image

https://learn.microsoft.com/en-us/windows/win32/api/comadmin/nf-comadmin-icatalogcollection-add

When I just do a naïve search for the terms (ICatalogCollection, add, E_NOTIMPL), I get this Stackoverflow answer
https://stackoverflow.com/a/78363380

Which is essentially

#import "c:\windows\system32\com\Comadmin.dll" no_namespace

Which results in plenty of compile errors about redefined types, and doesn't quite feel right (I'd expect more a tlb reference or such).

I'm very much open to ideas...

hmmm... actually it looks like COM+ partitions may be disabled by default
https://learn.microsoft.com/en-us/windows/win32/cossdk/creating-and-configuring-com--partitions
So I might need to look into how to automatically enable them if a Partition is to be created. fun...
Ok.. i've got code that enables the PartitionsEnabled flag if a Partition is to be created, however, this only works on Server OS. Desktop OS resets the PartitionsEnabled flag to 0 whenever an attempt is made to enable it.. (and fails creating a Partition if one is attempted).

I might run through a full test of it under Windows server to make sure that it's working in general terms.
And will flag an issue for future considerations of 'nicer behaviour' under Desktop OS.

@bevanweiss
Copy link
Contributor Author

I haven't yet quite gotten a full successful run through of this. I got the Partition stuff working on a Windows Server OS. But then it got stuck on registering the .NET Assembly. It appears this is caused by .NET 4.5 (the Windows installation default) not correctly registering the RegistrationHelper COM class, installing .NET v3 was required to get RegistrationHelper working..

Table names updated for Wix4 prefix.
Custom action names similarly updated.
Table names Wix4ComPlusUserInApplicationRole,
Wix4ComPlusGroupInApplicationRole and Wix4ComPlusApplicationRoleProperty
had to be shortened to fit within MSI 31 character table name limit.
Migrated from fixed GUID for RegistrationHelper to use CLSIDFromProgID in
an attempt to fix behaviour under .NET 4+ DLLs (however issues still exist).
Added setting of Partition enable if a Partition is configured in authoring,
new Windows config has Partitions disabled by default, and they don't work
at all under Windows workstation (non-server) versions.

Added a new Runtime condition for `RequireWindowsServer` which will skip
execution of Runtime test on workstation/desktop OSes, since COM+ Partitions
only work correctly under Windows Server.

Quite a lot of basic typos fixed also.


Signed-off-by: Bevan Weiss <[email protected]>
@bevanweiss
Copy link
Contributor Author

@robmen / @barnson I've still got two issues with this PR.

  1. On my fresh VM images without .NET FW 3.5 (only .NET FW 4.8) Registration Helper doesn't appear to work via COM. Even using OLEVIEW it fails to be able to get an instance of the RegistrationHelper class. However, regsvcs does work to register the DLLs, as does manually adding them via the Component Services tool.
  2. Once .NET FW 3.5 is installed, then it all works for both the .NET FW 3.5 DLL and a Native COM DLL (I've got the TestComponent solution for these if there's a good place to put them for reference). However trying to use a .NET FW 4.5 DLL results in an error (0x80020009) during RegistrationHelper.InstallAssembly.

I suspect both of these issues were present in the pre-WiX4+ also. So I think that the current PR is worth adding as-is.
The only bit is that perhaps the .NET 4 test should be disabled (perhaps using a SkippableFact for the time being), just so it doesn't fail other tests.

Any ideas on solutions to the two issues above?
Or anything that you'd like to see different in this PR?

@barnson
Copy link
Member

barnson commented Jul 28, 2024

Can't help with COM+ itself, sorry. The tests you added are just install/uninstall---how hard would it be to build them with WiX v3.14 and do a one-time test pass? The changes look fine but again, with no COM+ knowledge, I'd rather verify that the behavior in v3.14 is the same.

@bevanweiss
Copy link
Contributor Author

bevanweiss commented Jul 28, 2024

Can't help with COM+ itself, sorry. The tests you added are just install/uninstall---how hard would it be to build them with WiX v3.14 and do a one-time test pass? The changes look fine but again, with no COM+ knowledge, I'd rather verify that the behavior in v3.14 is the same.

Here's a bunch of install/uninstall logs for each of the invocations under this PR (Wix5) version, and the Wix3 version.
The only difference is that the WiX3 version failed on the Partition install under Windows Server, whilst the version in this PR works (because it enables Partitions if any are to be installed).
I did still have an Assert in for debugging on this PR versions, so there's an entry for them in the logs for this PR

Wix3_InstallUninstallNativeWithoutPartitions_Install.txt (success)
Wix5_InstallUninstallNativeWithoutPartitions_Install.log (success)

Wix3_InstallUninstallNativeWithoutPartitions_Uninstall.txt (success)
Wix5_InstallUninstallNativeWithoutPartitions_Uninstall.log (success)

Wix3_InstallUninstallNET3WithoutPartitions_Install.txt (success)
Wix5_InstallUninstallNET3WithoutPartitions_Install.log (success)

Wix3_InstallUninstallNET3WithoutPartitions_Uninstall.txt (success)
Wix5_InstallUninstallNET3WithoutPartitions_Uninstall.log (success)

Wix3_InstallUninstallNET4WithoutPartitions_Install.txt (fail)
Wix5_InstallUninstallNET4WithoutPartitions_Install.log (fail)

Wix3_InstallUninstallWithPartitions_Install.txt (fail)
Wix5_InstallUninstallWithPartitions_Install.log (success)

Install failed under Wix3, so no uninstall possible
Wix5_InstallUninstallWithPartitions_Uninstall.log (success)

Wix3_NET4WithoutNET35_Install.txt (fail)
Wix5_NET4WithoutNET35_Install.txt (fail)

@bevanweiss bevanweiss marked this pull request as ready for review July 28, 2024 06:14
@bevanweiss
Copy link
Contributor Author

bevanweiss commented Jul 28, 2024

I think I've got an app.config file that will allow the .NET 4 runtime to work also.
When I have this saved as oleview.exe.config and in the folder next to the oleview.exe executable then I can create an instance of the RegistrationHelper. Whilst without it I cannot (unless .NET 3.5 is installed also)

<?xml version ="1.0"?>
<configuration>
    <startup useLegacyV2RuntimeActivationPolicy="true">
        <supportedRuntime version="v2.0.50727"/>
        <supportedRuntime version="v4.0"/>
    </startup>
</configuration>

Any tips for how I might be able to get this 'next to' the ComPlus CA DLL? (perhaps it needs to be associated with the launching executable even)
I tried just using Orca to patch it in (I wasn't sure on naming, so I spammed both options), but this still resulted in the same error.
image

@barnson
Copy link
Member

barnson commented Jul 28, 2024

I think that only works for .exes.

@bevanweiss
Copy link
Contributor Author

bevanweiss commented Jul 28, 2024

I think that only works for .exes.

Yeah, I've experimented with another way in the native code, it's just a bit more annoying because of the split between .NET FW <4 and .NET FW >= 4.
I think there's three basic situations:

  1. only .NET <4 available and trying to register .NET < 4 (all is well)
  2. only .NET >=4 available and trying to register .NET >= 4 (breaks due to .NET binding for RegistrationHelper)
  3. Both .NET <4 and .NET >= 4 available, and possibly registering both types (breaks on .NET >= 4 due to CLR host being <4)

I'm hoping the solution might be as simple as just creating a .NET 4 host if available, but setting the legacy binding flags for it, so that it can support .NET <4 also (which just needs to be for RegistrationHelper)
If that doesn't work for .NET <4 assemblies, then it'll need to swap and change between .NET <4 and .NET >=4 hosts depending on the (to be registered) assembly version. Not sure what the execution timing impact of that might be...

UPDATE: ok, it looks like it worked. If a .NET v4 CLR is spun up before the CoCreateInstance, then things all work to get a v4 instance of the RegistrationHelper, which is able to register both v3 AND v4 assemblies.
If a v4 CLR isn't available, then the spin up will fail, and we just fall back to the CoCreateInstance trying to default construct a CLR (which will be v1/v2/v3 if available..)

Summary: This PR will fix the naming issues from Wix4 which prevented COM+ Extension from working at all. It also fixes up the three of the issues that the Wix3 COM+ had: (fixed) Failure to create Partitions on Windows Server, (fixed) Failure to work at all if .NET v4 ONLY CLR available, (fixed) Failure to register .NET v4 assemblies.

It also lets the .NET FW 4+ assemblies actually register.

So.. that's a win.
But it's a bit ugly.


Signed-off-by: Bevan Weiss <[email protected]>
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.

ComPlusApplication doesn't work for type ".net" Net 4.0 ComPlusInstallExecute fails on installing assembly
3 participants