Skip to content

Arcade powered source build #32790

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
merged 31 commits into from
Jun 3, 2021
Merged

Arcade powered source build #32790

merged 31 commits into from
Jun 3, 2021

Conversation

JunTaoLuo
Copy link
Contributor

@JunTaoLuo JunTaoLuo commented May 17, 2021 •
edited
Loading

Fixes: #31445

@ghost ghost added the area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework label May 17, 2021
@JunTaoLuo JunTaoLuo mentioned this pull request May 17, 2021
Closed
JunTaoLuo
JunTaoLuo commented May 18, 2021
View reviewed changes
Directory.Build.props
@@ -171,7 +178,8 @@
linux-musl-arm64;
linux-x64;
linux-arm;
linux-arm64
linux-arm64;
freebsd-x64
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@MichaelSimons Can you provide the background why this was added?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FreeBSD is a community effort which we want to foster.
Issue - dotnet/source-build#1139
Initial PR - dotnet/source-build#1362

JunTaoLuo
JunTaoLuo commented May 18, 2021
View reviewed changes
@JunTaoLuo
Copy link
Contributor Author

Another discussion item is whether we can remove the need to clone to artifacts and run the inner build in that subdirectory. The clone step seems redundant since we expect to only run source build on the CI and on the off chance we need to run it locally, directly patching or changing the files on disk is likely acceptable.

@JunTaoLuo
Copy link
Contributor Author

JunTaoLuo commented May 18, 2021 •
edited
Loading

@MichaelSimons One more question. I see we have been using $(DotNetBuildFromSource) in a lot of our build logic. Is this variable still being set? I ask since I'm not sure whether this variable should be replaced by $(ArcadeBuildFromSource).

JunTaoLuo
JunTaoLuo commented May 19, 2021
View reviewed changes
@JunTaoLuo JunTaoLuo marked this pull request as ready for review May 19, 2021 03:11
@JunTaoLuo JunTaoLuo requested review from dougbu and a team as code owners May 19, 2021 03:11
@JunTaoLuo
Copy link
Contributor Author

FYI, there are several unresolved questions/issues in this PR as marked out by comments, so I'm hesitant to merge as is. However, the build is passing so the PR is ready for initial review.

@MichaelSimons We confirmed that the build produced the Microsoft.SourceBuild.Intermediate.aspnetcore.6.0.0-ci.nupkg package as expected but we should also check that that nupkg contains the right set of contents. Can you provide a list of what's expected to be in it (maybe a list of what was built in preview3 or preview4) so I can check that against what's produced by the build?

@MichaelSimons
Copy link
Member

@MichaelSimons One more question. I see we have been using $(DotNetBuildFromSource) in a lot of our build logic. Is this variable still being set? I ask since I'm not sure whether this variable should be replaced by $(ArcadeBuildFromSource).

You should continue to use $(DotNetBuildFromSource) this gets set when the inner build is run.

MichaelSimons
MichaelSimons reviewed May 19, 2021
View reviewed changes
@MichaelSimons
Copy link
Member

Can you provide a list of what's expected to be in it (maybe a list of what was built in preview3 or preview4) so I can check that against what's produced by the build?

dotnet-dev-certs
dotnet-user-secrets
Microsoft.AspNetCore.Analyzers
Microsoft.AspNetCore.App.Runtime.linux-x64
Microsoft.AspNetCore.Authorization
Microsoft.AspNetCore.Components
Microsoft.AspNetCore.Components.Analyzers
Microsoft.AspNetCore.Components.Authorization
Microsoft.AspNetCore.Components.Forms
Microsoft.AspNetCore.Components.Web
Microsoft.AspNetCore.Connections.Abstractions
Microsoft.AspNetCore.Cryptography.Internal
Microsoft.AspNetCore.Cryptography.KeyDerivation
Microsoft.AspNetCore.DataProtection
Microsoft.AspNetCore.DataProtection.Abstractions
Microsoft.AspNetCore.DataProtection.Extensions
Microsoft.AspNetCore.DeveloperCertificates.XPlat
Microsoft.AspNetCore.Http.Connections.Common
Microsoft.AspNetCore.Http.Features
Microsoft.AspNetCore.Metadata
Microsoft.AspNetCore.Mvc.Analyzers
Microsoft.AspNetCore.Mvc.Api.Analyzers
Microsoft.AspNetCore.Mvc.Razor.Extensions
Microsoft.AspNetCore.Mvc.Razor.Extensions.Version1_X
Microsoft.AspNetCore.Mvc.Razor.Extensions.Version2_X
Microsoft.AspNetCore.Razor.Language
Microsoft.AspNetCore.SignalR.Common
Microsoft.AspNetCore.SignalR.Protocols.Json
Microsoft.CodeAnalysis.Razor
Microsoft.Extensions.Configuration.KeyPerFile
Microsoft.Extensions.Diagnostics.HealthChecks
Microsoft.Extensions.Diagnostics.HealthChecks.Abstractions
Microsoft.Extensions.FileProviders.Embedded
Microsoft.Extensions.Identity.Core
Microsoft.Extensions.Identity.Stores
Microsoft.Extensions.Localization
Microsoft.Extensions.Localization.Abstractions
Microsoft.Extensions.ObjectPool
Microsoft.Extensions.WebEncoders
Microsoft.JSInterop

@JunTaoLuo JunTaoLuo requested a review from Pilchie as a code owner May 19, 2021 17:44
JunTaoLuo
JunTaoLuo commented May 19, 2021
View reviewed changes
@JunTaoLuo
Copy link
Contributor Author

JunTaoLuo commented May 19, 2021 •
edited
Loading

Took a diff of the contents of M.SourceBuild.Intermediate.aspnetcore and what you provided:

image

The only diff is expected from this change: #32043

dougbu
dougbu suggested changes May 27, 2021
View reviewed changes
@JunTaoLuo
Copy link
Contributor Author

I've addressed all feedback items and this is ready for a final review.

MichaelSimons
MichaelSimons reviewed Jun 2, 2021
View reviewed changes
Copy link
Member

@MichaelSimons MichaelSimons left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove the Target from SourceBuild.props which applies the patches. This should not be needed going forward - https://github.com/dotnet/aspnetcore/blob/main/eng/SourceBuild.props#L9

dougbu
dougbu approved these changes Jun 3, 2021
View reviewed changes
Copy link
Contributor

@dougbu dougbu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

eng/scripts/prepare-sourcebuild-globaljson.sh is nice work 🥇

@JunTaoLuo JunTaoLuo merged commit 4d67e70 into main Jun 3, 2021
@JunTaoLuo JunTaoLuo deleted the johluo/arpow branch June 3, 2021 21:44
@ghost ghost added this to the 6.0-preview6 milestone Jun 3, 2021
@JunTaoLuo JunTaoLuo mentioned this pull request Jun 3, 2021
Closed
@MichaelSimons MichaelSimons mentioned this pull request Jun 7, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dougbu dougbu dougbu approved these changes

@MichaelSimons MichaelSimons MichaelSimons approved these changes

@Pilchie Pilchie Awaiting requested review from Pilchie

Assignees
No one assigned
Labels
area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework
Projects
None yet
Milestone
6.0-preview6
Development

Successfully merging this pull request may close these issues.

ArPow stage 2: implementing source-build CI
3 participants
@JunTaoLuo @MichaelSimons @dougbu