Safe Dependency Resolution For Side-By-Side Loading With Other Modules #585
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
peombwa
changed the title
Contributor
|
Can this PR lead us to a path of building and testing on linux? |
georgend
reviewed
Mar 19, 2021
| // Catalog our dependencies here to ensure we don't load anything else. | ||
| private static IReadOnlyDictionary<string, Version> Dependencies = new Dictionary<string, Version> | ||
| { | ||
| { "Microsoft.Identity.Client", new Version("4.23.0.0") }, |
Contributor
There was a problem hiding this comment.
Can the resolution logic use a file instead of being explicitly set here?
For example read the csproj to determine this list?
Member
Author
There was a problem hiding this comment.
We can't access csproj here since this is done at runtime and not build time. We need to know which assembly and version to load on our own when PowerShell imports a module thus the need for an in-memory dictionary.
Also, read and write speed is something to consider when resolving dependencies.
Member
Author
@finsharp, the scope of this PR is only limited to addressing dependency resolution at runtime. To build on Linux, we will need to overhaul our generation scripts. |
georgend
approved these changes
Mar 19, 2021
peombwa
added a commit
that referenced
this pull request
Apr 16, 2021
* Bump .NET Core version * Add client capability to MSAL. * Bump MS Graph core version. * Weekly OpenApiDocs Download * Safe Dependency Resolution For Side-By-Side Loading With Other Modules (#585) * Divide project into 2 assemblies. * Update build scripts to build, sign and pack auth module with core dll. * Updates ADO pipeline to also sign and strong name MSG.Authentication.Core.dll. * Add Pester tests for loading module side by side. * Add internals visible to test proj. * Load multiframework dependencies. * Weekly OpenApiDocs Download * Update agreementFile remove directive (#605) * Fix aggrement file directive. * Bump SDK version to 1.4.3. * Adds missing site permissions to sites module (#607) * Add sistes.permissions tag to module mappings. * Generate profile for sites. * Sort crawl logs. * Weekly OpenApiDocs Download (#616) Co-authored-by: Microsoft Graph DevX Tooling <GraphTooling@service.microsoft.com> * identity governance: reduce unneeded EM cmdlets (#577) * add suppression for autogenerated get cmdlets * remove New cmdlets * add remove set and update variants of those get cmdlets * remove search and ro cmdlets * remove cmdlets for read only links and objects * Update src/Identity.Governance/Identity.Governance/readme.md Co-authored-by: Peter Ombwa <peter.ombwa@microsoft.com> * Weekly OpenApiDocs Download (#619) Co-authored-by: Microsoft Graph DevX Tooling <GraphTooling@service.microsoft.com> * 1.5.0 Pre-Release (#617) * Bump SDK version to 1.5.0 * Handle duplicate cmdlet names across modules. * Update readme.md * Use version in manifest if module is not in repo. Co-authored-by: Microsoft Graph DevX Tooling <GraphTooling@service.microsoft.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Mark Wahl <mwahl@microsoft.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR closes #534 by implementing an assembly resolve handler that handles loading our dependencies for side-by-side loading with other modules. This change involves:
Microsoft.Graph.Authentication.CoreandMicrosoft.Graph.Authentication.Microsoft.Graph.Authentication.CoreandMicrosoft.Graph.Authenticationwill be loaded by PowerShell, and all our dependencies will be loaded byMicrosoft.Graph.Authentication.Coreinto the execution context.Microsoft.Graph.Authentication.Corespecifies and makes calls to our dependencies e.g. MSAL.Microsoft.Graph.Authenticationcontains implementations of our cmdlets and calls methods inMicrosoft.Graph.Authentication.Core..\azure-pipelines.This PR should unblock #555, #415, #334, and #517