dotnet / project-system

The .NET Project System for Visual Studio
MIT License
969 stars 387 forks source link

Third-party Windows Forms controls that use LicenseManager crash the designer with E_NOTIMPL #4938

Closed jnm2 closed 5 years ago

jnm2 commented 5 years ago

This is the last thing I know of that blocks SDK-style csproj from being used for .NET Framework Windows Forms projects in Visual Studio 2019 Update 1.

My only reluctance to file this is that I passionately dislike the LICX paradigm. The constantly-generated LICX cache file causes CI build failures and failures between developer machines due to outdated strongly-named versions of assemblies never being removed from the file when newer versions are referenced. This has been so much pain over the years that we banned it from source control. Any other way to license at all, or no licensing, would be extremely welcome for other reasons besides enabling the simplified csproj format.

Therefore, either of these two outcomes is ideal for me:

These outcomes would be sad:

See also: https://www.devexpress.com/Support/Center/Question/Details/T740667/designer-fails-to-load

Steps to Reproduce:

Create a control with the following code:

[LicenseProvider(typeof(LicFileLicenseProvider))]
public class SomeThirdPartyControl : Control
{
    private readonly License license;

    public SomeThirdPartyControl()
    {
        license = LicenseManager.Validate(typeof(SomeThirdPartyControl), this);
    }

    protected override void Dispose(bool disposing)
    {
        if (disposing) license?.Dispose();
        base.Dispose(disposing);
    }
}

Build the project and then add a new form, open its designer, and add this control using the toolbox.

Expected Behavior: The designer should open.

Actual Behavior:

Designer crashes with E_NOTIMPL.

at System.Runtime.InteropServices.Marshal.ThrowExceptionForHRInternal(Int32 errorCode, IntPtr errorInfo)
at Microsoft.VisualStudio.Design.Serialization.LicenseService.GetLicenseDocData(FileAccess access)
at Microsoft.VisualStudio.Design.Serialization.LicenseService.FillLicensedTypes()
at Microsoft.VisualStudio.Design.Serialization.LicenseService.get_LicensedTypes()
at Microsoft.VisualStudio.Design.Serialization.LicenseService.OnComponentAdded(Object sender, ComponentEventArgs e)
at System.ComponentModel.Design.ComponentEventHandler.Invoke(Object sender, ComponentEventArgs e)
at System.ComponentModel.Design.DesignerHost.AddToContainerPostProcess(IComponent component, String name, IContainer containerToAddTo)
at System.ComponentModel.Design.DesignerHost.PerformAdd(IComponent component, String name)
at System.ComponentModel.Design.DesignerHost.System.ComponentModel.Design.IDesignerHost.CreateComponent(Type componentType, String name)
at System.ComponentModel.Design.Serialization.DesignerSerializationManager.CreateInstance(Type type, ICollection arguments, String name, Boolean addToContainer)
at System.ComponentModel.Design.Serialization.DesignerSerializationManager.System.ComponentModel.Design.Serialization.IDesignerSerializationManager.CreateInstance(Type type, ICollection arguments, String name, Boolean addToContainer)
at System.ComponentModel.Design.Serialization.TypeCodeDomSerializer.Deserialize(IDesignerSerializationManager manager, CodeTypeDeclaration declaration)
at DevExpress.Utils.Serializers.ComponentCodeDomeSerializer.Deserialize(IDesignerSerializationManager manager, CodeTypeDeclaration codeTypeDeclaration)
at System.ComponentModel.Design.Serialization.CodeDomDesignerLoader.PerformLoad(IDesignerSerializationManager manager)
at Microsoft.VisualStudio.Design.Serialization.CodeDom.VSCodeDomDesignerLoader.PerformLoad(IDesignerSerializationManager serializationManager)
at Microsoft.VisualStudio.Design.Serialization.CodeDom.VSCodeDomDesignerLoader.DeferredLoadHandler.Microsoft.VisualStudio.TextManager.Interop.IVsTextBufferDataEvents.OnLoadCompleted(Int32 fReload)

