Skip to content

Conversation

@Pinguin2001
Copy link

@Pinguin2001 Pinguin2001 commented Dec 8, 2024

Resolved / Related Issues

To prevent extra work, all changes to the Files codebase must link to an approved issue marked as Ready to build. Please insert the issue number following the hashtag with the issue number that this Pull Request resolves.

Stability is a top priority for Files and all changes are required to go through testing before being merged into the repo. Please include a list of steps that you used to test this PR.

  1. Build files for all three architectures
  2. Verify only the architecture specific dll has been copied

While looking into the outdated 7z dlls, I noticed files bundles all three dlls regardless of the architecture being built. This pr addresses this issue

@yaira2
Copy link
Member

yaira2 commented Dec 8, 2024

Btw, I tried this once before, but it broke the installation. 7e28845

@yaira2 yaira2 added the ready for review Pull requests that are ready for review label Dec 8, 2024
@Pinguin2001
Copy link
Author

Btw, I tried this once before, but it broke the installation. 7e28845

Which issue occurred? I don't see how this could affect msix-based app installation

@yaira2
Copy link
Member

yaira2 commented Dec 8, 2024

Users couldn't update or install the new version due to resource mapping issue.

@Pinguin2001
Copy link
Author

Users couldn't update or install the new version due to resource mapping issue.

This seems odd, can you generate a signed test appx with and without the change and try it for yourself? This really doesn't seem like intended behavoir at all

@Pinguin2001
Copy link
Author

I will generate a test signed msix as well and update from that to the one with the other two dlls removed in a virtual machine and on a secondary pc. Will report back my tests

@0x5bfa
Copy link
Member

0x5bfa commented Dec 9, 2024

This doesn't work in CD (the side loading pipeline) because we configure it to generate x64|arm64 msixbundle and configure building to be only x64.
I like this size reduction improvement but unless we configure CD to build 2 times for each x64 and arm64 and then bundle them into msixbundle we can't enjoy this improvement.

If we want, we can re-configure CD to build two times and bundle them AND we don't have to use WAP project any longer, plus I doubt this would break auto-update since CDN package URL doesn't change.

  1. Build two times for x64 and arm64
  2. Bundle these msix files to msixbundle
  3. Bundle msixbundle and appxsym files for each configuration into msixupload into zip file

@yaira2
Copy link
Member

yaira2 commented Dec 22, 2024

Aside from the workaround to use msixbundler, are there any other paths forward?

@0x5bfa
Copy link
Member

0x5bfa commented Dec 22, 2024

I don't know. I guess it's easier.

@yaira2
Copy link
Member

yaira2 commented Jan 14, 2025

@Pinguin2001 sorry for the delay in responding. We're definitely interested in implementing this improvement, but our current packaging setup isn't compatible with this change, as @0x5bfa pointed out. We hope to make the necessary adjustments in the future, but right now, we lack the resources to do so, which means we'll have to put this on hold.

We'll keep you updated on our progress in #16583. Also, it's worth mentioning that we're looking forward to a significant reduction in package size once we enable Native AOT, something we've been working towards for some time now. Keep an eye on the issues related to Vanara and the Windows Community Toolkit for more details.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready for review Pull requests that are ready for review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants