Skip to content

Commit e70ae8c

Browse files
rolfbjarnerolfbjarne
committed
[msbuild] Revamp BundleResource.GetVirtualProjectPath and BundleResource.GetLogicalName.
* Completely rework the code to make it more readable add lots of comments explaining what it's doing. * Fix some issues on Windows: for each item whose virtual project path or logical name we want to compute, we need to know the path to the MSBuild file that defined the item in question ('DefiningProjectFullPath'), as well as the path to the project file ('MSBuildProjectFullPath'). Unfortunately, when executing remotely, these values are not the original values, which means we need to store them as additional metadata ('LocalDefiningProjectFullPath' and 'LocalMSBuildProjectFullPath', respectively) in our MSBuild logic, so that we can access them in the task. Do this by adding a new target ("_SetResourceMetadata") that sets these new metadata. * Add lots of comments explaining what the code does, since it's not the first time we've tinkered with these functions. Also add tests to make sure we don't regress. * Add optional tracing to ease future debugging. * Enable nullability and fix any issues.
1 parent 820f1ec commit e70ae8c

File tree

18 files changed

+427
-97
lines changed

18 files changed

+427
-97
lines changed

‎msbuild/Xamarin.Localization.MSBuild/MSBStrings.resx‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1623,6 +1623,14 @@
16231623
</comment>
16241624
</data>
16251625

1626+
<data name="E7133" xml:space="preserve">
1627+
<value>The item '{0}'' does not have a '{1}' value set.</value>
1628+
<comment>
1629+
{0}: a path to a file
1630+
{1}: the name of a MSBuild metadata
1631+
</comment>
1632+
</data>
1633+
16261634
<data name="XcodeBuild_CreateNativeRef" xml:space="preserve">
16271635
<value>Adding reference to Xcode project output: '{0}'. The '%(CreateNativeReference)' metadata can be set to 'false' to opt out of this behavior.</value>
16281636
<comment>

‎msbuild/Xamarin.MacDev.Tasks/BundleResource.cs‎

Lines changed: 114 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,20 @@
1+
#define TRACE
2+
13
using System;
4+
using System.Diagnostics;
25
using System.Diagnostics.CodeAnalysis;
36
using System.IO;
47
using System.Linq;
58
using System.Collections.Generic;
69

710
using Microsoft.Build.Framework;
11+
using Microsoft.Build.Utilities;
812

13+
using Xamarin.Localization.MSBuild;
914
using Xamarin.Utils;
15+
using Xamarin.MacDev.Tasks;
16+
17+
#nullable enable
1018

1119
namespace Xamarin.MacDev {
1220
public static class BundleResource {
@@ -48,74 +56,147 @@ public static bool IsIllegalName (string name, [NotNullWhen (true)] out string?
4856
return false;
4957
}
5058

51-
public static IList<string> SplitResourcePrefixes (string prefix)
59+
public static IList<string> SplitResourcePrefixes (string? prefix)
5260
{
61+
if (prefix is null)
62+
return Array.Empty<string> ();
63+
5364
return prefix.Split (new [] { ';' }, StringSplitOptions.RemoveEmptyEntries)
5465
.Select (s => s.Replace ('\\', Path.DirectorySeparatorChar).Trim () + Path.DirectorySeparatorChar)
5566
.Where (s => s.Length > 1)
5667
.ToList ();
5768
}
5869

59-
public static string GetVirtualProjectPath (string projectDir, ITaskItem item, bool isVSBuild)
70+
[Conditional ("TRACE")]
71+
static void Trace (Task task, string msg)
6072
{
61-
var link = item.GetMetadata ("Link");
73+
task.Log.LogMessage (MessageImportance.Low, msg);
74+
}
6275

63-
// Note: if the Link metadata exists, then it will be the equivalent of the ProjectVirtualPath
76+
// Compute the path of 'item' relative to the project.
77+
public static string GetVirtualProjectPath<T> (T task, ITaskItem item) where T: Task, IHasProjectDir, IHasSessionId
78+
{
79+
// If the Link metadata exists, use that, it takes precedence over anything else.
80+
var link = item.GetMetadata ("Link");
6481
if (!string.IsNullOrEmpty (link)) {
65-
if (Path.DirectorySeparatorChar != '\\')
66-
return link.Replace ('\\', '/');
67-
82+
// Canonicalize to use macOS-style directory separators.
83+
link = link.Replace ('\\', '/');
84+
Trace (task, $"BundleResource.GetVirtualProjectPath ({item.ItemSpec}) => Link={link}");
6885
return link;
6986
}
7087

71-
// HACK: This is for Visual Studio iOS projects
72-
if (isVSBuild) {
73-
if (item.GetMetadata ("DefiningProjectFullPath") != item.GetMetadata ("MSBuildProjectFullPath")) {
74-
return item.GetMetadata ("FullPath").Replace (item.GetMetadata ("DefiningProjectDirectory"), string.Empty);
75-
} else {
76-
return item.ItemSpec;
77-
}
78-
}
88+
// Note that '/' is a valid path separator on Windows (in addition to '\'), so canonicalize the paths to use '/' as the path separator.
7989

8090
var isDefaultItem = item.GetMetadata ("IsDefaultItem") == "true";
81-
var definingProjectFullPath = item.GetMetadata (isDefaultItem ? "MSBuildProjectFullPath" : "DefiningProjectFullPath");
82-
var path = item.GetMetadata ("FullPath");
83-
string baseDir;
91+
var localMSBuildProjectFullPath = item.GetMetadata ("LocalMSBuildProjectFullPath").Replace ('\\', '/');
92+
var localDefiningProjectFullPath = item.GetMetadata ("LocalDefiningProjectFullPath").Replace ('\\', '/');
93+
if (string.IsNullOrEmpty (localDefiningProjectFullPath)) {
94+
task.Log.LogError (null, null, null, item.ItemSpec, 0, 0, 0, 0, MSBStrings.E7133 /* The item '{0}'' does not have a '{1}' value set. */, item.ItemSpec, "LocalDefiningProjectFullPath");
95+
return "placeholder";
96+
}
97+
98+
if (string.IsNullOrEmpty (localMSBuildProjectFullPath)) {
99+
task.Log.LogError (null, null, null, item.ItemSpec, 0, 0, 0, 0, MSBStrings.E7133 /* The item '{0}'' does not have a '{1}' value set. */, item.ItemSpec, "LocalMSBuildProjectFullPath");
100+
return "placeholder";
101+
}
102+
103+
// * If we're not a default item, compute the path relative to the
104+
// file that declared the item in question.
105+
// * If we're a default item (IsDefaultItem=true), compute
106+
// relative to the user's project file (because the file that
107+
// declared the item is our Microsoft.Sdk.DefaultItems.template.props file,
108+
// and the path relative to that file is certainly not what we want).
109+
//
110+
// We use the 'LocalMSBuildProjectFullPath' and
111+
// 'LocalDefiningProjectFullPath' metadata because the
112+
// 'MSBuildProjectFullPath' and 'DefiningProjectFullPath' are not
113+
// necessarily correct when building remotely (the relative path
114+
// between files might not be the same on macOS once XVS has
115+
// copied them there, in particular for files outside the project
116+
// directory).
117+
//
118+
// The 'LocalMSBuildProjectFullPath' and 'LocalDefiningProjectFullPath'
119+
// values are set to the Windows version of 'MSBuildProjectFullPath'
120+
// and 'DefiningProjectFullPath' when building remotely, and the macOS
121+
// version when building on macOS.
122+
123+
// First find the absolute path to the item
124+
var projectAbsoluteDir = task.ProjectDir;
125+
var isRemoteBuild = !string.IsNullOrEmpty (task.SessionId);
126+
string itemAbsolutePath;
127+
if (isRemoteBuild) {
128+
itemAbsolutePath = PathUtils.PathCombineWindows (projectAbsoluteDir, item.ItemSpec);
129+
} else {
130+
itemAbsolutePath = Path.Combine (projectAbsoluteDir, item.ItemSpec);
131+
}
132+
var originalItemAbsolutePath = itemAbsolutePath;
84133

85-
if (!string.IsNullOrEmpty (definingProjectFullPath)) {
86-
baseDir = Path.GetDirectoryName (definingProjectFullPath);
134+
// Then find the directory we should use to compute the result relative to.
135+
string relativeToDirectory; // this is an absolute path.
136+
if (isDefaultItem) {
137+
relativeToDirectory = Path.GetDirectoryName (localMSBuildProjectFullPath);
87138
} else {
88-
baseDir = projectDir;
139+
relativeToDirectory = Path.GetDirectoryName (localDefiningProjectFullPath);
89140
}
141+
var originalRelativeToDirectory = relativeToDirectory;
90142

91-
baseDir = PathUtils.ResolveSymbolicLinks (baseDir);
92-
path = PathUtils.ResolveSymbolicLinks (path);
143+
// On macOS we need to resolve symlinks before computing the relative path.
144+
if (!isRemoteBuild) {
145+
relativeToDirectory = PathUtils.ResolveSymbolicLinks (relativeToDirectory);
146+
itemAbsolutePath = PathUtils.ResolveSymbolicLinks (itemAbsolutePath);
147+
}
93148

94-
return PathUtils.AbsoluteToRelative (baseDir, path);
149+
// Compute the relative path we want to return.
150+
string rv;
151+
if (isRemoteBuild) {
152+
rv = PathUtils.AbsoluteToRelativeWindows (relativeToDirectory, itemAbsolutePath);
153+
} else {
154+
rv = PathUtils.AbsoluteToRelative (relativeToDirectory, itemAbsolutePath);
155+
}
156+
// Make it a mac-style path
157+
rv = rv.Replace ('\\', '/');
158+
159+
Trace (task, $"BundleResource.GetVirtualProjectPath ({item.ItemSpec}) => {rv}\n" +
160+
$"\t\t\tprojectAbsoluteDir={projectAbsoluteDir}\n" +
161+
$"\t\t\tIsRemoteBuild={isRemoteBuild}\n" +
162+
$"\t\t\tisDefaultItem={isDefaultItem}\n" +
163+
$"\t\t\tLocalMSBuildProjectFullPath={localMSBuildProjectFullPath}\n" +
164+
$"\t\t\tLocalDefiningProjectFullPath={localDefiningProjectFullPath}\n" +
165+
$"\t\t\toriginalItemAbsolutePath={originalItemAbsolutePath}\n" +
166+
$"\t\t\titemAbsolutePath={itemAbsolutePath}\n" +
167+
$"\t\t\toriginalRelativeToDirectory={originalRelativeToDirectory}\n" +
168+
$"\t\t\trelativeToDirectory={relativeToDirectory}\n");
169+
170+
return rv;
95171
}
96172

97-
public static string GetLogicalName (string projectDir, IList<string> prefixes, ITaskItem item, bool isVSBuild)
173+
public static string GetLogicalName<T> (T task, ITaskItem item) where T: Task, IHasProjectDir, IHasResourcePrefix, IHasSessionId
98174
{
99175
var logicalName = item.GetMetadata ("LogicalName");
100176

177+
// If an item has the LogicalName metadata set, return that.
101178
if (!string.IsNullOrEmpty (logicalName)) {
102-
if (Path.DirectorySeparatorChar != '\\')
103-
return logicalName.Replace ('\\', '/');
104-
105-
return logicalName;
179+
Trace (task, $"BundleResource.GetLogicalName ({item.ItemSpec}) => has LogicalName={logicalName.Replace ('\\', '/')} (original {logicalName})");
180+
// Canonicalize to use macOS-style directory separators.
181+
return logicalName.Replace ('\\', '/');
106182
}
107183

108-
var vpath = GetVirtualProjectPath (projectDir, item, isVSBuild);
184+
// Check if the start of the item matches any of the resource prefixes, in which case choose
185+
// the longest resource prefix, and subtract it from the start of the item.
186+
var vpath = GetVirtualProjectPath (task, item);
109187
int matchlen = 0;
110-
188+
var prefixes = SplitResourcePrefixes (task.ResourcePrefix);
111189
foreach (var prefix in prefixes) {
112190
if (vpath.StartsWith (prefix, StringComparison.OrdinalIgnoreCase) && prefix.Length > matchlen)
113191
matchlen = prefix.Length;
114192
}
115-
116-
if (matchlen > 0)
193+
if (matchlen > 0) {
194+
Trace (task, $"BundleResource.GetLogicalName ({item.ItemSpec}) => LogicalName={vpath.Substring (matchlen)} (vpath={vpath} matchlen={matchlen} prefixes={string.Join (",", prefixes)})");
117195
return vpath.Substring (matchlen);
196+
}
118197

198+
// Otherwise return the item as-is.
199+
Trace (task, $"BundleResource.GetLogicalName ({item.ItemSpec}) => LogicalName={vpath} (prefixes={string.Join (",", prefixes)})");
119200
return vpath;
120201
}
121202
}

‎msbuild/Xamarin.MacDev.Tasks/Tasks/ACTool.cs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ bool IsMessagesExtension {
7474

7575
protected override void AppendCommandLineArguments (IDictionary<string, string?> environment, CommandLineArgumentBuilder args, ITaskItem [] items)
7676
{
77-
var assetDirs = new HashSet<string> (items.Select (x => BundleResource.GetVirtualProjectPath (ProjectDir, x, !string.IsNullOrEmpty (SessionId))));
77+
var assetDirs = new HashSet<string> (items.Select (x => BundleResource.GetVirtualProjectPath (this, x)));
7878

7979
if (!string.IsNullOrEmpty (XSAppIconAssets) && !string.IsNullOrEmpty (AppIcon)) {
8080
Log.LogError (MSBStrings.E7129 /* Can't specify both 'XSAppIconAssets' in the Info.plist and 'AppIcon' in the project file. Please select one or the other. */);
@@ -257,7 +257,7 @@ public override bool Execute ()
257257
var specs = new PArray ();
258258

259259
for (int i = 0; i < ImageAssets.Length; i++) {
260-
var vpath = BundleResource.GetVirtualProjectPath (ProjectDir, ImageAssets [i], !string.IsNullOrEmpty (SessionId));
260+
var vpath = BundleResource.GetVirtualProjectPath (this, ImageAssets [i]);
261261

262262
// Ignore MacOS .DS_Store files...
263263
if (Path.GetFileName (vpath).Equals (".DS_Store", StringComparison.OrdinalIgnoreCase))
@@ -298,7 +298,7 @@ public override bool Execute ()
298298
items.Clear ();
299299

300300
for (int i = 0; i < ImageAssets.Length; i++) {
301-
var vpath = BundleResource.GetVirtualProjectPath (ProjectDir, ImageAssets [i], !string.IsNullOrEmpty (SessionId));
301+
var vpath = BundleResource.GetVirtualProjectPath (this, ImageAssets [i]);
302302
var clone = false;
303303
ITaskItem item;
304304

@@ -349,7 +349,7 @@ public override bool Execute ()
349349

350350
// Note: `items` contains only the Contents.json files at this point
351351
for (int i = 0; i < items.Count; i++) {
352-
var vpath = BundleResource.GetVirtualProjectPath (ProjectDir, items [i], !string.IsNullOrEmpty (SessionId));
352+
var vpath = BundleResource.GetVirtualProjectPath (this, items [i]);
353353
var path = items [i].GetMetadata ("FullPath");
354354

355355
// get the parent (which will typically be .appiconset, .launchimage, .imageset, .iconset, etc)

‎msbuild/Xamarin.MacDev.Tasks/Tasks/CollectBundleResources.cs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
using System;
22
using System.IO;
33
using System.Collections.Generic;
4+
using System.Diagnostics.CodeAnalysis;
45

56
using Microsoft.Build.Framework;
67
using Microsoft.Build.Utilities;
@@ -9,7 +10,7 @@
910
using Xamarin.Utils;
1011

1112
namespace Xamarin.MacDev.Tasks {
12-
public class CollectBundleResources : XamarinTask, ICancelableTask {
13+
public class CollectBundleResources : XamarinTask, ICancelableTask, IHasProjectDir, IHasResourcePrefix {
1314
#region Inputs
1415

1516
public ITaskItem [] BundleResources { get; set; } = Array.Empty<ITaskItem> ();
@@ -64,7 +65,6 @@ public override bool Execute ()
6465

6566
bool ExecuteImpl ()
6667
{
67-
var prefixes = BundleResource.SplitResourcePrefixes (ResourcePrefix);
6868
var bundleResources = new List<ITaskItem> ();
6969

7070
foreach (var item in BundleResources) {
@@ -73,7 +73,7 @@ bool ExecuteImpl ()
7373
if (!string.IsNullOrEmpty (publishFolderType))
7474
continue;
7575

76-
var logicalName = BundleResource.GetLogicalName (ProjectDir, prefixes, item, !string.IsNullOrEmpty (SessionId));
76+
var logicalName = BundleResource.GetLogicalName (this, item);
7777
// We need a physical path here, ignore the Link element
7878
var path = item.GetMetadata ("FullPath");
7979

‎msbuild/Xamarin.MacDev.Tasks/Tasks/CompileAppManifest.cs‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
#nullable enable
1414

1515
namespace Xamarin.MacDev.Tasks {
16-
public class CompileAppManifest : XamarinTask, ITaskCallback, ICancelableTask {
16+
public class CompileAppManifest : XamarinTask, IHasProjectDir, IHasResourcePrefix, ITaskCallback, ICancelableTask {
1717
#region Inputs
1818

1919
// Single-project property that maps to CFBundleIdentifier for Apple platforms
@@ -212,10 +212,9 @@ void RegisterFonts (PDictionary plist)
212212
// https://developer.apple.com/documentation/swiftui/applying-custom-fonts-to-text
213213

214214
// Compute the relative location in the app bundle for each font file
215-
var prefixes = BundleResource.SplitResourcePrefixes (ResourcePrefix);
216215
const string logicalNameKey = "_ComputedLogicalName_";
217216
foreach (var item in FontFilesToRegister) {
218-
var logicalName = BundleResource.GetLogicalName (ProjectDir, prefixes, item, !string.IsNullOrEmpty (SessionId));
217+
var logicalName = BundleResource.GetLogicalName (this, item);
219218
item.SetMetadata (logicalNameKey, logicalName);
220219
}
221220

‎msbuild/Xamarin.MacDev.Tasks/Tasks/CompileSceneKitAssets.cs‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
#nullable disable
1717

1818
namespace Xamarin.MacDev.Tasks {
19-
public class CompileSceneKitAssets : XamarinTask, ICancelableTask {
19+
public class CompileSceneKitAssets : XamarinTask, ICancelableTask, IHasProjectDir, IHasResourcePrefix {
2020
string toolExe;
2121

2222
#region Inputs
@@ -130,7 +130,6 @@ public override bool Execute ()
130130
return taskRunner.RunAsync (this).Result;
131131
}
132132

133-
var prefixes = BundleResource.SplitResourcePrefixes (ResourcePrefix);
134133
var intermediate = Path.Combine (IntermediateOutputPath, ToolName, AppBundleName);
135134
var bundleResources = new List<ITaskItem> ();
136135
var modified = new HashSet<string> ();
@@ -150,7 +149,7 @@ public override bool Execute ()
150149

151150
asset.RemoveMetadata ("LogicalName");
152151

153-
var bundleName = BundleResource.GetLogicalName (ProjectDir, prefixes, asset, !string.IsNullOrEmpty (SessionId));
152+
var bundleName = BundleResource.GetLogicalName (this, asset);
154153
var output = new TaskItem (Path.Combine (intermediate, bundleName));
155154

156155
if (!modified.Contains (scnassets) && (!File.Exists (output.ItemSpec) || File.GetLastWriteTimeUtc (asset.ItemSpec) > File.GetLastWriteTimeUtc (output.ItemSpec))) {
@@ -202,7 +201,7 @@ public override bool Execute ()
202201

203202
var tasks = new List<Task> ();
204203
foreach (var item in items) {
205-
var bundleDir = BundleResource.GetLogicalName (ProjectDir, prefixes, new TaskItem (item), !string.IsNullOrEmpty (SessionId));
204+
var bundleDir = BundleResource.GetLogicalName (this, new TaskItem (item));
206205
var output = Path.Combine (intermediate, bundleDir);
207206

208207
tasks.Add (CopySceneKitAssets (item.ItemSpec, output, intermediate));

‎msbuild/Xamarin.MacDev.Tasks/Tasks/CoreMLCompiler.cs‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
#nullable disable
1313

1414
namespace Xamarin.MacDev.Tasks {
15-
public class CoreMLCompiler : XamarinTask, ICancelableTask {
15+
public class CoreMLCompiler : XamarinTask, ICancelableTask, IHasProjectDir, IHasResourcePrefix {
1616
string toolExe;
1717

1818
public string ToolName { get { return "coremlc"; } }
@@ -162,7 +162,6 @@ public override bool Execute ()
162162
return new TaskRunner (SessionId, BuildEngine4).RunAsync (this).Result;
163163

164164
var coremlcOutputDir = Path.Combine (IntermediateOutputPath, "coremlc");
165-
var prefixes = BundleResource.SplitResourcePrefixes (ResourcePrefix);
166165
var mapping = new Dictionary<string, IDictionary> ();
167166
var bundleResources = new List<ITaskItem> ();
168167
var partialPlists = new List<ITaskItem> ();
@@ -171,7 +170,7 @@ public override bool Execute ()
171170
Directory.CreateDirectory (coremlcOutputDir);
172171

173172
foreach (var model in Models) {
174-
var logicalName = BundleResource.GetLogicalName (ProjectDir, prefixes, model, !string.IsNullOrEmpty (SessionId));
173+
var logicalName = BundleResource.GetLogicalName (this, model);
175174
var bundleName = GetPathWithoutExtension (logicalName) + ".mlmodelc";
176175
var outputPath = Path.Combine (coremlcOutputDir, bundleName);
177176
var outputDir = Path.GetDirectoryName (outputPath);

‎msbuild/Xamarin.MacDev.Tasks/Tasks/FindItemWithLogicalName.cs‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
#nullable disable
99

1010
namespace Xamarin.MacDev.Tasks {
11-
public class FindItemWithLogicalName : XamarinTask {
11+
public class FindItemWithLogicalName : XamarinTask, IHasProjectDir, IHasResourcePrefix {
1212
#region Inputs
1313

1414
[Required]
@@ -34,10 +34,8 @@ public class FindItemWithLogicalName : XamarinTask {
3434
public override bool Execute ()
3535
{
3636
if (Items is not null) {
37-
var prefixes = BundleResource.SplitResourcePrefixes (ResourcePrefix);
38-
3937
foreach (var item in Items) {
40-
var logical = BundleResource.GetLogicalName (ProjectDir, prefixes, item, !string.IsNullOrEmpty (SessionId));
38+
var logical = BundleResource.GetLogicalName (this, item);
4139

4240
if (logical == LogicalName) {
4341
Log.LogMessage (MessageImportance.Low, MSBStrings.M0149, LogicalName, item.ItemSpec);

0 commit comments

Comments
 (0)