Use Microsoft.Net.Compilers.Toolset in coreclr/src/Common projects #901
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ViktorHofer
force-pushed
the
CoreClrTestFixes
branch
from
1d8edc8 to
191da91
Compare
ViktorHofer
changed the title
safern
reviewed
Dec 16, 2019
ViktorHofer
force-pushed
the
CoreClrTestFixes
branch
from
50acc07 to
750dee1
Compare
Versions.props is the Arcade entry point to manage version which are being used throughout the repository.
At the moment only Arcade Sdk.props are being imported in the common managed projects. Changing that to make sure that the same compiler is being used everywhere.
Restore and Build targets shouldn't run in the same msbuild invocation as that could cause issues with nuget auto-generated props/targets.
Intermediate paths are encoded wrong so that MSBuildProjectExtensionsPath which defaults to BaseIntermediateOutputPath is set wrong and the in-built msbuild Exists check fails. This is because of a mixture of different slashes.
The restore and build target invocations need to happen in separate msbuild task invocations and different global properties need to be passed in to mitigate issues with caching the invocations. For more context see dotnet/msbuild#2811.
ViktorHofer
force-pushed
the
CoreClrTestFixes
branch
3 times, most recently
from
1742717 to
2507255
Compare
ViktorHofer
force-pushed
the
CoreClrTestFixes
branch
from
2507255 to
c8745d8
Compare
safern
reviewed
Jan 14, 2020
janvorli
reviewed
Jan 14, 2020
safern
reviewed
Jan 14, 2020
safern
reviewed
Jan 14, 2020
safern
reviewed
Jan 14, 2020
safern
reviewed
Jan 14, 2020
safern
reviewed
Jan 14, 2020
safern
reviewed
Jan 14, 2020
safern
reviewed
Jan 14, 2020
Member
|
@ViktorHofer, what's the state of this? Thanks. |
Member
|
@ViktorHofer, given this has been open for two months, hasn't been touched in a month, has a bunch of conflicts, and there's been no response in over a week about status, I'm going to close this. It can obviously be re-opened when you're ready to proceed with it. Thanks. |
Member
Author
|
I've been working on this over the weekend but am still not in a state to push an update. Closing is fine, I'll reopen when this is ready again. |
Member
|
Thanks. |
ghost
locked as resolved and limited conversation to collaborators
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Remove dependencies.props favor of Versions.props
Versions.props is the Arcade entry point to manage version which are
being used throughout the repository.
Use Arcade Sdk.targets in coreclr/src/Common
At the moment only Arcade Sdk.props are being imported in the common
managed projects. Changing that to make sure that the same compiler is
being used everywhere.
Consolidate managed properties to OSGroup, ArchGroup and ConfigurationGroup. With the ongoing work done by @Anipik which will allow multi-targeting by OS this will be cleaned-up further.
Packaging fixes to point to the right license and third party notices
A lot of cleanup
cc @janvorli