DYN-7835 : adapt assembly check to ALC #15652
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
See the ticket for this pull request: https://jira.autodesk.com/browse/DYN-7835
UI Smoke TestsTest: success. 11 passed, 0 failed. |
| // Ignore some assemblies(Revit assemblies) that we know work and have changed their version number format or do not align | ||
| // with semantic versioning. | ||
| var loadedAssemblies = AppDomain.CurrentDomain.GetAssemblies() | ||
| .Where(a => !assemblyNamesToIgnore.Contains(a.GetName().Name)) | ||
| .ToList(); |
There was a problem hiding this comment.
@aparajit-pratap any idea if AppDomain.CurrentDomain.GetAssemblies() will return all assemblies from all load contexts?
There was a problem hiding this comment.
No, it's best to use AssemblyLoadContext.All.SelectMany(context => context.Assemblies); for that purpose.
In any case, we should avoid continuing to use AppDomain going forward.
| @@ -398,45 +399,55 @@ public static List<Exception> CheckAssemblyForVersionMismatches(Assembly assembl | |||
| private static List<Exception> GetVersionMismatchedReferencesInAppDomain(Assembly assembly, String[] assemblyNamesToIgnore) | |||
There was a problem hiding this comment.
@BogdanZavu sounds like it might be cleaner to get the ALC of the input argument assembly. And then only check for conflicts against all assemblies loaded in that ALC ?
@mjkkirschner would MetadataLoadCOntext help in this case ?
There was a problem hiding this comment.
I think MLC could work, but if I recall correctly, I think it may not force a load of the referenced assemblies so you may need to make that traversal yourself.
it looks like @BogdanZavu is already getting the ALC of the input assembly, so I agree it's a bit confusing why we even look in the other ALCs - is it because we're assuming the worst case, that some other referenced assembly might get loaded in the future into the current ALC?
There was a problem hiding this comment.
I think it may not force a load of the referenced assemblies so you may need to make that traversal yourself.
So I think we never want to force load the referenced assemblies.
is it because we're assuming the worst case, that some other referenced assembly might get loaded in the future into the current ALC?
That is indeed a possibility but actually we can't really tell for sure so I guess there's no point in issuing a warning.
So I think it's better to simplify and only look at the input ALC as @pinzart90 suggested.
Co-authored-by: Bogdan Zavu <[email protected]> Co-authored-by: Trygve Wastvedt <[email protected]> Co-authored-by: Trygve Wastvedt <[email protected]>
Purpose
Avoid issuing a warning when two assemblies with the same name but different versions are loaded in separate ALCs.
Declarations
Check these if you believe they are true
*.resxfilesRelease Notes
Avoid issuing a warning when two assemblies with the same name but different versions are loaded in separate assembly load contexts.
Reviewers
@twastvedt @zeusongit