Add --os and --arch in VMR build script

[am11]

Usage goes from:

./build.sh --clean-while-building -sb --rid fedora-riscv64 -p:TargetOS=linux -p:TargetArchitecture=riscv64 -p:Crossbuild=true 

to:

./build.sh --clean-while-building -sb -rid fedora-riscv64 --os linux --arch riscv64 --cross

[am11]

cc @ViktorHofer, @tmds

Current experience is not very nice when it comes to cross-build. Even for portable RID linux-riscv64, it doesn't infer architecture from --rid and we have to explicitly spell out the property name TargetArchitecture. I think making all constituents separate args would make it slightly pleasant to work with.

(Noticed it while testing dotnet/runtime#114285 here: https://github.com/am11/dotnet-riscv/actions/runs/14551082308/workflow)

[ViktorHofer]

Please add them to build.ps1 and add the long forms as well (target-os, target-architecture).

Would also be good if you could update the vmr-build.yml and use them.

[tmds]

When I'm working later this week, I'll share what flags we're passing in our builds to provide some empirical data.

[ViktorHofer]

Thanks 👍

[tmds]

Since @ViktorHofer already approved the PR, I wanted to have a look already at what we're passing in our builds.

When we cross-build on Linux with a cross-compilation toolchain we only set /p:TargetArchitecture=<arch>.
For that, we can use the --target-arch that gets added here.

We don't seem to be setting /p:CrossBuild=true. It seems not all cross-builds need this flag? When is this flag needed?

Can we also add an argument for /p:PortableBuild=true? We set this in our cross-builds for the same reasons mentioned by the FreeBSD maintainers in dotnet/source-build#5035.

[am11]

We don't seem to be setting /p:CrossBuild=true. It seems not all cross-builds need this flag? When is this flag needed?

CrossBuild=true is optional. I was testing with @jkoritzinsky's bootstrapping changes, it's automatically set when targetArch!=hostArch: https://github.com/jkoritzinsky/runtime/blob/dotnetbuild-local-props-bootstrap/eng/DotNetBuild.props#L37. I just wanted to be explicit as it's a required argument when building dotnet/runtime (and makes it clear).

Also, I opened dotnet/runtime#115002. We might need it in dotnet/sdk as a hint to pick host crossgen2 during the SDK build.

Can we also add an argument for /p:PortableBuild=true? We set this in our cross-builds for the same reasons mentioned by the FreeBSD maintainers in dotnet/source-build#5035.

The default for cross compilation is PortableBuild=true these days:

sdk/src/SourceBuild/content/Directory.Build.props

Lines 61 to 65 in 5d6ce56

	<!-- Source-only builds are non portable, except for cross-builds.
	Source-only cross-builds default to the portable configuration so the resulting SDK works on a wider range of distros. -->
	<PortableBuild Condition="'$(PortableBuild)' == '' and '$(DotNetBuildSourceOnly)' == 'true' and '$(BuildArchitecture)' == '$(TargetArchitecture)'">false</PortableBuild>
	<PortableBuild Condition="'$(PortableBuild)' == ''">true</PortableBuild>

I have added the argument.

[tmds]

CrossBuild=true is optional. I was testing with @jkoritzinsky's bootstrapping changes, it's automatically set when targetArch!=hostArch: https://github.com/jkoritzinsky/runtime/blob/dotnetbuild-local-props-bootstrap/eng/DotNetBuild.props#L37. I just wanted to be explicit as it's a required argument when building dotnet/runtime (and makes it clear).

Afaik before @jkoritzinsky's changes, nothing in our cross-builds caused CrossBuild to be set to true.

Why do some cross-builds need to have this set to true and others do not?

When you say it is optional, what does that mean?

[am11]

Why do some cross-builds need to have this set to true and others do not?

The underlying concept revolve around ROOTFS_DIR. It is required when cross-building runtime, CrossBuild validates it. On mobile platforms, such as Android, we use the compiler from NDK (rather than host compiler with -sysroot=$ROOTFS_DIR), so we don't need it there. Similarly iOS, macCatalyst etc, don't require $ROOTFS_DIR as they require corresponding SDKs to be installed on macOS host (at a per-determinied system location out of our control). Other Unix-likes (desktop platforms), require it, e.g. Linux, FreeBSD and illumos (and soon Haiku).

When you say it is optional, what does that mean?

I meant if we don't specify it, runtime/diagnostics repos infra will figure it out (and issue an error if directory pointed by $ROOTFS_DIR doesn't exist).

[am11]

