-
Notifications
You must be signed in to change notification settings - Fork 4.9k
Convert all projects to SDK-style #29831
Changes from 22 commits
91d7e62
be0421d
f60fa38
10b7151
3ecd281
ad505eb
be5ceb5
204a8db
1c91f96
3b36996
b09b0b5
67dacd6
0c7364e
53d5973
6d25667
e05508a
9f65193
ec546de
860888c
6ccae92
0343b37
1a4b8a2
9b7b8c0
e72643b
aa89672
b86cade
99e21e6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,13 @@ | ||
<?xml version="1.0" encoding="utf-8"?> | ||
<Project ToolsVersion="14.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003"> | ||
<Import Condition="Exists('..\dir.props')" Project="..\dir.props" /> | ||
|
||
<PropertyGroup> | ||
<!-- | ||
For non-SDK projects that import this file and then import Microsoft.Common.props, | ||
tell Microsoft.Common.props not to import Directory.Build.props again | ||
--> | ||
<ImportDirectoryBuildProps>false</ImportDirectoryBuildProps> | ||
</PropertyGroup> | ||
|
||
<!-- We shipped assembly file version 4.6.x up until the end of rc3. Version assembly as | ||
4.6.x to ensure compatability in Visual Studio for an in-place update. --> | ||
|
@@ -312,6 +319,15 @@ | |
<IncludeDllSafeSearchPathAttribute Condition="'$(TargetGroup)' == 'netstandard1.0'">false</IncludeDllSafeSearchPathAttribute> | ||
</PropertyGroup> | ||
|
||
<!-- Set up default paths for reference assemblies --> | ||
<PropertyGroup Condition="'$(IsReferenceAssembly)'=='true'"> | ||
<!-- Create a common root output directory for all reference assemblies --> | ||
<ReferenceAssemblyOutputPath Condition="'$(ReferenceAssemblyOutputPath)' == ''">$(BaseOutputPath)ref/</ReferenceAssemblyOutputPath> | ||
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. Why did these get moved here instead of using the ones in BuildTools? https://github.com/dotnet/buildtools/blob/16a6d1f39cf0a572041d51bd318913548ba41b34/src/Microsoft.DotNet.Build.Tasks/PackageFiles/ReferenceAssemblies.targets#L15 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. 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. I'd like to understand if it is needed here still. 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. I've investigated why this was being set. The reason looks to be that the BuildTools setting of This change was attempting to set these properties earlier, but aren't the real fix since |
||
<TargetOutputRelPath Condition="'$(IsCompatAssembly)'=='true'">$(TargetOutputRelPath)Compat/</TargetOutputRelPath> | ||
<OutputPath>$(ReferenceAssemblyOutputPath)$(MSBuildProjectName)/$(TargetOutputRelPath)</OutputPath> | ||
<IntermediateOutputPath>$(BaseIntermediateOutputPath)ref/$(AssemblyName)/$(TargetOutputRelPath)</IntermediateOutputPath> | ||
</PropertyGroup> | ||
|
||
<!-- Properties related to multi-file mode for ILC tests --> | ||
<PropertyGroup Condition="'$(TargetGroup)' == 'uapaot'"> | ||
<!-- Only enable multi-file for Release-x64 and Debug-x86 for now --> | ||
|
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.