Runtime: PlatformDetection IsWinRT and IsNetNative return confusing values when running uap or uap-aot tests

Created on 18 May 2017  路  20Comments  路  Source: dotnet/runtime

While exploring solutions to dotnet/runtime#21774, I discovered the PlatformDetection properties IsWinRT and IsNetNative return confusing values when running uap or uap-aot tests.

For example:

// Test against .NET Core (non-UAP)
msbuild /t:rebuildandtest

// Result (good)
IsWinRT=False, IsNetNative=False

// Test against .NET Core UAP without .NET Native (ILC) (F5 scenario)
msbuild /t:rebuildandtest /p:TargetGroup=uap

// Result (weird, why is IsNetNative true?)
IsWinRT=True, IsNetNative=True

// Test against .NET Core UAP with .NET Native (ILC)
msbuild /t:rebuildandtest /p:TargetGroup=uapaot

// Result (weird, why is IsWinRT false?  Is it because it is now a native app?)
IsWinRT=False, IsNetNative=True
area-Infrastructure-libraries test bug

All 20 comments

cc: @AlexGhiondea @stephentoub @karelz @weshaggard @ViktorHofer

IsWinRT should be read as IsRunningAsAppPackage but it's currently being used to mean IsRunningInAppContainer. We should probably rename it that, although the implementation is not directly testing for app container.

IsNetNative should be changed to false in the F5 scenario because it's currently being used to mean IsSubjectToProjectNRuntimeOrToolchainLimitations

@JeremyKuhne

@AtsushiKan any opinion about a better name than IsSubjectToProjectNRuntimeOrToolchainLimitations? And what is the best way to determine whether it should be true? Right now it's doing RuntimeInformation.FrameworkDescription.StartsWith(".NET Native"..) which returns true in the F5 scenario which I don't think is desired.

IsNetNative sounds like the best name to me but I guess it doesn't sync with our branding. IsNetNativeReleaseBuild is I guess how a VS customer would think of it.

IsRecipeForHoursOfRdXmlPain might be a good second choice.

As far as implementation, it's not a "good" way but this will do in a pinch:

IsNetNative => RuntimeInformation.FrameworkDescription.StartsWith(".NET Native"..) && typeof(int).GetType().GetType() != typeof(int[]).GetType().GetType();

@ViktorHofer do you want to take this one?

  1. Rename IsWinRT to IsInAppContainer. We can correctify the implementation another day.
  2. Fix IsNetNative to return RuntimeInformation.FrameworkDescription.StartsWith(".NET Native"..) && typeof(int).GetType().GetType() != typeof(int[]).GetType().GetType();. Be sure to run the tests that haev this today in the UAP configuration as they will start to run there.

// Result (weird, why is IsWinRT false? Is it because it is now a native app?)
IsWinRT=False, IsNetNative=True

If we rename IsWinRT to IsInAppContainer I would assume that the result is now correct as we are not running uapaot inside an AppContainer yet ;)

@ViktorHofer

Just realized the code snippet I gave has too many GetType()'s. The correct one to use is:

IsNetNative => RuntimeInformation.FrameworkDescription.StartsWith(".NET Native"..) && typeof(int).GetType() != typeof(int[]).GetType();

Is "NetNative" something restricted to UWP? I thought it'll be an option for any app, even non-UWP or even non-Windows.

@AtsushiKan Ya I wondered. But typeof RuntimeType is RuntimeType so it wouldn't have caused harm.

Is "NetNative" something restricted to UWP? I thought it'll be an option for any app, even non-UWP or even non-Windows.

Non-UWP uses of "NetNative" mean CoreRT scenarios.

Non-UWP uses of "NetNative" mean CoreRT scenarios.

Which I guess will also refer to the name "NetNative" (in future).

Which I guess will also refer to the name "NetNative" (in future).

I would think so. I think the "IsNetNative" property should return true anytime ILC.EXE has been used to convert the managed IL to native code.

@AtsushiKan what is special about typeof(int).GetType() != typeof(int[]).GetType(); that makes it look like .NET Native? If we use that we should at least leave a comment as to why.

CoreCLR uses a single type (RuntimeType) to represent all Types, CoreRT uses a type hierarchy. That's an implementation detail that isn't likely to change with any speed.

@davidsh @danmosemsft does this still apply?

I don't know if it is still repros.

With the latest addition of TargetFrameworkMoniker.UapNotUapAot, we now have some clarity for our tests.

TargetFrameworkMoniker.Uap (means both UAP and UAPAOT)
TargetFrameworkMoniker.UapAot (means just UAPAOT)
TargetFrameworkMoniker.UapNotUapAot (means just UAP)

And when we need to use PlatformDetection class, we use PlatformDetection.IsUap. And currently
PlatformDetection.IsUap means both UAP and UAPAOT which is good since this also matches the ifdef mechanism we use in source code for things like this:

```c#

if UAP

// source code to use only in UAP implementations of the binary

else

// source code to use in netstandard implementations of the binary

endif

```

I don't know if we have any need to use PlatformDetection.IsWinRT or PlatformDetection.IsNetNative anymore for most things.

Sometimes it's helpful or necessary to do dynamic checks, if you don't want to break a test into UAP and non UAP versions.
Also, if the code is specifically to handle being in app container, I prefer to use IsWinRT (which should be renamed) rather than TFM.UapNotUapAot because that code won't need fixing again if and when we want to run UapAot tests in app container.

We should do the rename now to IsAppContainer

Later we should ideally change the implementation to check "is low integrity" rather than "am I an app" . That's not worth changing now as they'll be the same thing for us for now. BTW, here's an implementation of "is in low integrity": https://referencesource.microsoft.com/#System/sys/System/EnvironmentHelpers.cs,15

Also, if the code is specifically to handle being in app container, I prefer to use IsWinRT (which should be renamed) rather than TFM.UapNotUapAot because that code won't need fixing again if and when we want to run UapAot tests in app container.

This one of course could be addressed by adding TFM.IsInAppContainer = TFM.UapNotUapAot and later change it to = TFM.Uap.

+1 what Dan said.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

bencz picture bencz  路  3Comments

chunseoklee picture chunseoklee  路  3Comments

Timovzl picture Timovzl  路  3Comments

EgorBo picture EgorBo  路  3Comments

jamesqo picture jamesqo  路  3Comments