Skip to content

Commit

Permalink
Allow Custom CopyToOutputDirectory Location With TargetPath (#6237)
Browse files Browse the repository at this point in the history 
Fixes #2795
and indirectly fixes https://developercommunity.visualstudio.com/t/copytooutputdirectorypreservenewest-ignored-inside/1332219?from=email&viewtype=all#T-ND1363347

Context
There's currently no way to include items in a project such that:

Visual studio sees them in a specific folder (via <Link>).
They are published to a user-defined path (currently controlled via <Link>)
Changes Made
Modify the AssignTargetPath task to return early if TargetPath metadata is already set on a particular item.

Testing
 Need to add one test covering this.
 Tested locally with bootstrapped MSBuild on command line
 Tested locally with a boostrapped msbuild on internal VS
Here's the repro I'm using:

<Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
    <TargetFramework>netstandard2.0</TargetFramework>
  </PropertyGroup>
  <ItemGroup>
    <Content Include="Files\**">
      <Link>Files\%(Filename)%(Extension)</Link>
      <TargetPath>%(Filename)%(Extension)</TargetPath>
      <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
    </Content>
  </ItemGroup>
</Project>
Notes
The other way of solving this problem has to do with Microsoft.Common.CurrentVersion.targets. We modify it so that the AssignTargetPath task look something like this:

    <AssignTargetPath Files="@(Content)" RootFolder="$(MSBuildProjectDirectory)" Condition="'%(Content.TargetPath)'==''">
      <Output TaskParameter="AssignedFiles" ItemName="ContentWithTargetPath" />
    </AssignTargetPath>
    <ItemGroup>
        <ContentWithTargetPath Include="@(Content)" Condition="'%(Content.TargetPath)'!=''"/>
    </ItemGroup>
This seems less efficient to me. AssignTargetPath is also called for all None, Content, and EmbeddedResource files. So if we go this batching route and want None or EmbeddedResource to have this feature, we'd need to batch those as well.
  • Loading branch information
@benvillalobos
benvillalobos committed Apr 5, 2021
1 parent b7476cc commit 7804350
Show file tree
Hide file tree
Showing 4 changed files with 88 additions and 31 deletions.
1 change: 1 addition & 0 deletions documentation/wiki/ChangeWaves.md
View file
Expand Up @@ -25,6 +25,7 @@ The opt-out comes in the form of setting the environment variable `MSBuildDisabl
- [Don't expand full drive globs with false condition](https://github.com/dotnet/msbuild/pull/5669)
### 16.10
- [Error when a property expansion in a condition has whitespace](https://github.com/dotnet/msbuild/pull/5672)
- [Allow Custom CopyToOutputDirectory Location With TargetPath](https://github.com/dotnet/msbuild/pull/6237)
### 17.0

## Change Waves No Longer In Rotation
4 changes: 4 additions & 0 deletions src/Shared/Constants.cs
View file
Expand Up @@ -166,6 +166,10 @@ internal static class ItemMetadataNames
internal const string subType = "SubType";
internal const string executableExtension = "ExecutableExtension";
internal const string embedInteropTypes = "EmbedInteropTypes";

/// <summary>
/// The output path for a given item.
/// </summary>
internal const string targetPath = "TargetPath";
internal const string dependentUpon = "DependentUpon";
internal const string msbuildSourceProjectFile = "MSBuildSourceProjectFile";
Expand Down
100 changes: 73 additions & 27 deletions src/Tasks.UnitTests/AssignTargetPath_Tests.cs
View file
@@ -1,10 +1,12 @@
// Copyright (c) Microsoft. All rights reserved.
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

using System.Collections.Generic;
using Microsoft.Build.Framework;
using Microsoft.Build.Shared;
using Microsoft.Build.Tasks;
using Microsoft.Build.Utilities;
using Shouldly;
using Xunit;

namespace Microsoft.Build.UnitTests
Expand All @@ -20,15 +22,10 @@ public void Regress314791()
{ new TaskItem(NativeMethodsShared.IsWindows ? @"c:\bin2\abc.efg" : "/bin2/abc.efg") };
t.RootFolder = NativeMethodsShared.IsWindows ? @"c:\bin" : "/bin";

bool success = t.Execute();

Assert.True(success);

Assert.Single(t.AssignedFiles);
Assert.Equal(
NativeMethodsShared.IsWindows ? @"c:\bin2\abc.efg" : "/bin2/abc.efg",
t.AssignedFiles[0].ItemSpec);
Assert.Equal(@"abc.efg", t.AssignedFiles[0].GetMetadata("TargetPath"));
t.Execute().ShouldBeTrue();
t.AssignedFiles.Length.ShouldBe(1);
t.AssignedFiles[0].ItemSpec.ShouldBe(NativeMethodsShared.IsWindows ? @"c:\bin2\abc.efg" : "/bin2/abc.efg");
t.AssignedFiles[0].GetMetadata("TargetPath").ShouldBe("abc.efg");
}

[Fact]
Expand All @@ -40,12 +37,9 @@ public void AtConeRoot()
{ new TaskItem(NativeMethodsShared.IsWindows ? @"c:\f1\f2\file.txt" : "/f1/f2/file.txt") };
t.RootFolder = NativeMethodsShared.IsWindows ? @"c:\f1\f2" : "/f1/f2";

bool success = t.Execute();

Assert.True(success);

Assert.Single(t.AssignedFiles);
Assert.Equal(@"file.txt", t.AssignedFiles[0].GetMetadata("TargetPath"));
t.Execute().ShouldBeTrue();
t.AssignedFiles.Length.ShouldBe(1);
t.AssignedFiles[0].GetMetadata("TargetPath").ShouldBe("file.txt");
}

[Fact]
Expand All @@ -64,12 +58,9 @@ public void OutOfCone()
// /f1 to /x1
t.RootFolder = NativeMethodsShared.IsWindows ? @"c:\f1" : "/x1";

bool success = t.Execute();

Assert.True(success);

Assert.Single(t.AssignedFiles);
Assert.Equal("file.txt", t.AssignedFiles[0].GetMetadata("TargetPath"));
t.Execute().ShouldBeTrue();
t.AssignedFiles.Length.ShouldBe(1);
t.AssignedFiles[0].GetMetadata("TargetPath").ShouldBe("file.txt");
}

[Fact]
Expand All @@ -84,14 +75,69 @@ public void InConeButAbsolute()
};
t.RootFolder = NativeMethodsShared.IsWindows ? @"c:\f1\f2" : "/f1/f2";

bool success = t.Execute();
t.Execute().ShouldBeTrue();
t.AssignedFiles.Length.ShouldBe(1);
t.AssignedFiles[0].GetMetadata("TargetPath").ShouldBe(NativeMethodsShared.IsWindows ? @"f3\f4\file.txt" : "f3/f4/file.txt");
}

[Theory]
[InlineData("c:/fully/qualified/path.txt")]
[InlineData("test/output/file.txt")]
[InlineData(@"some\dir\to\file.txt")]
[InlineData("file.txt")]
[InlineData("file")]
public void TargetPathAlreadySet(string targetPath)
{
AssignTargetPath t = new AssignTargetPath();
t.BuildEngine = new MockEngine();
Dictionary<string, string> metaData = new Dictionary<string, string>();
metaData.Add("TargetPath", targetPath);
metaData.Add("Link", "c:/foo/bar");
t.Files = new ITaskItem[]
{
new TaskItem(
itemSpec: NativeMethodsShared.IsWindows ? @"c:\f1\f2\file.txt" : "/f1/f2/file.txt",
itemMetadata: metaData)
};
t.RootFolder = NativeMethodsShared.IsWindows ? @"c:\f1\f2" : "/f1/f2";

t.Execute().ShouldBeTrue();
t.AssignedFiles.Length.ShouldBe(1);
t.AssignedFiles[0].GetMetadata("TargetPath").ShouldBe(targetPath);
}

[Theory]
[InlineData("c:/fully/qualified/path.txt")]
[InlineData("test/output/file.txt")]
[InlineData(@"some\dir\to\file.txt")]
[InlineData("file.txt")]
[InlineData("file")]
public void TargetPathAlreadySet_DisabledUnderChangeWave16_10(string targetPath)
{
using TestEnvironment env = TestEnvironment.Create();
string link = "c:/some/path";

ChangeWaves.ResetStateForTests();
env.SetEnvironmentVariable("MSBUILDDISABLEFEATURESFROMVERSION", ChangeWaves.Wave16_10.ToString());
BuildEnvironmentHelper.ResetInstance_ForUnitTestsOnly();

Assert.True(success);
AssignTargetPath t = new AssignTargetPath();
t.BuildEngine = new MockEngine();
Dictionary<string, string> metaData = new Dictionary<string, string>();
metaData.Add("TargetPath", targetPath);
metaData.Add("Link", link);
t.Files = new ITaskItem[]
{
new TaskItem(
itemSpec: NativeMethodsShared.IsWindows ? @"c:\f1\f2\file.txt" : "/f1/f2/file.txt",
itemMetadata: metaData)
};
t.RootFolder = NativeMethodsShared.IsWindows ? @"c:\f1\f2" : "/f1/f2";

Assert.Single(t.AssignedFiles);
Assert.Equal(
NativeMethodsShared.IsWindows ? @"f3\f4\file.txt" : "f3/f4/file.txt",
t.AssignedFiles[0].GetMetadata("TargetPath"));
t.Execute().ShouldBeTrue();
t.AssignedFiles.Length.ShouldBe(1);
t.AssignedFiles[0].GetMetadata("TargetPath").ShouldBe(link);
ChangeWaves.ResetStateForTests();
}
}
}
Expand Down
14 changes: 10 additions & 4 deletions src/Tasks/AssignTargetPath.cs
View file
Expand Up @@ -71,13 +71,19 @@ public override bool Execute()

for (int i = 0; i < Files.Length; ++i)
{
string link = Files[i].GetMetadata(ItemMetadataNames.link);
AssignedFiles[i] = new TaskItem(Files[i]);

// If file has a link, use that.
string targetPath = link;
// If TargetPath is already set, it takes priority.
// https://github.com/dotnet/msbuild/issues/2795
string targetPath = ChangeWaves.AreFeaturesEnabled(ChangeWaves.Wave16_10) ? Files[i].GetMetadata(ItemMetadataNames.targetPath) : null;

if (string.IsNullOrEmpty(link))
// If TargetPath not already set, fall back to default behavior.
if (string.IsNullOrEmpty(targetPath))
{
targetPath = Files[i].GetMetadata(ItemMetadataNames.link);
}

if (string.IsNullOrEmpty(targetPath))
{
if (// if the file path is relative
!Path.IsPathRooted(Files[i].ItemSpec) &&
Expand Down

0 comments on commit 7804350

Please sign in to comment.