Skip to content

Bug fixes in packaging command#344

Merged
azchohfi merged 9 commits intomainfrom
nmetulev/packaging-fixes
Mar 6, 2026
Merged

Bug fixes in packaging command#344
azchohfi merged 9 commits intomainfrom
nmetulev/packaging-fixes

Conversation

@nmetulev
Copy link
Member

@nmetulev nmetulev commented Mar 5, 2026

Description

This PR fixes package command issue found when attempting to package a blank new winui project from visual studio.

  • Fixes an issue where existing pri resources were overwritten,
  • Adds check for executable architecture to inlcude in manifest,
  • Ability to use package.appxmanifest for packaging

WIP

Usage Example

Related Issue

Type of Change

  • 🐛 Bug fix
  • ✨ New feature
  • 💥 Breaking change
  • 📝 Documentation
  • 🔧 Config/build
  • ♻️ Refactoring
  • 🧪 Test update

Checklist

  • New tests added for new functionality (if applicable)
  • Tested locally on Windows
  • Main README.md updated (if applicable)
  • docs/usage.md updated (if CLI commands changed)
  • Language-specific guides updated (if applicable)
  • Sample projects updated to reflect changes (if applicable)
  • Agent skill templates updated in docs/fragments/skills/ (if CLI commands/workflows changed)

Screenshots / Demo

Additional Notes

AI Description

This PR addresses several bugs in the packaging command for WinApp CLI, specifically improving the handling of existing PRI resources, checking executable architecture for inclusion in the manifest, and allowing the use of package.appxmanifest. The command now supports either appxmanifest.xml or package.appxmanifest as a manifest file.

winapp package ./dist --manifest appxmanifest.xml --cert ./devcert.pfx
# or 
winapp package ./dist --manifest package.appxmanifest --cert ./devcert.pfx

@github-actions github-actions bot added the bug Something isn't working label Mar 5, 2026
@github-actions
Copy link

github-actions bot commented Mar 5, 2026

Build Metrics Report

Binary Sizes

Artifact Baseline Current Delta
CLI (ARM64) 13.92 MB 13.94 MB 📈 +26.5 KB (+0.19%)
CLI (x64) 13.21 MB 13.23 MB 📈 +25.5 KB (+0.19%)
MSIX (ARM64) 6.05 MB 6.06 MB 📈 +10.8 KB (+0.17%)
MSIX (x64) 6.29 MB 6.30 MB 📈 +12.2 KB (+0.19%)
NPM Package 12.32 MB 12.35 MB 📈 +22.6 KB (+0.18%)

Test Results

402 passed out of 402 tests in 361.6s (+20 tests, +8.3s vs. baseline)

Test Coverage

42% line coverage, 45.8% branch coverage · ⚠️ -0.9% vs. baseline

CLI Startup Time

35ms median (x64, winapp --version) · ✅ no change vs. baseline


Updated 2026-03-06 23:32:27 UTC · commit 754f710 · workflow run

@nmetulev nmetulev marked this pull request as ready for review March 5, 2026 21:57
@nmetulev nmetulev requested a review from Copilot March 5, 2026 21:57
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes several WinApp CLI packaging edge cases seen with new/blank WinUI projects by improving manifest discovery, preserving existing PRI resources, and enhancing manifest/packaging metadata.

Changes:

  • Support package.appxmanifest in addition to appxmanifest.xml (search + CLI/docs text updates).
  • Avoid overwriting existing resources.pri; resolve x-generate Languages from existing PRI; improve PRI config defaults (scale).
  • Detect executable architecture to populate/warn about ProcessorArchitecture and include it in default MSIX filename; remove WinAppSDK bootstrapper DLLs from staging.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
src/winapp-CLI/WinApp.Cli/Services/MsixService.cs Adds manifest discovery, PRI/language handling, arch detection, and staging cleanup to fix packaging behavior
src/winapp-CLI/WinApp.Cli/Commands/PackageCommand.cs Updates command help text to reflect manifest options
src/winapp-CLI/WinApp.Cli.Tests/MsixServiceTests.cs Adds unit tests for new helper methods (manifest search, scale detection, x-generate replacement, bootstrapper removal)
docs/cli-schema.json Updates package command description in generated schema
.github/plugin/skills/winapp-cli/package/SKILL.md Updates package skill docs to reflect manifest options
.gitattributes Adds LF normalization for plugin/docs files to match generation output

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +1904 to +1905
file.Delete();
taskContext.AddDebugMessage($"{UiSymbols.Note} Removed {fileName} (not needed in MSIX package)");
Copy link

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

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

FileInfo.Delete() can throw (locked file, ACL/readonly, AV scanning, etc.), which would fail packaging even though removing these DLLs is an optimization/safety step. Consider wrapping the delete in try/catch and logging a warning (and continuing), possibly clearing the readonly attribute first if needed.

Suggested change
file.Delete();
taskContext.AddDebugMessage($"{UiSymbols.Note} Removed {fileName} (not needed in MSIX package)");
try
{
// Ensure the file is not read-only before attempting deletion.
if (file.Attributes.HasFlag(FileAttributes.ReadOnly))
{
file.Attributes &= ~FileAttributes.ReadOnly;
}
file.Delete();
taskContext.AddDebugMessage($"{UiSymbols.Note} Removed {fileName} (not needed in MSIX package)");
}
catch (IOException ex)
{
taskContext.AddDebugMessage($"{UiSymbols.Note} Failed to remove {fileName} (optional cleanup step): {ex.Message}");
}
catch (UnauthorizedAccessException ex)
{
taskContext.AddDebugMessage($"{UiSymbols.Note} Failed to remove {fileName} due to access restrictions (optional cleanup step): {ex.Message}");
}
catch (SecurityException ex)
{
taskContext.AddDebugMessage($"{UiSymbols.Note} Failed to remove {fileName} due to security restrictions (optional cleanup step): {ex.Message}");
}

Copilot uses AI. Check for mistakes.
@azchohfi azchohfi merged commit c346ef6 into main Mar 6, 2026
11 checks passed
@azchohfi azchohfi deleted the nmetulev/packaging-fixes branch March 6, 2026 23:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants