Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

[Infrastructure] Switch to arcade and use SDK Style projects - WIP#23988

Closed
jashook wants to merge 33 commits intodotnet:masterfrom
jashook:arcade_tests_build
Closed

[Infrastructure] Switch to arcade and use SDK Style projects - WIP#23988
jashook wants to merge 33 commits intodotnet:masterfrom
jashook:arcade_tests_build

Conversation

@jashook
Copy link

@jashook jashook commented Apr 15, 2019

This is a fairly large change to our build infrastructure that does several key/important things.

Overview

General

  1. Remove all dependencies on BuildTools
  2. Updates our dotnet sdk to the latest 3.0 preview4 sdk
  3. Imports and uses the arcade SDK

build.sh/build.cmd

  1. Changes all the managed components, SPC, SOS, package restore to use SDK style projects and import arcade
  2. Changes our output location from bin/Product/${__BuildOS}.${__BuildArch}.${__BuildConfig} to artifacts/Product/${__BuildOS}/${__BuildArch}/${__BuildConfig}
  3. Updates our native components to reference the new output locations

build-test.sh/build-test.cmd

  1. Change our tests to build as SDK style projects
  2. Change the traversal logic to use the built in multi-project build solution provided by the dotnet sdk, which is sln files with project references.
  3. Due to the sln files requiring unique test names, several dozen tests have been renamed from either lib.csproj or test.csproj to their parent directories name.

Remaining work

This PR is accurately a work in progress, building both the product and the tests complete; however, the following are still not working:

  1. Creating the Core_Root
    a. This should only require a default build target to depend on the existing runtest.proj targets.
  2. Changes to runtest.py to use the artifact directory
  3. Changes to helix publish to use the artifact directory
  4. Documentation changes:
    a. The usage of sln files will change our addition of tests to the repository.
    b. Now in order to add a test, a dotnet sdk command will need to be called. An example call to add a test would be ~/coreclr/.dotnet/dotnet sln tests/src/priority_0-1.sln JIT/Regression/newTest/newTest.csproj

The following is not investigated, and will probably require more work:

  1. Changes to signing to either use the artifacts directory, or rely on the built in signing of the arcade sdk
  2. Version.h/c has been changed, we should change the sem ver2 and directly use the arcade SDK to create the version information for the native components.

Cleanup work

The following would be nice to have cleanup work that is enabled by this change:

  1. Removal of dir.props, this have been superseded by Directory.Build.props
  2. Removal of dir.targets, this has been superseded by Directory.Build.targets

jashook and others added 30 commits March 15, 2019 09:51
* Add RestoreArcade target and some other properties to SOS NETCore project
* Remove the hack to Skip SoS for arcade builds
This allows us to get a clean SPC with ILLink
@jashook jashook added area-Infrastructure-coreclr * NO MERGE * The PR is not ready for merge yet (see discussion for detailed reasons) labels Apr 15, 2019
@jashook
Copy link
Author

jashook commented Apr 15, 2019

This PR currently has conflicts, I will rebase.

@mikedn
Copy link

mikedn commented Apr 15, 2019

Due to the sln files requiring unique test names, several dozen tests have been renamed from either lib.csproj or test.csproj to their parent directories name.

It looks like you added a tests.sln, are you sure you want to do that? There was one in the past that got deleted because it wasn't maintained and it's impractical to load so many projects in VS. Besides, it's not really needed.

@jashook
Copy link
Author

jashook commented Apr 15, 2019

@mikedn I am not sold on the usage of the .sln files. Currently it has been added because it is the only OOB multi-project build support provided by the dotnet sdk. It adds complexity, and in some ways fragility to the system. The benefit is that this is supported by the dotnet sdk, and instead of our custom msbuild logic, it should be improved incrementally with further sdk changes.

@sbomer @echesakovMSFT also have strong opinions, I believe mostly against the change. If it is decided not to make the change, that is fine too. At the very minimum it allowed me to make quick forward progress on porting the tests to SDK style projects.

@jashook
Copy link
Author

jashook commented Apr 15, 2019

/cc @RussKeldorph

@jkoritzinsky
Copy link
Member

An alternative option to solution files may be to use the Microsoft.Build.Traversal MSBuild SDK from https://github.com/Microsoft/MSBuildSdks/tree/master/src/Traversal

It allows you to do multi-project builds via simpler <ProjectReference /> includes and a normal MSBuild project file instead of dealing with the solution format.

@am11
Copy link
Member

am11 commented Apr 16, 2019

An alternative option to solution files may be to use the Microsoft.Build.Traversal MSBuild SDK from https://github.com/Microsoft/MSBuildSdks/tree/master/src/Traversal

Here is another one: dotnet/msbuild#1730 (comment). Hopefully VS team will pick it up and refresh the SLN file format based on XML.

@AaronRobinsonMSFT
Copy link
Member

@jashook I am going to carve off some of this work and apply it directly to master. Some of the minor changes I made can be found in https://github.com/AaronRobinsonMSFT/coreclr/tree/arcade_tests_build

@jashook
Copy link
Author

jashook commented Apr 25, 2019

I agree, when you put out a pr feel free to include me on the review. I am curious to see where the split is.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-Infrastructure-coreclr * NO MERGE * The PR is not ready for merge yet (see discussion for detailed reasons)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants