-
Notifications
You must be signed in to change notification settings - Fork 4.2k
Add a source package that can be used to connect to the roslyn build server #78986
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 all commits
0b46306
f0494fa
0f82cda
3ff6ebb
0558174
14fcc76
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 |
|---|---|---|
|
|
@@ -2,6 +2,8 @@ | |
| // The .NET Foundation licenses this file to you under the MIT license. | ||
| // See the LICENSE file in the project root for more information. | ||
|
|
||
| #nullable enable | ||
|
|
||
| using System; | ||
| using System.Collections.Generic; | ||
| using System.Diagnostics; | ||
|
|
@@ -63,7 +65,8 @@ internal static BuildRequest CreateBuildRequest( | |
| string workingDirectory, | ||
| string? tempDirectory, | ||
| string? keepAlive, | ||
| string? libDirectory) | ||
| string? libDirectory, | ||
| string? compilerHash = null) | ||
| { | ||
| Debug.Assert(workingDirectory is object); | ||
|
|
||
|
|
@@ -72,7 +75,7 @@ internal static BuildRequest CreateBuildRequest( | |
| arguments, | ||
| workingDirectory: workingDirectory, | ||
| tempDirectory: tempDirectory, | ||
| compilerHash: BuildProtocolConstants.GetCommitHash() ?? "", | ||
| compilerHash: compilerHash ?? BuildProtocolConstants.GetCommitHash() ?? "", | ||
|
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. How is it intended for the SDK to get the compiler hash?
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. Similarly to how
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 take this back, the utility is included as source in different projects and it's not possible to make it grab the CommitHash attribute from one assembly which all would reference, so probably best to keep as I have it now. |
||
| requestId: requestId, | ||
| keepAlive: keepAlive, | ||
| libDirectory: libDirectory); | ||
|
|
@@ -178,12 +181,6 @@ internal static async Task<BuildResponse> RunServerBuildRequestAsync( | |
| { | ||
| Debug.Assert(pipeName is object); | ||
|
|
||
| // early check for the build hash. If we can't find it something is wrong; no point even trying to go to the server | ||
| if (string.IsNullOrWhiteSpace(BuildProtocolConstants.GetCommitHash())) | ||
| { | ||
| return new IncorrectHashBuildResponse(); | ||
| } | ||
|
|
||
| using var pipe = await tryConnectToServerAsync(pipeName, timeoutOverride, logger, tryCreateServerFunc, cancellationToken).ConfigureAwait(false); | ||
| if (pipe is null) | ||
| { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| <?xml version="1.0" encoding="utf-8"?> | ||
| <!-- 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. --> | ||
| <Project Sdk="Microsoft.NET.Sdk"> | ||
|
|
||
| <PropertyGroup> | ||
| <TargetFrameworks>$(NetRoslynSourceBuild)</TargetFrameworks> | ||
|
|
||
| <!-- NuGet --> | ||
| <IsPackable>true</IsPackable> | ||
| <IsSourcePackage>true</IsSourcePackage> | ||
| <PackageId>Microsoft.CodeAnalysis.BuildClient</PackageId> | ||
| <IncludeBuildOutput>false</IncludeBuildOutput> | ||
| <PackageDescription> | ||
| Package containing sources of Microsoft .NET Compiler Platform ("Roslyn") build client API. For internal use only. | ||
|
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. Please add a bit how we can and will break the APIs here. |
||
| Relying on APIs from this package is explicitly not supported, they can and will break on a regular basis. | ||
| </PackageDescription> | ||
| </PropertyGroup> | ||
|
|
||
| <Import Project="$(MSBuildThisFileDirectory)Microsoft.CodeAnalysis.BuildClient.targets" /> | ||
|
|
||
| </Project> | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| <Project> | ||
| <ItemGroup> | ||
| <Compile Include="$(MSBuildThisFileDirectory)..\..\Compilers\Core\Portable\CommitHashAttribute.cs" /> | ||
| <Compile Include="$(MSBuildThisFileDirectory)..\..\Compilers\Core\Portable\InternalUtilities\Debug.cs" /> | ||
| <Compile Include="$(MSBuildThisFileDirectory)..\..\Compilers\Core\Portable\InternalUtilities\PlatformInformation.cs" /> | ||
| <Compile Include="$(MSBuildThisFileDirectory)..\..\Compilers\Shared\BuildProtocol.cs" /> | ||
| <Compile Include="$(MSBuildThisFileDirectory)..\..\Compilers\Shared\BuildServerConnection.cs" /> | ||
| <Compile Include="$(MSBuildThisFileDirectory)..\..\Compilers\Shared\CompilerServerLogger.cs" /> | ||
| <Compile Include="$(MSBuildThisFileDirectory)..\..\Compilers\Shared\NamedPipeUtil.cs" /> | ||
| <Compile Include="$(MSBuildThisFileDirectory)..\..\Compilers\Shared\NativeMethods.cs" /> | ||
| <Compile Include="$(MSBuildThisFileDirectory)..\..\Compilers\Shared\RuntimeHostInfo.cs" /> | ||
|
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. It's always bugged me how this logic is spread out among so many files in different directories. Did you consider as part of this pulling it into a singe isolation directory?
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. Sounds good, I can move them all to the Shared directory. Not sure if I should move those being shared from |
||
| </ItemGroup> | ||
| </Project> | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason we didn't have effectively the following instead of manually listig the files?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that can be done here. But for example in VBCSCompilerCommandLine.projitems which also shares some of these files, it can't be done since there would be conflicts (because it doesn't currently share all these files and some of the types it gets from the referenced projects, maybe that could be avoided as well, but it seems to be a deep rabbit hole).