Skip to content

Switch to MIT license for files copied from Roslyn #13305

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
anmenaga merged 1 commit into from
Aug 5, 2020
Merged

Switch to MIT license for files copied from Roslyn #13305

anmenaga merged 1 commit into from
Aug 5, 2020

Conversation

@xtqqczze
Copy link
Contributor

@xtqqczze xtqqczze commented Jul 29, 2020 •
edited
Loading

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
Copy link
Collaborator

iSazonov commented Jul 30, 2020

@xtqqczze Please rebase to pass CIs.

@xtqqczze
Copy link
Contributor Author

xtqqczze commented Jul 30, 2020

@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
Copy link
Member

daxian-dbw commented Jul 31, 2020

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

@xtqqczze
Copy link
Contributor Author

xtqqczze commented Jul 31, 2020

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
Copy link
Collaborator

iSazonov commented Jul 31, 2020

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
Copy link
Collaborator

vexx32 commented Jul 31, 2020

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

@xtqqczze
Copy link
Contributor Author

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

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
Copy link
Collaborator

iSazonov commented Aug 2, 2020

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
Copy link
Contributor Author

xtqqczze commented Aug 5, 2020

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
Copy link
Contributor Author

xtqqczze commented Aug 9, 2020 •
edited
Loading

@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
Copy link
Collaborator

iSazonov commented Aug 10, 2020

@xtqqczze Feel free to open new issue.

@xtqqczze xtqqczze mentioned this pull request Aug 10, 2020
Closed
@xtqqczze
Copy link
Contributor Author

xtqqczze commented Aug 10, 2020

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

@rjmholt
Copy link
Collaborator

rjmholt commented Aug 10, 2020 •
edited
Loading

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
Copy link

ghost commented Aug 17, 2020

🎉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