User Impact: This is the last thing I know of that blocks SDK-style csproj from being used for .NET Framework Windows Forms projects as of Visual Studio 2019 Update 1.

davkean commented 5 years ago

@dmonroym Can you take a look at this and follow up with the state of this when compared to legacy? We should figure out run time support too for .NET Core with WinForms.

At a minimum, we should behave like legacy - we have to regardless of whether we improve the scenario for compat reasons as we bring up the old format.

chucker commented 5 years ago

Since the issue calls out DevExpress several times, I thought I would point out that they're not the only vendor still using this mechanism. I ran into this due to Infragistics, not DevExpress.

The new project system implements what LicenseManager needs exactly the way the old project system does. We're back to the LICX baggage.

That would be sad. Can it be implemented but with an information (or even warning) in Error List that marks the mechanism as deprecated or at least discouraged?

In that case, is there actual guidance for the aforementioned vendors on what system they should move to?

chucker commented 5 years ago

Also, out of curiosity:

These outcomes would be sad:

Neither Microsoft or DevExpress changes anything. It is never possible to use the SDK-style csproj for .NET Framework Windows Forms applications.

Does this "only" block the designer, or does it also break shipping builds (i.e. are they never treated as properly licensed on customer machines)?

MattJeanes commented 5 years ago

Also running into this issue with Telerik components

jnm2 commented 5 years ago

So far as I can tell, the embedded LICX is entirely optional. Our CI server builds without it since we ban it from source control. I don't think DevExpress has a licensing mechanism at runtime on our customer's machines. It's just when the designer opens. I could be wrong.

davkean commented 5 years ago

This is occuring because WinForms is asking for PSFFILEID_Licenses via IVsProjectSpecialFiles but we are failing to return it. Based on my walk through the legacy code, it's almost identical to AppConfigFileSpecialFileProvider, except:

1) It's created under AppDesigner folder. 2) There's no template for it if it doesn't exist, a zero'd out file is written on disk. Base doesn't support this, so we'll need to introduce this concept./ 3) The file is called "licenses.licx"

It's uncleared to me how the .licenses file gets generated and that needs comparison/investigation when compared to legacy.

davidwengier commented 5 years ago

This might be nothing to do with what you're talking about, but I used to have to deal with these files in a past life. The .licenses file is generated my lc.exe, called at build time here: https://github.com/microsoft/msbuild/blob/master/src/Tasks/Microsoft.Common.CurrentVersion.targets#L3167. It looks at any .licx file that is an embedded resource.

davkean commented 5 years ago

Yep that's exactly what I'm after. @nguerrera is license support on the radar for the SDK? The license files ".licx" probably needs to be picked up by the Desktop SDK.

nguerrera commented 5 years ago

We do not have anything officially on the radar yet, but I think there's basically two issues on the build side:

  1. Should the licx be globbed by default? If so, I think it should go in WindowsDesktop SDK and a tracking issue on dotnet/wpf should be filed? Did classic projects have an explicit licx embedded resource item?

  2. Is dotnet build expected to work? This will require porting the code for lc.exe to core and bundling it somewhere in the sdk. I'm not sure where to track that as I don't know who owns that code. Depending on the complexity of the code, folding it into a build task in the windows desktop sdk might be the simplest deployment strategy.

My expectation is that neither of these are blocking other progress on the scenario as testing can be done against the other parts using VS / desktop msbuild + explicit item.

weltkante commented 5 years ago

Did classic projects have an explicit licx embedded resource item?

You don't embed the licx, its just a text file listing controls requiring licensing. The .licx files get compiled into a .license resource by executing control-specific code for each control listed in the licx files (this is done via LC.exe) - the generated .license data is similar to the .resource data generated from resx files and gets embedded into the assembly, it is a container for binary blobs which carry control specific data. At runtime the controls will look for the .license resource to verify the data which was stored at build-time.

The problem licx files try to solve is to provide a list of controls actually used in the project, so you can run a build task on them to generate and store licensing data. How this data is used is up to the control vendor, he might show popups, throw exceptions, terminate the process, lazily create a trial license and fail months later. I have seen every single one of those options.

Since we now got Roslyn it may be technically feasible to to inspect the source code for references to control constructors for types carrying the licensing attributes, that would solve the problem of the user manually having to maintain licx files to determine which controls are used. This would make licx files unnecessary and be entirely backwards compatible by still generating .license resources

nguerrera commented 5 years ago

I got that. Above it was shown that the compilation from licx -> licenses is triggered by an EmbeddedResource item for the licx. This is like resx: you don't embed the resx, you compile to resources and embed that.

But in the classic csproj, you do still see <EmbeddedResource Include="blah.resx">. So my question was “does <EmbeddedResource Include=“blah.licx”> appear in classic csproj ?” and it is still relevant.

nguerrera commented 5 years ago

https://github.com/microsoft/msbuild/blob/93fdeb63001a40c5694611051b05f8972cb7e823/src/Tasks/Microsoft.Common.CurrentVersion.targets#L2913

weltkante commented 5 years ago

Yes they do appear as <EmbeddedResource Include="Properties\licenses.licx" /> in classic projects, and when you drag new licx files into the project they are also added as such, so you probably should treat them the same as you do with resx.

jnm2 commented 5 years ago

Including the LICX in the project leads to stale versions of GAC references which fails the build, so often that we keep the LICX out of source control. All our builds happen with no LICX even though the designer will start one any time we open it. If there is any opportunity along the way to put the LICX cache in the .vs or obj folder, please do that.

nguerrera commented 5 years ago

I'm unclear on why that is ok. Do some things not use the license at runtime while other things do? It feels like we have to support what exists, especially for .net framework, and separately we can discuss and develop alternatives.

jnm2 commented 5 years ago

I'm also unclear on this. That approach makes sense. Do you know anyone who would have a clear idea?

weltkante commented 5 years ago

