search

kekyo / IL2C

IL2C - A translator for ECMA-335 CIL/MSIL to C language.
Apache License 2.0
402 stars 36 forks source link

Fix alpine build #106

Closed am11 closed 3 years ago

am11 commented 3 years ago

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

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 commented 3 years ago

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.. :slightly_smiling_face:).

kekyo commented 3 years ago

(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 commented 3 years ago

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 commented 3 years ago

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 andNETCoreSdkRuntimeIdentifier is split ('-'), it will be $"{elem[0]}-{elem[2]}". How about using a value?

am11 commented 3 years ago

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 (https://github.com/dotnet/runtime/pull/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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 :slightly_smiling_face:

  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 commented 3 years ago

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 commented 3 years ago

@am11 Will be merged, moving on #100