Conversation
| throw new InvalidOperationException(string.Format("Can't import {0}, {1} is already imported as {2}, namespace support needed.", type.FullName, type.Name, otherType.FullName)); | ||
| } | ||
|
|
||
| mTypes.Add(type, mtype); |
There was a problem hiding this comment.
if I add the other type in the type map (even though they are equivalent) Dynamo engine will complain and fail to compile the graph
If I skip the other type (and only keep the first equivalent type) then we get into stackoverflows (types are expected to get stored in ther type map)
There was a problem hiding this comment.
so - does this mean that we cannot even the more general case where 2 packages depend on the same binary?
If I skip the other type (and only keep the first equivalent type) then we get into stackoverflows (types are expected to get stored in ther type map)
It seems it must be possible - consider the Revit API, many packages reference it, but none distribute it.
OR are you trying to solve a different problem with his PR?
There was a problem hiding this comment.
The problem is visible only when you embed types in the managed assembly.
"Normal" references will work just fine (like Revit API references)
| { | ||
| if (!mTypes.TryGetValue(type, out mtype)) | ||
| mtype = new CLRModuleType(type); | ||
| //Now check that a type with same name is not imported. |
There was a problem hiding this comment.
is this comment entirely accurate anymore?
| <Reference Include="System.Xml" /> | ||
| <Reference Include="$(PkgMicrosoft_Office_Interop_Excel)\lib\net20\Microsoft.Office.Interop.Excel.dll"> | ||
| <EmbedInteropTypes>True</EmbedInteropTypes> | ||
| <WrapperTool>tlbimp</WrapperTool> |
| <Reference Include="Microsoft.CSharp" /> | ||
| <Reference Include="System.Data" /> | ||
| <Reference Include="System.Xml" /> | ||
| <Reference Include="$(PkgMicrosoft_Office_Interop_Excel)\lib\net20\Microsoft.Office.Interop.Excel.dll"> |
There was a problem hiding this comment.
should this reference be wrapped in a condition for now? (I mean the one below)
There was a problem hiding this comment.
will this make it to the bin folder? I think we purposefully removed it at some point.
There was a problem hiding this comment.
totally forgot about test dlls. Will work on removing them
| <PackageReference Include="Microsoft.Office.Interop.Excel" Version="15.0.4795.1000" GeneratePathProperty="true"> | ||
| <ExcludeAssets>all</ExcludeAssets> | ||
| </PackageReference> | ||
| <Reference Include="$(PkgMicrosoft_Office_Interop_Excel)\lib\net20\Microsoft.Office.Interop.Excel.dll"> |
There was a problem hiding this comment.
so is there anything in this binary except for the office types?
There was a problem hiding this comment.
no, just office types and a wrapping class
mjkkirschner
left a comment
mjkkirschner
left a comment
There was a problem hiding this comment.
provided the tests pass, I think it seems great except my question about the test binaries making their way to the bin folder.
I also am still not sure if the normal reference case works fine - but let's table that until we get a report from customers - maybe it was fixed a while ago and I missed it.
maybe we can discuss this tomorrow, maybe I am missing something |
This reverts commit cab284e.
* update * Update CLRDLLModule.cs * update * Create EmbeddedInterop.dll * update * Update ProtoTest.csproj * update * update * Update FFITarget.csproj * update * Update ClassFunctionality.cs * update * Update DynamoCore.csproj * Revert "Update DynamoCore.csproj" This reverts commit cab284e. --------- Co-authored-by: pinzart <[email protected]>
* update * Update CLRDLLModule.cs * update * Create EmbeddedInterop.dll * update * Update ProtoTest.csproj * update * update * Update FFITarget.csproj * update * Update ClassFunctionality.cs * update * Update DynamoCore.csproj * Revert "Update DynamoCore.csproj" This reverts commit cab284e. --------- Co-authored-by: pinzart <[email protected]>
2 participants
Changes:
Interop types equivalence is checked when importing types from assemblies.
Embedded interop types will appear as distinct when you check type.Equals(other).
Dotnet provides a new API to check equivalence = IsEquivalentTo
https://learn.microsoft.com/en-us/dotnet/framework/interop/type-equivalence-and-embedded-interop-types
Potential issues:
Even though the main part of dynamo that deals with import of types is now updated, there is a risk that other parts of dynamo still have code that compare type.Equals(other). This sort of code will not work as expected for embedded interop types