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

Improvements to single-file analysis #44488

Closed
16 of 18 tasks
agocke opened this issue Nov 10, 2020 · 7 comments · Fixed by dotnet/winforms#5426
Closed
16 of 18 tasks

Improvements to single-file analysis #44488

agocke opened this issue Nov 10, 2020 · 7 comments · Fixed by dotnet/winforms#5426
Assignees
Labels
area-Single-File Priority:0 Work that we can't release without tracking This issue is tracking the completion of other related issues.

Comments

@agocke
Copy link
Member

agocke commented Nov 10, 2020

The existing single-file analyzer covers the top issues customers will likely hit with single-file, but it has some shortcomings.

Goals

  • Single-file analyzer is moved next to the trimming analyzer in the mono/linker repo
  • Single-file analyzer is enabled in the dotnet/runtime repo to identify problems in the framework
  • Users can annotate their own APIs as being not compatible with single-file

Work

  • Add an attribute similar to RequiresUnreferencedCode (code, ref assemblies)
  • Annotate low-level APIs with the attribute - One more run over all "dangerous" APIs
    • Make sure that the attributes show up in ref assemblies.
    • Validate that the ref assembly validator in dotnet/runtime enforces inclusion of this attribute (we had issues with this for RequiresUnreferencedCode)
  • At least plan on adding a docs page on the topic with some thoughts/guides on how to “fix” – ideally link to this from the attributes (see https://docs.microsoft.com/en-us/dotnet/core/deploying/single-file#api-incompatibility)
  • Synchronize with mono/Xamarin to annotate low level APIs which are problematic because of mono/Xamarin single-bundle publishing (mono itself can do a single-file bundle similar to CoreCLR's single-file, but Xamarin has other single-file like bundles for devices - Android, iOS).
  • Improve existing Roslyn analyzer
    • Move the analyzer next to the trimming analyzer in the mono/linker repo
    • Support for the new attribute
      • Implicit suppression on annotated methods
  • Integrate the analyzers with SDK - probably conditioned on PublishSingleFile property - warnings on-by-default??? (they already are for user code, what about whole-program analysis done by linker)
  • Enable Roslyn analyzer on dotnet/runtime repo (similar to TrimAnalysis) - probably add baselines as well (not sure how to do baselines for Roslyn analyzer - .cs file with SuppressMessage attributes which will be removed by compilation?)
  • Go over reported warnings from within the framework in dotnet/runtime and “do something about them” (suppress, propagate to public API via attributes)
    • Ideally also look through other features which might be affected by single-file and annotate
  • ?? Run the analysis on higher-level frameworks – ASP.NET, WinForms, WPF and file issues as appropriate
@ghost
Copy link

ghost commented Nov 10, 2020

Tagging subscribers to this area: @agocke, @vitek-karas
See info in area-owners.md if you want to be subscribed.


Issue meta data
Issue content: The existing single-file analyzer covers the top issues customers will likely hit with single-file, but it has some shortcomings.

Goals

  • Single-file problems are detected using linker whole-program analysis
  • Single-file analyzer is moved next to the trimming analyzer in the mono/linker repo
  • Single-file analyzer is enabled in the dotnet/runtime repo to identify problems in the framework
  • Users can annotate their own APIs as being not compatible with single-file
Issue author: agocke
Assignees: -
Milestone: -

@vitek-karas
Copy link
Member

As part of the "Users can annotated their own APIs" we will introduce an attribute similar to RequiresUnreferencedCode and it will need almost identical treatment by the linker.

dotnet/linker#1607 will probably force us to move analysis around RequiresUnreferencedCode into a different place in the linker - so we might as well make it an extensibility point (of sorts) and then implement both there.

@danmoseley
Copy link
Member

Should something like this be parented under a single file user story?

@agocke
Copy link
Member Author

agocke commented Nov 10, 2020

Probably a question for @samsp-msft

@danmoseley
Copy link
Member

danmoseley commented Nov 10, 2020

I think we're all just figuring out what is useful to us, hence the question. I'm going to be bold and make it a user story under the single file epic. I think in general 'significant work items' make sense to have in the tree. It's certainly helpful to me to see all the major work. Feel free to change.

@danmoseley danmoseley added the User Story A single user-facing feature. Can be grouped under an epic. label Nov 10, 2020
@maryamariyan maryamariyan added the untriaged New issue has not been triaged by the area owner label Nov 11, 2020
@vitek-karas vitek-karas changed the title Improve single-file analyzer integration with linker Improvements to single-file analysis Nov 16, 2020
@marek-safar marek-safar added tracking This issue is tracking the completion of other related issues. and removed User Story A single user-facing feature. Can be grouped under an epic. labels Nov 16, 2020
@agocke agocke added Priority:0 Work that we can't release without and removed untriaged New issue has not been triaged by the area owner labels Nov 16, 2020
@danmoseley danmoseley added the User Story A single user-facing feature. Can be grouped under an epic. label Nov 24, 2020
@danmoseley
Copy link
Member

Making story so it appears in the tree -- but now I see you have removed it @marek-safar ?

@marek-safar marek-safar removed the User Story A single user-facing feature. Can be grouped under an epic. label Nov 27, 2020
@agocke agocke added the User Story A single user-facing feature. Can be grouped under an epic. label Nov 30, 2020
@agocke
Copy link
Member Author

agocke commented Jan 27, 2021

Update: cutting whole-program analysis of single-file for 6.0. Telemetry shows that most users are not using linking in conjunction with PublishSingleFile, so adding single-file analysis in the linker could produce several downsides, including slowing down publish significantly.

As a longer-term plan, adding the analysis to either a separate tool or inside crossgen is likely a better path here.

@marek-safar marek-safar removed the User Story A single user-facing feature. Can be grouped under an epic. label Jan 28, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Sep 15, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-Single-File Priority:0 Work that we can't release without tracking This issue is tracking the completion of other related issues.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants