Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fast up-to-date check supports TargetPath for CopyToOutputDirectory items #7470

Merged
merged 3 commits into from
Aug 12, 2021
Merged

Fast up-to-date check supports TargetPath for CopyToOutputDirectory items #7470

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
bfbee26
Improve FUTD log message
drewnoakes Aug 9, 2021
e8dca67
Rename `Link` to `TargetPath`
drewnoakes Aug 9, 2021
4d45beb
Up-to-date check supports `TargetPath` metadata
drewnoakes Aug 9, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 12 additions & 0 deletions src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/Rules/Items/Content.xaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,18 @@
</StringProperty.DataSource>
</StringProperty>

<StringProperty Name="TargetPath"
Visible="false">
<StringProperty.DataSource>
<DataSource PersistenceStyle="Attribute"
SourceOfDefaultValue="AfterContext" />
</StringProperty.DataSource>
<StringProperty.Metadata>
<NameValuePair Name="DoNotCopyAcrossProjects"
Value="true" />
</StringProperty.Metadata>
</StringProperty>

<BoolProperty Name="Visible"
Visible="false" />

Expand Down
12 changes: 12 additions & 0 deletions ...rosoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/Rules/Items/EmbeddedResource.xaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,18 @@
</StringProperty.DataSource>
</StringProperty>

<StringProperty Name="TargetPath"
Visible="false">
<StringProperty.DataSource>
<DataSource PersistenceStyle="Attribute"
SourceOfDefaultValue="AfterContext" />
</StringProperty.DataSource>
<StringProperty.Metadata>
<NameValuePair Name="DoNotCopyAcrossProjects"
Value="true" />
</StringProperty.Metadata>
</StringProperty>

<BoolProperty Name="Visible"
Visible="false" />

Expand Down
12 changes: 12 additions & 0 deletions src/Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/Rules/Items/None.xaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,18 @@
</StringProperty.DataSource>
</StringProperty>

<StringProperty Name="TargetPath"
Visible="false">
<StringProperty.DataSource>
<DataSource PersistenceStyle="Attribute"
SourceOfDefaultValue="AfterContext" />
</StringProperty.DataSource>
<StringProperty.Metadata>
<NameValuePair Name="DoNotCopyAcrossProjects"
Value="true" />
</StringProperty.Metadata>
</StringProperty>

<BoolProperty Name="Visible"
Visible="false" />

