Allow explicitly skipping crossgen

[trungnt2910]

Add a -skipCrossgen option to build.sh, similar to the option in build.ps1.

This allows the user to both enable -pack and skip Crossgen.

[Copilot]

Pull request overview

Adds a new -skipCrossgen command-line option to eng/build.sh to let users skip Crossgen when invoking the repo build script, aligning the shell script’s surface area more closely with eng/build.ps1.

Changes:

• Add -skipcrossgen option parsing in eng/build.sh to set SkipUsingCrossgen=true for the MSBuild invocation.

[am11]

Currently, it's dotnet/build.sh -p:SkipUsingCrossgen=true .. from VMR. It's only used on two platforms which don't support crossgen2: haiku and illumos (I don't count netbsd (abandondned port for over half a decade now), openbsd (still at early stages of port; libunwind pending among other things)). Making it a top level script option require substantial usecase. The reason here is port-in-progress. I suggest we keep focusing on completing the port and meanwhile rely on -p:SkipUsingCrossgen to get-by the issue.

[trungnt2910]

There might be other use cases, since the Powershell script also supports this option:

sdk/eng/build.ps1

Line 23 in 1ec3763

Write-Host " -skipCrossgen Skip crossgen during layout generation"

[trungnt2910]

I suggest we keep focusing on completing the port and meanwhile rely on -p:SkipUsingCrossgen to get-by the issue.

Ideally yes, but right now, -p:SkipUsingCrossgen is getting overwritten by ./build.sh:

sdk/eng/build.sh

Lines 22 to 27 in 1ec3763

	-os) target_os="$2"; shift ;;
	-arch|-a) target_arch="$2"; shift ;;
	-pack) skip_crossgen=false; skip_installers=false; arguments+=(-pack) ;;
	-test|-t) arguments+=(-test) ;;
	-configuration|-c) arguments+=(-configuration "$2"); shift ;;
	*) arguments+=("$1") ;;

sdk/eng/build.sh

Line 39 in 1ec3763

arguments+=("/p:SkipUsingCrossgen=$skip_crossgen")

[am11]

IMO, crossgen2 and NativeAOT are becoming exceedingly “essential” components of .NET that I think we shouldn’t consider them optional but rather mandatory part of port. One reason is their “support=false” modes are not exercised in CI so they go stale from time to time. As you noted in another thread that it was not too much of a problem to port them for new OS (for new architecture like riscv, it took me a few weeks last year, but only because AI was poor back then).

Maybe it’s best to keep these patches local for now until your PRs are merged (and we don’t need to disable it for haiku anyway)?

[akoeplinger]

This got refactored in 0a74d57, before that there was a comment: skip crossgen for inner-loop builds to save a ton of time. I think it's fine if we restore parity with the powershell version of the script.

[am11]

I think it's fine if we restore parity with the powershell version of the script.

Bash and powershell version are doing the same thing in main branch, PR is changing that. It is just a workaround for local dev. For full build (with packs), crossgen2 is mandatory even in powershell version.

[akoeplinger]

I think this makes sense but we should make sure the powershell version has matching behavior (i.e. also allow overriding crossgen skipping when packing)

[am11]

What we aren't realizing is skipping crossgen2 step during build is a major performance degradation (every method in corelib won't be precompiled and start jitting). This shouldn't be an option for production at all, especially when the only use-case here is to support work-in-progress port which is already blocked on multiple other things.

[trungnt2910]

I think this makes sense but we should make sure the powershell version has matching behavior (i.e. also allow overriding crossgen skipping when packing)

The latest commit should do it.

I had a closer look and it seems that -skipCrossgen and -skipInstallers are currently useless? -oring them with -not $pack in the else block means that the conditions become always true. Meanwhile, the if block ignores these flags entirely.

sdk/eng/build.ps1

Lines 36 to 45 in b1aec5e

	if ($pack) {
	$arguments += " -pack /p:SkipUsingCrossgen=false /p:SkipBuildingInstallers=false"
	} else {
	if ($skipCrossgen -or -not $pack) {
	$arguments += " /p:SkipUsingCrossgen=true"
	}
	if ($skipInstallers -or -not $pack) {
	$arguments += " /p:SkipBuildingInstallers=true"
	}
	}

[trungnt2910]

What we aren't realizing is skipping crossgen2 step during build is a major performance degradation (every method in corelib won't be precompiled and start jitting).

You are making it sound like the end of the world when it has been the reality for every .NET version until .NET 8?

Also, aren't build scripts meant to also support development work, and not just CI machines?

This shouldn't be an option for production at all

If the team is aware that skipping Crossgen in production script is harmful, then why should there be any release script that passes -skipCrossgen (or the underlying MSBuild property) at all?

[am11]

it has been the reality for every .NET version until .NET 8?

