Skip to content

Commit

Permalink
Conditions for targets/props imports
Browse files Browse the repository at this point in the history
Framework conditions for targets/props are added only when a project uses cross targeting. For scenarios with a single framework under $(TargetFramework) buildCrossTargeting will be left out and there will be no framework conditions to block the imports.

In addition to this a new property has been added to allow control over package imports: ExcludeRestorePackageImports. This flag is used at restore time to avoid imports from packages changing the inputs to restore, without this it is possible to get different results between the first and second restore.

This change also contains support for nooping when writing out targets/props files. Currently the check is a simple string compare on the files since these files are small. Nooping is now a major benefit since all NETCore projects reference the SDK package, without nooping on the targets/props files MSBuild will always end up rebuilding everything for NETCore solutions.

Fixes NuGet/Home#3588
Fixes NuGet/Home#3637
Fixes NuGet/Home#3604
Fixes NuGet/Home#3641
Fixes NuGet/Home#3199
  • Loading branch information
emgarten committed Oct 11, 2016
1 parent fa3f3ae commit 3fc5aff
Show file tree
Hide file tree
Showing 15 changed files with 1,440 additions and 159 deletions.
3 changes: 3 additions & 0 deletions src/NuGet.Clients/NuGet.CommandLine/MsBuildUtility.cs
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,9 @@ public static DependencyGraphSpec GetProjectReferences(
argumentBuilder.Append(" /p:RestoreGraphOutputPath=");
AppendQuoted(argumentBuilder, resultsPath);

// Disallow the import of targets/props from packages
argumentBuilder.Append(" /p:ExcludeRestorePackageImports=true ");

// Projects to restore
argumentBuilder.Append(" /p:RestoreGraphProjectInput=\"");
for (var i = 0; i < projectPaths.Length; i++)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,11 @@ public class MSBuildShellOutNuGetProject : NuGetProject, INuGetIntegratedProject
/// </summary>
private const string TargetFrameworksName = "TargetFrameworks";

/// <summary>
/// Single Framework
/// </summary>
private const string TargetFrameworkName = "TargetFramework";

/// <summary>
/// The MSBuild property for the path to MSBuild.
/// </summary>
Expand Down Expand Up @@ -103,8 +108,9 @@ public static MSBuildShellOutNuGetProject Create(EnvDTEProject project)
return null;
}

// The project must have the "TargetFrameworks" property.
if (GetMSBuildProperty(buildPropertyStorage, TargetFrameworksName) == null)
// The project must have the "TargetFrameworks" or "TargetFramework" property.
if (GetMSBuildProperty(buildPropertyStorage, TargetFrameworksName) == null
&& GetMSBuildProperty(buildPropertyStorage, TargetFrameworkName) == null)
{
return null;
}
Expand Down Expand Up @@ -480,6 +486,9 @@ public static DependencyGraphSpec GetProjectReferences(
argumentBuilder.Append($" {msbuildAdditionalArgs} ");
}

// Disallow the import of targets/props from packages
argumentBuilder.Append(" /p:ExcludeRestorePackageImports=true ");

argumentBuilder.Append(" /p:RestoreTaskAssemblyFile=");
AppendQuoted(argumentBuilder, buildTasksPath);

Expand Down
75 changes: 36 additions & 39 deletions src/NuGet.Core/NuGet.Build.Tasks/NuGet.targets
Original file line number Diff line number Diff line change
Expand Up @@ -133,47 +133,36 @@ Copyright (c) .NET Foundation. All rights reserved.
<Message Text="%(RestoreGraphProjectInputItems.Identity)" Importance="low" />

<!-- Generate a restore graph for each entry point project -->
<PropertyGroup>
<_GenerateRestoreGraphProjectEntryInputProperties>
%(_MSBuildProjectReferenceExistent.SetConfiguration);
%(_MSBuildProjectReferenceExistent.SetPlatform);
RestoreUseCustomAfterTargets=$(RestoreUseCustomAfterTargets);
NuGetRestoreTargets=$(MSBuildThisFileFullPath);
BuildProjectReferences=false;
ExcludeRestorePackageImports=true;
</_GenerateRestoreGraphProjectEntryInputProperties>

<!-- Standalone mode
This is used by NuGet.exe or to directly override NuGet.targets with the current file.
Override the ImportAfter targets with NuGetRestoreTargets if they exist.
Set CustomAfterMicrosoftCommonCrossTargetingTargets and CustomAfterMicrosoftCommonTargets
for both the inner and outer builds.
-->
<_GenerateRestoreGraphProjectEntryInputProperties Condition=" '$(RestoreUseCustomAfterTargets)' == 'true' ">
$(_GenerateRestoreGraphProjectEntryInputProperties);
CustomAfterMicrosoftCommonCrossTargetingTargets=$(MSBuildThisFileFullPath);
CustomAfterMicrosoftCommonTargets=$(MSBuildThisFileFullPath);
</_GenerateRestoreGraphProjectEntryInputProperties>
</PropertyGroup>

<!-- Using targets imported with ImportsAfter -->
<MsBuild
Condition=" '$(RestoreUseCustomAfterTargets)' != 'true' "
Projects="@(RestoreGraphProjectInputItems)"
Targets="_GenerateRestoreGraphProjectEntry"
BuildInParallel="false"
Properties="
%(_MSBuildProjectReferenceExistent.SetConfiguration);
%(_MSBuildProjectReferenceExistent.SetPlatform);
RestoreUseCustomAfterTargets=$(RestoreUseCustomAfterTargets);
NuGetRestoreTargets=$(MSBuildThisFileFullPath);
BuildProjectReferences=false;"
RemoveProperties="%(_MSBuildProjectReferenceExistent.GlobalPropertiesToRemove)">

<Output
TaskParameter="TargetOutputs"
ItemName="_RestoreGraphEntry" />
</MsBuild>

<!-- Standalone mode
This is used by NuGet.exe or to directly override NuGet.targets with the current file.
Override the ImportAfter targets with NuGetRestoreTargets if they exist.
Set CustomAfterMicrosoftCommonCrossTargetingTargets and CustomAfterMicrosoftCommonTargets
for both the inner and outer builds.
-->
<MsBuild
Condition=" '$(RestoreUseCustomAfterTargets)' == 'true' "
Projects="@(RestoreGraphProjectInputItems)"
Targets="_GenerateRestoreGraphProjectEntry"
BuildInParallel="false"
Properties="
%(_MSBuildProjectReferenceExistent.SetConfiguration);
%(_MSBuildProjectReferenceExistent.SetPlatform);
CustomAfterMicrosoftCommonCrossTargetingTargets=$(MSBuildThisFileFullPath);
CustomAfterMicrosoftCommonTargets=$(MSBuildThisFileFullPath);
RestoreUseCustomAfterTargets=$(RestoreUseCustomAfterTargets);
NuGetRestoreTargets=$(MSBuildThisFileFullPath);
BuildProjectReferences=false;"
Properties="$(_GenerateRestoreGraphProjectEntryInputProperties)"
RemoveProperties="%(_MSBuildProjectReferenceExistent.GlobalPropertiesToRemove)">

<Output
TaskParameter="TargetOutputs"
ItemName="_RestoreGraphEntry" />
Expand Down Expand Up @@ -332,7 +321,11 @@ Copyright (c) .NET Foundation. All rights reserved.
<_RestoreProjectName Condition=" '$(_ProjectRestoreType)' == 'NETCore' AND '$(AssemblyName)' != '' ">$(AssemblyName)</_RestoreProjectName>
<_RestoreProjectName Condition=" '$(_ProjectRestoreType)' == 'NETCore' AND '$(PackageId)' != '' ">$(PackageId)</_RestoreProjectName>
</PropertyGroup>


