RegAsm completely replaced by ServerRegistrationManager

[falahati]

Regarding #211,

These changes will update and replace every usage of RegAsm with ServerRegistrationManager.

Also extends the ServerRegistrationManager class with a piece of code (refactored from ServerRegistrationApi.cs) for server extraction from an assembly and some other minor changes and cleanups regarding this issue.

Couldn't find any unit test and didn't bother to test sample servers as nothing really changed about them; however, registration and un-registration processes both with ServerRegistrationManager.exe and ServerManager.exe works in my environment.

In the case of ServerRegistrationManager.exe, it is still possible to force RegAsm by a new parameter named -regasm, in case someone wants to use it for some reason. However, the default behaviour is to use ServerRegistrationManager class.

[codecov]

Codecov Report

Merging #259 into master will decrease coverage by 0.1%.
The diff coverage is 0%.

@@            Coverage Diff             @@
##           master     #259      +/-   ##
==========================================
- Coverage   12.54%   12.43%   -0.11%     
==========================================
  Files          86       86              
  Lines        3021     3047      +26     
==========================================
  Hits          379      379              
- Misses       2642     2668      +26

Impacted Files	Coverage Δ
SharpShell/SharpShell/SharpShellServer.cs	0% <0%> (ø)	⬆️
...ll/ServerRegistration/ServerRegistrationManager.cs	7.53% <0%> (-1.45%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7fdfa87...cd54c64. Read the comment docs.

[Countryen]

Great job for sharing and improving on the issue :) I hope this will improve other registration issues as well. But I think it'd be wise to split this up as the changes have side effects (not only changes to replace RegAsm) as I see it.

Also this thing here is bugging me, maybe you can explain?

public abstract class SharpShellServer : ISharpShellServer
    {
        ...
        [ComRegisterFunction]
        internal static void __Register__(Type type)
        {
           ...
            //  Register the type, use the operating system architecture to determine
            //  what registration type to perform.
            ServerRegistrationManager.__UnregisterServerType__(
                type,
                Environment.Is64BitOperatingSystem ? RegistrationType.OS64Bit : RegistrationType.OS32Bit
            );
        }
        ...
        [ComUnregisterFunction]
        internal static void __Unregister__(Type type)
        {
            ...
            //  Unregister the type, use the operating system architecture to determine
            //  what registration type to unregister.
            ServerRegistrationManager.__RegisterSeerverType__(
                type,
                Environment.Is64BitOperatingSystem ? RegistrationType.OS64Bit : RegistrationType.OS32Bit
            );
        }

Shouldn't this be reversed?
So that Register() calls RegisterServerType instead of UnregisterServerType?
So that UnRegister() calls UnRegisterServerType instead of RegisterServerType?
Confusing if not :)

The MEF approach for finding the Servers isn't that suitable for ever, though. If you use MEF you can only load servers from the same version of SharpShell used, as discussed in issue #217.

Will test the changes at the weekend and report back if all my extensions (registration/unreg.) work as well.

[falahati]

Sharp eyes @Countryen,
Just added a new commit to fix that.

But I think it'd be wise to split this up as the changes have side effects (not only changes to replace RegAsm) as I see it.

Well, the changes are all about the registration process with ServerManager.exe and ServerRegistrationManager.exe. Not much else is changed except maybe some minor tweaks to disable or enable a menu item and such; which was too annoying for me to ignore.

The MEF approach for finding the Servers isn't that suitable forever, though. If you use MEF you can only load servers from the same version of SharpShell used, as discussed in issue #217.

This isn't much of a problem with ServerRegistrationManager.exe as the SS version published with the tool is probably the same as the one the developer used in his/her extension. But then we probably should load the SharpShell library from the library's folder instead of publishing one with ServerRegistrationManager.exe. Especially because the NuGet packages are different and it is possible in theory for someone to use two different versions of NuGet packages.

However, I see your point regarding ServerManager.exe especially now that I moved the code for finding classes in an assembly to the main library and others might use it inside their code.

The problem, however, is not just the use of MEF, we can simply go for Reflection with Assembly.ReflectionOnlyLoad or something; but I think we are going to still have some problems with it.

ServerManager.exe should be able to load the extensions from registry instead of loading them from assembly files. This is a much bigger problem for now. Solving this, we can uninstall and unregister all version of SharpShell extensions and even manage other extensions.

