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

Remove reference to PhysicalFileProvider from WebView #35750

Closed
wants to merge 5 commits into from

Conversation

Eilon
Copy link
Member

@Eilon Eilon commented Aug 26, 2021

Fixes #35707

Description

.NET MAUI apps currently have a reference to MS.Extensions PhysicalFileProvider that comes from ASP.NET's WebViewManager. The PhysicalFileProvider includes reference to system file watchers (whether you're actually watching for file changes or not). This reference prevents it from being accepted in certain app stores that restrict usage of file system watchers.

In this change the base WebViewManager class has a new virtual method that can be overridden on platforms that use static web assets and need physical file system access. .NET MAUI will not override this method, avoiding the reference. (Other platforms, such as WPF/WinForms Blazor Desktop will override it and use PhysicalFileProvider.)

Note: None of Blazor Desktop actually uses file system watchers, but the API call graph means that even with trimming, the reference to the watchers makes it to the published app, potentially causing app store rejection.

Customer Impact

Cannot publish Blazor Desktop apps to the iOS App Store (this was confirmed by a customer).

Regression?

  • Yes
  • No

[If yes, specify the version the behavior has regressed from]

Risk

  • High
  • Medium
  • Low

[Justify the selection above]

Verification

  • Manual (required) - the only way to truly verify this fix does what we expect is to publish another app to the app store and wait ~1 week, but that is not feasible. So, I verified that there are no more references to PhysicalFileProvider or file watchers from WebViewManager (a core class in Blazor Desktop)
  • Automated - CI tests pass

Packaging changes reviewed?

  • Yes
  • No
  • N/A

Addresses #35707

dotnet-maestro bot and others added 4 commits August 25, 2021 23:05
…me (#35736)

[release/6.0-rc1] Update dependencies from dotnet/efcore dotnet/runtime
…825.16 (#35745)

[release/6.0-rc1] Update dependencies from dotnet/efcore
… previous version (#35731)

* Update RazorPage NET 6 template to be able to be SxS with previous versions.
  - Update Web.ItemTemplates to use a versioned package identity
  - Bump up the precedence
  - Make unique identities
  - Add Framework symbol for RazorPage template
* Remove Framework parameter
* Revert to block namespace
* Remove unused using statements from RazorPage Index.cshtml.cs

Co-authored-by: Phil Henning <[email protected]>
The reference prevents it from being usable in certain app stores that restrict usage of file system watchers.
@Pilchie Pilchie added the Servicing-consider Shiproom approval is required for the issue label Aug 26, 2021
@ghost
Copy link

ghost commented Aug 26, 2021

Hello human! Please make sure you've included the Shiproom Template in a comment or (preferably) the PR description. Also, make sure this PR is not marked as a draft and is ready-to-merge.

@Pilchie
Copy link
Member

Pilchie commented Aug 26, 2021

Can you fill out the servicing template? For RC1, we need tactics approval at this point.

@leecow leecow added Servicing-approved Shiproom has approved the issue and removed Servicing-consider Shiproom approval is required for the issue labels Aug 26, 2021
@Pilchie
Copy link
Member

Pilchie commented Aug 26, 2021

This is approved by Tactics, looks like you hit a build race that has been plaguing us lately :(

@Eilon Eilon removed the Servicing-approved Shiproom has approved the issue label Aug 26, 2021
@Eilon Eilon changed the base branch from release/6.0-rc1 to release/6.0 August 26, 2021 21:01
@Eilon Eilon requested a review from dougbu as a code owner August 26, 2021 21:01
@Eilon
Copy link
Member Author

Eilon commented Aug 26, 2021

I'm removing from RC1 because it turns out this change isn't strictly needed. In dotnet/runtime#57931 (which is in RC1), the disallowed iOS API isn't even called on iOS builds anymore, so using PhysicalFileProvider is perfectly safe from this perspective. However, we might still want to remove calls to PhysicalFileProvider simply because it isn't even needed on any .NET MAUI platforms.

@dougbu
Copy link
Member

dougbu commented Aug 26, 2021

Suspect versions should be resolved in favour of what's in 'release/6.0' already. Is that correct @Eilon i.e. you didn't intentionally update the dependencies when targeting 'release/6.0'❔

Separately, has this change already been done in 'main'❔ Our current practise is to get things working in 'main' then use /backport to release/6.0-rc1 or /backport to release/6.0.

@github-actions
Copy link
Contributor

Started backporting to release/6.0-rc1: https://github.com/dotnet/aspnetcore/actions/runs/1172020491

@github-actions
Copy link
Contributor

@dougbu backporting to release/6.0-rc1 failed, the patch most likely resulted in conflicts:

$ git am --3way --ignore-whitespace --keep-non-patch changes.patch

Applying: [release/6.0-rc1] Update dependencies from dotnet/efcore dotnet/runtime (#35736)
Using index info to reconstruct a base tree...
M	eng/Version.Details.xml
M	eng/Versions.props
Falling back to patching base and 3-way merge...
Auto-merging eng/Versions.props
CONFLICT (content): Merge conflict in eng/Versions.props
Auto-merging eng/Version.Details.xml
CONFLICT (content): Merge conflict in eng/Version.Details.xml
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 [release/6.0-rc1] Update dependencies from dotnet/efcore dotnet/runtime (#35736)
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
Error: The process '/usr/bin/git' failed with exit code 128

Please backport manually!

@dougbu
Copy link
Member

dougbu commented Aug 26, 2021

Started backporting to release/6.0-rc1: https://github.com/dotnet/aspnetcore/actions/runs/1172020491

Now that's annoying. @wtgodbe is our comment parser too forgiving❔

@Eilon
Copy link
Member Author

Eilon commented Aug 26, 2021

Oh I just re-targetted the PR, which totally messed it up. I'll close this and send a new PR when ready.

@Eilon Eilon closed this Aug 26, 2021
@dougbu
Copy link
Member

dougbu commented Aug 26, 2021

Should we delete the branch❔

@Eilon
Copy link
Member Author

Eilon commented Aug 26, 2021

No, I need the code in that branch.

@ghost
Copy link

ghost commented Aug 26, 2021

Hi @Eilon. It looks like you just commented on a closed PR. The team will most probably miss it. If you'd like to bring something important up to their attention, consider filing a new issue and add enough details to build context.

@dougbu dougbu deleted the eilon/remove-physicalfile-ref branch August 30, 2021 23:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-blazor Includes: Blazor, Razor Components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants