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
Convert all projects to SDK-style #29831
Merged
Merged
Changes from 1 commit
Commits
Show all changes
27 commits
Select commit
Hold shift + click to select a range
91d7e62
Convert all .csproj files in src to SDK-style projects
wtgodbe be0421d
Add Directory.build.props/targets
wtgodbe f60fa38
Add project.assets.json & edit properties
wtgodbe 10b7151
Remove project.assets.json file
wtgodbe 3ecd281
Get netcoreapp build working.
eerhardt ad505eb
Merge master into SdkProj
eerhardt be5ceb5
Clean up projects. Get all projects importing clean with no warnings…
eerhardt 204a8db
Rename dir files to Directory.Build
eerhardt 1c91f96
merge master into SdkProj
eerhardt 3b36996
Small fixes to get the build clean.
eerhardt b09b0b5
Fix AssemblySearchPaths on the Microsoft.NET.Sdk.
eerhardt 67dacd6
Ensure csc command line is the same as before converting to Sdk proje…
eerhardt 0c7364e
Merge master into SdkProj
eerhardt 53d5973
Fix FileVersionInfo build
eerhardt 6d25667
Rephrase Directory.Build comments to better describe why ImportDirect…
eerhardt e05508a
Change .csproj to new Configurations property
eerhardt 9f65193
Turn off ILLinkTrimAssembly during design-time builds.
eerhardt ec546de
Merge master into SdkProj
eerhardt 860888c
Fix up Reflection.Context dir.props/targets to Directory.Build.
eerhardt 6ccae92
Revert WebServer change, since it cannot be an SDK style project.
eerhardt 0343b37
Merge master into SdkProj
eerhardt 1a4b8a2
Run UpdateVSConfigurations on the current code.
eerhardt 9b7b8c0
Fix up new projects and usages of dir.props/targets.
eerhardt e72643b
Merge master into SdkProj
eerhardt aa89672
Set TargetFramework to workaround project system bug.
eerhardt b86cade
Merge remote-tracking branch 'upstream/master' into SdkProj
eerhardt 99e21e6
Merge master into SdkProj
eerhardt 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
Rephrase Directory.Build comments to better describe why ImportDirect…
…oryBuild properties need to be set
- Loading branch information
commit 6d25667aec315fb4705195e53ff8cb59290a986b
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
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.
Why is this necessary?
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.
Because of the projects that aren't SDK style projects (like depproj or pkgproj). What they do:
Directory.Build.props
at the top of the projectMicrosoft.Common.props
from MSBuild (probably from importing BuildTools stuff)Microsoft.Common.props
will try to importDirectory.Build.props
again, and thus you get a warning.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.
Should I make the comment better here?
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 comment is definitely added confusion to me as I was expecting this to automatically be imported. Is there any way we can just let the SDK import these in those project types as well? Or are they too much work right now to convert to SDK?
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 way
Directory.Build.props|targets
files get automatically imported is by someone importingMicrosoft.Common.props|targets
. If your project isn't an "SDK-style" project, then somewhere in your project, you need an explicit<Import>
statement in order to get theMicrosoft.Common.props|targets
imported.So in order to do that, we would either need to:
Microsoft.Common.props|targets
.Import
a different.props|.targets
set of files at the top and bottom of the .depproj and .pkgproj files. Then that other set of.props|.targets
files would importMicrosoft.Common.props|targets
at the right place, which would then import theDirectory.Build.props|targets
correctly.I haven't tried (1) above, yet. You said @joperezr was working on the pkgproj projects, so I didn't want to do any work there. Also, it feels like we should try to make incremental progress here - get the .csproj's converted first, then worry about .depproj and .pkgproj.
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.
For now I'm fine if you could please update the comment to call out this is about dealing with the non-sdk 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.
Done.