And for installation, maybe we should try to do what I suggested with ServerRegistrationManager.exe above with loading the right version of SharpShell published with the library we are trying to load. This, however, requires us to move the code regarding registration to a separate library so we can have access to these methods without actually loading the SharpShell library for ourselves. This is a big change and might break some older implementations, but I think a necessary one that can be published as part of major version 3.x.x not necessarily promising backwards compatibility for the custom registration codes.

All that being said, the bigger problem, in my opinion, is the clarity of the code. It is hard to follow what happens. Namings are sometimes kind of hard to understand. In one class I saw three different namings for one thing, DisplayName, className and serverName; another example being classId and ServerCLSID which combined with the fuzzy definition of a Server in the same class made it hard to know just from the code if they are exactly one thing or not. The code should also be refactored in a lot of places. We should also enforce some styling rules. Sometimes, things as simple as having a bracket after one line if conditions when used differently each time, can make it hard to read the code. I know for sure my ReSharper had a bad time xD

[Countryen]

@falahati Thanks for changing that re/unregister thing around :)

I now agree with you. Still need to test it, though.

All that being said, the bigger problem, in my opinion, is the clarity of the code. It is hard to follow what happens. Namings are sometimes kind of hard to understand. In one class I saw three different namings for one thing, DisplayName, className and serverName; another example being classId and ServerCLSID which combined with the fuzzy definition of a Server in the same class made it hard to know just from the code if they are exactly one thing or not.

I think that has to do with the difference between WindowsAPI and .NET / SharpShell / COM.
Imho, different classes should name "things" according to their own purpose, not the overall purpose
So, for instance, the serverName can become DisplayName when the Server's name is used as a DisplayName of a Control. Also, the Server's name can be the className of the Server, if the server is about to be registered for classes.

It is hard to follow "along" but not if you only look at one part of the code (e.G. implement just a single interface). You don't need to know, that the DisplayName is actually the class name and the serverName most of the time, just use it as smth. to display.

The code tends to use most expressions from msdn/windows source code in deeper classes and changes that for .NET developers for better understanding in higher classes, as I see it. And in WindowsAPI (and especially the shell) most things have multiple names all the time.

I don't think it's necessary to change that at most places just to be equal.

[falahati]

I agree with you on the points you mentioned regarding namings.

However, the problem is not with an entity having different names throughout the library, but rather throughout a class. In my case, this being the ServerRegistrationManager class.

I think we both can agree that in one class, we should'nt be able to see these and if necessary, the name changes should happen somewhere that makes it easy to understand. For example, an internal private method handling the logic and one or more public methods to point to right variables based on their respected arguments, being a server or an already registered extension or similar. I had this exact problem when I tried to make it possible to unregister a registered server without the file hosting the server actually being accessible.

[Countryen]

Had time to test it now.
Everything compiles fine and the registration doesn't throw errors.
All tests run sucessfully.

But I get bad results from testing the Sample servers.
I can not really say that it isn't my machine, though - as I have no "clean" env. to test on.
I know that the examples from SharpShell@master worked - but that was a few months ago so...

Maybe someone else got a clean environment to test the Samples?
At least the official sample server tests should be done :)

[falahati]

I am just curious about the type of errors you got. Maybe I can double check it.

Now, to be honest, I tested the changes only with one my own servers (ContextMenu) but my environment wasn't especially clean either; same [but older versions of the] server was already installed and uninstalled multiple times. I just trusted that the ServerRegistrationManager class is functional at this point.

I have a clean Win7 virtual machine tho; so I might be able to check it there. But knowing the exact problem and exact server to test would help a lot for sure.

[Countryen]

I am just curious about the type of errors you got. Maybe I can double check it.

Did the "Full Regression Test.txt" thing, tested the following sample servers (with SRM.exe):

• FileTimesPropertySheet -> worked well
• IconPreviewHandler -> no icons shown in preview
• ReadOnlyFileIconOverlayHandler -> The icon was just the same default txt-icon with a black frame, but the differentiation between readonly and readwrite did work.
  
• TxtThumbnailHandler -> no different thumbnail (but again the black frame as above)

As it's mostly just related to the icon / thumbnail I am pretty sure it's just my pc but, please test it with the vm :)

[falahati]

I have tested almost all of them (Except "AbcPreviewHandler.dll" and "WebSearchDeskBand.dll" for obvious reasons) using the new method and the old one (with and without "-regasm" argument) on a clean installation of Win7x64. Here are the results:

General Problems:

