[release/5.0] Backport System.Speech to compat pack #46729

danmoseley · 2021-01-08T04:35:27Z

Backport System.Speech including its entry in the windows compat pack.
Original PR #45941

Customer Impact

The System.Speech assembly on .NET Framework does not work on .NET Core/.NET 5 because it uses private reflection. Many customers rely on generating speech from .NET code to make their apps or scripts accessible to them or their users. They have given us clear and extensive feedback on the impact this has on them, eg in the issue https://github.com/dotnet/wpf/issues/2935. In particular this means they cannot generate speech from Powershell scripts, as recent versions of Powershell depend on .NET Core.

This change backports the port of System.Speech that we have completed in master, so that we can ship it in an updated Windows Compatibility Pack and enable this speech functionality for our customers using .NET 2.1+.

Testing

Unit tests that give ~50% coverage. That isn't great, but they cover the key synthesis/recognition paths. Much of the uncovered code are paths like event handlers, or code relating to generating or parsing custom grammars, etc that is not as relevant to generating basic speech which is the key use case we are trying to enable. There aren't quick wins to increase the coverage substantially, and the original tests are not feasible to port due to their structure.
Manually tested the System.Speech package as follows: net472->ns20, net472->net472, netcoreapp2.1->ns20, netcoreapp2.2->ns20, net50->ns2.0. Where a P2P is shown, code was exercised in both. Also verified a build error for netcoreapp2.0.
Manually tested the Windows Compat pack package (containing it) as follows: net50->ns20, netcoreapp21->ns20

Risk

The code forms its own library, not previously shipped, so it's highly unlikely that existing .NET Core users would be broken by shipping this library. If there was a break, it would presumably relate to a packaging issue (since it's included into the compatibility pack). We tested key scenarios around consuming the package.

* Initial sources, with banners * Run the code formatter * Fix hang in XUnit due to failing to complete all AsyncOperation-s * Remove reflection over RegistryKey * Add ref and packaging * Add tests * Add sln * Fix CS1584 * Fix CA1823 * Fix CA1834 * Unnecessary suppressions * Fix SA1028 * Fix CA1507 * Fix CA1810 * Fix CA1825 * Fix CA1825 * Unnecessary suppressions * Fix CA1805 * Fix IDE0004 * Fix IDE0090 * Remove CAS * Remove tabs and dead code * Unnecessary suppressions * Fix SA1212 * Fix SA1121 * Disable SA1129 * Fix SA1206 * Fix SA1518 * Fix SA1617 * Fix SA1001 * Fix CS0618 * Remove unnecessary comments * Remove unnecessary whitespace * Remove low value xml doc comments * Unused usings * dead files * Remove CAS * More junk * Fix obvious original bug * Remove/insert newlines * Remove reference to old design document * Fix spacing * Fix typo name * Fix file casing * Remove dead code * Add to compat pack * Remove AppDomain etc * Fix casing of .NET * Remove low value XML docs * Remove code that relies on compiling assemblies * Fix inadvertently removed padding * Use EDI to preserve stack when rethrowing * Fix misaligned resource ID's to match sperror.h * Skip SpeechRecognitionEngine tests if no installed recognizers * Fix misformatted string bug * Logging for CI error * Fix NRE trying to map phonemes for voice for culture for which we do not have phoneme map * Fix 153 spelling errors in comments using `Visual Studio Spell Checker` * Remove extraneous file * Fix spacing * Fix project reference * Reorder properties in csproj * Change from netcoreapp2.0 to netcoreapp2.1 * Update src/libraries/System.Speech/pkg/System.Speech.pkgproj Co-authored-by: Jose Perez Rodriguez <joperezr@microsoft.com> * Add build error for targeting netcoreapp2.0 * Suppress new error during packaging testing * Update System.Speech.targets * Remove ref comments * Update pkgproj * Remove placeholder Co-authored-by: Jose Perez Rodriguez <joperezr@microsoft.com> Co-authored-by: Viktor Hofer <viktor.hofer@microsoft.com>

Dotnet-GitSync-Bot · 2021-01-08T04:35:32Z

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, to please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

src/libraries/pkg/baseline/packageIndex.json

danmoseley · 2021-01-08T16:22:26Z

Test failure are #43981 and #11063

ericstj · 2021-01-08T19:01:29Z

I suspect you'll need to cherry-pick bc24a14 from #46688 to get package testing to pass.

danmoseley · 2021-01-08T21:01:39Z

@ericstj apparently I didn't, unless it's something not run in CI.

danmoseley · 2021-01-08T21:04:12Z

Running pkg/tests locally i do get a bunch of errors like

C:\git\50runtime\artifacts\bin\testPkg\projects\System.Diagnostics.DiagnosticSource\netcoreapp1.0\project.csproj : error NU3005: Package 'Microsoft.NETCore.Platforms 1.0.2' from source 'C:\Program Files\dotnet\sdk\NuGetFallbackFolder': The package signature file entry is invalid. The central directory header field 'compression method' has an invalid value (8). [C:\git\50runtime\src\libraries\pkg\test\testPackages.proj]

danmoseley · 2021-01-08T21:06:23Z

Oh, silly mistake, I didn't remove the 5.0.2.

ericstj · 2021-01-08T21:15:07Z

@ericstj apparently I didn't, unless it's something not run in CI.

Did you add the package to

runtime/src/libraries/libraries-packages.proj

Line 21 in 988fa3b

    
           <ProjectReference Include="$(LibrariesProjectRoot)\pkg\Microsoft.Windows.Compatibility\Microsoft.Windows.Compatibility.pkgproj" Condition="'$(BuildAllConfigurations)' == 'true'" />

? If not it won't build during servicing.

Double check the steps here: https://github.com/dotnet/runtime/blob/release/5.0/docs/project/library-servicing.md

danmoseley · 2021-01-09T02:02:23Z

OK added to libraries-packages.proj. Checked library-servicing.md.

Also found it didn't work on .NET Framework -- fixed public key to MicrosoftShared to fix that. Also tested net472->ns20, net472->net472, netcoreapp2.1->ns20, netcoreapp2.2->ns20, netcoreapp2.0 (build error), net50->ns2.0.

Strangely something is updating packageIndex.json by itself to add lots of "5.0.2" entries. I'll see whether it's the build..

danmoseley · 2021-01-09T02:22:47Z

Yes, when I do build clr+libs+libs.tests -rc release -allconfigurations -pack I get this edit created for me:

diff --git a/src/libraries/pkg/baseline/packageIndex.json b/src/libraries/pkg/baseline/packageIndex.json
index cff6d7f372d..176476355a1 100644
--- a/src/libraries/pkg/baseline/packageIndex.json
+++ b/src/libraries/pkg/baseline/packageIndex.json
@@ -23,7 +23,8 @@
     "Microsoft.Bcl.AsyncInterfaces": {
       "StableVersions": [
         "1.0.0",
-        "5.0.0"
+        "5.0.0",
+        "5.0.2"
       ],
       "BaselineVersion": "5.0.0",
       "InboxOn": {},
@@ -137,7 +138,8 @@
         "2.0.0",
         "2.0.1",
         "2.1.0",
-        "5.0.0"
+        "5.0.0",
+        "5.0.2"
       ],
       "BaselineVersion": "5.0.0",
       "InboxOn": {},
@@ -170,7 +172,8 @@
         "3.1.0",
         "3.1.1",
         "3.1.2",
-        "5.0.0"
+        "5.0.0",
+        "5.0.2"
       ],
       "InboxOn": {},
       "AssemblyVersionInPackageVersion": {
@@ -199,7 +202,8 @@
         "3.1.0",
         "3.1.1",
         "3.1.2",
-        "5.0.0"
+        "5.0.0",
+        "5.0.2"
       ],
       "InboxOn": {},
etc etc

is that an issue?

janvorli · 2021-01-11T23:44:50Z

Yes, when I do build clr+libs+libs.tests -rc release -allconfigurations -pack I get this edit created for me

I've also noticed it happens whenever I try to build the 5.0 branch locally.

danmoseley · 2021-01-12T01:57:46Z

Need to cherry pick #46841

danmoseley · 2021-01-12T18:25:24Z

Marking no merge until I can cherry pick #46841 shortly

* Fix PKT * Fix SAPI errors * Disable test as appropriate * enable more tests to fail to get messages * reenable tests * Update src/libraries/System.Speech/tests/SpeechRecognizerTests.cs Co-authored-by: Stephen Toub <stoub@microsoft.com> Co-authored-by: Stephen Toub <stoub@microsoft.com>

src/libraries/System.Speech/tests/SpeechRecognizerTests.cs

danmoseley · 2021-01-13T01:25:29Z

@ericstj any other feedback? if you can approve, we can presumably merge.

ericstj

LGTM. I think @Anipik needs to merge this once he does branding and will need to address merge conflicts.

danmoseley · 2021-01-13T03:11:38Z

OK thanks! @Anipik I will be happy to validate this when we have product installer that includes it.

Anipik · 2021-01-13T03:14:44Z

OK thanks! @Anipik I will be happy to validate this when we have product installer that includes it.

i am waiting on this #46888 to be green. After that i will start merging the changes.

Anipik · 2021-01-14T06:33:29Z

Yes, when I do build clr+libs+libs.tests -rc release -allconfigurations -pack I get this edit created for me:

This is being fixed here #46224

Dotnet-GitSync-Bot added area-Meta new-api-needs-documentation labels Jan 8, 2021

danmoseley requested a review from ericstj January 8, 2021 04:35

danmoseley added area-System.Speech Servicing-consider Issue for next servicing release review and removed area-Meta new-api-needs-documentation labels Jan 8, 2021

Fix port

b47f75b

danmoseley commented Jan 8, 2021

View reviewed changes

src/libraries/pkg/baseline/packageIndex.json Outdated Show resolved Hide resolved

runfoapp bot mentioned this pull request Jan 8, 2021

System.Threading.Tests.ThreadLocalTests.RunThreadLocalTest8_Values failing on Mono on SLES 12 AMD64 #43981

Open

danmoseley added 2 commits January 8, 2021 10:28

Disable some Speech tests on Server Core

b0f606f

Set PackageVersion in Directory.Build.props

f14cbd6

danmoseley and others added 2 commits January 8, 2021 13:08

Change packageIndex back to 5.0.0

de4d121

Don't rev the platforms package unless we ship it

236382e

danmoseley added 2 commits January 8, 2021 18:00

Fix PKT

e9a7ec3

Add to libraries-packages.proj

00cdd67

runfoapp bot mentioned this pull request Jan 11, 2021

Inability to unzip assets during build on Unix x64 #32805

Closed

danmoseley changed the title ~~[release/5.0] Backport System.Speech~~ [release/5.0] Backport System.Speech to compat pack Jan 12, 2021

danmoseley mentioned this pull request Jan 12, 2021

Fix SAPI error code mappings #46841

Merged

danmoseley added the NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) label Jan 12, 2021

leecow added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Jan 12, 2021

leecow added this to the 5.0.3 milestone Jan 12, 2021

danmoseley removed the NO-MERGE The PR is not ready for merge yet (see discussion for detailed reasons) label Jan 12, 2021

ericstj reviewed Jan 12, 2021

View reviewed changes

src/libraries/System.Speech/tests/SpeechRecognizerTests.cs Show resolved Hide resolved

runfoapp bot mentioned this pull request Jan 12, 2021

Can_Run_App_With_StatiHost installer test failure in CI #42101

Closed

ericstj approved these changes Jan 13, 2021

View reviewed changes

ericstj requested a review from Anipik January 13, 2021 03:07

Anipik approved these changes Jan 14, 2021

View reviewed changes

Anipik added 2 commits January 13, 2021 19:50

Merge branch 'release/5.0' into port.speech

bbe5d49

Merge branch 'release/5.0' into port.speech

f0ee366

Anipik merged commit f5f2e4f into dotnet:release/5.0 Jan 14, 2021

ghost locked as resolved and limited conversation to collaborators Feb 13, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[release/5.0] Backport System.Speech to compat pack #46729

[release/5.0] Backport System.Speech to compat pack #46729

danmoseley commented Jan 8, 2021 •

edited

Loading

Dotnet-GitSync-Bot commented Jan 8, 2021

danmoseley commented Jan 8, 2021

ericstj commented Jan 8, 2021

danmoseley commented Jan 8, 2021

danmoseley commented Jan 8, 2021

danmoseley commented Jan 8, 2021

ericstj commented Jan 8, 2021

danmoseley commented Jan 9, 2021 •

edited

Loading

danmoseley commented Jan 9, 2021

janvorli commented Jan 11, 2021

danmoseley commented Jan 12, 2021

danmoseley commented Jan 12, 2021

danmoseley commented Jan 13, 2021

ericstj left a comment •

edited

Loading

danmoseley commented Jan 13, 2021

Anipik commented Jan 13, 2021

Anipik commented Jan 14, 2021

[release/5.0] Backport System.Speech to compat pack #46729

[release/5.0] Backport System.Speech to compat pack #46729

Conversation

danmoseley commented Jan 8, 2021 • edited Loading

Customer Impact

Testing

Risk

Dotnet-GitSync-Bot commented Jan 8, 2021

danmoseley commented Jan 8, 2021

ericstj commented Jan 8, 2021

danmoseley commented Jan 8, 2021

danmoseley commented Jan 8, 2021

danmoseley commented Jan 8, 2021

ericstj commented Jan 8, 2021

danmoseley commented Jan 9, 2021 • edited Loading

danmoseley commented Jan 9, 2021

janvorli commented Jan 11, 2021

danmoseley commented Jan 12, 2021

danmoseley commented Jan 12, 2021

danmoseley commented Jan 13, 2021

ericstj left a comment • edited Loading

Choose a reason for hiding this comment

danmoseley commented Jan 13, 2021

Anipik commented Jan 13, 2021

Anipik commented Jan 14, 2021

danmoseley commented Jan 8, 2021 •

edited

Loading

danmoseley commented Jan 9, 2021 •

edited

Loading

ericstj left a comment •

edited

Loading