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

Use our local-built instances of tools from Microsoft.DotNet.BuildTools.CoreClr instead of using the package.#24347

Merged
jkoritzinsky merged 6 commits into
dotnet:masterfrom
jkoritzinsky:local-over-buildtoools
May 4, 2019
Merged

Use our local-built instances of tools from Microsoft.DotNet.BuildTools.CoreClr instead of using the package.#24347
jkoritzinsky merged 6 commits into
dotnet:masterfrom
jkoritzinsky:local-over-buildtoools

Conversation

@jkoritzinsky
Copy link
Copy Markdown
Member

Three years ago we moved the all of the sources for the tools in Microsoft.DotNet.BuildTools.CoreClr into the coreclr repo. For most of the tools, we never actually hooked up our CMake build to build the tools locally and use them.

This PR changes our build to use the local tool builds and removes our dependency on the Microsoft.DotNet.BuildTools.CoreClr package.

@jkoritzinsky jkoritzinsky added this to the 3.0 milestone May 1, 2019
@jkoritzinsky jkoritzinsky requested a review from RussKeldorph May 1, 2019 23:12
@RussKeldorph RussKeldorph requested a review from mikem8361 May 1, 2019 23:21
@jkoritzinsky
Copy link
Copy Markdown
Member Author

@mikem8361 I was looking at one of @vancem's old PRs in this area (#17004) and he commented that we should look at moving to the Linux way of generating the DAC information so we can move off all of these tools that support the Windows method. Any chance you have some insight on to why that wasn't done or if we should do it?

@jkoritzinsky
Copy link
Copy Markdown
Member Author

Looks like we need to figure out a way to build these tools targetting the host architecture instead of the target architecture. That might explain why this was never done.

@mikem8361
Copy link
Copy Markdown

We have talked about doing what we do in Linux with the DAC table which doesn't require these tools but we really never have any time to do this work. It could introduce race conditions on when the DAC table is available (it did on Linux) made the change some what risky.

I don't know all the reasons or problems with building and using the tools in coreclr like this PR is doing. I only had experience with the Linux mechanism.

…ross-building and import the targets into the cross-build.
@jkoritzinsky
Copy link
Copy Markdown
Member Author

The diff in build.cmd looks pretty ugly. What I did there was move the cross-native build before the native build (and added a few extra CMake defines).

@jkoritzinsky
Copy link
Copy Markdown
Member Author

@BruceForstall the _il_relthread-race test timed out on the Linux arm64 checked run.

@jkoritzinsky
Copy link
Copy Markdown
Member Author

As this change is Windows-only I'm going to merge this in.

@jkoritzinsky jkoritzinsky merged commit 1ffcf98 into dotnet:master May 4, 2019
@jkoritzinsky jkoritzinsky deleted the local-over-buildtoools branch May 4, 2019 00:56
@BruceForstall
Copy link
Copy Markdown

@BruceForstall the _il_relthread-race test timed out on the Linux arm64 checked run.

Ugh. I recently disable it, but only under GCStress: #24174

picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022
…ls.CoreClr instead of using the package. (dotnet/coreclr#24347)

* Use our local-built instances of tools from Microsoft.DotNet.BuildTools.CoreClr instead of using the package.

* Fix const-correctness in InjectResource.

* Build cross-arch native components before building native components for target arch.

* Build InjectResource and GenClrDebugResource for the host arch when cross-building and import the targets into the cross-build.

* install(EXPORT) in the directory where the target is created


Commit migrated from dotnet/coreclr@1ffcf98
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants