Upstream changes from Vertical Build PoC #71666

mmitche · 2024-01-17T00:03:10Z

This largely enables roslyn to build in the VMR, without much of the source build exclusions. Some current exclusions may also be removed at some point.

Backport IsTestUtilityProject arcade features into roslyn's targets. Can be removed when roslyn is on Arcade 9
Sprinkle on test utility projects, removing IsShipping where specified.
Exclude projects from the vertical build where they were excluded for the PoC work.
Tweak eng/build.ps1 to use common tooling to initialize the CLI rather than force installing it under -ci.
Remove extra / to roslyn solution in source-build.props

This largely enables roslyn to build in the VMR, without much of the source build exclusions. Some current exclusions may also be removed at some point. - Backport IsTestUtilityProject arcade features into roslyn's targets. Can be removed when roslyn is on Arcade 9 - Sprinkle on test utility projects, removing IsShipping where specified. - Exclude projects from the vertical build where they were excluded for the PoC work. - Tweak eng/build.ps1 to use common tooling to initialize the CLI rather than force installing it under -ci. - Remove extra / to roslyn solution in source-build.props

mmitche · 2024-01-17T00:06:52Z

@ViktorHofer

eng/targets/Imports.BeforeArcade.targets

…reaming

eng/targets/Imports.BeforeArcade.targets

… vertical-build-upstreaming

mmitche · 2024-01-23T21:27:23Z

azure-pipelines.yml

        displayName: Validate Generated Syntax Files
+        inputs:
+          filePath: eng/validate-code-formatting.ps1
+          arguments: -rootDirectory $(Build.SourcesDirectory)\src -includeDirectories Compilers\CSharp\Portable\Generated\, Compilers\VisualBasic\Portable\Generated\, ExpressionEvaluator\VisualBasic\Source\ResultProvider\Generated\


@jaredpar I tweaked this so it doesn't assume that the SDK is installed at the root of the repo (instead, uses Ensure-DotNetSDK). I set the validation tooling to diag for this PR iteration to ensure that things are operating as expected.

I'm pretty sure that the current format command is incorrect. It appears that the include directories are expected to be paths relative to the root dir ($(Build.SourcesDirectory)\src) and not absolute paths. When I was testing it locally it was not formatting any files.

Looking good now. 11 generated files from those directories checked. I changed the verbosity back down to detailed. It prints some details (number of files) that are useful.

…reaming

mmitche · 2024-01-24T15:11:31Z

Ready to go. Waiting on @dotnet/roslyn-infrastructure to verify.

jaredpar · 2024-01-24T17:00:54Z

azure-pipelines.yml

@@ -403,8 +403,11 @@ stages:
          arguments: -test -configuration Release
        condition: or(ne(variables['Build.Reason'], 'PullRequest'), eq(variables['compilerChange'], 'true'))

-      - script: $(Build.SourcesDirectory)\.dotnet\dotnet.exe tool run dotnet-format whitespace  $(Build.SourcesDirectory)\src --folder --include-generated --include $(Build.SourcesDirectory)\src\Compilers\CSharp\Portable\Generated\ $(Build.SourcesDirectory)\src\Compilers\VisualBasic\Portable\Generated\ $(Build.SourcesDirectory)\src\ExpressionEvaluator\VisualBasic\Source\ResultProvider\Generated\ --verify-no-changes
+      - task: PowerShell@2


Curious: why did you pick the PowerShell task here vs using pwsh? That is installed in our repo as a local tool. Generally I try and use that when possible.

jaredpar · 2024-01-24T17:04:24Z

eng/generate-compiler-code.ps1

@@ -42,6 +42,7 @@ function Run-LanguageCore($language, $languageSuffix, $languageDir, $syntaxProje
 function Test-GeneratedContent($generatedDir, $scratchDir) {
  $algo = "MD5"
  foreach ($fileName in (Get-ChildItem $scratchDir)) { 
+    $fileName = $fileName.Name


Why was this change needed?

On my local powershell installation (powershell core 7), Get-ChildItem is returning full file objects. So when you do Join-Path below you actually get:

C:\r\roslyn\src\generatedfoobar\C:\r\roslyn\src\generatedfoobar\Foo.cs

I can definitely revert this if you want.

I tweaked this so that it selects Name explicitly.

eng/validate-code-formatting.ps1

jaredpar · 2024-01-24T17:08:52Z

src/NuGet/VS.ExternalAPIs.Roslyn.Package/VS.ExternalAPIs.Roslyn.Package.csproj

+
+    <!-- Work around missing project dependencies. VS features not needed at this time:
+         https://github.com/dotnet/source-build/issues/3981. -->
+    <ExcludeFromVerticalBuild>true</ExcludeFromVerticalBuild>


Curious: if we set something as excluded from vertical build is it automatically excluded from source build? That would save us a lot of duplicate properties if true.

It will with the new unified build controls. "ExcludeFromVerticalBuild" is the PoC switch. It becomes a general ExcludeFromDotNetBuild switch, while ExcludeFromSourceOnlyBuild becomes the specific switch that says; "When you have access to no outside binaries, exclude this project".

jaredpar · 2024-01-24T17:09:37Z

src/Setup/Directory.Build.props

@@ -2,5 +2,8 @@
  <Import Project="$([MSBuild]::GetPathOfFileAbove('Directory.Build.props', '$(MSBuildThisFileDirectory)../'))" />
  <PropertyGroup>
    <ExcludeFromSourceBuild>true</ExcludeFromSourceBuild>
+    <!-- TODO: https://github.com/dotnet/source-build/issues/3981
+          Remove if necessary. -->
+    <ExcludeFromVerticalBuild>true</ExcludeFromVerticalBuild>


Having both properties seems unnecessary. What code is excuded from the vertical build but included in source build?

Expect this to go away when the new Unified build controls come to this repo. That will happen after the implementation is complete (very soon) and when Roslyn moves to .NET 9 Arcade (late). Whenever you see both of these conditions, they would be replaced by ExcludeFromDotNetBuild

Co-authored-by: Jared Parsons <[email protected]>

… vertical-build-upstreaming

mmitche · 2024-01-25T16:15:42Z

@jaredpar Dealt with feedback.

jaredpar · 2024-01-25T16:41:14Z

eng/validate-code-formatting.ps1

+  $dotnet = Ensure-DotnetSdk
+  Exec-Console $dotnet "tool run dotnet-format -v detailed whitespace $rootDirectory --folder --include-generated --include $includeDirectories --verify-no-changes"
+
+  exit 0


Why did you use exit here vs. ExitWithCode?

Was following the generate-compiler.code.ps1 patterns https://github.com/dotnet/roslyn/pull/71666/files#diff-64ff4bf5168c08ddeb40d75a8889a9ad53f4f2bf3ff1b320a320cdf3567227c0R139

jaredpar · 2024-01-25T16:41:54Z

azure-pipelines.yml

@@ -396,14 +396,13 @@ stages:
      - script: eng/validate-rules-missing-documentation.cmd -ci
        displayName: Validate rules missing documentation

-      - task: PowerShell@2
+      - pwsh: |


Does that use a pwsh that is controlled by AzDO or shell out to the pwsh that we installed via local tools?

pwsh is the Powershell task with pwsh set to true.

mmitche requested review from a team as code owners January 17, 2024 00:03

dotnet-issue-labeler bot added Area-Infrastructure untriaged Issues and PRs which have not yet been triaged by a lead labels Jan 17, 2024

ghost added the Community The pull request was submitted by a contributor who is not a Microsoft employee. label Jan 17, 2024

jaredpar self-assigned this Jan 17, 2024

jaredpar reviewed Jan 17, 2024

View reviewed changes

eng/targets/Imports.BeforeArcade.targets Outdated Show resolved Hide resolved

mmitche added 2 commits January 17, 2024 13:57

Tweak conditionals to not exclude if explicitly included

8a40d54

Remove erroneous exclusion

6c9c891

mmitche commented Jan 17, 2024

View reviewed changes

eng/targets/Imports.BeforeArcade.targets Outdated Show resolved Hide resolved

mmitche added 4 commits January 17, 2024 14:54

Keep IsShipping properties

d50da23

Merge remote-tracking branch 'upstream/main' into vertical-build-upst…

e6093d0

…reaming

Keep IsShipping properties

6abad35

Remove redundant ExcludeFromSourceBuild properties

670c398

mmitche commented Jan 18, 2024

View reviewed changes

eng/targets/Imports.BeforeArcade.targets Outdated Show resolved Hide resolved

mmitche added 2 commits January 17, 2024 16:03

Revert some additional changes

273422b

Merge branch 'main' into vertical-build-upstreaming

fc0e653

ViktorHofer approved these changes Jan 18, 2024

View reviewed changes

Merge branch 'main' into vertical-build-upstreaming

cdada0f

ViktorHofer mentioned this pull request Jan 23, 2024

Windows Proof of Concept dotnet/dotnet#46

Closed

mmitche added 2 commits January 23, 2024 13:21

Fix generated code formatting check

01f12f2

Merge remote-tracking branch 'origin/vertical-build-upstreaming' into…

2d1d923

… vertical-build-upstreaming

mmitche commented Jan 23, 2024

View reviewed changes

mmitche added 2 commits January 23, 2024 13:34

Merge remote-tracking branch 'upstream/main' into vertical-build-upst…

c1b1501

…reaming

Revert verbosity to normal

b1335d2

mmitche enabled auto-merge (squash) January 24, 2024 00:19

ViktorHofer approved these changes Jan 24, 2024

View reviewed changes

jaredpar reviewed Jan 24, 2024

View reviewed changes

auto-merge was automatically disabled January 24, 2024 17:55
Head branch was pushed to by a user without write access

mmitche and others added 3 commits January 24, 2024 09:55

Update eng/validate-code-formatting.ps1

0e038a5

Co-authored-by: Jared Parsons <[email protected]>

Respond to feedback

da05c1a

Merge remote-tracking branch 'origin/vertical-build-upstreaming' into…

7a2ec71

… vertical-build-upstreaming

jaredpar reviewed Jan 25, 2024

View reviewed changes

jaredpar approved these changes Jan 25, 2024

View reviewed changes

mmitche merged commit c31c76c into dotnet:main Jan 25, 2024
31 checks passed

ghost added this to the Next milestone Jan 25, 2024

Cosifne modified the milestones: Next, 17.10 P1 Jan 30, 2024

Cosifne mentioned this pull request Feb 2, 2024

[Automated] PRs inserted in VS build main-34526.79 #71943

Closed

jjonescz mentioned this pull request Feb 7, 2024

Revert dotnet installation #71988

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Upstream changes from Vertical Build PoC #71666

Upstream changes from Vertical Build PoC #71666

mmitche commented Jan 17, 2024

mmitche commented Jan 17, 2024

mmitche Jan 23, 2024

mmitche Jan 23, 2024

mmitche commented Jan 24, 2024

jaredpar Jan 24, 2024

mmitche Jan 24, 2024

jaredpar Jan 24, 2024

mmitche Jan 24, 2024

mmitche Jan 24, 2024

mmitche Jan 25, 2024

jaredpar Jan 24, 2024

mmitche Jan 24, 2024

jaredpar Jan 24, 2024

mmitche Jan 24, 2024

mmitche commented Jan 25, 2024

jaredpar Jan 25, 2024

mmitche Jan 25, 2024

jaredpar Jan 25, 2024

mmitche Jan 25, 2024

Upstream changes from Vertical Build PoC #71666

Upstream changes from Vertical Build PoC #71666

Conversation

mmitche commented Jan 17, 2024

mmitche commented Jan 17, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mmitche commented Jan 24, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mmitche commented Jan 25, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment