-
Notifications
You must be signed in to change notification settings - Fork 179
Support for source only dev/ci builds #400
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
Support for source only dev/ci builds #400
Conversation
b427802
to
8401a4f
Compare
bb9a5a2
to
8a37d68
Compare
Suggestions:
Dev builds will look like: - ./build.sh --clean-while-building -sb --os linux --arch riscv64 -p:DisableDevBuildAsDefaultForSourceOnly=true
+ ./build.sh --clean-while-building -sb --os linux --arch riscv64 |
For official builds, we will either not require a build ID as input at all or address the UX with dotnet/source-build#5109. |
bc58e04
to
1de58cb
Compare
1de58cb
to
37f23f5
Compare
37f23f5
to
1911617
Compare
@@ -8,7 +8,7 @@ | |||
Restore would conclude that there is a cyclic dependency between Microsoft.CodeAnalysis and Microsoft.CodeAnalysis.Analyzers. | |||
--> | |||
<PackageId>*$(MSBuildProjectFile)*</PackageId> | |||
<MicrosoftCodeAnalysisVersion Condition="'$(MicrosoftCodeAnalysisVersion)' == ''">$(MicrosoftCodeAnalysisVersionForCodeAnalysisAnalyzers)</MicrosoftCodeAnalysisVersion> | |||
<MicrosoftCodeAnalysisVersion>$(MicrosoftCodeAnalysisVersionForCodeAnalysisAnalyzers)</MicrosoftCodeAnalysisVersion> |
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.
:)
|
||
<PackageReference Include="System.Composition" /> | ||
<ItemGroup Condition="'$(DotNetBuildSourceOnly)' == 'true' and '$(OfficialBuild)' == 'false'"> |
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.
Have the roslyn repo-SB legs been removed? If not, has this been tested in that context?
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.
Testing these changes here: dotnet/roslyn#78608
analyzer assembly version is not the same as the compiler assembly version being used by the sdk that the project | ||
is being built with. But this is fine because the analyzer is just being used here to distribute it with the tool. | ||
--> | ||
<NoWarn Condition="'$(DotNetBuildSourceOnly)' == 'true' and '$(OfficialBuild)' != 'true'">$(NoWarn);CS9057</NoWarn> |
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.
Really appreciate the in-depth comments here.
Is that a fundamental problem that needs attention regardless of the changes in this PR?
Why don't we want it to be lifted and prefer SBRP? Because of the above semver issue or because of a design decision?
Is that a sustainable pattern and if not, do we have ideas how to counter future issues around that? |
a source generator which must be built targeting a version of Roslyn contained in the executing SDK. Otherwise, because of how | ||
Arcade works for non-official builds, M.CA will get an assembly version of 42.42.42.42 and the source generator would target that | ||
version leading to an error like the following: | ||
CSC error CS9057: Analyzer assembly '/repos/dotnet/src/aspnetcore/artifacts/bin/Microsoft.AspNetCore.Http.RequestDelegateGenerator/Release/netstandard2.0/Microsoft.AspNetCore.Http.RequestDelegateGenerator.dll' cannot be used because it references version '42.42.42.42' of the compiler, which is newer than the currently running version '5.0.0.0'. |
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.
CSC error CS9057: Analyzer assembly '/repos/dotnet/src/aspnetcore/artifacts/bin/Microsoft.AspNetCore.Http.RequestDelegateGenerator/Release/netstandard2.0/Microsoft.AspNetCore.Http.RequestDelegateGenerator.dll' cannot be used because it references version '42.42.42.42' of the compiler, which is newer than the currently running version '5.0.0.0'. | |
CSC error CS9057: Analyzer assembly '/repos/dotnet/src/aspnetcore/artifacts/bin/Microsoft.AspNetCore.Http.RequestDelegateGenerator/Release/netstandard2.0/Microsoft.AspNetCore.Http.RequestDelegateGenerator.dll' | |
cannot be used because it references version '42.42.42.42' of the compiler, which is newer than the currently running version '5.0.0.0'. |
Hey folks, I'm going to get this prioritized to be merged and will address feedback in a follow-up. Dev builds need to be enabled to unblock things (see dotnet/source-build#5135 (comment)). |
Getting a prebuilt for Microsoft.CodeAnalysis.Common.5.0.0-1.25230.108 coming from aspnetcore. This is due to now being dependent on the PSB and the PSB has some incorrect dependencies. The PSB contains Microsoft.CodeAnalysis.AnalyzerUtilities.5.0.0-1.25265.106 which has a dependency on Microsoft.CodeAnalysis.Common.5.0.0-1.25230.108. That Microsoft.CodeAnalysis.Common version isn't right, it should have the same version as AnalyzerUtilities. |
It's because Microsoft.CodeAnalysis.AnalyzerUtilities has a package reference to Microsoft.CodeAnalysis.Common instead of a project reference: Line 21 in c7f1d4f
This causes it to have a dependency on the PSB version of Microsoft.CodeAnalysis.Common (5.0.0-1.25230.108) instead of the live version. |
This seems to be temporarily resolved by using the output of the stage 2 build as rebootstrap source: #598. This makes sense because the issue is that it's using the wrong version from the PSB artifacts. But in stage 2 the correct version is being output from stage 1 which is then selected as the PSB artifact in the stage 2 build. |
Merging this now. I'll address the comments in a follow-up. |
Fixes dotnet/source-build#5061
Adds support for source-only dev/ci (non-official) builds which allows source-only jobs to run in PR validation within the UB pipeline.
The changes can be classified in the following groups:
CS9057
in the changed files for a description of why that was needed.