Repository navigation
Migrate Csc to multithreaded MSBuild execution #84701
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.
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
Changes from 29 commits
0bae975
c5509d6
f39df64
b64e6b3
3ad846e
0255db0
fb98f60
ec9fe01
c175fa7
b941367
a15813d
fab0b1c
c533da2
f3288fb
bd3f2d0
1a92c26
9448129
e47dd8c
da0fc32
80d8ad3
5258959
e2033b5
d282f14
fa0cf21
3ba8a2f
7b28515
dab569d
2f7cafc
5fac7bd
52b773a
305eb0c
d29f6d5
58b9fc3
6e0c557
f8ddfd6
fd30eaf
775c916
259bfda
6e50163
ce86426
627bbdc
3b1378e
9feef99
cb14d46
77108f1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
|
|
@@ -53,6 +53,20 @@ | |||
| "problemMatcher": "$msCompile", | ||||
| "group": "build" | ||||
| }, | ||||
| { | ||||
| "label": "build Compilers.slnf with analyzers", | ||||
| "command": "dotnet", | ||||
| "type": "shell", | ||||
| "args": [ | ||||
| "build", | ||||
| "-p:RunAnalyzersDuringBuild=true", | ||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we set
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was just following the other patterns in this file. Got to be honest ... I'm surprised that these are separate settings. I would assume that enabling analyzers turned on code style enforcement. Is that not the case?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure why is that but I think they are separate. Our CI checks set both: Line 585 in 9c32699
|
||||
| "-p:GenerateFullPaths=true", | ||||
| "-tl:off", | ||||
| "Compilers.slnf" | ||||
| ], | ||||
| "problemMatcher": "$msCompile", | ||||
| "group": "build" | ||||
| }, | ||||
| { | ||||
| "label": "build Roslyn.slnx", | ||||
| "command": "dotnet", | ||||
|
|
||||
|
jaredpar marked this conversation as resolved.
Resolved
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| P:System.Environment.CurrentDirectory | ||
| M:System.Environment.GetEnvironmentVariable(System.String); | ||
| M:System.Environment.GetEnvironmentVariables(); | ||
| M:System.Environment.GetCommandLineArgs(); | ||
| M:System.Environment.GetFolderPath(); | ||
| M:System.Environment.GetLogicalDrives(); | ||
| M:System.Environment.SetEnvironmentVariable(); | ||
| M:System.IO.Directory.GetCurrentDirectory(); | ||
| M:System.IO.Path.GetTempPath(); | ||
| M:System.IO.Path.GetFullPath(System.String); | ||
| M:System.IO.Path.GetFullPath(System.String, System.String); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,91 @@ | ||
| // Licensed to the .NET Foundation under one or more agreements. | ||
| // The .NET Foundation licenses this file to you under the MIT license. | ||
| // See the LICENSE file in the project root for more information. | ||
|
|
||
| using System; | ||
| using System.Collections.Generic; | ||
| using System.IO; | ||
| using System.Runtime.InteropServices; | ||
| using Microsoft.Build.Framework; | ||
| using Microsoft.CodeAnalysis.BuildTasks; | ||
| using Roslyn.Utilities; | ||
| namespace Microsoft.CodeAnalysis.CommandLine; | ||
|
|
||
| internal static class Extensions | ||
| { | ||
| extension(TaskEnvironment taskEnvironment) | ||
| { | ||
| internal string GetFullPath(string path) | ||
| { | ||
| var fullPath = taskEnvironment.GetAbsolutePath(path).Value; | ||
| return fullPath; | ||
| } | ||
|
|
||
| internal string GetFullPathNoThrow(string path) | ||
| { | ||
| try | ||
| { | ||
| path = taskEnvironment.GetFullPath(path); | ||
| } | ||
| catch (Exception e) when (Utilities.IsIoRelatedException(e)) { } | ||
| return path; | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Gets the value of the temporary path for the provided environment settings. This behavior | ||
| /// is OS specific. | ||
| /// - On Windows it seeks to emulate Path.GetTempPath as closely as possible with | ||
| /// provided working directory. | ||
| /// </summary> | ||
| internal string? GetTempPath() | ||
| { | ||
| return RuntimeInformation.IsOSPlatform(OSPlatform.Windows) | ||
| ? getTempPathWindows() | ||
| : getTempPathLinux(); | ||
|
jaredpar marked this conversation as resolved.
Resolved
|
||
|
|
||
| string? getTempPathLinux() | ||
| { | ||
| // Unix temp path is fine: it does not use the working directory | ||
| // (it uses ${TMPDIR} if set, otherwise, it returns /tmp) | ||
| var tempPath = taskEnvironment.GetEnvironmentVariable("TMPDIR"); | ||
| return !string.IsNullOrEmpty(tempPath) ? tempPath : "/tmp"; | ||
| } | ||
|
|
||
| string? getTempPathWindows() | ||
| { | ||
| var tmp = taskEnvironment.GetEnvironmentVariable("TMP"); | ||
| if (Path.IsPathRooted(tmp)) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Path like
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good catch ... have a few places I need to fix this. I think that I need to keep the @baronfel FYI ... more weirdness on replacing
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Consider adding a test for this case. |
||
| { | ||
| return tmp; | ||
| } | ||
|
|
||
| var temp = taskEnvironment.GetEnvironmentVariable("TEMP"); | ||
| if (Path.IsPathRooted(temp)) | ||
| { | ||
| return temp; | ||
| } | ||
|
|
||
| if (!string.IsNullOrEmpty(taskEnvironment.ProjectDirectory)) | ||
| { | ||
| if (!string.IsNullOrEmpty(tmp)) | ||
| { | ||
| return Path.Combine(taskEnvironment.ProjectDirectory, tmp); | ||
| } | ||
|
|
||
| if (!string.IsNullOrEmpty(temp)) | ||
| { | ||
| return Path.Combine(taskEnvironment.ProjectDirectory, temp); | ||
| } | ||
| } | ||
|
|
||
| var userProfile = taskEnvironment.GetEnvironmentVariable("USERPROFILE"); | ||
| if (Path.IsPathRooted(userProfile)) | ||
| { | ||
| return userProfile; | ||
| } | ||
|
|
||
| return taskEnvironment.GetEnvironmentVariable("SYSTEMROOT"); | ||
| } | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -75,7 +75,24 @@ private static class Names | |
| [return: NotNullIfNotNull(nameof(path))] | ||
| private static string? NormalizePath(string? path) | ||
| { | ||
| return string.IsNullOrEmpty(path) ? path : EnsureEndsWithSlash(Utilities.GetFullPathNoThrow(path)); | ||
| // Delete this and call TaskEnvironment.GetFullPathNoThrow. | ||
| return string.IsNullOrEmpty(path) ? path : EnsureEndsWithSlash(GetFullPathNoThrow(path)); | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @OvesN when I merge this PR it will create merge conflicts in yours. But your PR has access to |
||
| } | ||
|
|
||
| /// <summary> | ||
| /// This will be removed in PR https://github.com/dotnet/roslyn/pull/84421 once these changes are | ||
| /// merged into the main branch. The TaskEnvironment.GetFullPathNoThrow method should be used instead. | ||
| /// </summary> | ||
| internal static string GetFullPathNoThrow(string path) | ||
| { | ||
| try | ||
| { | ||
| #pragma warning disable RS0030 // Do not used banned APIs | ||
| path = Path.GetFullPath(path); | ||
| #pragma warning restore RS0030 // Do not used banned APIs | ||
| } | ||
| catch (Exception e) when (Utilities.IsIoRelatedException(e)) { } | ||
| return path; | ||
| } | ||
|
|
||
| private static string EnsureEndsWithSlash(string path) | ||
|
|
@@ -186,7 +203,7 @@ void setTopLevelMappedPaths(bool sourceControlled) | |
| // Normalize nested root. | ||
| if (Utilities.TryCombine(containingRoot, nestedRoot, out var combinedPath)) | ||
| { | ||
| var fullOriginalPath = Utilities.GetFullPathNoThrow(combinedPath); | ||
| var fullOriginalPath = GetFullPathNoThrow(combinedPath); | ||
| if (fullOriginalPath.StartsWith(containingRoot, StringComparison.OrdinalIgnoreCase)) | ||
| { | ||
| nestedRoot = fullOriginalPath.Substring(containingRoot.Length); | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.