Fix alpine build

[am11]

Changes in tests/IL2C.Core.Test.Common/ILSupport.Standard.targets:

• On Unix-like OS, il{d}asm treat / character as a filepath and gives errors where options are passed. Replacing / with - fixes the problem.
• On musl-libc based distros like Alpine, the runtime being used linux-x64 rather than linux-musl-x64 because of hardcoded list of runtimes. The fix for that is to use NETCoreSdkPortableRuntimeIdentifier.
  • Since portable identifier is new in .NET 5 onwards (dotnet/installer@003b1d1), also added a fallback to NETCoreSdkRuntimeIdentifier.
  • This is to support older SDK (do we need netcore 3x, 2x and netfx support?), which is why that part of patch looks a bit different than Use SDK-selected runtime in IL SDK dotnet/runtime#60400.

Change in tests/IL2C.Core.Test.Common/IL2C.Core.Test.Common.csproj was based on feedback I got upstream. This allows us to use .entrypoint in IL if we ever wanted.

[am11]

With dotnet 5+ in PATH

$ git clone https://github.com/kekyo/IL2C --single-branch --depth 1 --branch devel
$ cd IL2C
$ ./build-runtime.sh
$ dotnet test il2c.sln

With this patch it succeeds the build but fails when "running" the tests because I don't have "mono" installed. We could update MSTest package which supports modern .NET, or perhaps better yet, switch tests to xunit or nunit (since .NET teams are using xunit as well rather than MStest.. 🙂).

[am11]

• portable runtime identifer $(NETCoreSdkPortableRuntimeIdentifier) is a distro-agnostic ID which we need for MicrosoftNetCoreIlasmPackageRuntimeId.
• non-portable runtime identifier $(NETCoreSdkRuntimeIdentifier) on the other hand, is distro-aware. For instance, if the SDK was built with -p:PortableBuild=false. This is the case when some distros (like RHEL, Fedora etc.) build the SDK in their packages. But the one provided by .NET team is always portable.

MicrosoftNetCoreIlasmPackageRuntimeId requires a portable RID, since the packages are published by .NET team for only portable flavors. Since NETCoreSdkPortableRuntimeIdentifier is new in .NET 5, if we are building this project on, e.g. Fedora, and we installed dotnet-sdk 3.1 package from the distro, we would need to pass the correct portable RID explicitly dotnet test il2c.proj -p:MicrosoftNetCoreIlasmPackageRuntimeId=linux-x64 in such environment (I think we can live with that since it's not a very common setup and only effects older version of runtimes). Otherwise if we are using SDK 3x provided by .NET team, it will return the correct value of portable RID (linux-x64) from NETCoreSdkRuntimeIdentifier.

[kekyo]

(Thank you PR, I'm reading now...)

On Unix-like OS, il{d}asm treat / character as a filepath and gives errors where options are passed. Replacing / with - fixes the problem.

I understood after last merged my PR commit, I applied same patch :)

On musl-libc based distros like Alpine, the runtime being used linux-x64 rather than linux-musl-x64 because of hardcoded list of runtimes. The fix for that is to use NETCoreSdkPortableRuntimeIdentifier.

I think it's a good change. I don't know, why the scripts in the Sdk IL directive 5.0.0 package were hard-coded to identify from _OSPlatform instead of using NETCoreSdkRuntimeIdentifier? Do you know?

(To be honest, when I saw that code, I thought it was dirty hardcoding :)

I'm trying to figure out if I should leave that hardcoding in place when NETCoreSdkRuntimeIdentifier is undefined....

[kekyo]

With this patch it succeeds the build but fails when "running" the tests because I don't have "mono" installed. We could update MSTest package which supports modern .NET, or perhaps better yet, switch tests to xunit or nunit (since .NET teams are using xunit as well rather than MStest.. 🙂).

Oh... I forgot that the mono runtime was installed.

Is mono used because of Microsoft.NET.Test.Sdk? I hadn't thought about it seriously, but I thought it was because the test project tfm was net45. (Untested) I think that migrating the test code project to netstandard2.0 and the test fixture project to net5.0 will solve the problem.... (?)

[kekyo]

MicrosoftNetCoreIlasmPackageRuntimeId requires a portable RID, since the packages are...

(I agree that this code is for test projects, so it may not make sense to ponder it)

It's a very ugly and dirty method :) : If NETCoreSdkPortableRuntimeIdentifier is not defined and NETCoreSdkRuntimeIdentifier is split ('-'), it will be $"{elem[0]}-{elem[2]}". How about using a value?

[am11]

After .NET 3.1, three repositories were combined (coreclr, corefx and core-setup) into one (runtime). After that there was a series of consolidation work done in runtime repo. The whole RID stuff is now calculated in the runtime in one place https://github.com/dotnet/runtime/blob/53e8c7/Directory.Build.props#L157. It is to support platforms, which are new and don't have the official identifier assigned. To port runtime to a new platform, we first define the _DistroRID in runtime, OSPlatform and so forth to be able to compile stuff before SDK can be ported. The stuff in IL SDK .targets was a leftover of that consolidation work, which is used in the fallback path.

IL SDK was primarily created by runtime team for a shipping assembly: System.Runtime.CompilerServices.Unsafe. This is a real assembly part of Shared Framework (sfx). The said SDK was then used in runtime for test projects like ilverify tests etc. It was also published to nuget.org for public consumption, but there aren't many consumers of it yet, besides the runtime repository itself. I found a bug in Alpine Linux (my personal desktop environment installed on a laptop) when testing out IL2C's devel branch, so I proposed a change upstream (dotnet/runtime#60400). It is approved and will merge soon. This PR is basically pieces took from that PR. :)

For public consumption, it makes sense to use NETCoreSdkPortableRuntimeIdentifier because consumer is always running it in the context of .NET SDK (which the IL SDK inherits).

[kekyo]

I encounted tfm specific bugs on porting from Azure Pipeline to GitHub Actions working (#107)...

(Maybe bit relating this topic)

Could you wait continue reviewing? I will back to discussion immediately.

[kekyo]

#107 closed with some regression errors on ubuntu environment. But maybe ready to build-and-test on linux ;)

I will approve your opinion using NETCoreSdkPortableRuntimeIdentifier variable, and requires set variable manually on some environment and using older SDK version.

Is there anything else? I will merge PR if not.

[am11]

Is mono used because of Microsoft.NET.Test.Sdk?

It looks like a netfx specific project is requiring mono. When I run dotnet test il2c.sln, it gives the following error on running the test:

Starting test execution, please wait...
A total of 1 test files matched the specified pattern.
System.IO.FileNotFoundException: Could not find 'mono' host. Make sure that 'mono' is installed on the machine and is available in PATH environment variable.
   at Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.Helpers.DotnetHostHelper.GetMonoPath()
   at Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.Hosting.DefaultTestHostManager.GetTestHostProcessStartInfo(IEnumerable`1 sources, IDictionary`2 environmentVariables, TestRunnerConnectionInfo connectionInfo)
   at Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.Client.ProxyOperationManager.SetupChannel(IEnumerable`1 sources, String runSettings)
   at Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.Client.ProxyExecutionManager.StartTestRun(TestRunCriteria testRunCriteria, ITestRunEventsHandler eventHandler)

Test Run Aborted.

but if I pass --framework, dotnet test il2c.sln -f net5.0, then it doesn't complain about it, which is ok if we want to keep netfx for building and testing.

There are other test failures, which we can later fix. Some failures are funny as expected and actual values are same 🙂

  Failed ToString(-1.7976931348623157E+308) [1 s]
  Error Message:
   System.Exception : test [ExitCode=1]: Failed: System_String*: expected="-", actual="-"

  Stack Trace:
     at IL2C.CMakeDriver.BuildAsync(String binPath, String configuration, String sourcePath, String il2cRuntimePath) in /home/am11/projects/IL2C/tests/IL2C.Core.Test.Fixture/CMakeDriver.cs:line 179
   at IL2C.TestFramework.ExecuteTestAsync(TestCaseInformation caseInfo) in /home/am11/projects/IL2C/tests/IL2C.Core.Test.Fixture/TestFramework.cs:line 420
   at NUnit.Framework.Internal.TaskAwaitAdapter.GenericAdapter`1.BlockUntilCompleted()
   at NUnit.Framework.Internal.MessagePumpStrategy.NoMessagePumpStrategy.WaitForCompletion(AwaitAdapter awaiter)
   at NUnit.Framework.Internal.AsyncToSyncAdapter.Await(Func`1 invoke)
   at NUnit.Framework.Internal.Commands.TestMethodCommand.RunTestMethod(TestExecutionContext context)
   at NUnit.Framework.Internal.Commands.TestMethodCommand.Execute(TestExecutionContext context)
   at NUnit.Framework.Internal.Execution.SimpleWorkItem.<>c__DisplayClass4_0.<PerformWork>b__0()
   at NUnit.Framework.Internal.ContextUtils.<>c__DisplayClass1_0`1.<DoIsolated>b__0(Object _)
   at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state)
--- End of stack trace from previous location ---
   at System.Threading.ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, Object state)
   at NUnit.Framework.Internal.ContextUtils.DoIsolated(ContextCallback callback, Object state)
   at NUnit.Framework.Internal.ContextUtils.DoIsolated[T](Func`1 func)
   at NUnit.Framework.Internal.Execution.SimpleWorkItem.PerformWork()

Is there anything else? I will merge PR if not.

From my point of view, it is ready. :)

[kekyo]

but if I pass --framework, dotnet test il2c.sln -f net5.0, then it doesn't complain about it, which is ok if we want to keep netfx for building and testing.

I recommend it, because I found difference regression test results between net462 and net5.0. I will suppress net462 regression test in future commit.

There are other test failures, which we can later fix. Some failures are funny as expected and actual values are same 🙂

I feel it too, (unverified) I think the unicode bytes was applied with ansi (standard) C string functions...? ( swprintf() --> sprintf() )

[kekyo]

@am11 Will be merged, moving on #100