Revive #48505

[ViktorHofer]

Revive #48505 and try to find out why the change caused official builds to break.

I couldn't apply the patch in src/tests/build.sh as sources changed: https://github.com/dotnet/runtime/pull/48505/files#diff-056300437704f257a4c8567b61dfdb647da75c5ef93baefec47709796792f182.

cc @am11 @ericstj

[dotnet-issue-labeler]

I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label.

[ViktorHofer]

Internal build which should then contain the binlog: https://dnceng.visualstudio.com/internal/_build/results?buildId=1211606&view=results.

[ViktorHofer]

@ericstj the binlog is available in case you want to take a look.

[am11]

Thanks @ViktorHofer!

Does it fix the official build issue which was encountered previously (#48647)?
Few things have changed since, I am looking into FreeBSD cross-build error:

/__w/1/s/.dotnet/sdk/6.0.100-preview.4.21255.9/Sdks/Microsoft.NET.Sdk/targets/Microsoft.NET.Sdk.FrameworkReferenceResolution.targets(101,5): error NETSDK1084: There is no application host available for the specified RuntimeIdentifier 'freebsd-x64'. [/__w/1/s/Build.proj]

[ViktorHofer]

Does it fix the official build issue which was encountered previously (#48647)?

Unfortunately not yet. I opened the PR so that we can take a look at the binlog that is produced in the internal build to figure out what went wrong. @ericstj mentioned that a binlog should tell.

[am11]

FreeBSD error is due to crossgen2's dependency on internal property, the one which starts with underscore (which as I understood it, is discouraged)

runtime/src/coreclr/tools/aot/crossgen2/crossgen2.csproj

Line 10 in 53e6b47

<AppHostRuntimeIdentifier>$(_runtimeOS)-$(TargetArchitectureAppHost)</AppHostRuntimeIdentifier>

changing this line to <AppHostRuntimeIdentifier>$(PackageRID)</AppHostRuntimeIdentifier> fixes the error on FreeBSD cross leg.

[ViktorHofer]

Great observation. Feel free to push into the branch if you want to :)

[am11]

I do not have push access to this branch's remote, pushed to mine am11@f7a6ca2.

[am11]

I couldn't apply the patch in src/tests/build.sh as sources changed: https://github.com/dotnet/runtime/pull/48505/files#diff-056300437704f257a4c8567b61dfdb647da75c5ef93baefec47709796792f182.

To make the test legs green, we would still need to apply the modified form of that patch to make it propagate /p:CrossBuild which we set in top level shell script (eng/build.sh) and want it to be available in all child contexts:

--- a/src/tests/build.sh
+++ b/src/tests/build.sh
@@ -573,6 +573,10 @@ if [[ "${__BuildArch}" != "${__HostArch}" ]]; then
     __CrossBuild=1
 fi

+if (( __CrossBuild == 1 )); then
+    __UnprocessedBuildArgs+=("/p:CrossBuild=true")
+fi
+

[ViktorHofer]

To make the test legs green, we would still need to apply the modified form of that patch to make it propagate /p:CrossBuild which we set in top level shell script (eng/build.sh) and want it to be available in all child contexts:

Applied a minor variation of your patch. Does that look right?

[am11]

Yup, yours is more consistent with the repo style 👍 (it was just numerical comparison instead of string-y one but consistency takes precedence)

[am11]

Remaining failures are unrelated to PR changes. It also does not break the developer workflow, including Windows/VisualStudio and Alpine Linux.

Internal build which should then contain the binlog: https://dnceng.visualstudio.com/internal/_build/results?buildId=1211606&view=results.

@ViktorHofer, (since I cannot access this link) could you please paste the build command which fails on that leg? I will also have a look.

[am11]

One difference between public CI legs and internal build could be that public CI is passing OutputRid explicitly to the cross compilation legs, and perhaps the internal build is not? The reason why we can't rely on the "calculated OutputRid" is that it has two different meaning, which is what this change aims to align.

This change is:

• PackageRID: used to restore packages; in cross compilation, its value is the RID of host (or build) machine.
• OutputRid: used to "create" .nupkgs; in cross compilation, its value is the RID of target machine.

(without any weird exceptional cases, like we have in main branch..)

If you could cherry-pick am11@c01d07c commit on your PR's branch. that should fix the internal build without modifying its .yml file. If not, then I would need binlog / more info to understand the problem.

[ViktorHofer]

Triggered a new internal build: https://dnceng.visualstudio.com/internal/_build/results?buildId=1220899&view=results.

I don't think sharing the internal binlog is a good idea because of potential secrets/env vars but will at least share the command output to let you know if the patch was successful.

Thanks @am11 for your continuous help :)

[Thefrank]

Will this also resolve the issue of native builds (in my case FreeBSD) trying to restore Linux ILASM/ILDASM instead of FreeBSD?
The binlog that was generated seemed to indicate a similar (maybe exact?) RID/variable issue.

[am11]

@Thefrank, if you are referring to #55133, then yes. 🙂

[ViktorHofer]

Awesome, the internal build succeeded 💯

[akoeplinger]

Looks like a good step in the right direction, but would love to simplify it even more :)

[akoeplinger]

from what I understood _hostOS is the OS where the build runs right? in that case Browser doesn't make sense here (I assume we have some other wrong assumption about this somewhere else so this is needed right now).

[ViktorHofer]

Makes sense. Let's untangle this when we eliminate remaining properties which are conflicting with each other.

[am11]

I think in case of browser, value of _hostOS is used in PackageRID below. For other platforms, it is also used to calculate OutputRid (through _portableOS) and MicrosoftNetCoreIlasmPackageRuntimeId when cross-building.

[ViktorHofer]

I see a lot work items stopping during execution and the console log doesn't tell what went wrong. @dotnet/dnceng can you please take a look?

I.e. https://dev.azure.com/dnceng/public/_build/results?buildId=1220900&view=ms.vss-test-web.build-test-results-tab&runId=36394826&resultId=176021&paneView=debug

[ilyas1974]

Thank you for letting us know. I have created https://github.com/dotnet/core-eng/issues/13596 to investigate this.

[clamp03]

ARMEL build is broken because target architecture is not updated to arm for crossgen2.
Please fix it. Thank you.

[ViktorHofer]

@clamp03 can you please open a new issue for that? @am11 do you have time to look at the regression?

[am11]

armel is another community supported platform, which gets broken time to time. For community related things to sustain in the continuous development (for the busy repo like runtime), there should be a CI leg IMO (as we added one last year for GCC build validation). I have the next batch of changes in progress, and will fix it in in that batch soon. Meanwhile, you can apply a local patch and get by it (and please consider adding CI leg as it won't be the last armel breakage otherwise).

[ViktorHofer]

cc @jkotas @safern regarding the armel CI protection discussion above.

[am11]

Armel fix is pushed in 55279: https://github.com/dotnet/runtime/pull/55279/files#diff-9da24614831c308827a1ae533ffea392c97638c261dd42bd0f5226baa136d16eR137-R169.

[safern]

I think it is fine to add an armel CI build that just does a traversal build. Maybe in the dev-innerloop pipeline?

[jkotas]

Adding armel + Tizen CI build leg is tracked by #2394

As @safern said, we are open to adding CI build legs to protect community supported configurations. We expect the community to set them up. The steps for how to do that are in #2394 (comment)

[safern]

LGTM