-
Notifications
You must be signed in to change notification settings - Fork 442
Enable build.sh for non source build scenarios #18358
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
Conversation
1. Define argument consistently between build.ps1 and buils.sh 2. Add a Directory.Build.rsp file to consistently disable the new msbuild terminal logger as it doesn't work well with orchestrated builds. 3. Make the default non source build and add the "--source-only" switch. 4. Update the YML to pass the new switch in.
3a86816
to
da88fc0
Compare
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 it looks good. Logic in the new build.sh looks solid. May want to have @mthalman or @MichaelSimons give that a once over too.
echo " --excludeCIBinarylog Don't output binary log (short: -nobl)" | ||
echo " --prepareMachine Prepare machine for CI run, clean up processes after build" |
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.
For consistency, shouldn't these two use kebab-casing?
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.
In theory yes, but Arcade already exposes them differently and we want to eventually just use Arcade's build script: https://github.com/dotnet/arcade/blob/8853b56b0f1a9f223563e5879d3f01f409c6515a/eng/common/build.sh#L41-L42
As those are part of eng/common/build.ps1 and build.sh, they are already exposed in every .NET core stack repository. I would rather stay consistent with those and live with the local difference here.
Merging to unblock @akoeplinger's CI work and @directhex's short-stack work. I'm happy to react to additional feedback post-merge via a follow-up. |
This unblocks dotnet/source-build#3682 & dotnet/source-build#3823