Skip to content

Commit 72f2b54

Browse files
azchohfiazchohfi
authored
Fixed bug where cert was always re-generated on restore and improved logging. (#251)
## Description Improve logs for restore and fixed a bug where cert was always re-generated on restore even if the cert file already existed. ## Related Issue Fixes #245 ## Type of Change <!-- Mark the appropriate option(s) with an "x" --> - [x] 🐛 Bug fix (non-breaking change that fixes an issue) - [ ] ✨ New feature (non-breaking change that adds functionality) - [ ] 💥 Breaking change (fix or feature that would cause existing functionality to change) - [ ] 📝 Documentation update - [ ] 🔧 Configuration/build change - [ ] ♻️ Refactoring (no functional changes) - [ ] 🧪 Test update ## Checklist ### Build & Tests - [X] All CI builds pass (Build and Package workflow) - [X] All existing tests pass - [ ] New tests added for new functionality (if applicable) - [X] Tested locally on Windows ### README & Guides <!-- Check all that apply to your changes --> - [ ] Main [README.md](../README.md) updated (if applicable) - [ ] [docs/usage.md](../docs/usage.md) updated (if CLI commands changed) - [ ] [Language-specific guides](../docs/guides) updated (if applicable) - [ ] [Sample projects updated](../samples) to reflect changes (if applicable) - [X] No documentation updates needed
1 parent bff7f5a commit 72f2b54

File tree

5 files changed

+117
-99
lines changed

5 files changed

+117
-99
lines changed

‎src/winapp-CLI/WinApp.Cli.Tests/DirectoryPackagesServiceTests.cs‎

Lines changed: 14 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -39,19 +39,17 @@ public void Cleanup()
3939
}
4040

4141
[TestMethod]
42-
public void UpdatePackageVersionsNoFileExistsReturnsFalse()
42+
public void UpdatePackageVersionsNoFileExistsThrowsFileNotFoundException()
4343
{
4444
// Arrange
4545
var packageVersions = new Dictionary<string, string>
4646
{
4747
{ "Microsoft.Windows.SDK.BuildTools", "10.0.22621.3233" }
4848
};
4949

50-
// Act
51-
var result = _directoryPackagesService.UpdatePackageVersions(new DirectoryInfo(_testTempDirectory), packageVersions, TestTaskContext);
52-
53-
// Assert
54-
Assert.IsFalse(result, "Should return false when Directory.Packages.props doesn't exist");
50+
// Act & Assert
51+
Assert.ThrowsExactly<FileNotFoundException>(() =>
52+
_directoryPackagesService.UpdatePackageVersions(new DirectoryInfo(_testTempDirectory), packageVersions, TestTaskContext));
5553
}
5654

5755
[TestMethod]
@@ -148,7 +146,7 @@ public void UpdatePackageVersionsPreservesWhitespaceReturnsTrue()
148146
}
149147

150148
[TestMethod]
151-
public void UpdatePackageVersionsNoChangesNeededReturnsTrue()
149+
public void UpdatePackageVersionsNoChangesNeededReturnsFalse()
152150
{
153151
// Arrange
154152
var propsFilePath = Path.Combine(_testTempDirectory, "Directory.Packages.props");
@@ -168,7 +166,7 @@ public void UpdatePackageVersionsNoChangesNeededReturnsTrue()
168166
var result = _directoryPackagesService.UpdatePackageVersions(new DirectoryInfo(_testTempDirectory), packageVersions, TestTaskContext);
169167

170168
// Assert
171-
Assert.IsTrue(result, "Should return true even when no changes are needed");
169+
Assert.IsFalse(result, "Should return false when no changes are needed");
172170

173171
var updatedContent = File.ReadAllText(propsFilePath);
174172
Assert.AreEqual(originalContent, updatedContent, "Content should remain unchanged");
@@ -206,7 +204,7 @@ public void UpdatePackageVersionsPartialMatchUpdatesOnlyMatchingPackages()
206204
}
207205

208206
[TestMethod]
209-
public void UpdatePackageVersionsEmptyFileReturnsFalse()
207+
public void UpdatePackageVersionsEmptyFileThrowsXmlException()
210208
{
211209
// Arrange
212210
var propsFilePath = Path.Combine(_testTempDirectory, "Directory.Packages.props");
@@ -217,15 +215,13 @@ public void UpdatePackageVersionsEmptyFileReturnsFalse()
217215
{ "Microsoft.Windows.SDK.BuildTools", "10.0.22621.3233" }
218216
};
219217