• [CODEBASE]: None of the sample servers works without the "codebase" argument. Doesn't matter if RegAsm or ServerRegistrationManager is used. Maybe a problem with dependency to the "SharpShell.dll"?
• CopyPathDataHandler.dll: breaks everything. Can't even double-click on a file, nor can open the properties dialogue of any and all files (even executables). This happens with both RegAsm and ServerRegistrationManager with "codebase" argument and without. So there must be something wrong with the handler itself or the sharpshell bridge.
• EnviromentVariablesNamespaceExtension.dll: throws a Could not load type 'System.Runtime.CompilerServices.IteratorStateMachineAttribute' from assembly 'mscorlib, Version=4.0.0.0' error with RegAsm and ServerRegistrationManager ;also again with and without the "codebase" argument.
• RegistryNamespaceExtension.dll: same as above.
• GithubNamespaceExtension.dll: Didn't try, but supposedly same as above.

ServerRegistrationManager specific problems:

• IconPreviewHandler.dll: When registered with ServerRegistrationManager (with and without "codebase" argument) it fails with a "This file can't be previewed because of an error in the Icon Preview Handler" message inside the preview panel. With RegAsm, it works if "codebase" argument is present; otherwise you get the same message. Therefore it seems that the registration process is ok, but there is a problem with one of the dll's dependencies as this is one of the few handlers with a third party dependency.

• TxtThumbnailHandler.dll: Only works with RegAsm. With ServerRegistrationManager, unlike IconPreviewHandler.dll, there is no indication that this server is even installed and registered properly. Could be a problem with the registration code of "Thumbnail Handler"s. Should investigate the difference between RegAsm and ServerRegistrationManager class regarding "Thumbnail Handler" registration procedure.

---

I think; at this point, considering the fact that RegAsm still seems more stable than the ServerRegistrationManager class, I should change the proposed code to use RegAsm as default and add an argument to use ServerRegistrationManager in case someone had to use it (reverse of what happens now). For example, even tho RegAsm works well inside the Virtual Machine with Win7, in my own environment (Win10) it fails and I have to use ServerRegistrationManager. It might be due to the fact that I have tons of .Net versions installed.

We can also use both RegAsm and ServerRegistrationManager back to back and this should also decrease the failure rate. But this needs further testings.

[falahati]

BTW, didn't test with "srm.exe" as it isn't part of the official release as of version 2.2 as far as I know. Is it? Seems to be only available from the "SharpShellTools" NuGet package.

[Countryen]

I think; at this point, considering the fact that RegAsm still seems more stable than the ServerRegistrationManager class, I should change the proposed code to use RegAsm as default and add an argument to use ServerRegistrationManager in case someone had to use it (reverse of what happens now).

Yes, I agree, that'd be the best to do.

BTW, didn't test with "srm.exe" as it isn't part of the official release as of version 2.2 as far as I know. Is it? Seems to be only available from the "SharpShellTools" NuGet package.

I just used the built project's "Tools/ServerRegistrationManager" executable, renamed to SRM for shorter scripts. ServerRegistrationManger.exe is the successor of srm.exe (I think).

[falahati]

Was checking the ServerRegistrationManager code and it seems we need to add the assembly to GAC before trying to register without "codebase" argument. So the part about the "codebase" argument in my last post is irrelevant. I thought ServerRegistrationManager already do that as part of registration.

[falahati]

Default installation provider changed back to "RegAsm".

@dwmkerr, Hey Dave, any idea on these changes?

[dwmkerr]

Hi @falahati and everyone, thanks so much for the hard work on this! I like the approach, the reliance on regasm has been bugging me for ages, particular as directly manipulating the registry should be a perfectly valid solution to the problem.

It'll probably be next week before I can look at this in detail and continue to test in more scenarios, looking over all the comments so far, I hope that's OK!

[dwmkerr]

As a quick reminder to myself:

https://docs.microsoft.com/en-us/dotnet/api/system.runtime.interopservices.registrationservices.registerassembly?view=netframework-4.0

RegisterAssembly provides the same functionality as regasm (including the option to specify /codebase). However, it makes an assumption that the registration type is the same bit-ness as the calling process. This is probably fine for 99% of the cases but I'm not sure if it would be for all of them.

But frankly, manually creating the keys as done in this PR is pretty easy to do, and also would allow us to test more effectively by using some kind of mocked registry object.

I'm going to merge this into a develop branch shortly and experiment, running the manual test scripts before and after the change...

[falahati]

@dwmkerr, I suggest taking a look at #260 too since it can install and uninstall different versions and different bitness of extensions without using any third-party tool. In fact, it can even give you information about native extensions and uninstall them.

#260 is essentially the next version of this PR.

[dwmkerr]

Closing as #323 contains the latest version of this code.