-
Notifications
You must be signed in to change notification settings - Fork 126
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
Enable source-build through arcade #1823
Enable source-build through arcade #1823
Conversation
It looks like the submodule wasn't initialized by the Checkout step. The arcade-powered source-build CI template doesn't automatically provide submodule checkout yet: dotnet/source-build#2049. There's a workaround in there that I used for dotnet/source-build that can be copied here to start with, and getting rid of the workaround can be tracked there. |
@@ -0,0 +1,148 @@ | |||
From 194c172bb1d8e0c0fa0596062ccdd3c28a3923b4 Mon Sep 17 00:00:00 2001 |
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.
There are ways how to do nicely. Could you do it without patching sln?
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.
Any suggestions how? Looking at build.sh
, it directly kicks off illnk.sln
. I dont see too many options for conditionalization. Are you thinking of adding a separate sln file that we can use in a source-build 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.
we could add a build.proj file to do the actual build instead of building the .sln (and keep the .sln just for the IDE).
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.
what Alexander said or add another configuration (e.g. Debug, Release, SourceBuild)to sln which does not include tests
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.
Done, I think.
@@ -0,0 +1,73 @@ | |||
From e2f6d8d0ff79a23433d205b9eb0c7526949bbe6b Mon Sep 17 00:00:00 2001 |
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.
This analyzer ships with SDK don't we want to build it as well?
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.
Hm...it ships with the SDK? Can you point me to an SDK that includes it? I am looking at 5.0.103 and I can't find the analzyer:
$ tar tf dotnet-microsoft-built-sdk-5.0.103.tar.gz | grep -i illink
./sdk/5.0.103/Sdks/Microsoft.NET.Sdk/targets/Microsoft.NET.ILLink.targets
./sdk/5.0.103/Sdks/Microsoft.NET.ILLink.Tasks/
./sdk/5.0.103/Sdks/Microsoft.NET.ILLink.Tasks/Sdk/
./sdk/5.0.103/Sdks/Microsoft.NET.ILLink.Tasks/Sdk/Sdk.props
./sdk/5.0.103/Sdks/Microsoft.NET.ILLink.Tasks/Icon.png
./sdk/5.0.103/Sdks/Microsoft.NET.ILLink.Tasks/tools/
./sdk/5.0.103/Sdks/Microsoft.NET.ILLink.Tasks/tools/net472/
./sdk/5.0.103/Sdks/Microsoft.NET.ILLink.Tasks/tools/net472/System.Collections.Immutable.dll
./sdk/5.0.103/Sdks/Microsoft.NET.ILLink.Tasks/tools/net472/ILLink.Tasks.dll
./sdk/5.0.103/Sdks/Microsoft.NET.ILLink.Tasks/tools/net472/System.Reflection.Metadata.dll
./sdk/5.0.103/Sdks/Microsoft.NET.ILLink.Tasks/tools/net472/Mono.Cecil.dll
./sdk/5.0.103/Sdks/Microsoft.NET.ILLink.Tasks/tools/netcoreapp3.0/
./sdk/5.0.103/Sdks/Microsoft.NET.ILLink.Tasks/tools/netcoreapp3.0/illink.dll
./sdk/5.0.103/Sdks/Microsoft.NET.ILLink.Tasks/tools/netcoreapp3.0/illink.deps.json
./sdk/5.0.103/Sdks/Microsoft.NET.ILLink.Tasks/tools/netcoreapp3.0/ILLink.Tasks.deps.json
./sdk/5.0.103/Sdks/Microsoft.NET.ILLink.Tasks/tools/netcoreapp3.0/illink.runtimeconfig.json
./sdk/5.0.103/Sdks/Microsoft.NET.ILLink.Tasks/tools/netcoreapp3.0/ILLink.Tasks.dll
./sdk/5.0.103/Sdks/Microsoft.NET.ILLink.Tasks/tools/netcoreapp3.0/Mono.Cecil.dll
./sdk/5.0.103/Sdks/Microsoft.NET.ILLink.Tasks/tools/netcoreapp3.0/Mono.Cecil.Pdb.dll
If it ships with the SDK, we would definitely want to build it.
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.
It's a new in .NET6
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.
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 we should merge this PR as-is, and fix up the patches and this functional issue as followup. This PR represents the state of dotnet/source-build, it's not necessary to solve every source-build issue right here and now.
FYI @dseefeld this will also need to be fixed up in the old-infra-style .NET 6 preview1 work, once we find a resolution.
7864fe2
to
d44a5ec
Compare
d44a5ec
to
db815a4
Compare
@@ -84,6 +86,21 @@ stages: | |||
${{ if eq(variables.officialBuild, 'true') }}: | |||
displayName: Build and publish illink.sln $(_BuildConfig) | |||
|
|||
- job: SourceBuild_Managed |
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 this should disable tests processing and publishing if they are not built
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.
Sorry, I am lost. Can you please help me understand how to do that?
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.
Right now, the flag comes in through here:
and get passed thorough the jobs template into the job template here:
Maybe you can add a enablePublishTestResults: false
to the source-build job and it would override the "outer scope" enablePublishTestResults: true
parameter. But AzDO might just throw an error if it's defined twice.
A more reliable solution might be to completely remove the "outer scope" enablePublishBuildArtifacts: true
, and put it on each job that should publish test results.
7617c69
to
a671d58
Compare
This enables 'source-build', which makes it easier to build the entire shipping .NET SDK from source. This is the first and second step of arcade-powered-source-build: https://github.com/dotnet/source-build/blob/master/Documentation/planning/arcade-powered-source-build/README.md See dotnet/sourcelink#692 for a similar PR, that this is based on. The `LICENSE` to `LICENSE.txt` rename is hack to work around NuGet/Home#7601 for now.
a671d58
to
d19d9ed
Compare
Co-authored-by: Marek Safar <marek.safar@gmail.com> Commit migrated from dotnet/linker@6b3a305
This enables 'source-build', which makes it easier to build the entire shipping .NET SDK from source.
This is the first and second step of arcade-powered-source-build: https://github.com/dotnet/source-build/blob/master/Documentation/planning/arcade-powered-source-build/README.md
See dotnet/sourcelink#692 for a similar PR, that this is based on.
The
LICENSE
toLICENSE.txt
rename is hack to work around NuGet/Home#7601 for now.