Skip to content

Handle renamed or deleted MauiImage files #13251

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

Merged
merged 8 commits into from
Feb 27, 2023
+135 −6
Merged

Handle renamed or deleted MauiImage files #13251

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
79babb8
Handle renamed or deleted MauiImage files
dellis1972 Feb 10, 2023
7be55f8
Fix build errors
dellis1972 Feb 10, 2023
d197115
Fix NRE
dellis1972 Feb 10, 2023
689aa0a
Back out the zip changes for now
dellis1972 Feb 13, 2023
5b8bbf1
Add Unit Tests
dellis1972 Feb 13, 2023
472792b
Change to use Count
dellis1972 Feb 15, 2023
49f088e
Auto-format source code
Feb 15, 2023
dfba4e8
Update src/SingleProject/Resizetizer/src/nuget/buildTransitive/Micros…
dellis1972 Feb 21, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion src/SingleProject/Resizetizer/src/ResizetizeImages.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,9 @@ public override System.Threading.Tasks.Task ExecuteAsync()
if (PlatformType == "tizen")
{
var tizenResourceXmlGenerator = new TizenResourceXmlGenerator(IntermediateOutputPath, Logger);
tizenResourceXmlGenerator.Generate();
var r = tizenResourceXmlGenerator.Generate();
if (r is not null)
resizedImages.Add(r);
}

var copiedResources = new List<TaskItem>();
Expand Down Expand Up @@ -178,10 +180,13 @@ void ProcessAppIcon(ResizeImageInfo img, ConcurrentBag<ResizedImageInfo> resized
if (destinationModified > sourceModified)
{
Logger.Log($"Skipping `{img.Filename}` => `{destination}` file is up to date.");
resizedImages.Add(new ResizedImageInfo() { Dpi = dpi, Filename = destination });
continue;
}

appTool.Resize(dpi, destination);
var r = appTool.Resize(dpi, destination);
resizedImages.Add(r);
}
}

Expand Down
8 changes: 5 additions & 3 deletions src/SingleProject/Resizetizer/src/TizenResourceXmlGenerator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ public TizenResourceXmlGenerator(string intermediateOutputPath, ILogger logger)

public ILogger Logger { get; private set; }

public void Generate()
public ResizedImageInfo Generate()
{
XmlDocument doc = new XmlDocument();
XmlNode docNode = doc.CreateXmlDeclaration("1.0", "UTF-8", "yes");
Expand All @@ -54,12 +54,13 @@ public void Generate()

string outputResourceDir = Path.Combine(IntermediateOutputPath, "res");
string outputContentsDir = Path.Combine(outputResourceDir, "contents");
string destination = Path.Combine(outputResourceDir, "res.xml");

var contentsDirInfo = new DirectoryInfo(outputContentsDir);
if (!contentsDirInfo.Exists)
{
Logger.Log("No 'res/contents/' directory to generate res.xml.");
return;
return null;
}
foreach (DirectoryInfo subDir in contentsDirInfo.GetDirectories())
{
Expand All @@ -80,8 +81,9 @@ public void Generate()
}
}
}
doc.Save(Path.Combine(outputResourceDir, "res.xml"));
doc.Save(destination);
Logger.Log($"res.xml file has been saved in {outputResourceDir}");
return new ResizedImageInfo() { Dpi = DpiPath.Tizen.Original, Filename = destination };
}
}
}
15 changes: 13 additions & 2 deletions ...gleProject/Resizetizer/src/nuget/buildTransitive/Microsoft.Maui.Resizetizer.After.targets
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -386,6 +386,7 @@
MauiSplashScreen="@(MauiSplashScreen)"
InputsFile="$(_MauiSplashInputsFile)"
/>

<ItemGroup Condition="'$(_ResizetizerIsAndroidApp)' == 'True' and '$(_MauiHasSplashScreens)' == 'true'">
<_MauiSplashAssets Include="$(_MauiIntermediateSplashScreen)**\*" />
<LibraryResourceDirectories Condition="Exists('$(_MauiIntermediateSplashScreen)')" Include="$(_MauiIntermediateSplashScreen)">
Expand Down Expand Up @@ -579,15 +580,25 @@
IntermediateOutputPath="$(_MauiIntermediateImages)"
InputsFile="$(_ResizetizerInputsFile)"
Images="@(MauiImage->Distinct())">
<Output TaskParameter="CopiedResources" ItemName="_CopiedResources" />
</ResizetizeImages>

<ItemGroup>
<!-- Get Images that were generated -->
<!-- Either from the task, or if the task was skipped (up to date), use the wildcard lookup -->
<_ResizetizerCollectedImages Condition="'@(CopiedResources)' != ''" Include="@(CopiedResources)" />
<_ResizetizerCollectedImages Condition="'@(CopiedResources)' == ''" Include="$(_MauiIntermediateImages)**\*" />
<_ResizetizerCollectedImages Condition="'@(_CopiedResources->Count())' != '0'" Include="@(_CopiedResources)" />
<_ResizetizerExistingImages Include="$(_MauiIntermediateImages)\**\*" />
Copy link
Member

@jonathanpeppers jonathanpeppers Feb 21, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only other thing, is it possible for $(_MauiIntermediateImages) to ever be blank?

If it ever was, this would wildcard your entire drive like \**\*.

Copy link
Contributor Author

@dellis1972 dellis1972 Feb 22, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is set here

maui/src/SingleProject/Resizetizer/src/nuget/buildTransitive/Microsoft.Maui.Resizetizer.After.targets

Line 71 in dfba4e8

<_MauiIntermediateImages>$(_ResizetizerIntermediateOutputRoot)r\</_MauiIntermediateImages>
so it should always be set.

Copy link
Member

@jonathanpeppers jonathanpeppers Feb 22, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, we could rely on it ending with a trailing \, then at worst it would wildcard **\* in the current directory instead of the whole drive.

But this is kind of unrelated to this change, we don't really have to fix it here.

<_ResizetizerImagesToDelete Include="@(_ResizetizerExistingImages->'%(FullPath)')" />
<_ResizetizerCollectedImages Condition="'@(_CopiedResources)' == ''" Include="@(_ResizetizerExistingImages->'%(FullPath)')" />
Comment on lines +591 to +592
Copy link
Member

@jonathanpeppers jonathanpeppers Feb 21, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do these actually need ->'%(FullPath)'? I think the <Delete/> task below is all that uses these item groups? It can use the relative path.

Copy link
Contributor Author

@dellis1972 dellis1972 Feb 21, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have to use FullPath because the wildcard Include in the case where the task is skipped returns full paths. In that case comparing with relative ones causes nothing to match and all the files get deleted. So we need to compare like with like.

<_ResizetizerImagesToDelete Remove="@(_ResizetizerCollectedImages)" />
</ItemGroup>

<!-- Remove files which are no longer needed -->
<Delete
Condition="'@(_ResizetizerImagesToDelete->Count())' != '0'"
Files="@(_ResizetizerImagesToDelete)"
/>

<!-- iOS -->
<ItemGroup Condition="'$(_ResizetizerIsiOSApp)' == 'True'">
<!-- Batch the collectd items into BundleResource which iOS expects -->
Expand Down
111 changes: 111 additions & 0 deletions src/SingleProject/Resizetizer/test/UnitTests/ResizetizeImagesTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -676,6 +676,43 @@ public void DiffPropoprtionAppIconWithoutBaseUseBackgroundSize(double fgScale, s
AssertFileMatches($"mipmap-xhdpi/{Path.GetFileNameWithoutExtension(bg)}_foreground.png", new object[] { fgScale, bg, fg, "xh", "f" });
}

[Fact]
public void NonExistantFilesAreDeleted()
{
var items = new[]
{
new TaskItem("images/camera.svg", new Dictionary<string, string>
{
["Link"] = "dog",
}),
};

var task = GetNewTask(items);
var success = task.Execute();
Assert.True(success, LogErrorEvents.FirstOrDefault()?.Message);

AssertFileNotExists("drawable-hdpi/cat.png");
AssertFileExists("drawable-hdpi/dog.png");

LogErrorEvents.Clear();
LogMessageEvents.Clear();

items = new[]
{
new TaskItem("images/camera.svg", new Dictionary<string, string>
{
["Link"] = "cat",
}),
};

task = GetNewTask(items);
success = task.Execute();
Assert.True(success, LogErrorEvents.FirstOrDefault()?.Message);

AssertFileNotExists("drawable-hdpi/dot.png");
AssertFileExists("drawable-hdpi/cat.png");
}

//[Theory]
//[InlineData(1, 1, "dotnet_background.svg", "tall_image.png", 300, 300)]
//[InlineData(1, 1, "dotnet_background.svg", "wide_image.png", 300, 300)]
Expand Down Expand Up @@ -1015,6 +1052,43 @@ public void SingleVectorAppIconWithOnlyPathSucceedsWithVectors(string name, stri
$"\"filename\": \"{outputName}[email protected]\"",
$"\"size\": \"20x20\",");
}

[Fact]
public void NonExistantFilesAreDeleted()
{
var items = new[]
{
new TaskItem("images/camera.svg", new Dictionary<string, string>
{
["Link"] = "dog",
}),
};

var task = GetNewTask(items);
var success = task.Execute();
Assert.True(success, LogErrorEvents.FirstOrDefault()?.Message);

AssertFileNotExists("cat.png");
AssertFileExists("dog.png");

LogErrorEvents.Clear();
LogMessageEvents.Clear();

items = new[]
{
new TaskItem("images/camera.svg", new Dictionary<string, string>
{
["Link"] = "cat",
}),
};

task = GetNewTask(items);
success = task.Execute();
Assert.True(success, LogErrorEvents.FirstOrDefault()?.Message);

AssertFileNotExists("dot.png");
AssertFileExists("cat.png");
}
}

public class ExecuteForWindows : ExecuteForApp
Expand Down Expand Up @@ -1335,6 +1409,43 @@ public void ColorsInCssCanBeUsed()

AssertFileContains("not_working.scale-100.png", 0xFF71559B, 2, 6);
}

[Fact]
public void NonExistantFilesAreDeleted()
{
var items = new[]
{
new TaskItem("images/camera.svg", new Dictionary<string, string>
{
["Link"] = "dog",
}),
};

var task = GetNewTask(items);
var success = task.Execute();
Assert.True(success, LogErrorEvents.FirstOrDefault()?.Message);

AssertFileNotExists("cat.scale-150.png");
AssertFileExists("dog.scale-150.png");

LogErrorEvents.Clear();
LogMessageEvents.Clear();

items = new[]
{
new TaskItem("images/camera.svg", new Dictionary<string, string>
{
["Link"] = "cat",
}),
};

task = GetNewTask(items);
success = task.Execute();
Assert.True(success, LogErrorEvents.FirstOrDefault()?.Message);

AssertFileNotExists("dot.scale-150.png");
AssertFileExists("cat.scale-150.png");
}
}

public class ExecuteForAny : ExecuteForApp
Expand Down