This repository has been archived by the owner on Jan 23, 2023. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 4.9k
Update required software and developer workflow #30919
Merged
Merged
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
bb51f3b
Update required software and developer workflow
ViktorHofer 4d4ec68
Address PR feedback
ViktorHofer 7a7dd0b
Fix debugging packages instructions for builds
ViktorHofer ffcf9e9
Add desktop vs core msbuild disclaimer
ViktorHofer 8b41363
Delete obsolete file
ViktorHofer File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains 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
This file contains 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
This file contains 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
This file contains 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
This file contains 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
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -131,41 +131,29 @@ Temporary versions are at https://github.com/dotnet/corefx/blob/dev/eng/src/Tool | |
- UAP F5 -> `uap-Windows_NT` | ||
|
||
## Project configurations for VS | ||
For each unique configuration needed for a given library project a configuration property group should be added to the project so it can be selected and built in VS and also clearly identify the various configurations.<BR/> | ||
|
||
`<PropertyGroup Condition="'$(Configuration)|$(Platform)' == '$(OSGroup)-$(TargetGroup)-$(ConfigurationGroup)|$(Platform)'">` | ||
For each unique configuration needed for a given library project a configuration entry separated by a ';' should be added to the project so it can be selected and built in VS and also clearly identify the various configurations.<BR/> | ||
|
||
`$(TargetGroup)-$(OSGroup)-$(ConfigurationGroup)|$(Platform` | ||
- Note that the majority of managed projects, currently all in corefx, $(Platform) is overridden to be AnyCPU. | ||
|
||
`<Configurations>netcoreapp-Unix-Debug;netcoreapp-Unix-Release;netcoreapp-Windows_NT-Debug;netcoreapp-Windows_NT-Release;uap-Windows_NT-Debug;uap-Windows_NT-Release;uapaot-Windows_NT-Debug;uapaot-Windows_NT-Release</Configurations>` | ||
|
||
####*Examples* | ||
Project configurations for a pure IL library project which targets the defaults. | ||
```xml | ||
<PropertyGroup Condition="'$(Configuration)|$(Platform)' == 'Debug|AnyCPU'" /> | ||
<PropertyGroup Condition="'$(Configuration)|$(Platform)' == 'Release|AnyCPU'" /> | ||
``` | ||
Project configurations with a unique implementation on Unix and Windows | ||
```xml | ||
<PropertyGroup Condition="'$(Configuration)|$(Platform)' == 'Unix-Debug|AnyCPU'" /> | ||
<PropertyGroup Condition="'$(Configuration)|$(Platform)' == 'Unix-Release|AnyCPU'" /> | ||
<PropertyGroup Condition="'$(Configuration)|$(Platform)' == 'Windows_NT-Debug|AnyCPU'" /> | ||
<PropertyGroup Condition="'$(Configuration)|$(Platform)' == 'Windows_NT-Release|AnyCPU'" /> | ||
<Configurations>netcoreapp-Unix-Debug;netcoreapp-Unix-Release;netcoreapp-Windows_NT-Debug;netcoreapp-Windows_NT-Release</Configurations> | ||
``` | ||
Project configurations that are unique for a few different target frameworks and runtimes | ||
```xml | ||
<PropertyGroup Condition="'$(Configuration)|$(Platform)' == 'Debug|AnyCPU'" /> | ||
<PropertyGroup Condition="'$(Configuration)|$(Platform)' == 'Release|AnyCPU'" /> | ||
<PropertyGroup Condition="'$(Configuration)|$(Platform)' == 'uap101aot-Debug|AnyCPU'" /> | ||
<PropertyGroup Condition="'$(Configuration)|$(Platform)' == 'uap101aot-Release|AnyCPU'" /> | ||
<PropertyGroup Condition="'$(Configuration)|$(Platform)' == 'uap101-Debug|AnyCPU'" /> | ||
<PropertyGroup Condition="'$(Configuration)|$(Platform)' == 'uap101-Release|AnyCPU'" /> | ||
<Configurations>netcoreapp-Windows_NT-Debug;netcoreapp-Windows_NT-Release;uap-Windows_NT-Debug;uap-Windows_NT-Release;uapaot-Windows_NT-Debug;uapaot-Windows_NT-Release</Configurations> | ||
``` | ||
|
||
## Updating Configurations | ||
|
||
We have a build task that you can run to automatically update all the projects with the above boilerplate as well as updating all the solution files for the libraries. Whenever you change the list of configurations for a project you can regenerate all these for the entire repo by running: | ||
|
||
``` | ||
msbuild build.proj /t:UpdateVSConfigurations | ||
dotnet msbuild build.proj /t:UpdateVSConfigurations | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @eerhardt is the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, he updated it in: dotnet/buildtools@94f18eb There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. thanks |
||
``` | ||
|
||
If you want to scope the geneneration you can either undo changes that you don't need or you can temporally limit the set of projects or directories by updating the item set in the UpdateVSConfigurations target in https://github.com/dotnet/corefx/blob/master/build.proj | ||
|
Oops, something went wrong.
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.
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.
The plan is to keep both
msbuild
anddotnet msbuild
working so I wonder if we should simply make a note of that somewhere and just referencemsbuild
in the instructions.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.
Question, why should we continue advertising the Desktop msbuild if using the dotnet msbuild is (finally) possible in corefx (from a feature perspective). I don't see any compelling reason to stick with the Desktop flavor. We now already require the .NET Core SDK to be installed which means in 99% of the cases dotnet will be in the path.
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.
That is a fair question. Last I checked even if you called
dotnet msbuild
on a windows machine with full msbuild it would use the full version so I think we are going to need to keep our builds working for both and in such a case we should allow folks to use either. In the past I know the performance was much better with full msbuild over core msbuild but I'm not sure if that is still the case or not, but another potential reason.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 discussed that with @eerhardt offline and when he switched building from the project root with dotnet msbuild he measured performance and it was equal: #30675 (comment). I agree that the dotnet msbuild was slower in the past but I believe it's now on par with the Desktop flavor.
That's interesting. Would be great if we could find out if that's still the case. I just did a quick invocation of dotnet msbuild:
and the Task Manager shows
.NET Core Host
. I believe it's not using the Desktop msbuild anymore underneath but we should verify.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.
While I thought dotnet msbuild would use the full msbuild your correct that doesn't seem to be the case from some quick testing. However the full msbuild is still used when building from VS, so we will need to maintain our build infrastructure to work with both. So it is really just a matter of which do we advertise in our docs. I'm OK advertising the
dotnet msbuild
and just adding a note that says you can also call msbuild on our projects.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.
Also, I'd still be against mandatory pinning for local builds, but my objection wouldn't be as visceral.
I think official builds should be fully repeatable, but freedom to bring my own tools for unofficial, local work is valuable.
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 is also SUPER important for source-build and platform boot-strapping purposes. It is great that official builds mandate that this specific set of tools MUST be used. But we also need a way where I can tell this build "this is the toolset to use".
That way we can bootstrap source-build by building once using a pre-built toolset. Then discarding the pre-built toolset, and turning around and re-building using the toolset I just built from source. That's a requirement in most distros that I've seen.
And for platform boot-strapping, it is obvious that I can't use some already built version of the SDK to build on "PlatformX", when there is no existing version that can run on "PlatformX".
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.
Wow that escalated quickly. I recommend creating an issue to discuss updating the SDK via maestro PRs more frequently further and keep the discussion here streamlined on how to invoke the same SDK for both root and individual project builds. Ideally with dotnet build to be consistent as @stephentoub also noted.
I guess the right approach for now is to modify the path to /Tools/dotnetcli/. I'll update the docs and try it myself locally to check if I run into any errors.
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 will do so.
This actually makes my point much more concrete. We have external reasons to ensure that we can build with some particular toolset, but those shouldn't prevent local dogfooding with newer bits. CI can validate that I haven't taken a dependency on something too new without cutting off my ability to dogfood the new stuff.
Yes, and some one-off gesture shouldn't be needed to dogfood toolsets locally. Source-build will do what it needs to do, but nobody will bother opting in to dogfooding if it doesn't happen naturally/automatically IMHO.
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.
dotnet/arcade#320