Skip to content

Conversation

@kant2002
Copy link
Contributor

This PR is just ask to start producing list of ILLink warnings
for System.Speech. I would like to eventually make ComWrappers for this
library, but for now, I think there other low-hanging fruit which can
be reached here.

This PR is just ask to start producing list of ILLink warnings
for System.Speech. I would like to eventually make ComWrappers for this
library, but for now, I think there other low-hanging fruit which can
be reached here.
@ghost ghost added community-contribution Indicates that the PR has been added by a community member area-Infrastructure-libraries labels Nov 20, 2021
@ghost
Copy link

ghost commented Nov 20, 2021

Tagging subscribers to this area: @Anipik, @safern, @ViktorHofer
See info in area-owners.md if you want to be subscribed.

Issue Details

This PR is just ask to start producing list of ILLink warnings
for System.Speech. I would like to eventually make ComWrappers for this
library, but for now, I think there other low-hanging fruit which can
be reached here.

Author: kant2002
Assignees: -
Labels:

area-Infrastructure-libraries, community-contribution

Milestone: -

@marek-safar marek-safar added the linkable-framework Issues associated with delivering a linker friendly framework label Nov 22, 2021
@ghost
Copy link

ghost commented Nov 22, 2021

Tagging subscribers to 'linkable-framework': @eerhardt, @vitek-karas, @LakshanF, @sbomer, @joperezr
See info in area-owners.md if you want to be subscribed.

Issue Details

This PR is just ask to start producing list of ILLink warnings
for System.Speech. I would like to eventually make ComWrappers for this
library, but for now, I think there other low-hanging fruit which can
be reached here.

Author: kant2002
Assignees: -
Labels:

area-Infrastructure-libraries, linkable-framework, community-contribution

Milestone: -

@vitek-karas
Copy link
Member

There could be other ways to do this, but my trick to get all the warnings baselined is:

  • Run the build where the OOB trimmer is executed (should be enough to build.cmd -s Libs -bl) with binlong enabled
  • Open the binlog and find the linker invocation (search for $task ILLink)
  • Copy the command line from the task into a text file and call it for example linker-oob.rsp
  • The first line of that file will contain the "/dotnet.exe /illink.dll" cut that out and run it from the command line like so: <path>/dotnet.exe <path>/illink.dll @linker-oob.rsp.
  • This should now fail the same way as in the build (this should do exactly the same thing as it did inside build)
  • Now add this to command line: --generate-warning-suppressions xml and run it again
  • Now there should be .xml files in the output folder (you can find the folder by looking for the --out in the response file)

Take the xml file for Speech and put it in place of the ILLink.Suppressions.xml. Try to build again - now it should not produce any warnings from the Speech assembly.

@kant2002
Copy link
Contributor Author

Nice, thanks a lot. I suspect that there was some way to make that file automatically! will try.

@eerhardt
Copy link
Member

There could be other ways to do this

See the changes here: #44264. This PR stalled on writing docs and wasn't picked up again.

@eerhardt
Copy link
Member

I would like to eventually make ComWrappers for this library

In #62160 (comment) @stephentoub mentions

We've been avoiding churning the code, as it's here only for compat and doesn't have a robust set of tests.

I'm concerned that refactoring the interop layer here without a decent set of tests could introduce bugs.

@kant2002 - can you describe why you would like to make System.Speech trimmable?

cc @danmoseley

@kant2002
Copy link
Contributor Author

kant2002 commented Dec 5, 2021

Personally my case maybe not very compelling. I just try everything under NativeAOT and "fix" what's possible to smooth experience of NativeAOT users. I trying do that in the areas where MS does not invest resources, right now, but at least consider this is important (to not have my efforts axed). CoreRT was suffering from the assumption that MS do everything and eventually that tech will works. I think this is mostly education problem, and I want to cover as much of .NET world as possible. "System.Speech" IMO is important part of ecosystem enough. It is very inspirational library, even if it's lacking of documentation and support.

I understand that taking any changes without tests is a risk, but there no tests. So maybe I can start breaking that cycle and add some tests first?

@eerhardt
Copy link
Member

eerhardt commented Dec 5, 2021

So maybe I can start breaking that cycle and add some tests first?

That would be very welcomed, at least IMO.

I understand that taking any changes without tests is a risk

Even with tests, changes have risk. We have tests for Icon.Save, but the move to COMWrappers in System.Drawing exposed a pretty interesting bug: dotnet/winforms#8853.

So we have to weigh the risk vs. reward in these kinds of changes. I think we can make them. Starting with a good set of tests is definitely a good first step.

@ghost
Copy link

ghost commented Dec 6, 2021

Tagging subscribers to this area: @danmoseley
See info in area-owners.md if you want to be subscribed.

Issue Details

This PR is just ask to start producing list of ILLink warnings
for System.Speech. I would like to eventually make ComWrappers for this
library, but for now, I think there other low-hanging fruit which can
be reached here.

Author: kant2002
Assignees: -
Labels:

linkable-framework, area-System.Speech, community-contribution

Milestone: -

@danmoseley
Copy link
Member

Looks like the errors need baselining to fix the build

@danmoseley
Copy link
Member

@kant2002 thanks for this start. I will close this PR for now pending updates, feel free to reopen if you plan to continue.

@danmoseley danmoseley closed this Jan 3, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Feb 2, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-System.Speech community-contribution Indicates that the PR has been added by a community member linkable-framework Issues associated with delivering a linker friendly framework

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants