Fix TargetOS value linux instead of Unix

[am11]

Regression from 4ece8f0 (see src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.targets).

Daily build of SDK (8.0.100-alpha.1.23068.3) is failing with:

$  dotnet publish -o dist -p:PublishAot=true
...
  foo -> /foo/bin/Debug/net8.0/linux-arm64/foo.dll
  Generating native code
  Unhandled exception. System.CommandLine.CommandLineException: Target OS 'Unix' is not supported
     at System.CommandLine.Helpers.GetTargetOS(String) in /_/src/coreclr/tools/Common/CommandLineHelpers.cs:line 82
     at System.CommandLine.Argument`1.<>c__DisplayClass5_0.<.ctor>b__1(ArgumentResult, Object& )
     at System.CommandLine.Parsing.ArgumentResult.Convert(Argument)
     at System.CommandLine.Parsing.ArgumentResult.GetArgumentConversionResult()
     at System.CommandLine.Parsing.ParseResultVisitor.ValidateAndConvertArgumentResult(ArgumentResult)
     at System.CommandLine.Parsing.ParseResultVisitor.ValidateAndConvertOptionResult(OptionResult)
     at System.CommandLine.Parsing.ParseResultVisitor.Stop()
     at System.CommandLine.Parsing.Parser.Parse(IReadOnlyList`1, String )
     at ILCompiler.Program.Main(String[]) in /_/src/coreclr/tools/aot/ILCompiler/Program.cs:line 672
  Aborted
/root/.nuget/packages/microsoft.dotnet.ilcompiler/8.0.0-alpha.1.23067.2/build/Microsoft.NETCore.Native.targets(281,5): error MSB3073: The command ""/root/.nuget/packages/runtime.linux-arm64.microsoft.dotnet.ilcompiler/8.0.0-alpha.1.23067.2/tools/ilc" @"obj/Debug/net8.0/linux-arm64/native/foo.ilc.rsp"" exited with code 134. [/foo/foo.csproj]

copied this block: https://github.com/am11/runtime/blob/feature/nativeaot/build-integration2/src/coreclr/nativeaot/BuildIntegration/Microsoft.DotNet.ILCompiler.SingleEntry.targets#L5-L8 plus a line from @Thefrank's PR.

[ghost]

Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Regression from 4ece8f0 (see src/coreclr/nativeaot/BuildIntegration/Microsoft.NETCore.Native.targets).

Daily build of SDK (8.0.100-alpha.1.23068.3) is failing with:

$  dotnet publish -o dist -p:PublishAot=true
...
  foo -> /foo/bin/Debug/net8.0/linux-arm64/foo.dll
  Generating native code
  Unhandled exception. System.CommandLine.CommandLineException: Target OS 'Unix' is not supported
     at System.CommandLine.Helpers.GetTargetOS(String) in /_/src/coreclr/tools/Common/CommandLineHelpers.cs:line 82
     at System.CommandLine.Argument`1.<>c__DisplayClass5_0.<.ctor>b__1(ArgumentResult, Object& )
     at System.CommandLine.Parsing.ArgumentResult.Convert(Argument)
     at System.CommandLine.Parsing.ArgumentResult.GetArgumentConversionResult()
     at System.CommandLine.Parsing.ParseResultVisitor.ValidateAndConvertArgumentResult(ArgumentResult)
     at System.CommandLine.Parsing.ParseResultVisitor.ValidateAndConvertOptionResult(OptionResult)
     at System.CommandLine.Parsing.ParseResultVisitor.Stop()
     at System.CommandLine.Parsing.Parser.Parse(IReadOnlyList`1, String )
     at ILCompiler.Program.Main(String[]) in /_/src/coreclr/tools/aot/ILCompiler/Program.cs:line 672
  Aborted
/root/.nuget/packages/microsoft.dotnet.ilcompiler/8.0.0-alpha.1.23067.2/build/Microsoft.NETCore.Native.targets(281,5): error MSB3073: The command ""/root/.nuget/packages/runtime.linux-arm64.microsoft.dotnet.ilcompiler/8.0.0-alpha.1.23067.2/tools/ilc" @"obj/Debug/net8.0/linux-arm64/native/foo.ilc.rsp"" exited with code 134. [/foo/foo.csproj]

copied this block: https://github.com/am11/runtime/blob/feature/nativeaot/build-integration2/src/coreclr/nativeaot/BuildIntegration/Microsoft.DotNet.ILCompiler.SingleEntry.targets#L5-L8 plus a line from @Thefrank's PR.

Author:	am11
Assignees:	-
Labels:	area-NativeAOT-coreclr
Milestone:	-

[am11]

@jkotas this is an alt. fix for #80791.

[ViktorHofer]

Why do we need to pass a target OS value to ILC at all? Before 4ece8f0, we didn't do that as ILC already has OS runtime detection code.

[MichalStrehovsky]

Why do we need to pass a target OS value to ILC at all? Before 4ece8f0, we didn't do that as ILC already has OS runtime detection code.

FreeBSD is a first instance of where we need to do cross-OS build. In our infrastructure, the build machine is Linux. The target is FreeBSD. Before FreeBSD the most advanced thing we needed was a cross-architecture build (from x64 to arm64, but same OS).

[MichalStrehovsky]

Since the revert merged, could you add a revert of the revert (or just the extra IlcArg to specify target os)?

[MichalStrehovsky]

Let's also wait for dotnet/sdk#29800 to merge so that if there's something wrong, we don't break it again. I'll mark this no merge for now.

[am11]

could you add a revert of the revert

This PR is targeting the main branch. We do not support NativeAOT FreeBSD in current main branch. First PR doing the actual work is #80323.

FreeBSD is a first instance of where we need to do cross-OS build.

It's not supported even with that PR, cross compilation is disabled: https://github.com/dotnet/runtime/pull/80323/files#diff-115c40d5f6cdfb8ebea1490c13581e76b60c3430cbf13f45c270fc5ec383daffR22

[MichalStrehovsky]

could you add a revert of the revert

This PR is targeting the main branch. We do not support NativeAOT FreeBSD in current main branch. First PR doing the actual work is #80323.

I meant that #80791 removed passing TargetOS to ILC. So with this PR we would fill out the property, but still not actually use it.

[am11]

I mean that revert is best suited for #80323, not this PR. (i.e. we shouldn't have taken that one-off NativeAOT related change in #71486 in first place)

[MichalStrehovsky]

I mean that revert is best suited for #80323, not this PR. (i.e. we shouldn't have taken that one-off NativeAOT related change in #71486 in first place)

Agreed on both counts but I'm also a bit worried about PRs that only update dead code. If we make this actually use the value of target OS we get at least some coverage.

[am11]

Yup, TargetOS is also used in conditions to distinguish between Windows, OSX and everything_else. It is not completely unused. @ViktorHofer was suggesting to prefer lower-casing the OS name #80791 (comment), which this PR is doing (Unix -> linux as a default value; follow up of #80164).

[MichalStrehovsky]

Makes sense. Let's wait for the SDK update to merge and we can merge this too.

[Thefrank]

As both my pending PR and me look responsible for this one...how do I prevent BreakEverything() from happening in the future?

In all cases I came across the os logic looks like windows -> osx -> linux -> I_have_no_idea_what_this_is_so_nonportable_linux (or windows -> linux -> osx -> ?)
I tried to make sure to leave the defaults in place and only insert freebsd things where needed, but it appears I overlooked something, or worse, poked something that did not need poking.

[am11]

how do I prevent BreakEverything() from happening

@Thefrank, at least this issue was avoidable if #71486 hadn't touched anything related to "NativeAOT on FreeBSD" and left it for #80323.

Generally, yes it is sometimes a pain as some of these issues are revealed during the integration or code flow. The runtime CI does some testing, but since there are growing number of configurations in NativeAOT, it is not possible to test all combinations in the runtime repo.

[jkotas]

how do I prevent BreakEverything() from happening

We have tiered validation system. Some issues are only going to get caught in secondary tiers.

[sec]

@Thefrank, at least this issue was avoidable if #71486 hadn't touched anything related to "NativeAOT on FreeBSD" and left it for #80323.

Yeah that's on me - I was thinking those were needed for crossgen to work as it was failing/generating Linux binaries instead of FreeBSD ones, hence the TargetOS changes :( Sorry for that

[am11]

Let's also wait for dotnet/sdk#29800 to merge

It's merged. Removed the labels.