All versions of .NET Core have crossgened version of corelib in production. crossgen2 replaced ngen/crossgen(1) at one point. This hasn't changed.

Also, aren't build scripts meant to also support development work, and not just CI machines?

Following the right order of porting prevents blocking on these things. First port runtime (which you did), then shared framework/BCL (which is almost done for Haiku +/- your opened PRs in runtime), then these tools and then SDK and other repos are involved. Besides porting crossgen2 and NativeAOT are pretty trivial for new OS as you've already done in dotnet/runtime#127431.

[trungnt2910]

All versions of .NET Core have crossgened version of corelib in production. crossgen2 replaced ngen/crossgen(1) at one point. This hasn't changed.

Doesn't this flag only disable Crossgen for the SDK?

From what I understand, even with a Crossgen runtime present, we would still be blocked on that change being released and published before the normal build process starts working.

[am11]

From what I understand, even with a Crossgen runtime present, we would still be blocked on that change being released and published before the normal build process starts working.

Codeflow is automatic; runtime PR merged, few hours later bot will open PR in VMR repo (dotnet/dotnet), once that one is merged, bot will open PR in SDK and other repos with "from dotnet/dotnet". That's a continuous sync process.

My suggestion is that we align build.sh with build.ps1 by keeping the option to skip this step for debug builds, which saves a few minutes and is primarily intended to improve developer inner-loop times. Using the flag as a workaround because “crossgen2 is not ported” seems like the wrong abstraction and purpose for the option.

[trungnt2910]

porting crossgen2 and NativeAOT are pretty trivial for new OS as you've already done in dotnet/runtime#127431.

The non-trivial thing here would be the review process - which is perfectly understandable given the massive scale of this project and the great work they are doing - so some quick unblocker like this is desirable.

[am11]

Haiku port is going on since 2021, we are clearly not in a rush that we can't live with a local patch for a few more weeks.

[akoeplinger]

Using the flag as a workaround because “crossgen2 is not ported” seems like the wrong abstraction and purpose for the option.

you could say the same thing about many other options across the stack, while porting you often need to make choices that are not optimal to make progress.

I think there are two things here:

1. Having an option to disable crossgen2 in sdk build scripts
2. Using that option to workaround a porting issue

This PR is about number one and it makes sense to me to have it. Whether it will be used for number two as well is a separate topic IMO

[am11]

while porting you often need to make choices that are not optimal to make progress.

The port which is still in progress in runtime shouldn't need SDK in first place.

Having an option to disable crossgen2 in sdk build scripts

Do we have this option in other repos in VMR? e.g. aspnetcore also uses crossgen2.

[trungnt2910]

The port which is still in progress in runtime shouldn't need SDK in first place.

SDK is what half of the dotnet command user experience relies on. You cannot expect the average user to scp assemblies and then corerun them.

The runtime port is, once again, usable(-ish?), but is undergoing a long review process that can only proceed when everybody has time.

[akoeplinger]

Do we have this option in other repos in VMR? e.g. aspnetcore also uses crossgen2.

no, but we do have the option here so why not make it actually useful?

let me know if you feel strongly about this being wrong, otherwise I'd be inclined to merge it.

[am11]

let me know if you feel strongly about this being wrong

No problem with getting this merged, just trying to prevent this mode from spreading since it's not what gets exercised in the CI (and not the configuration any platform/distro maintainer ships .NET with).

For porting of .NET to new platform, the high-level order is:

• port coreclr+libunwind (this takes most time for new OS and even longer for new arch)
• port libraries (only a few of those require platform-specific stuff)
• port AOT tools (R2R, AOT... for new OS it's ~50-200 lines of delta, for new arch it's a bit more work but for riscv64, NativeAOT took me a week of collab with humans and AI.. pretty manageable)
• port SDK (basically add OS/arch name in a few lists "run git grep -i freebsd and follow the trai")
• add RID in asp.net crossgen2 target
• validate end-to-end with VMR
• [after the .NET port] there are few semi-thirdparty libs like https://github.com/ericsink/cb, libskiasharp etc.

I know when @trungnt2910 started Haiku's port, this process was different and a bit unclear for "new platform bringup", now things more streamlined (still there is margin for improvement).

You cannot expect the average user to scp assemblies and then corerun them.

We can use dotnet with scp workflow as well; added steps to doc in ongoing PR: https://github.com/dotnet/runtime/pull/127909/changes#diff-a6f2ef85aaf1e9fe4f8bd5696e25492211e215a8daa946daa78cffdcab9a8d10. I remember Jan mentioned the "supported" option for port is cross-build. After the initial port is completed and we have a working SDK out of VMR, we can use it on the target platform and run ./build.sh without --cross.

[akoeplinger]

No problem with getting this merged, just trying to prevent this mode from spreading since it's not what gets exercised in the CI

ok, yeah we're in agreement here.