Skip to content
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.

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

Trimming defaults don't use advised trimming behavior #2644

Closed
breyed opened this issue Nov 25, 2024 · 5 comments · Fixed by #2688 or #2689
Closed

Trimming defaults don't use advised trimming behavior #2644

breyed opened this issue Nov 25, 2024 · 5 comments · Fixed by #2688 or #2689
Assignees
Labels
doc-enhancement Improve the current content [org] Pri1 High priority, do before Pri2 and Pri3 📌 seQUESTered Identifies that an issue has been imported into Quest.

Comments

@breyed
Copy link
Contributor

breyed commented Nov 25, 2024

Trimming defaults vary by build configuration:

By default, Android and Mac Catalyst builds use partial trimming when the build configuration is set to a release build.

However, the trimming behavior advice says not to:

The $(TrimMode) build property shouldn't be conditioned by build configuration.

Either the behavior should be changed to match the advice or the advice should be updated to indicate when it is and is not recommended to condition TrimMode by configuration.


Associated WorkItem - 350513

@dotnetrepoman dotnetrepoman bot added the ⌚ Not Triaged Not triaged label Nov 25, 2024
@davidbritch davidbritch self-assigned this Dec 3, 2024
@davidbritch davidbritch added doc-enhancement Improve the current content [org] 🗺️ reQUEST Triggers an issue to be imported into Quest. labels Dec 3, 2024
@dotnetrepoman dotnetrepoman bot removed the ⌚ Not Triaged Not triaged label Dec 3, 2024
@davidbritch davidbritch added Pri1 High priority, do before Pri2 and Pri3 and removed Pri3 labels Dec 3, 2024
@sequestor sequestor bot added 📌 seQUESTered Identifies that an issue has been imported into Quest. and removed 🗺️ reQUEST Triggers an issue to be imported into Quest. labels Dec 3, 2024
@dotnetrepoman dotnetrepoman bot added ⌚ Not Triaged Not triaged and removed ⌚ Not Triaged Not triaged labels Dec 3, 2024
@davidbritch
Copy link
Contributor

Hi @breyed

The intent here is that users should either accept the trimming defaults or if they're going to change the defaults, do so across all build configurations. I appreciate that the language could use some clarification.

@breyed
Copy link
Contributor Author

breyed commented Dec 3, 2024

@davidbritch What are the underlying technical reasons for the default of partial trimming for release but not debug on Android and Mac Catalyst? Intuitively, I would expect that if for example the user switched to full trimming, the same release/debug distinction would apply. I'm trying to understand why the guidance would be in the example to use full trimming in both release and debug.

@breyed
Copy link
Contributor Author

breyed commented Dec 10, 2024

The guidance in the docs should be reconciled with guidance from @jonathanpeppers in dotnet/maui#26509 to not use "<PublishTrimmed>true</PublishTrimmed> in Debug mode on Android".

@jonathanpeppers
Copy link
Member

Generally, I don't think you should have to set $(PublishTrimmed) on mobile or know this setting exists. It should pick the right thing by default for you. On Android, there isn't really a good reason to turn it on in Debug mode, or turn it off in Release mode. Doing either of those will cause issues. And then behavior differs on iOS or other platforms.

This has some details about the current behavior, that we might improve in .NET 10:

@davidbritch davidbritch moved this from 🔖 Ready to 🏗 In progress in dotnet/docs-maui 2024 Sprints Dec 20, 2024
This was referenced Dec 20, 2024
@davidbritch davidbritch moved this from 🏗 In progress to ✅ Done in dotnet/docs-maui 2024 Sprints Dec 20, 2024
@davidbritch
Copy link
Contributor

Fixed by #2688 and #2689

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc-enhancement Improve the current content [org] Pri1 High priority, do before Pri2 and Pri3 📌 seQUESTered Identifies that an issue has been imported into Quest.
Projects
Status: ✅ Done
Development

Successfully merging a pull request may close this issue.

3 participants