220-
// Act
221-
var result = _directoryPackagesService.UpdatePackageVersions(new DirectoryInfo(_testTempDirectory), packageVersions, TestTaskContext);
222-
223-
// Assert
224-
Assert.IsFalse(result, "Should return false for empty/invalid XML file");
218+
// Act & Assert
219+
Assert.ThrowsExactly<System.Xml.XmlException>(() =>
220+
_directoryPackagesService.UpdatePackageVersions(new DirectoryInfo(_testTempDirectory), packageVersions, TestTaskContext));
225221
}
226222

227223
[TestMethod]
228-
public void UpdatePackageVersionsNoPackageVersionElementsReturnsFalse()
224+
public void UpdatePackageVersionsNoPackageVersionElementsThrowsInvalidOperationException()
229225
{
230226
// Arrange
231227
var propsFilePath = Path.Combine(_testTempDirectory, "Directory.Packages.props");
@@ -241,11 +237,9 @@ public void UpdatePackageVersionsNoPackageVersionElementsReturnsFalse()
241237
{ "Microsoft.Windows.SDK.BuildTools", "10.0.22621.3233" }
242238
};
243239

244-
// Act
245-
var result = _directoryPackagesService.UpdatePackageVersions(new DirectoryInfo(_testTempDirectory), packageVersions, TestTaskContext);
246-
247-
// Assert
248-
Assert.IsFalse(result, "Should return false when no PackageVersion elements exist");
240+
// Act & Assert
241+
Assert.ThrowsExactly<InvalidOperationException>(() =>
242+
_directoryPackagesService.UpdatePackageVersions(new DirectoryInfo(_testTempDirectory), packageVersions, TestTaskContext));
249243
}
250244

251245
[TestMethod]

‎src/winapp-CLI/WinApp.Cli/Services/DirectoryPackagesService.cs‎

Lines changed: 50 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -25,100 +25,86 @@ public bool Exists(DirectoryInfo configDir)
2525
/// </summary>
2626
/// <param name="configDir">Directory containing winapp.yaml and potentially Directory.Packages.props</param>
2727
/// <param name="packageVersions">Dictionary of package names to versions from winapp.yaml</param>
28-
/// <returns>True if file was found and updated, false otherwise</returns>
28+
/// <returns>True if any package versions were changed, false if no changes were needed</returns>
29+
/// <exception cref="FileNotFoundException">Thrown when Directory.Packages.props does not exist</exception>
30+
/// <exception cref="InvalidOperationException">Thrown when the file is invalid or has no PackageVersion elements</exception>
2931
public bool UpdatePackageVersions(DirectoryInfo configDir, Dictionary<string, string> packageVersions, TaskContext taskContext)
3032
{
3133
var propsFilePath = Path.Combine(configDir.FullName, DirectoryPackagesFileName);
3234

3335
if (!File.Exists(propsFilePath))
3436
{
35-
taskContext.AddDebugMessage($"{UiSymbols.Note} No {DirectoryPackagesFileName} found in {configDir.FullName}");
36-
return false;
37+
throw new FileNotFoundException($"No {DirectoryPackagesFileName} found in {configDir.FullName}", propsFilePath);
38+
}
39+
40+
taskContext.AddStatusMessage($"{UiSymbols.Wrench} Updating {DirectoryPackagesFileName} to match winapp.yaml versions...");
41+
42+
// Load the XML document with whitespace preservation
43+
var doc = new XmlDocument();
44+
doc.PreserveWhitespace = true;
45+
doc.Load(propsFilePath);
46+
47+
if (doc.DocumentElement == null)
48+
{
49+
throw new InvalidOperationException($"{DirectoryPackagesFileName} has no root element");
3750
}
3851

39-
try
52+
// Find all PackageVersion elements using XPath
53+
var packageVersionNodes = doc.SelectNodes("//PackageVersion");
54+
55+
if (packageVersionNodes == null || packageVersionNodes.Count == 0)
4056
{
41-
taskContext.AddStatusMessage($"{UiSymbols.Wrench} Updating {DirectoryPackagesFileName} to match winapp.yaml versions...");
57+
throw new InvalidOperationException($"No PackageVersion elements found in {DirectoryPackagesFileName}");
58+
}
4259

43-
// Load the XML document with whitespace preservation
44-
var doc = new XmlDocument();
45-
doc.PreserveWhitespace = true;
46-
doc.Load(propsFilePath);
60+
var updated = 0;
4761

48-
if (doc.DocumentElement == null)
62+
foreach (XmlNode packageVersion in packageVersionNodes)
63+
{
64+
if (packageVersion.Attributes == null)
4965
{
50-
taskContext.AddStatusMessage($"{UiSymbols.Note} {DirectoryPackagesFileName} has no root element");
51-
return false;
66+
continue;
5267
}
5368

54-
// Find all PackageVersion elements using XPath
55-
var packageVersionNodes = doc.SelectNodes("//PackageVersion");
69+
var includeAttr = packageVersion.Attributes["Include"];
70+
var versionAttr = packageVersion.Attributes["Version"];
5671

57-
if (packageVersionNodes == null || packageVersionNodes.Count == 0)
72+
if (includeAttr == null || versionAttr == null)
5873
{
59-
taskContext.AddDebugMessage($"{UiSymbols.Note} No PackageVersion elements found in {DirectoryPackagesFileName}");
60-
return false;
74+
continue;
6175
}
6276

63-
var updated = 0;
77+
var packageName = includeAttr.Value;
6478

65-
foreach (XmlNode packageVersion in packageVersionNodes)
79+
// Check if this package is in our winapp.yaml config
80+
if (packageVersions.TryGetValue(packageName, out var newVersion))
6681
{
67-
if (packageVersion.Attributes == null)
68-
{
69-
continue;
70-
}
71-
72-
var includeAttr = packageVersion.Attributes["Include"];
73-
var versionAttr = packageVersion.Attributes["Version"];
82+
var oldVersion = versionAttr.Value;
7483

75-
if (includeAttr == null || versionAttr == null)
84+
if (oldVersion != newVersion)
7685
{
77-
continue;
86+
versionAttr.Value = newVersion;
87+
updated++;
88+
taskContext.AddStatusMessage($"{UiSymbols.Check} Updated {packageName}: {oldVersion} → {newVersion}");
7889
}
79-
80-
var packageName = includeAttr.Value;
81-
82-
// Check if this package is in our winapp.yaml config
83-
if (packageVersions.TryGetValue(packageName, out var newVersion))
90+
else
8491
{
85-
var oldVersion = versionAttr.Value;
86-
87-
if (oldVersion != newVersion)
88-
{
89-
versionAttr.Value = newVersion;
90-
updated++;
91-
taskContext.AddStatusMessage($"{UiSymbols.Check} Updated {packageName}: {oldVersion} → {newVersion}");
92-
}
93-
else
94-
{
95-
taskContext.AddDebugMessage($"{UiSymbols.Check} {packageName} already at version {newVersion}");
96-
}
92+
taskContext.AddDebugMessage($"{UiSymbols.Check} {packageName} already at version {newVersion}");
9793
}
9894
}
95+
}
9996

100-
if (updated > 0)
101-
{
102-
// Save the document - PreserveWhitespace will maintain original formatting
103-
doc.Save(propsFilePath);
97+
if (updated > 0)
98+
{
99+
// Save the document - PreserveWhitespace will maintain original formatting
100+
doc.Save(propsFilePath);
104101

105-
taskContext.AddStatusMessage($"{UiSymbols.Save} Updated {updated} package version(s) in {DirectoryPackagesFileName}");
106-
return true;
107-
}
108-
else
109-
{
110-
taskContext.AddStatusMessage($"{UiSymbols.Check} No package versions needed updating in {DirectoryPackagesFileName}");
111-
return true;
112-
}
102+
taskContext.AddStatusMessage($"{UiSymbols.Save} Updated {updated} package version(s) in {DirectoryPackagesFileName}");
103+
return true;
113104
}
114-
catch (Exception ex)
105+
else
115106
{
116-
taskContext.AddStatusMessage($"{UiSymbols.Note} Failed to update {DirectoryPackagesFileName}: {ex.Message}");
117-
if (ex.StackTrace != null)
118-
{
119-
taskContext.AddDebugMessage(ex.StackTrace);
120-
}
121-
107+
taskContext.AddStatusMessage($"{UiSymbols.Check} No package versions needed updating in {DirectoryPackagesFileName}");
122108
return false;
123109
}
124110
}