<!-- Determine if this will use cross targeting -->
<PropertyGroup Condition=" '$(_ProjectRestoreType)' == 'NETCore' AND '$(TargetFrameworks)' != '' ">
<_RestoreCrossTargeting>true</_RestoreCrossTargeting>
</PropertyGroup>

<!-- Write properties for the top level entry point -->
<ItemGroup Condition=" '$(_ProjectRestoreType)' == 'NETCore' ">
Expand All @@ -348,8 +341,9 @@ Copyright (c) .NET Foundation. All rights reserved.
<OutputType>$(_ProjectRestoreType)</OutputType>
<OutputPath>$(RestoreOutputAbsolutePath)</OutputPath>
<TargetFrameworks>@(_RestoreTargetFrameworksOutputFiltered)</TargetFrameworks>
<RuntimeIdentifiers>$(RuntimeIdentifiers)</RuntimeIdentifiers>
<RuntimeIdentifiers>$(RuntimeIdentifiers);$(RuntimeIdentifier)</RuntimeIdentifiers>
<RuntimeSupports>$(RuntimeSupports)</RuntimeSupports>
<CrossTargeting>$(_RestoreCrossTargeting)</CrossTargeting>
</_RestoreGraphEntry>
</ItemGroup>

Expand Down Expand Up @@ -400,7 +394,8 @@ Copyright (c) .NET Foundation. All rights reserved.
Properties="TargetFramework=%(_RestoreTargetFrameworksOutputFiltered.Identity);
%(_MSBuildProjectReferenceExistent.SetConfiguration);
%(_MSBuildProjectReferenceExistent.SetPlatform);
BuildProjectReferences=false;"
BuildProjectReferences=false;
ExcludeRestorePackageImports=true;"
RemoveProperties="%(_MSBuildProjectReferenceExistent.GlobalPropertiesToRemove)">

<Output
Expand Down Expand Up @@ -428,7 +423,8 @@ Copyright (c) .NET Foundation. All rights reserved.
%(_MSBuildProjectReferenceExistent.SetPlatform);
RestoreUseCustomAfterTargets=$(RestoreUseCustomAfterTargets);
NuGetRestoreTargets=$(MSBuildThisFileFullPath);
BuildProjectReferences=false;"
BuildProjectReferences=false;
ExcludeRestorePackageImports=true;"
RemoveProperties="%(_MSBuildProjectReferenceExistent.GlobalPropertiesToRemove)">

<Output
Expand All @@ -455,7 +451,8 @@ Copyright (c) .NET Foundation. All rights reserved.
%(_MSBuildProjectReferenceExistent.SetPlatform);
RestoreUseCustomAfterTargets=$(RestoreUseCustomAfterTargets);
NuGetRestoreTargets=$(MSBuildThisFileFullPath);
BuildProjectReferences=false"
BuildProjectReferences=false;
ExcludeRestorePackageImports=true;"
RemoveProperties="%(_MSBuildProjectReferenceExistent.GlobalPropertiesToRemove)">

<Output
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using System.Collections.Generic;
using System.Linq;

namespace NuGet.Commands
{
public class MSBuildRestoreImportGroup
{
/// <summary>
/// Optional position arguement used when ordering groups in the output file.
/// </summary>
public int Position { get; set; } = 1;

/// <summary>
/// Conditions applied to the item group. These will be AND'd together.
/// </summary>
public List<string> Conditions { get; set; } = new List<string>();

/// <summary>
/// Project paths to import.
/// </summary>
public List<string> Imports { get; set; } = new List<string>();

/// <summary>
/// Combined conditions
/// </summary>
public string Condition
{
get
{
if (Conditions.Count > 0)
{
return " " + string.Join(" AND ", Conditions.Select(s => s.Trim())) + " ";
}
else
{
return string.Empty;
}
}
}
}
}
Loading

0 comments on commit 3fc5aff

Please sign in to comment.