Expand Down
8 changes: 4 additions & 4 deletions ...ualStudio.ProjectSystem.Managed/ProjectSystem/UpToDate/BuildUpToDateCheck.ItemComparer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ namespace Microsoft.VisualStudio.ProjectSystem.UpToDate
{
internal sealed partial class BuildUpToDateCheck
{
internal sealed class ItemComparer : IEqualityComparer<(string path, string? link, CopyType copyType)>
internal sealed class ItemComparer : IEqualityComparer<(string path, string? targetPath, CopyType copyType)>
{
public static readonly ItemComparer Instance = new();

Expand All @@ -15,14 +15,14 @@ private ItemComparer()
}

public bool Equals(
(string path, string? link, CopyType copyType) x,
(string path, string? link, CopyType copyType) y)
(string path, string? targetPath, CopyType copyType) x,
(string path, string? targetPath, CopyType copyType) y)
{
return StringComparers.Paths.Equals(x.path, y.path);
}

public int GetHashCode(
(string path, string? link, CopyType copyType) obj)
(string path, string? targetPath, CopyType copyType) obj)
{
return StringComparers.Paths.GetHashCode(obj.path);
}
Expand Down
4 changes: 2 additions & 2 deletions ...sualStudio.ProjectSystem.Managed/ProjectSystem/UpToDate/BuildUpToDateCheck.ItemHashing.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ namespace Microsoft.VisualStudio.ProjectSystem.UpToDate
{
internal sealed partial class BuildUpToDateCheck
{
public static int ComputeItemHash(ImmutableDictionary<string, ImmutableArray<(string Path, string? Link, CopyType CopyType)>> itemsByItemType)
public static int ComputeItemHash(ImmutableDictionary<string, ImmutableArray<(string Path, string? TargetPath, CopyType CopyType)>> itemsByItemType)
{
int hash = 0;

Expand All @@ -17,7 +17,7 @@ public static int ComputeItemHash(ImmutableDictionary<string, ImmutableArray<(st
// This approach also assumes each path is only included once in the data structure. If a path
// were to exist twice, its hash would be XORed with itself, which produces zero net change.

foreach ((string itemType, ImmutableArray<(string Path, string? Link, CopyType CopyType)> items) in itemsByItemType)
foreach ((string itemType, ImmutableArray<(string Path, string? TargetPath, CopyType CopyType)> items) in itemsByItemType)
{
int itemHash = 0;

Expand Down
18 changes: 9 additions & 9 deletions ...Microsoft.VisualStudio.ProjectSystem.Managed/ProjectSystem/UpToDate/BuildUpToDateCheck.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ private bool CheckGlobalConditions(Log log, DateTime lastCheckedAtUtc, UpToDateC
return log.Fail("FirstRun", "The up-to-date check has not yet run for this project. Not up-to-date.");
}

foreach ((_, ImmutableArray<(string Path, string? Link, CopyType CopyType)> items) in state.ItemsByItemType)
foreach ((_, ImmutableArray<(string Path, string? TargetPath, CopyType CopyType)> items) in state.ItemsByItemType)
{
foreach ((string path, _, CopyType copyType) in items)
{
Expand Down Expand Up @@ -204,12 +204,12 @@ bool CheckInputsAndOutputs(IEnumerable<(string Path, bool IsRequired)> inputs, I

if (log.Level <= LogLevel.Info)
{
foreach ((bool isAdd, string itemType, string path, string? link, CopyType copyType) in state.LastItemChanges.OrderBy(change => change.ItemType).ThenBy(change => change.Path))
foreach ((bool isAdd, string itemType, string path, string? targetPath, CopyType copyType) in state.LastItemChanges.OrderBy(change => change.ItemType).ThenBy(change => change.Path))
{
if (Strings.IsNullOrEmpty(link))
if (Strings.IsNullOrEmpty(targetPath))
log.Info(" {0} item {1} '{2}' (CopyType={3})", itemType, isAdd ? "added" : "removed", path, copyType);
else
log.Info(" {0} item {1} '{2}' (CopyType={3}, Link='{4}')", itemType, isAdd ? "added" : "removed", path, copyType, link);
log.Info(" {0} item {1} '{2}' (CopyType={3}, TargetPath='{4}')", itemType, isAdd ? "added" : "removed", path, copyType, targetPath);
}
}

Expand Down Expand Up @@ -291,7 +291,7 @@ bool CheckInputsAndOutputs(IEnumerable<(string Path, bool IsRequired)> inputs, I
yield return (Path: state.NewestImportInput, IsRequired: true);
}

foreach ((string itemType, ImmutableArray<(string path, string? link, CopyType copyType)> changes) in state.ItemsByItemType)
foreach ((string itemType, ImmutableArray<(string path, string? targetPath, CopyType copyType)> changes) in state.ItemsByItemType)
{
if (!NonCompilationItemTypes.Contains(itemType))
{
Expand Down Expand Up @@ -595,9 +595,9 @@ private bool CheckCopyToOutputDirectoryFiles(Log log, in TimestampCache timestam
{
string outputFullPath = Path.Combine(state.MSBuildProjectDirectory, state.OutputRelativeOrFullPath);

foreach ((_, ImmutableArray<(string Path, string? Link, CopyType CopyType)> items) in state.ItemsByItemType)
foreach ((_, ImmutableArray<(string Path, string? TargetPath, CopyType CopyType)> items) in state.ItemsByItemType)
{
foreach ((string path, string? link, CopyType copyType) in items)
foreach ((string path, string? targetPath, CopyType copyType) in items)
{
// Only consider items with CopyType of CopyIfNewer
if (copyType != CopyType.CopyIfNewer)
Expand All @@ -608,7 +608,7 @@ private bool CheckCopyToOutputDirectoryFiles(Log log, in TimestampCache timestam
token.ThrowIfCancellationRequested();

string rootedPath = _configuredProject.UnconfiguredProject.MakeRooted(path);
string filename = Strings.IsNullOrEmpty(link) ? rootedPath : link;
string filename = Strings.IsNullOrEmpty(targetPath) ? rootedPath : targetPath;

if (string.IsNullOrEmpty(filename))
{
Expand Down Expand Up @@ -644,7 +644,7 @@ private bool CheckCopyToOutputDirectoryFiles(Log log, in TimestampCache timestam

if (destinationTime < itemTime)
{
return log.Fail("CopyToOutputDirectory", "PreserveNewest source is newer than destination, not up to date.");
return log.Fail("CopyToOutputDirectory", "PreserveNewest source '{0}' is newer than destination '{1}', not up to date.", rootedPath, destination);
}
}
}
Expand Down
67 changes: 48 additions & 19 deletions ...udio.ProjectSystem.Managed/ProjectSystem/UpToDate/UpToDateCheckImplicitConfiguredInput.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,9 @@ internal sealed class UpToDateCheckImplicitConfiguredInput
public string? MSBuildProjectFullPath { get; }

public string? MSBuildProjectDirectory { get; }

public string? CopyUpToDateMarkerItem { get; }

public string? OutputRelativeOrFullPath { get; }

/// <summary>
Expand Down Expand Up @@ -52,15 +52,15 @@ internal sealed class UpToDateCheckImplicitConfiguredInput
/// </remarks>
public DateTime LastItemsChangedAtUtc { get; }

public ImmutableArray<(bool IsAdd, string ItemType, string Path, string? Link, BuildUpToDateCheck.CopyType CopyType)> LastItemChanges { get; }
public ImmutableArray<(bool IsAdd, string ItemType, string Path, string? TargetPath, BuildUpToDateCheck.CopyType CopyType)> LastItemChanges { get; }

public int? ItemHash { get; }

public bool WasStateRestored { get; }

public ImmutableArray<string> ItemTypes { get; }

public ImmutableDictionary<string, ImmutableArray<(string Path, string? Link, BuildUpToDateCheck.CopyType CopyType)>> ItemsByItemType { get; }
public ImmutableDictionary<string, ImmutableArray<(string Path, string? TargetPath, BuildUpToDateCheck.CopyType CopyType)>> ItemsByItemType { get; }

public ImmutableArray<string> SetNames { get; }

Expand Down Expand Up @@ -128,7 +128,7 @@ private UpToDateCheckImplicitConfiguredInput()

LastItemsChangedAtUtc = DateTime.MinValue;
ItemTypes = ImmutableArray<string>.Empty;
ItemsByItemType = ImmutableDictionary.Create<string, ImmutableArray<(string Path, string? Link, BuildUpToDateCheck.CopyType CopyType)>>(StringComparers.ItemTypes);
ItemsByItemType = ImmutableDictionary.Create<string, ImmutableArray<(string Path, string? TargetPath, BuildUpToDateCheck.CopyType CopyType)>>(StringComparers.ItemTypes);
SetNames = ImmutableArray<string>.Empty;
UpToDateCheckInputItemsByKindBySetName = emptyItemBySetName;
UpToDateCheckOutputItemsByKindBySetName = emptyItemBySetName;
Expand Down Expand Up @@ -161,7 +161,7 @@ private UpToDateCheckImplicitConfiguredInput()
IImmutableDictionary<string, DateTime> additionalDependentFileTimes,
DateTime lastAdditionalDependentFileTimesChangedAtUtc,
DateTime lastItemsChangedAtUtc,
ImmutableArray<(bool IsAdd, string ItemType, string Path, string? Link, BuildUpToDateCheck.CopyType CopyType)> lastItemChanges,
ImmutableArray<(bool IsAdd, string ItemType, string Path, string? TargetPath, BuildUpToDateCheck.CopyType CopyType)> lastItemChanges,
int? itemHash,
bool wasStateRestored)
{
Expand Down Expand Up @@ -380,7 +380,7 @@ void AddKeys(ImmutableDictionary<string, ImmutableDictionary<string, ImmutableAr

bool itemTypesChanged = false;

List<(bool IsAdd, string ItemType, string Path, string? Link, BuildUpToDateCheck.CopyType)> changes = new();
List<(bool IsAdd, string ItemType, string Path, string? TargetPath, BuildUpToDateCheck.CopyType)> changes = new();

// If an item type was removed, remove all items of that type
foreach (string removedItemType in itemTypeDiff.Removed)
Expand All @@ -389,9 +389,9 @@ void AddKeys(ImmutableDictionary<string, ImmutableDictionary<string, ImmutableAr

if (itemsByItemTypeBuilder.TryGetValue(removedItemType, out var removedItems))
{
foreach ((string path, string? link, BuildUpToDateCheck.CopyType copyType) in removedItems)
foreach ((string path, string? targetPath, BuildUpToDateCheck.CopyType copyType) in removedItems)
{
changes.Add((false, removedItemType, path, link, copyType));
changes.Add((false, removedItemType, path, targetPath, copyType));
}

itemsByItemTypeBuilder.Remove(removedItemType);
Expand All @@ -418,29 +418,31 @@ void AddKeys(ImmutableDictionary<string, ImmutableDictionary<string, ImmutableAr
if (projectChange.After.Items.Count == 0)
continue;

ImmutableArray<(string Path, string? Link, BuildUpToDateCheck.CopyType)> before = ImmutableArray<(string Path, string? Link, BuildUpToDateCheck.CopyType)>.Empty;
if (itemsByItemTypeBuilder.TryGetValue(itemType, out ImmutableArray<(string Path, string? Link, BuildUpToDateCheck.CopyType CopyType)> beforeItems))
ImmutableArray<(string Path, string? TargetPath, BuildUpToDateCheck.CopyType)> before = ImmutableArray<(string Path, string? TargetPath, BuildUpToDateCheck.CopyType)>.Empty;
if (itemsByItemTypeBuilder.TryGetValue(itemType, out ImmutableArray<(string Path, string? TargetPath, BuildUpToDateCheck.CopyType CopyType)> beforeItems))
before = beforeItems;

var after = projectChange.After.Items.Select(item => (Path: item.Key, GetLink(item.Value), GetCopyType(item.Value))).ToHashSet(BuildUpToDateCheck.ItemComparer.Instance);
var after = projectChange.After.Items
.Select(item => (Path: item.Key, TargetPath: GetTargetPath(item.Value), CopyType: GetCopyType(item.Value)))
.ToHashSet(BuildUpToDateCheck.ItemComparer.Instance);

var diff = new SetDiff<(string, string?, BuildUpToDateCheck.CopyType)>(before, after, BuildUpToDateCheck.ItemComparer.Instance);

foreach ((string path, string? link, BuildUpToDateCheck.CopyType copyType) in diff.Added)
foreach ((string path, string? targetPath, BuildUpToDateCheck.CopyType copyType) in diff.Added)
{
changes.Add((true, itemType, path, link, copyType));
changes.Add((true, itemType, path, targetPath, copyType));
}

foreach ((string path, string? link, BuildUpToDateCheck.CopyType copyType) in diff.Removed)
foreach ((string path, string? targetPath, BuildUpToDateCheck.CopyType copyType) in diff.Removed)
{
changes.Add((false, itemType, path, link, copyType));
changes.Add((false, itemType, path, targetPath, copyType));
}

itemsByItemTypeBuilder[itemType] = after.ToImmutableArray();
itemsChanged = true;
}

ImmutableDictionary<string, ImmutableArray<(string Path, string? Link, BuildUpToDateCheck.CopyType CopyType)>> itemsByItemType = itemsByItemTypeBuilder.ToImmutable();
ImmutableDictionary<string, ImmutableArray<(string Path, string? TargetPath, BuildUpToDateCheck.CopyType CopyType)>> itemsByItemType = itemsByItemTypeBuilder.ToImmutable();

int itemHash = BuildUpToDateCheck.ComputeItemHash(itemsByItemType);

Expand Down Expand Up @@ -521,9 +523,36 @@ static BuildUpToDateCheck.CopyType GetCopyType(IImmutableDictionary<string, stri
return BuildUpToDateCheck.CopyType.CopyNever;
}

static string? GetLink(IImmutableDictionary<string, string> itemMetadata)
static string? GetTargetPath(IImmutableDictionary<string, string> itemMetadata)
{
return itemMetadata.TryGetValue(BuildUpToDateCheck.Link, out string link) ? link : null;
// "Link" is an optional path and file name under which the item should be copied.
// It allows a source file to be moved to a different relative path, or to be renamed.
//
// From the perspective of the FUTD check, it is only relevant on CopyToOutputDirectory items.
//
// Two properties can provide this feature: "Link" and "TargetPath".
//
// If specified, "TargetPath" metadata controls the path of the target file, relative to the output
// folder.
//
// "Link" controls the location under the project in Solution Explorer where the item appears.
// If "TargetPath" is not specified, then "Link" can also serve the role of "TargetPath".
//
// If both are specified, we only use "TargetPath". The use case for specifying both is wanting
// to control the location of the item in Solution Explorer, as well as in the output directory.
// The former is not relevant to us here.

if (itemMetadata.TryGetValue(None.TargetPathProperty, out string? targetPath) && !string.IsNullOrWhiteSpace(targetPath))
{
return targetPath;
}

if (itemMetadata.TryGetValue(None.LinkProperty, out string link) && !string.IsNullOrWhiteSpace(link))
{
return link;
}

return null;
}

static ImmutableDictionary<string, ImmutableDictionary<string, ImmutableArray<string>>> BuildItemsByKindBySetName(IProjectChangeDescription projectChangeDescription, string kindPropertyName, string setPropertyName)
Expand Down
11 changes: 10 additions & 1 deletion ...crosoft.VisualStudio.ProjectSystem.Managed.UnitTests/ProjectSystem/Rules/ItemRuleTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using System.IO;
using System.Linq;
using System.Xml.Linq;
using System.Xml.XPath;
using Xunit;

namespace Microsoft.VisualStudio.ProjectSystem.Rules
Expand Down Expand Up @@ -35,12 +36,20 @@ public void FileRulesShouldMatchNone(string ruleName, string fullPath)

string noneFile = Path.Combine(fullPath, "..", "None.xaml");

XElement none = LoadXamlRule(noneFile);
XElement none = LoadXamlRule(noneFile, out var namespaceManager);
XElement rule = LoadXamlRule(fullPath);

// First fix up the Name as we know they'll differ.
rule.Attribute("Name").Value = "None";

if (ruleName is "Compile" or "EditorConfigFiles")
{
// Remove the "TargetPath" element for these types
var targetPathElement = none.XPathSelectElement(@"/msb:Rule/msb:StringProperty[@Name=""TargetPath""]", namespaceManager);
Assert.NotNull(targetPathElement);
targetPathElement!.Remove();
}

AssertXmlEqual(none, rule);
}

Expand Down
10 changes: 10 additions & 0 deletions ...udio.ProjectSystem.Managed.UnitTests/ProjectSystem/UpToDate/BuildUpToDateCheckTestBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,16 @@ private protected static IProjectRuleSnapshotModel ItemWithMetadata(string itemS
};
}

private protected static IProjectRuleSnapshotModel ItemWithMetadata(string itemSpec, params (string MetadataName, string MetadataValue)[] metadata)
{
return new IProjectRuleSnapshotModel
{
Items = ImmutableStringDictionary<IImmutableDictionary<string, string>>.EmptyOrdinal.Add(
itemSpec,
metadata.ToImmutableDictionary(pair => pair.MetadataName, pair => pair.MetadataValue, StringComparer.Ordinal))
};
}

private protected static IProjectRuleSnapshotModel ItemsWithMetadata(params (string itemSpec, string metadataName, string metadataValue)[] items)
{
return new IProjectRuleSnapshotModel
Expand Down