Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Switch to MIT license for files copied from Roslyn#13305

Merged
anmenaga merged 1 commit into
PowerShell:masterPowerShell/PowerShell:masterfrom
xtqqczze:copyright-roslynxtqqczze/PowerShell-PowerShell:copyright-roslynCopy head branch name to clipboard
Aug 5, 2020
Merged

Switch to MIT license for files copied from Roslyn#13305
anmenaga merged 1 commit into
PowerShell:masterPowerShell/PowerShell:masterfrom
xtqqczze:copyright-roslynxtqqczze/PowerShell-PowerShell:copyright-roslynCopy head branch name to clipboard

Conversation

@xtqqczze

@xtqqczze xtqqczze commented Jul 29, 2020 •
edited
Loading

Copy link
Copy Markdown
Contributor

PR Summary

Since dotnet/roslyn#37797 Roslyn has used MIT license. Presumably we can now update the copyright header of these files.

Also add a comment explaining this code is from Roslyn.

There may be additional code copied from Roslyn, I found the details on these files from #6134 which I cross-checked with https://github.com/dotnet/roslyn

PR Context

Follow-up: #6134

PR Checklist

@xtqqczze xtqqczze requested a review from daxian-dbw as a code owner July 29, 2020 20:32
@ghost ghost assigned anmenaga Jul 29, 2020
@iSazonov

iSazonov commented Jul 30, 2020

Copy link
Copy Markdown
Collaborator

@xtqqczze Please rebase to pass CIs.

@xtqqczze

xtqqczze commented Jul 30, 2020

Copy link
Copy Markdown
Contributor Author

@iSazonov @daxian-dbw These files are wrapped with #if !CORECLR, we should review the code to see whether they are is still needed for .NET 5.0.

@xtqqczze
Switch to MIT license for files copied from Roslyn
a74d902 
Since dotnet/roslyn#37797 Roslyn has used MIT license. Presumably we can now change the copright header of these files.

Follow-up: PowerShell#6134
@daxian-dbw

daxian-dbw commented Jul 31, 2020

Copy link
Copy Markdown
Member

They are for searching assemblies in GAC in the Parser, but I cannot remember exactly for what now.

@xtqqczze

xtqqczze commented Jul 31, 2020

Copy link
Copy Markdown
Contributor Author

They are for searching assemblies in GAC in the Parser, but I cannot remember exactly for what now.

They are used in these files:

PowerShell/src/System.Management.Automation/engine/parser/Compiler.cs

Line 2791 in 4b9b078

Microsoft.CodeAnalysis.GlobalAssemblyCache.ResolvePartialName(assemblyName, out assemblyFileName);

PowerShell/src/System.Management.Automation/engine/parser/Parser.cs

Line 5079 in 4b9b078

GlobalAssemblyCache.ResolvePartialName(assemblyName, out assemblyFileName);

@iSazonov

iSazonov commented Jul 31, 2020

Copy link
Copy Markdown
Collaborator

Our ALC Resolver uses TryFindInGAC() method to search in GAC.

The referenced code is for using assembly statement - I'd expect it would search in GAC too.

@vexx32

vexx32 commented Jul 31, 2020

Copy link
Copy Markdown
Collaborator

I was under the impression that the GAC functionality is effectively nonexistent for .NET Core? Has that changed again in .NET 5?

@xtqqczze

xtqqczze commented Aug 2, 2020 •
edited by unfurl-links Bot
Loading

Copy link
Copy Markdown
Contributor Author

System.Reflection.Assembly.GlobalAssemblyCache will be obsoleted in .NET 5 (https://github.com/dotnet/designs/blob/master/accepted/2020/better-obsoletion/obsoletions-in-net5.md)

GitHub
This repo is used for reviewing new .NET designs. Contribute to dotnet/designs development by creating an account on GitHub.

@iSazonov

iSazonov commented Aug 2, 2020

Copy link
Copy Markdown
Collaborator

We have GlobalAssemblyCache in GlobalAssemblyCache.cs file.

I see the .Net document mentions UTF7 - we should review this.

@anmenaga anmenaga added CL-CodeCleanup Indicates that a PR should be marked as a Code Cleanup change in the Change Log CL-Docs Indicates that a PR should be marked as a documentation change in the Change Log CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log and removed CL-CodeCleanup Indicates that a PR should be marked as a Code Cleanup change in the Change Log labels Aug 5, 2020
@anmenaga anmenaga merged commit aa43d86 into PowerShell:master Aug 5, 2020
@xtqqczze xtqqczze deleted the copyright-roslyn branch August 5, 2020 21:39
@xtqqczze

xtqqczze commented Aug 5, 2020

Copy link
Copy Markdown
Contributor Author

I see the .Net document mentions UTF7 - we should review this.

Issue opened: #13358

@iSazonov iSazonov added this to the 7.1.0-preview.6 milestone Aug 6, 2020
@xtqqczze

xtqqczze commented Aug 9, 2020 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

@iSazonov MicrosoftDocs/PowerShell-Docs#6422, MicrosoftDocs/PowerShell-Docs#6446 suggests there is no GAC in PowerShell 6 and higher, should we open an issue to review GAC functionality?

@iSazonov

iSazonov commented Aug 10, 2020

Copy link
Copy Markdown
Collaborator

@xtqqczze Feel free to open new issue.

@xtqqczze xtqqczze mentioned this pull request Aug 10, 2020
Closed
@xtqqczze

xtqqczze commented Aug 10, 2020

Copy link
Copy Markdown
Contributor Author

I don't know enough about this to open an issue, pinging @rjmholt.

@rjmholt

rjmholt commented Aug 10, 2020 •
edited
Loading

Copy link
Copy Markdown
Collaborator

I was under the impression that the GAC functionality is effectively nonexistent for .NET Core? Has that changed again in .NET 5?

This is effectively true, but I believe pwsh.exe still keeps some .NET Framework GAC fallback functionality on Windows to support particular Windows modules... @adityapatwardhan is the right person to ask here

@rjmholt rjmholt removed the CL-Docs Indicates that a PR should be marked as a documentation change in the Change Log label Aug 14, 2020
@ghost

ghost commented Aug 17, 2020

Copy link
Copy Markdown

🎉v7.1.0-preview.6 has been released which incorporates this pull request.:tada:

Handy links:

@xtqqczze xtqqczze mentioned this pull request Oct 21, 2022
Merged
22 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@daxian-dbw daxian-dbw daxian-dbw approved these changes

@iSazonov iSazonov iSazonov approved these changes

Assignees

@anmenaga anmenaga

Labels

CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log

Projects

None yet

Milestone

7.1.0-preview.6

Development

Successfully merging this pull request may close these issues.

6 participants

@xtqqczze @iSazonov @daxian-dbw @vexx32 @rjmholt @anmenaga
Morty Proxy This is a proxified and sanitized view of the page, visit original site.