‎src/winapp-CLI/WinApp.Cli/Services/IDirectoryPackagesService.cs‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@ internal interface IDirectoryPackagesService
2222
/// </summary>
2323
/// <param name="configDir">Directory containing winapp.yaml and potentially Directory.Packages.props</param>
2424
/// <param name="packageVersions">Dictionary of package names to versions from winapp.yaml</param>
25-
/// <returns>True if file was found and updated, false otherwise</returns>
25+
/// <returns>True if any package versions were changed, false if no changes were needed</returns>
26+
/// <exception cref="FileNotFoundException">Thrown when Directory.Packages.props does not exist</exception>
27+
/// <exception cref="InvalidOperationException">Thrown when the file is invalid or has no PackageVersion elements</exception>
2628
bool UpdatePackageVersions(DirectoryInfo configDir, Dictionary<string, string> packageVersions, TaskContext taskContext);
2729
}

‎src/winapp-CLI/WinApp.Cli/Services/IWorkspaceSetupService.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,5 +9,5 @@ internal interface IWorkspaceSetupService
99
{
1010
public DirectoryInfo? FindWindowsAppSdkMsixDirectory(Dictionary<string, string>? usedVersions = null);
1111
public Task<int> SetupWorkspaceAsync(WorkspaceSetupOptions options, CancellationToken cancellationToken = default);
12-
public Task InstallWindowsAppRuntimeAsync(DirectoryInfo msixDir, TaskContext taskContext, CancellationToken cancellationToken);
12+
public Task<(int InstalledCount, int ErrorCount)> InstallWindowsAppRuntimeAsync(DirectoryInfo msixDir, TaskContext taskContext, CancellationToken cancellationToken);
1313
}

‎src/winapp-CLI/WinApp.Cli/Services/WorkspaceSetupService.cs‎