If we can figure out the "kind" of cross build in code, then we can remove this confusion. i.e. CrossBuild / -cross to always mean "build platform and target platform are different" and validate ROOTFS_DIR presence only when applicable. Architecture check is not enough as we cross-build stuff like freebsd-x64 on linux-x64 (e.g. in runtime CI).

[tmds]

Would this help?

<InnerBuildArgs Condition="'$(CrossBuild)' == 'true' or ('$(TargetArchitecture)' != '$(BuildArchitecture)' and '$(TargetsMobile)' != 'true')">$(InnerBuildArgs) $(FlagParameterPrefix)cross</InnerBuildArgs>

to

<InnerBuildArgs Condition="'$(CrossBuild)' == 'true' or ('$(TargetOS)-$(TargetArchitecture)' != '$(BuildOS)-$(BuildArchitecture)' and '$(TargetsMobile)' != 'true')">$(InnerBuildArgs) $(FlagParameterPrefix)cross</InnerBuildArgs>

[am11]

Currently, BuildOS is an RID instead of the OS: dotnet/runtime#114755 (comment).

[tmds]

And if it were made to match its Arcade definition?

[tmds]

I see you linked to BaseOS, not BuildOS. I assume the latter actually is an OS?

[am11]

Ah yes. We can update it in runtime once bootstrap changes go in.

[tmds]

Sounds good. Let's try that instead of adding a -cross flag here.

[am11]

I think it’s ok to have an explicit —cross option.

[ViktorHofer]

We can't take any potentially breaking changes right now. Several things need to happen before we drastically change entry points:

• VMR ships P4
• Back flow and forward flow is enabled meaning dotnet/dotnet is the new home for these types of changes
• Inner build complexity is removed.

I'm happy to take this if we keep it scoped down to OS+Arch. I would recommend to revert the addition of the cross flag if we agree that we can eventually infer it.

I'm not yet sold on the necessity for a portable flag CLI arg. Can you please elaborate on why it's usedul as a VMR input parameter?

Overall, let's keep in mind that the VMR is an msbuild orchestrator with a super thin script based entry point that only translates args.

[tmds]

I don't think this PR is required for the next preview. We can merge it when timing is appropriate.

I would recommend to revert the addition of the cross flag if we agree that we can eventually infer it.

Based on the discussion, we can defer it.

I'm not yet sold on the necessity for a portable flag CLI arg. Can you please elaborate on why it's usedul as a VMR input parameter?

The flag triggers a build configuratie that is expected to work across a larger set of distros.

I forgot I already made it part of the default behavior for cross-build. I still think it is useful to increase visibility based on the discussion with the FreeBSD maintainers in dotnet/source-build#5035.

[tmds]

Overall, let's keep in mind that the VMR is an msbuild orchestrator with a super thin script based entry point that only translates args.

Also, we like it to be straight forward to use for maintainers.

[am11]

@ViktorHofer, I can remove the last commit added after your approval, would that be sufficient to complete this PR? As for cross, as I have explained above, we may need it in SDK as well to resolve dotnet/runtime#115002 and possibly other repos too. Lets keep it explicit it's really not that big of a deal.

[tmds]

I'm not a fan of requiring arguments that can be deferred from other arguments.

I'm fine with removing portable if the use-case is not considered strong enough.

I think we should leave out -cross and make an attempt at inferring it. If it turns out too complex, we can still push it to the user. In the meanwhile, adding /p:CrossBuild=true is still an option for anyone that needs this.

[am11]

I've reverted portable. None of them are required arguments.

[tmds]

None of them are required arguments.

By required I meant: we require the user to specify an argument the build needs to succeed (which the build can figure out on its own).

make an attempt at inferring it. If it turns out too complex

I expect this to be doable as I think the main condition is '$(TargetOS)-$(TargetArchitecture)' != '$(BuildOS)-$(BuildArchitecture)'.

[am11]

we require the user to specify an argument the build needs to succeed

This PR is not forcing a new requirement either, it's giving -p:CrossBuild=true a familiar alias we've been using for years in runtime.

[tmds]

it's giving -p:CrossBuild=true a familiar alias we've been using for years in runtime.

This means something to someone that is familiar with building runtime repo. What does it mean to the target user of the vmr.

[ViktorHofer]

Right, that's my thinking here as well. Let's leave cross out for now.

[am11]

I've tested linux-riscv64 with am11/dotnet-riscv@194e55a and build succeeded https://github.com/am11/dotnet-riscv/actions/runs/14571025407. So CrossBuild wasn't needed for linux on linux with different arch, but needed for different OS on same arch until we modify the CrossBuild condition in runtime.