BinaryCompatibility type returning the wrong values

[stephentoub]

When corefx upgraded to a newer coreclr in dotnet/corefx#12836, some async-related tests started breaking. It appears to be due to System.Runtime.Versioning.BinaryCompatibility.TargetsAtLeast_Desktop_V4_5 now returning false, which has the effect of reverting several fixes made in coreclr years ago.

[jkotas]

Looks like regression introduced by dotnet/coreclr#5708

[stephentoub]

Looks like regression introduced by dotnet/coreclr#5708

Yup. With this program:

using System;
using System.Reflection;

class Test
{
    public static void Main()
    {
        foreach (var pi in typeof(object).Assembly.GetType("System.Runtime.Versioning.BinaryCompatibility").GetProperties(BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Static))
        {
            Console.WriteLine(pi.Name + ": " + pi.GetValue(null, null));
        }
    }
}

if I run this against what's in master, I get:

TargetsAtLeast_Desktop_V4_5: False
TargetsAtLeast_Desktop_V4_5_1: False
AppWasBuiltForFramework: Unspecified
AppWasBuiltForVersion: 0

If I then revert dotnet/coreclr@0e4869a and run again, I get:

TargetsAtLeast_Desktop_V4_5: True
TargetsAtLeast_Desktop_V4_5_1: True
AppWasBuiltForFramework: NetFramework
AppWasBuiltForVersion: 50000

so that commit does appear to be the cause.

[AlexGhiondea]

Indeed. The commit also defaults the tfm to unspecified instead of netframework and version 50000.

I can change the behavior for the BinaryCompatibility class to default to 50000.

Should we consider changing the quirks behavior to use AppContext?

[jkotas]

The BinaryCompatibility quirks should be deleted (once the github / TFS mirror is turned off).

[KristinXie1]

Disabled test, reopen it.

[AlexGhiondea]

@JiayiLi1 I am not sure what test you are referring to.

[jkotas]

@AlexGhiondea https://github.com/dotnet/corefx/blob/b57805bd336892e707bf5238e86e31aef98e4bce/src/System.Threading.Tasks/tests/TaskFactory/TaskFactory_FromAsyncTests.cs#L563

[AlexGhiondea]

Thanks for the link @jkotas -- I didn't realize we added a test for this. I will submit a PR to re-enable this.