Lines changed: 49 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -54,15 +54,16 @@ public async Task<int> SetupWorkspaceAsync(WorkspaceSetupOptions options, Cancel
5454
configService.ConfigPath = new FileInfo(Path.Combine(options.ConfigDir.FullName, "winapp.yaml"));
5555

5656
bool hadExistingConfig = default;
57-
bool shouldGenerateManifest = true;
58-
bool shouldGenerateCert = !options.NoCert;
5957

60-
(var initializationResult, WinappConfig? config, hadExistingConfig, shouldGenerateManifest, var manifestGenerationInfo, shouldGenerateCert, bool shouldEnableDeveloperMode) = await InitializeConfigurationAsync(options, cancellationToken);
58+
59+
(var initializationResult, WinappConfig? config, hadExistingConfig, bool shouldGenerateManifest, var manifestGenerationInfo, bool shouldGenerateCert, bool shouldEnableDeveloperMode) = await InitializeConfigurationAsync(options, cancellationToken);
6160
if (initializationResult != 0)
6261
{
6362
return initializationResult;
6463
}
6564

65+
var certPath = new FileInfo(Path.Combine(options.BaseDirectory.FullName, CertificateService.DefaultCertFileName));
66+
6667
// Handle config-only mode: just create/validate config file and exit
6768
if (options.ConfigOnly)
6869
{
@@ -391,10 +392,20 @@ await taskContext.AddSubTaskAsync("Configuring developer mode", async (taskConte
391392
if (msixDir != null)
392393
{
393394
// Install Windows App SDK runtime packages
394-
await InstallWindowsAppRuntimeAsync(msixDir, taskContext, cancellationToken);
395+
(int installedCount, int errorCount) = await InstallWindowsAppRuntimeAsync(msixDir, taskContext, cancellationToken);
396+
397+
string version = usedVersions[BuildToolsService.WINAPP_SDK_RUNTIME_PACKAGE];
398+
399+
if (errorCount > 0)
400+
{
401+
return (1, "Some Windows App Runtime packages failed to install.");
402+
}
403+
else if (installedCount == 0)
404+
{
405+
return (0, "Windows App SDK Runtime ([underline]{version}[/]) already installed");
406+
}
395407

396-
taskContext.AddDebugMessage($"{UiSymbols.Check} Windows App Runtime installation complete");
397-
return (0, $"WinAppSDK Runtime installed: [underline]{usedVersions[BuildToolsService.WINAPP_SDK_RUNTIME_PACKAGE]}[/]");
408+
return (0, $"WinAppSDK Runtime installed: [underline]{version}[/]");
398409
}
399410
else
400411
{
@@ -472,8 +483,6 @@ await taskContext.AddSubTaskAsync("Saving configuration", (taskContext, cancella
472483
{
473484
await taskContext.AddSubTaskAsync("Generating development certificate", async (taskContext, cancellationToken) =>
474485
{
475-
var certPath = new FileInfo(Path.Combine(options.BaseDirectory.FullName, CertificateService.DefaultCertFileName));
476-
477486
var result = await certificateService.GenerateDevCertificateWithInferenceAsync(
478487
outputPath: certPath,
479488
taskContext: taskContext,
@@ -490,10 +499,27 @@ await taskContext.AddSubTaskAsync("Generating development certificate", async (t
490499
addedCertToGitignore = true;
491500
}
492501

502+
if (result == null || !result.CertificatePath.Exists)
503+
{
504+
return (1, "Development certificate generation failed.");
505+
}
506+
493507
return (0, $"Development certificate created: [underline]{certPath.Name}[/]");
494508

495509
}, cancellationToken);
496510
}
511+
else
512+
{
513+
// Should not generate cert
514+
if (certPath.Exists)
515+
{
516+
taskContext.AddStatusMessage($"{UiSymbols.Check} Development certificate already exists → {certPath.FullName}");
517+
}
518+
else
519+
{
520+
taskContext.AddStatusMessage($"{UiSymbols.Skip} Development certificate generation skipped");
521+
}
522+
}
497523
}
498524

499525
bool showedGitignoreMessage = false;
@@ -547,8 +573,10 @@ await taskContext.AddSubTaskAsync("Updating Directory.Packages.props", (taskCont
547573
p => p.Version,
548574
StringComparer.OrdinalIgnoreCase);
549575

550-
directoryPackagesService.UpdatePackageVersions(options.ConfigDir, packageVersions, taskContext);
551-
return Task.FromResult((0, "Directory.Packages.props updated"));
576+
var wasUpdated = directoryPackagesService.UpdatePackageVersions(options.ConfigDir, packageVersions, taskContext);
577+
return Task.FromResult((0, message: wasUpdated
578+
? "Directory.Packages.props updated"
579+
: "Directory.Packages.props is up to date"));
552580
}
553581
catch (Exception ex)
554582
{
@@ -650,6 +678,12 @@ await taskContext.AddSubTaskAsync("Updating Directory.Packages.props", (taskCont
650678
}
651679
}
652680
}
681+
else
682+
{
683+
var certPath = new FileInfo(Path.Combine(options.BaseDirectory.FullName, CertificateService.DefaultCertFileName));
684+
685+
shouldGenerateCert = !options.NoCert && !certPath.Exists;
686+
}
653687
}
654688
else
655689
{
@@ -886,15 +920,15 @@ public class MsixPackageEntry
886920
/// </summary>
887921
/// <param name="msixDir">Directory containing the MSIX packages</param>
888922
/// <param name="cancellationToken">Cancellation token</param>
889-
public async Task InstallWindowsAppRuntimeAsync(DirectoryInfo msixDir, TaskContext taskContext, CancellationToken cancellationToken)
923+
public async Task<(int InstalledCount, int ErrorCount)> InstallWindowsAppRuntimeAsync(DirectoryInfo msixDir, TaskContext taskContext, CancellationToken cancellationToken)
890924
{
891925
var architecture = GetSystemArchitecture();
892926

893927
// Get package entries from MSIX inventory
894928
var packageEntries = await ParseMsixInventoryAsync(taskContext, msixDir, cancellationToken);
895929
if (packageEntries == null || packageEntries.Count == 0)
896930
{
897-
return;
931+
return (0, 0);
898932
}
899933

900934
var msixArchDir = Path.Combine(msixDir.FullName, $"win10-{architecture}");
@@ -920,7 +954,7 @@ public async Task InstallWindowsAppRuntimeAsync(DirectoryInfo msixDir, TaskConte
920954

921955
if (packageData.Count == 0)
922956
{
923-
return;
957+
return (0, 0);
924958
}
925959

926960
// Create compact PowerShell script with reusable function
@@ -1033,6 +1067,8 @@ public async Task InstallWindowsAppRuntimeAsync(DirectoryInfo msixDir, TaskConte
10331067
{
10341068
taskContext.AddDebugMessage($"{UiSymbols.Note} {errorCount} packages failed to install");
10351069
}
1070+
1071+
return (installedCount, errorCount);
10361072
}
10371073

10381074
/// <summary>

0 commit comments

Comments
 (0)