-
Notifications
You must be signed in to change notification settings - Fork 1.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implement imports for project extensions #994
Changes from all commits
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 |
---|---|---|
|
@@ -31,6 +31,33 @@ Copyright (C) Microsoft Corporation. All rights reserved. | |
|
||
<Import Project="$(DirectoryBuildPropsPath)" Condition="'$(ImportDirectoryBuildProps)' == 'true' and exists('$(DirectoryBuildPropsPath)')"/> | ||
|
||
<!-- | ||
Prepare to import project extensions which usually come from packages. Package management systems will create a file at: | ||
$(MSBuildProjectExtensionsPath)\$(MSBuildProjectFile).<SomethingUnique>.props | ||
|
||
Each package management system should use a unique moniker to avoid collisions. It is a wild-card import so the package | ||
management system can write out multiple files but the order of the import is alphabetic because MSBuild sorts the list. | ||
--> | ||
<PropertyGroup> | ||
<!-- | ||
The declaration of $(BaseIntermediateOutputPath) had to be moved up from Microsoft.Common.CurrentVersion.targets | ||
in order for the $(MSBuildProjectExtensionsPath) to use it as a default. | ||
--> | ||
<BaseIntermediateOutputPath Condition="'$(BaseIntermediateOutputPath)'=='' ">obj\</BaseIntermediateOutputPath> | ||
<BaseIntermediateOutputPath Condition="!HasTrailingSlash('$(BaseIntermediateOutputPath)')">$(BaseIntermediateOutputPath)\</BaseIntermediateOutputPath> | ||
<MSBuildProjectExtensionsPath Condition="'$(MSBuildProjectExtensionsPath)' == '' ">$(BaseIntermediateOutputPath)</MSBuildProjectExtensionsPath> | ||
<!-- | ||
Import paths that are relative default to be relative to the importing file. However, since MSBuildExtensionsPath | ||
defaults to BaseIntermediateOutputPath we expect it to be relative to the project directory. So if the path is relative | ||
it needs to be made absolute based on the project directory. | ||
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. 👍 #Closed |
||
--> | ||
<MSBuildProjectExtensionsPath Condition="'$([System.IO.Path]::IsPathRooted($(MSBuildProjectExtensionsPath)))' == 'false'">$([System.IO.Path]::Combine('$(MSBuildProjectDirectory)', '$(MSBuildProjectExtensionsPath)'))</MSBuildProjectExtensionsPath> | ||
<MSBuildProjectExtensionsPath Condition="!HasTrailingSlash('$(MSBuildProjectExtensionsPath)')">$(MSBuildProjectExtensionsPath)\</MSBuildProjectExtensionsPath> | ||
<ImportProjectExtensionProps Condition="'$(ImportProjectExtensionProps)' == ''">true</ImportProjectExtensionProps> | ||
</PropertyGroup> | ||
|
||
<Import Project="$(MSBuildProjectExtensionsPath)$(MSBuildProjectFile).*.props" Condition="'$(ImportProjectExtensionProps)' == 'true' and exists('$(MSBuildProjectExtensionsPath)')" /> | ||
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. Have we decided on an ordering mechanism here? Or is it up to each package manager to order inside their generated .props file? (please don't block this PR on this question) What is the ordering of a wildcard import? #Closed 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 thought it was up to the OS but it turns out we sort them... In reply to: 77725944 [](ancestors = 77725944) |
||
|
||
<!-- | ||
Import wildcard "ImportBefore" props files if we're actually in a 12.0+ project (rather than a project being | ||
treated as 4.0) | ||
|
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 says "moved" but I don't see a corresponding delete. Intentional? #Closed
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.
Yeah I wasn't sure if it was safe to remove it from Microsoft.Common.CurrentVersion.targets. Is there a way to confirm that it is safe?
In reply to: 77846900 [](ancestors = 77846900)
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.
Remove it and listen for screams?
Seriously: I don't know of a good way to guarantee that. But since common.targets imports common.props if it's not already in the project it seems safe, because the new codepath should always get hit. #Closed
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.
Okay I'll keep my ears open :)
In reply to: 77850989 [](ancestors = 77850989)