FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Added logic in AssemblyLoadContext to load assembly from GAC. by adityapatwardhan · Pull Request #3981 · PowerShell/PowerShell · GitHub

/ PowerShell Public

Added logic in AssemblyLoadContext to load assembly from GAC. - #3981

Merged
Dongbo Wang (daxian-dbw) merged 4 commits into
PowerShell:masterfrom
adityapatwardhan:FullClrAssemblyLoad
Jun 15, 2017
ConversationCommits (4)ChecksFiles changed
Merged

Added logic in AssemblyLoadContext to load assembly from GAC.#3981
Dongbo Wang (daxian-dbw) merged 4 commits into
PowerShell:masterfrom
adityapatwardhan:FullClrAssemblyLoad

Conversation

Aditya Patwardhan (adityapatwardhan) commented Jun 9, 2017 •
edited
Loading

Copy link
Copy Markdown
Member

Fixes #2592

When Resolve method is not able to find the assembly, we try to look it up from GAC.
If found, the assembly is loaded and cached.
There is a TestHook to disable GAC loading, but it is enabled by default.

Aditya Patwardhan (adityapatwardhan) commented Jun 9, 2017

Copy link
Copy Markdown
Member Author

Rebasing did not get rid of the changes in test unrelated to this commit.

Steve Lee (SteveL-MSFT) commented Jun 9, 2017

Copy link
Copy Markdown
Member

Aditya Patwardhan (@adityapatwardhan) if you click the TestCases tab, the test failure is your new test loading PSScheduledJob module. The other error output isn't fatal and fixed by #3968

Steve Lee (SteveL-MSFT) requested changes Jun 10, 2017
View reviewed changes
Comment thread src/Microsoft.PowerShell.CoreCLR.AssemblyLoadContext/CoreCLR/CorePsAssemblyLoadContext.cs Outdated

Steve Lee (SteveL-MSFT) Jun 10, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

convention is to use leading underscore for private fields

Comment thread src/Microsoft.PowerShell.CoreCLR.AssemblyLoadContext/CoreCLR/CorePsAssemblyLoadContext.cs Outdated

Steve Lee (SteveL-MSFT) Jun 10, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Wouldn't it be better to #ifdef !UNIX line 265 (calling this method) above as well as this whole method?

Comment thread src/Microsoft.PowerShell.CoreCLR.AssemblyLoadContext/CoreCLR/CorePsAssemblyLoadContext.cs Outdated

Steve Lee (SteveL-MSFT) Jun 10, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I think this is a typo: chosenVersionDirectory

Aditya Patwardhan (adityapatwardhan) commented Jun 10, 2017

Copy link
Copy Markdown
Member Author

Steve Lee (@SteveL-MSFT) Fixed all CR comments.

Steve Lee (SteveL-MSFT) requested changes Jun 10, 2017
View reviewed changes
Comment thread src/Microsoft.PowerShell.CoreCLR.AssemblyLoadContext/CoreCLR/CorePsAssemblyLoadContext.cs Outdated

Steve Lee (SteveL-MSFT) Jun 10, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

You still have to return null if UNIX right?

Added logic in AssemblyLoadContext to load assembly from GAC.
528d55c 
When Resolve method is not able to find the assembly, we try to look it up from GAC.
If found, the assembly is loaded and cached.
There is a TestHook to disable GAC loading, but it is enabled by default.
Resolved CR comments and fix test
a3c0fff

Aditya Patwardhan (adityapatwardhan) commented Jun 14, 2017

Copy link
Copy Markdown
Member Author

Having a look at the test failure.

Aditya Patwardhan (adityapatwardhan) commented Jun 14, 2017

Copy link
Copy Markdown
Member Author

Steve Lee (@SteveL-MSFT) All comments / issues fixed. Please have a look.

Steve Lee (SteveL-MSFT) approved these changes Jun 15, 2017
View reviewed changes

Steve Lee (SteveL-MSFT) left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM

Aditya Patwardhan (adityapatwardhan) commented Jun 15, 2017

Copy link
Copy Markdown
Member Author

Dongbo Wang (@daxian-dbw) Can you also have a look. Thanks!

Dongbo Wang (daxian-dbw) commented Jun 15, 2017

Copy link
Copy Markdown
Member

Looking now.

Dongbo Wang (daxian-dbw) merged commit 26a44ab into PowerShell:master Jun 15, 2017
Aditya Patwardhan (adityapatwardhan) deleted the FullClrAssemblyLoad branch June 15, 2017 20:28
Ilya (iSazonov) reviewed Jun 16, 2017
View reviewed changes
Comment thread src/Microsoft.PowerShell.CoreCLR.AssemblyLoadContext/CoreCLR/CorePsAssemblyLoadContext.cs
/// <summary>
/// Test hooks for PowershellAssemblyLoadContext
/// </summary>
public static class PowerShellAssemblyLoadContextTestHooks

Ilya (iSazonov) Jun 16, 2017

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

We have public static class InternalTestHooks for test hooks.

Aditya Patwardhan (adityapatwardhan) Jun 16, 2017

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Ilya (@iSazonov) That is in a different assembly, so could not re-use it.

40a mentioned this pull request Jun 27, 2017
Open
Thatgfsj (Thatgfsj) pushed a commit to Thatgfsj/PowerShell that referenced this pull request Aug 6, 2026
Added logic in AssemblyLoadContext to load assembly from GAC. (PowerS…
efe09a8 
…hell#3981)
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
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Ilya (iSazonov) iSazonov left review comments
Dongbo Wang (daxian-dbw) daxian-dbw approved these changes
Steve Lee (SteveL-MSFT) SteveL-MSFT approved these changes
Jason Shirk (lzybkr) Awaiting requested review from lzybkr

Assignees

Dongbo Wang (daxian-dbw)

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

Back | FazBrowse Home | New Git URL