Skip to content

Commit da42b1c

Browse files
JesseColnmetulevazchohfi
authored
When a tool fails, attempt to show lines that contain error information (#152)
Before this change, when there's an error with a tool it's unclear why it's happening. Users must know to re-run with "--verbose" option to see the error text. After this change, here's what the output looks like when there's an error in MakeAppx: ``` PS D:\temp\rigvgl1m.zgh> winapp.exe pack . [ERROR] - MakeAppx : error: Error info: error C00CE169: App manifest validation error: The app manifest must be valid as per schema: Line 38, Column 7, Reason: 'InvalidValue' violates enumeration constraint of 'appContainer mediumIL'. [ERROR] - The attribute '{http://schemas.microsoft.com/appx/manifest/uap/windows10/10}TrustLevel' with value 'InvalidValue' failed to parse. [ERROR] - MakeAppx : error: Package creation failed. [ERROR] - MakeAppx : error: 0x80080204 - The specified package format is not valid: The package manifest is not valid. [ERROR] - makeappx.exe reported an error. The above text is only part of the tool output. Please re-run with --verbose to see the full output. ❌ Failed to create MSIX package: Failed to create MSIX package: makeappx.exe execution failed with exit code 1 ``` Here's what it looks like when there's an error in MakePri: ``` PS D:\temp\rigvgl1m.zgh> winapp.exe pack . [ERROR] - ERROR: PRI191: 0x80004005 - Appx manifest not found or is invalid. Please ensure well-formed manifest file is present. Or specify an index name with /in switch. End tag 'InvalidEndTag' does not match the start tag 'InvalidTag'. [ERROR] - makepri.exe reported an error. The above text is only part of the tool output. Please re-run with --verbose to see the full output. ❌ Failed to create MSIX package: Failed to create MSIX package: Failed to generate PRI file: makepri.exe execution failed with exit code -2147467259 ``` --------- Co-authored-by: Nikola Metulev <[email protected]> Co-authored-by: Alexandre Zollinger Chohfi <[email protected]>
1 parent 3605370 commit da42b1c

File tree

9 files changed

+197
-15
lines changed

9 files changed

+197
-15
lines changed

src/winapp-CLI/WinApp.Cli.Tests/BuildToolsServiceTests.cs

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
// Copyright (c) Microsoft Corporation. All rights reserved.
22
// Licensed under the MIT License.
33

4+
using WinApp.Cli.Tools;
5+
46
namespace WinApp.Cli.Tests;
57

68
[TestClass]
@@ -157,7 +159,7 @@ public async Task RunBuildToolAsync_WithValidTool_ReturnsOutput()
157159
File.WriteAllText(fakeToolPath, "@echo Hello from fake tool");
158160

159161
// Act
160-
var (stdout, stderr) = await _buildToolsService.RunBuildToolAsync("echo.cmd", "", TestContext.CancellationToken);
162+
var (stdout, stderr) = await _buildToolsService.RunBuildToolAsync(new GenericTool("echo.cmd"), "", TestContext.CancellationToken);
161163

162164
// Assert
163165
Assert.Contains("Hello from fake tool", stdout);
@@ -173,7 +175,7 @@ public async Task RunBuildToolAsync_WithNonExistentTool_ThrowsFileNotFoundExcept
173175
// Act & Assert
174176
await Assert.ThrowsExactlyAsync<FileNotFoundException>(async () =>
175177
{
176-
await _buildToolsService.RunBuildToolAsync("nonexistent.exe", "", TestContext.CancellationToken);
178+
await _buildToolsService.RunBuildToolAsync(new GenericTool("nonexistent.exe"), "", TestContext.CancellationToken);
177179
});
178180
}
179181

@@ -320,7 +322,7 @@ public async Task RunBuildToolAsync_WithNoExistingPackage_AutoInstallsAndRuns()
320322
{
321323
// Create a simple batch command that outputs something
322324
// This will either succeed (if BuildTools installs successfully) or throw an exception
323-
await _buildToolsService.RunBuildToolAsync("echo.cmd", "test", TestContext.CancellationToken);
325+
await _buildToolsService.RunBuildToolAsync(new GenericTool("echo.cmd"), "test", TestContext.CancellationToken);
324326

325327
// If we reach here, the auto-installation worked - test passes
326328
}
@@ -347,7 +349,7 @@ public async Task RunBuildToolAsync_WithExistingTool_RunsDirectly()
347349
File.WriteAllText(batchFile, "@echo Hello from test tool");
348350

349351
// Act
350-
var (stdout, stderr) = await _buildToolsService.RunBuildToolAsync("test.cmd", "", TestContext.CancellationToken);
352+
var (stdout, stderr) = await _buildToolsService.RunBuildToolAsync(new GenericTool("test.cmd"), "", TestContext.CancellationToken);
351353

352354
// Assert
353355
Assert.Contains("Hello from test tool", stdout);

src/winapp-CLI/WinApp.Cli/Services/BuildToolsService.cs

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
using System.Text.RegularExpressions;
77
using WinApp.Cli.Helpers;
88
using WinApp.Cli.Models;
9+
using WinApp.Cli.Tools;
910

1011
namespace WinApp.Cli.Services;
1112

@@ -285,16 +286,16 @@ public async Task<FileInfo> EnsureBuildToolAvailableAsync(string toolName, Cance
285286
/// <summary>
286287
/// Execute a build tool with the specified arguments
287288
/// </summary>
288-
/// <param name="toolName">Name of the tool (e.g., 'mt.exe', 'signtool.exe')</param>
289+
/// <param name="tool">The tool to execute</param>
289290
/// <param name="arguments">Arguments to pass to the tool</param>
290291
/// <param name="cancellationToken">Cancellation token</param>
291292
/// <returns>Tuple containing (stdout, stderr)</returns>
292-
public async Task<(string stdout, string stderr)> RunBuildToolAsync(string toolName, string arguments, CancellationToken cancellationToken = default)
293+
public async Task<(string stdout, string stderr)> RunBuildToolAsync(Tool tool, string arguments, CancellationToken cancellationToken = default)
293294
{
294295
cancellationToken.ThrowIfCancellationRequested();
295296

296297
// Ensure the build tool is available, installing BuildTools if necessary
297-
var toolPath = await EnsureBuildToolAvailableAsync(toolName, cancellationToken: cancellationToken);
298+
var toolPath = await EnsureBuildToolAvailableAsync(tool.ExecutableName, cancellationToken: cancellationToken);
298299

299300
var psi = new ProcessStartInfo
300301
{
@@ -308,7 +309,7 @@ public async Task<FileInfo> EnsureBuildToolAvailableAsync(string toolName, Cance
308309

309310
cancellationToken.ThrowIfCancellationRequested();
310311

311-
using var p = Process.Start(psi) ?? throw new InvalidOperationException($"Failed to start {toolName} process");
312+
using var p = Process.Start(psi) ?? throw new InvalidOperationException($"Failed to start {tool.ExecutableName} process");
312313
var stdout = await p.StandardOutput.ReadToEndAsync(cancellationToken);
313314
var stderr = await p.StandardError.ReadToEndAsync(cancellationToken);
314315
await p.WaitForExitAsync(cancellationToken);
@@ -325,7 +326,14 @@ public async Task<FileInfo> EnsureBuildToolAvailableAsync(string toolName, Cance
325326

326327
if (p.ExitCode != 0)
327328
{
328-
throw new InvalidBuildToolException(p.Id, stdout, stderr, $"{toolName} execution failed with exit code {p.ExitCode}");
329+
// Print tool-specific error output when not in verbose mode
330+
// In verbose mode, all output is already visible via LogDebug above
331+
if (!logger.IsEnabled(LogLevel.Debug))
332+
{
333+
tool.PrintErrorText(stdout, stderr, logger);
334+
}
335+
336+
throw new InvalidBuildToolException(p.Id, stdout, stderr, $"{tool.ExecutableName} execution failed with exit code {p.ExitCode}");
329337
}
330338

331339
return (stdout, stderr);

src/winapp-CLI/WinApp.Cli/Services/CertificateService.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// Licensed under the MIT License.
33

44
using WinApp.Cli.Helpers;
5+
using WinApp.Cli.Tools;
56
using System.Diagnostics.Eventing.Reader;
67
using System.Text.RegularExpressions;
78
using Microsoft.Extensions.Logging;
@@ -207,7 +208,7 @@ public async Task SignFileAsync(FileInfo filePath, FileInfo certificatePath, str
207208

208209
try
209210
{
210-
await buildToolsService.RunBuildToolAsync("signtool.exe", arguments, cancellationToken: cancellationToken);
211+
await buildToolsService.RunBuildToolAsync(new GenericTool("signtool.exe"), arguments, cancellationToken: cancellationToken);
211212

212213
logger.LogDebug("File signed successfully");
213214
}

src/winapp-CLI/WinApp.Cli/Services/IBuildToolsService.cs

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
// Copyright (c) Microsoft Corporation. All rights reserved.
22
// Licensed under the MIT License.
33

4+
using WinApp.Cli.Tools;
5+
46
namespace WinApp.Cli.Services;
57

68
internal interface IBuildToolsService
@@ -26,5 +28,13 @@ internal interface IBuildToolsService
2628
Task<FileInfo> EnsureBuildToolAvailableAsync(string toolName, CancellationToken cancellationToken = default);
2729

2830
Task<DirectoryInfo?> EnsureBuildToolsAsync(bool forceLatest = false, CancellationToken cancellationToken = default);
29-
Task<(string stdout, string stderr)> RunBuildToolAsync(string toolName, string arguments, CancellationToken cancellationToken = default);
31+
32+
/// <summary>
33+
/// Execute a build tool with the specified arguments
34+
/// </summary>
35+
/// <param name="tool">The tool to execute</param>
36+
/// <param name="arguments">Arguments to pass to the tool</param>
37+
/// <param name="cancellationToken">Cancellation token</param>
38+
/// <returns>Tuple containing (stdout, stderr)</returns>
39+
Task<(string stdout, string stderr)> RunBuildToolAsync(Tool tool, string arguments, CancellationToken cancellationToken = default);
3040
}

src/winapp-CLI/WinApp.Cli/Services/MsixService.cs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
using System.Xml;
1010
using WinApp.Cli.Helpers;
1111
using WinApp.Cli.Models;
12+
using WinApp.Cli.Tools;
1213

1314
namespace WinApp.Cli.Services;
1415

@@ -522,7 +523,7 @@ public async Task<FileInfo> CreatePriConfigAsync(DirectoryInfo packageDir, strin
522523
523524
try
524525
{
525-
await buildToolsService.RunBuildToolAsync("makepri.exe", arguments, cancellationToken: cancellationToken);
526+
await buildToolsService.RunBuildToolAsync(new MakePriTool(), arguments, cancellationToken: cancellationToken);
526527
527528
logger.LogDebug("PRI configuration created: {ConfigPath}", configPath);
528529
@@ -577,7 +578,7 @@ public async Task<List<FileInfo>> GeneratePriFileAsync(DirectoryInfo packageDir,
577578
578579
try
579580
{
580-
var (stdout, stderr) = await buildToolsService.RunBuildToolAsync("makepri.exe", arguments, cancellationToken: cancellationToken);
581+
var (stdout, stderr) = await buildToolsService.RunBuildToolAsync(new MakePriTool(), arguments, cancellationToken: cancellationToken);
581582
582583
// Parse the output to extract resource files
583584
var resourceFiles = new List<FileInfo>();
@@ -1129,13 +1130,13 @@ private async Task CreateMsixPackageFromFolderAsync(DirectoryInfo inputFolder, F
11291130

11301131
logger.LogDebug("Creating MSIX package...");
11311132

1132-
await buildToolsService.RunBuildToolAsync("makeappx.exe", makeappxArguments, cancellationToken: cancellationToken);
1133+
await buildToolsService.RunBuildToolAsync(new MakeAppxTool(), makeappxArguments, cancellationToken: cancellationToken);
11331134
}
11341135

11351136
private async Task RunMtToolAsync(string arguments, CancellationToken cancellationToken = default)
11361137
{
11371138
// Use BuildToolsService to run mt.exe
1138-
await buildToolsService.RunBuildToolAsync("mt.exe", arguments, cancellationToken: cancellationToken);
1139+
await buildToolsService.RunBuildToolAsync(new GenericTool("mt.exe"), arguments, cancellationToken: cancellationToken);
11391140
}
11401141

11411142
private static void TryDeleteFile(FileInfo path)
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
// Licensed under the MIT License.
3+
4+
namespace WinApp.Cli.Tools;
5+
6+
/// <summary>
7+
/// Generic tool wrapper for build tools without specific error parsing logic
8+
/// </summary>
9+
public class GenericTool : Tool
10+
{
11+
private readonly string _executableName;
12+
13+
public GenericTool(string executableName)
14+
{
15+
_executableName = executableName;
16+
}
17+
18+
public override string ExecutableName => _executableName;
19+
20+
// Uses default PrintErrorText implementation (prints stdout and stderr)
21+
}
Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
// Licensed under the MIT License.
3+
4+
using Microsoft.Extensions.Logging;
5+
6+
namespace WinApp.Cli.Tools;
7+
8+
/// <summary>
9+
/// Tool wrapper for makeappx.exe with specific error parsing
10+
/// </summary>
11+
public class MakeAppxTool : Tool
12+
{
13+
public override string ExecutableName => "makeappx.exe";
14+
15+
/// <summary>
16+
/// Print error text from makeappx.exe output, extracting relevant error lines
17+
/// </summary>
18+
public override void PrintErrorText(string stdout, string stderr, ILogger logger)
19+
{
20+
// Example makeappx.exe error output -- this goes to stdout:
21+
// Processing "\\?\D:\temp\rigvgl1m.zgh\priconfig.xml" as a payload file. Its path in the package will be "priconfig.xml".
22+
// Processing "\\?\D:\temp\rigvgl1m.zgh\resources.pri" as a payload file. Its path in the package will be "resources.pri".
23+
// MakeAppx : error: Error info: error C00CE169: App manifest validation error: The app manifest must be valid as per schema: Line 38, Column 7, Reason: 'InvalidValue' violates enumeration constraint of 'appContainer mediumIL'.
24+
// The attribute '{http://schemas.microsoft.com/appx/manifest/uap/windows10/10}TrustLevel' with value 'InvalidValue' failed to parse.
25+
26+
// MakeAppx : error: Package creation failed.
27+
// MakeAppx : error: 0x80080204 - The specified package format is not valid: The package manifest is not valid.
28+
29+
// Now, find the first error line in stdout and print out the rest of the output from there.
30+
if (!string.IsNullOrWhiteSpace(stdout))
31+
{
32+
var lines = stdout.Split(['\r', '\n'], StringSplitOptions.RemoveEmptyEntries);
33+
var firstErrorIndex = -1;
34+
35+
// Find the first line containing "error"
36+
for (int i = 0; i < lines.Length; i++)
37+
{
38+
if (lines[i].Contains("error", StringComparison.OrdinalIgnoreCase))
39+
{
40+
firstErrorIndex = i;
41+
break;
42+
}
43+
}
44+
45+
// Print from the first error line onwards
46+
if (firstErrorIndex >= 0)
47+
{
48+
for (int i = firstErrorIndex; i < lines.Length; i++)
49+
{
50+
logger.LogError("{ErrorLine}", lines[i]);
51+
}
52+
logger.LogError("{ExeName} reported an error. The above text is only part of the tool output. Please re-run with --verbose to see the full output.", ExecutableName);
53+
return;
54+
}
55+
}
56+
57+
// If no error pattern found or stderr has content, use default behavior
58+
if (!string.IsNullOrWhiteSpace(stderr))
59+
{
60+
logger.LogError("{Stderr}", stderr);
61+
logger.LogError("{ExeName} reported an error. The above text is only part of the tool output. Please re-run with --verbose to see the full output.", ExecutableName);
62+
}
63+
else
64+
{
65+
// Fallback to base implementation
66+
base.PrintErrorText(stdout, stderr, logger);
67+
}
68+
}
69+
}
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
// Licensed under the MIT License.
3+
4+
using Microsoft.Extensions.Logging;
5+
using System.Text.RegularExpressions;
6+
7+
namespace WinApp.Cli.Tools;
8+
9+
/// <summary>
10+
/// Tool wrapper for makepri.exe with specific error parsing
11+
/// </summary>
12+
public partial class MakePriTool : Tool
13+
{
14+
public override string ExecutableName => "makepri.exe";
15+
16+
/// <summary>
17+
/// Print error text from makepri.exe output, extracting relevant error lines
18+
/// </summary>
19+
public override void PrintErrorText(string stdout, string stderr, ILogger logger)
20+
{
21+
// makepri.exe writes errors to stderr, so just print stderr if there is any.
22+
if (string.IsNullOrWhiteSpace(stderr))
23+
{
24+
base.PrintErrorText(stdout, stderr, logger);
25+
}
26+
else
27+
{
28+
logger.LogError("{Stderr}", stderr);
29+
logger.LogError("{ExeName} reported an error. The above text is only part of the tool output. Please re-run with --verbose to see the full output.", ExecutableName);
30+
}
31+
}
32+
}
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
// Licensed under the MIT License.
3+
4+
using Microsoft.Extensions.Logging;
5+
6+
namespace WinApp.Cli.Tools;
7+
8+
/// <summary>
9+
/// Base class for Windows SDK build tools
10+
/// </summary>
11+
public abstract class Tool
12+
{
13+
/// <summary>
14+
/// The executable name (e.g., "signtool.exe", "makeappx.exe")
15+
/// </summary>
16+
public abstract string ExecutableName { get; }
17+
18+
/// <summary>
19+
/// Print error text from stdout/stderr.
20+
/// Tool can parse and print what it believes to be the error output.
21+
/// Default implementation prints both stdout and stderr.
22+
/// </summary>
23+
/// <param name="stdout">Standard output from the tool</param>
24+
/// <param name="stderr">Standard error from the tool</param>
25+
/// <param name="logger">Logger for outputting error messages</param>
26+
public virtual void PrintErrorText(string stdout, string stderr, ILogger logger)
27+
{
28+
// We don't know this tool, so print all output.
29+
if (!string.IsNullOrWhiteSpace(stdout))
30+
{
31+
logger.LogError("{Stdout}", stdout);
32+
}
33+
if (!string.IsNullOrWhiteSpace(stderr))
34+
{
35+
logger.LogError("{Stderr}", stderr);
36+
}
37+
}
38+
}

0 commit comments

Comments
 (0)