It appears you are not clear what licx does, I tried to explain above (please reread I'm not gonna repeat everything but try to expand on it). The license resource is a storage for control vendors to store binary evidence that the build machine had a license for this control. It does have nothing to do with design time licensing, it is about runtime licensing. What to do when runtime licensing is not present is up to the vendor. Furthermore, the WinForms designer does know nothing about how controls implement licensing and will add any control which has a license provider attribute to the licx list, so it can generate runtime licenses on builds.

Missing licx entries will not fail builds, they'll be detected when the controls constructor runs. Not every control vendor choses to do something. Many control vendors check additional sources besides the license resource, so if you test on a development machine it might consider the control licensed even without a license resource entry.

I explained above that licx files are just a way of listing controls potentially requiring licensing and with Roslyn it would be possible to detect that in a build step and generate more accurate licx files automatically.

If you have concrete questions feel free to ask, I've researched the whole process extensively over several years to customize our own build pipeline without breaking control licensing (we use a whole bunch of 3rd party controls all requiring licensing)

davidwengier commented 5 years ago

All our builds happen with no LICX even though the designer will start one any time we open it.

Since the .licx file is compiled into the .licenses file, the above works if you have the .licenses file pre-compiled and available to CI, or in source control, or just don't need it. If its just available for CI, you would/could get license issues on developer machines, if it's checked in to source control then you wouldn't see issues as long as you are embedded that file correctly. At some point there has to be some machine or person that updates the .licenses file if anything changes presumably.

There are a few reasons why someone might take this approach IMO.

Given the above motivations (and feel free to enlighten me @weltkante if I've missed something) I don't think its a scenario we should really worry about, and I think a default glob for <EmbeddedResource Include="licenses.licx" /> makes sense.

Stale references in the licx could be an issue (its just a list of full assembly names really) though it presumably could be a feature request on the designer to update the file, if indeed that doesn't work already.

weltkante commented 5 years ago

though it presumably could be a feature request on the designer to update the file, if indeed that doesn't work already

The designer can't do it perfectly, it really has only a view on a single form at a time, so it doesn't know if a control is still requiring a license as far as the whole project is concerned. Thats why it only ever adds controls to the file.

What could (and probably should) happen is validating the licx file and removing any line for which you don't have a matching assembly reference, since obviously without reference the control cannot be in use in this assembly.

davkean commented 5 years ago

I think we should split this bug into two; the one here will represent enabling the existing behavior as is to expendiate compat and unblock folks, one on http://github.com/dotnet/winforms for future design changes. Can someone with alittle more knowledge on how this works file a bug over on winforms?

davidwengier commented 5 years ago

so it doesn't know if a control is still requiring a license as far as the whole project is concerned

You're right, I was only thinking of the case where an assembly name would need to be updated. Removing unused assemblies is definitely a different case, and not something that anyone is well set up for at the moment I don't think, though we are talking about it in general for another scenario, so its a timely reminder, thank you.

Can someone with alittle more knowledge on how this works file a bug over on winforms?

@jnm2 I think your original post at the start of this issue gives a pretty good overview of what you don't like about LICX, perhaps you wouldn't mind creating it as an issue in https://github.com/dotnet/winforms as a feature request for a new licensing mechanism, based on the assumption that the bug (ie, the error when opening the designer) will be fixed. Unfortunately that means we're at your sad outcome of "The new project system implements what LicenseManager needs exactly the way the old project system does." but any changes to the system really should be driven from the winforms side of things.

weltkante commented 5 years ago

@davidwengier updating is and will always be a problem since you invalidate the licx file without running the designer for each form again. You can only solve this by making the licx build stage look at the whole project and determine which control constructors are called. You could generate the list of controls on the fly instead of relying on an explicit listing of controls.

I agree that the whole process should be split into different issues

davkean commented 5 years ago

The VS support for the existing feature has been added in 16.4, SDK support still needs to be added to make sure that it ends up in the right item type: https://github.com/dotnet/sdk/issues/3631.

weltkante commented 5 years ago

I'll create a follow-up issue for future improvements once it's clear who will own LC.exe (or whatever the equivalent will be for Core), so the ideas/suggestions here won't be dropped.

ilbrando commented 5 years ago

I hope someone can help me with this. I'm waiting for this fix to be included in Visual Studio, but I don't know how to find out which version of the product-system is used by VS. So how do I know when this is included in a Visual Studio release or pre-release?

jnm2 commented 5 years ago

@ilbrando The milestone in the sidebar on this page is 16.4, so Visual Studio version 16 (2019) Update 4.

Update 3 is releasing today for https://www.dotnetconf.net/. The preview of Update 4 would typically be available around the same time. That may have this change already, or maybe a following preview of Update 4.

The time between update releases seems to average around two months, so my completely unofficial guess is that Update 4 will release at the end of November. That's the date I'm waiting for.

drewnoakes commented 5 years ago

This was merged very close to our internal cut off for 16.4 preview 1. I suspect it'll be included in that first preview, though if not it'll definitely be in 16.4 preview 2.

jnm2 commented 5 years ago

I think it'll be Preview 2 then. 16.4 Preview 1 repros the same E_NOTIMPL at LicenseService.GetLicenseDocData.

ilbrando commented 5 years ago

I have just tried it in 16.4 preview 1 and it works like a charm :-)

ilbrando commented 5 years ago

Sorry I was in the wrong branch of my code. It doesn't work in 16.4 preview 1.

dmonroym commented 5 years ago

Yes, this didn't make the cut for preview 1 so it'll be in preview 2

jnm2 commented 5 years ago

Just tried preview 2 and designers are now loading! After poking around a bit, it looks like there won't be anything keeping our Windows Forms projects back from the new-style csproj. I'm really happy about this! Thank you!

weltkante commented 5 years ago

FYI at anyone still watching this issue, seems LC.exe will not be ported and there will be something new instead. You probably should follow dotnet/winforms#1462 and dotnet/sdk#3631 if you care about licensing, I assume someone will post there with more information.

jnm2 commented 4 years ago

Visual Studio 16.4 is out now!