Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

support building mono on x64 as it gets built on s390x #62440

Closed
tmds opened this issue Dec 6, 2021 · 32 comments · Fixed by #68424
Closed

support building mono on x64 as it gets built on s390x #62440

tmds opened this issue Dec 6, 2021 · 32 comments · Fixed by #68424

Comments

@tmds
Copy link
Member

tmds commented Dec 6, 2021

I'd like to build this repo using mono as runtime similar to how it is built on s390x.

The build script doesn't provide a way to do that.

The s390x case is specially handled using PrimaryRuntimeFlavor:

<PropertyGroup>
<PrimaryRuntimeFlavor>CoreCLR</PrimaryRuntimeFlavor>
<PrimaryRuntimeFlavor Condition="'$(TargetArchitecture)' == 's390x'">Mono</PrimaryRuntimeFlavor>
</PropertyGroup>

Then subsequent checking occurs as:

'$(RuntimeFlavor)' == '$(PrimaryRuntimeFlavor)')" -> coreCLR or mono on s390x
'$(RuntimeFlavor)' != '$(PrimaryRuntimeFlavor)')" -> mono not on s390x

cc @ViktorHofer @uweigand @omajid @akoeplinger

@dotnet-issue-labeler
Copy link

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

@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label Dec 6, 2021
@omajid
Copy link
Member

omajid commented Dec 6, 2021

This would be a requirement for dotnet/source-build#2596

@ghost
Copy link

ghost commented Dec 6, 2021

Tagging subscribers to this area: @dotnet/runtime-infrastructure
See info in area-owners.md if you want to be subscribed.

Issue Details

I'd like to build this repo using mono as runtime similar to how it is built on s390x.

The build script doesn't provide a way to do that.

The s390x case is specially handled using PrimaryRuntimeFlavor:

<PropertyGroup>
<PrimaryRuntimeFlavor>CoreCLR</PrimaryRuntimeFlavor>
<PrimaryRuntimeFlavor Condition="'$(TargetArchitecture)' == 's390x'">Mono</PrimaryRuntimeFlavor>
</PropertyGroup>

Then subsequent checking occurs as:

'$(RuntimeFlavor)' == '$(PrimaryRuntimeFlavor)')" -> coreCLR or mono on s390x
'$(RuntimeFlavor)' != '$(PrimaryRuntimeFlavor)')" -> mono not on s390x

cc @ViktorHofer @uweigand @omajid @akoeplinger

Author: tmds
Assignees: -
Labels:

area-Infrastructure, untriaged

Milestone: -

@steveisok
Copy link
Member

@directhex Is there much else to do other than conditionally set PrimaryRuntimeFlavor?

@directhex
Copy link
Contributor

There may be some other conditionals to fix up (e.g. the stuff around building the correct coreclr subsets), but that'd the main one

@tmds
Copy link
Member Author

tmds commented Dec 7, 2021

Is there much else to do other than conditionally set PrimaryRuntimeFlavor?

./build.sh already has args for controlling the subsets and the runtime flavor.
Personally, I'd rather not add another for PrimaryRuntimeFlavor.

Maybe we can get rid of PrimaryRuntimeFlavor by picking appropriate default subsets when ('$(DotNetBuildFromSource)' == 'true' and '$(RuntimeFlavor)' == 'Mono').

dotnet/source-build#2596 then is about passing RuntimeFlavor to the building dotnet/runtime build.

The default subset for ./build.sh -rf mono would not build the same subsets as source-build, but a user can specify them on the command-line.

@steveisok @directhex wdyt?

@uweigand
Copy link
Contributor

uweigand commented Dec 7, 2021

You cannot get rid of PrimaryRuntimeFlavor just by picking subsets. Before we added PrimaryRuntimeFlavor, many components (like e.g. building the main dotnet binary) were guarded by hard-coded RuntimeFlavor == CoreCLR checks. These have now been replaced by RuntimeFlavor == PrimaryRuntimeFlavor to enable building those also with Mono on platforms that do not support CoreCLR (s390x).

Also, we want to be able to build the full runtime (including dotnet etc.) on s390x in a stand-alone build as well, so this behavior cannot be guarded by DotNetBuildFromSource.

I guess it might be possible to move building everything that is now guarded by RuntimeFlavor == PrimaryRuntimeFlavor into dedicated subsets, but that seems a more significant change (those guards currently are not at the main Subsets.props level, but spread out across multiple csproj files).

@tmds
Copy link
Member Author

tmds commented Dec 7, 2021

I prefer working with runtime flavor and subsets rather than adding an additional flavor argument if that is possible, and I am not sure it is.

Also, we want to be able to build the full runtime (including dotnet etc.) on s390x in a stand-alone build as well, so this behavior cannot be guarded by DotNetBuildFromSource.

You can add it to build.sh as an argument:

./build.sh ... /p:DotnetBuildFromSource=true

I guess it might be possible to move building everything that is now guarded by RuntimeFlavor == PrimaryRuntimeFlavor into dedicated subsets

Maybe it is not needed to split it out completely, and it is fine that some things get built for the default Mono runtime flavor.

@tmds
Copy link
Member Author

tmds commented Dec 7, 2021

My main concern with PrimaryRuntimeFlavor is the name/value. It sounds like it controls the runtime flavor, but it actually controls what gets built.

@uweigand
Copy link
Contributor

uweigand commented Dec 7, 2021

My rationale for the naming was this: while you can switch between two runtime flavors (CoreCLR and Mono), those are not actually equivalent. Rather, one of those flavors builds a lot more components than the other. Conceptually, I was thinking of those as the "primary" and "secondary" runtime flavors. Now, before my change, the "primary" flavor always was CoreCLR. What I intended to add was a way to swap those around so that on some targets, Mono can be the primary flavor.

But it seems that naming convention isn't as obvious as I thought, so if you have alternative suggestions, I'd be interested ...

@tmds
Copy link
Member Author

tmds commented Dec 7, 2021

If we can't express it through subsets, something that expresses: I'm building .NET with the interest of building a .NET SDK/installation. Something like TargetsDotnet. I've used Targets as prefix similar to the existing TargetsMobile.

@tmds
Copy link
Member Author

tmds commented Dec 7, 2021

I tried putting something together to get an idea: tmds@44d4a56. It's untested and probably has some bugs.

@uweigand
Copy link
Contributor

uweigand commented Dec 7, 2021

Added some comments there.

@tmds
Copy link
Member Author

tmds commented Dec 10, 2021

Thanks @uweigand. Your comments make sense. They need to be addressed.
I'm not going to make more changes, I was exploring the option.

@ViktorHofer maybe you have some inputs? The top-level use-case is to enable building '.NET Core' but with mono runtime instead of coreclr.
Currently such a build is supporter for s390x through checks against PrimaryRuntimeFlavor. We'd like to do it on different architectures. Does it make sense to make PrimaryRuntimeFlavor settable via ./build.sh, or how should this be handled?

@danmoseley
Copy link
Member

Viktor is out for a little while. Perhaps @safern can help.

@tmds
Copy link
Member Author

tmds commented Jan 12, 2022

@safern We'd like to build .NET Core with mono instead of coreclr runtime. Can you give some input on how to expose this feature from the top-level build script?

@tmds
Copy link
Member Author

tmds commented Jan 25, 2022

@ViktorHofer I see you're back :) Can you chime in?

@ViktorHofer
Copy link
Member

Not back until April. Just doing some stuff on my spare time 😄 I'm not up to date so it would be better for @ericstj and @safern to comment.

@ericstj
Copy link
Member

ericstj commented Jan 25, 2022

So this question is more about how the different runtimes are exposed from the build. cc @agocke @jkotas @jkoritzinsky @trylek for runtime infra

The extent of my opinion here is make something that minimally captures the amount of new state you'd like to communicate, as if you were designing the interface from the start, then determine what would need to change in how the build scripts select subsets. Avoid building onto concepts that are redundant or grew organically.

Background question: if someone used this flag to build the runtime would it be producing shared-framework installers and runtime-packs that include the mono runtime instead of CoreCLR -- but with the same names as the CoreCLR ones? That seems problematic since it could cause a clash of product state. I would hope that we build different things if they have different content.

@steveisok
Copy link
Member

The extent of my opinion here is make something that minimally captures the amount of new state you'd like to communicate, as if you were designing the interface from the start, then determine what would need to change in how the build scripts select subsets. Avoid building onto a concepts that are redundant or grew organically.

Agreed. Tbh, I can't suggest a better alternative than PrimaryRuntimeFlavor. Thinking it through as Eric suggested may bring a better solution to light.

Background question: if someone used this flag to build the runtime would it be producing shared-framework installers and runtime-packs that include the mono runtime instead of CoreCLR -- but with the same names as the CoreCLR ones? That seems problematic since it could cause a clash of product state. I would hope that we build different things if they have different content.

For the configurations mono actively supports, the names of the runtime are different than coreclr. I believe it's only within the shared framework where the name is the same. In the past, there hasn't been a need for a mono runtime shared framework outside of running benchmarks internally. There is an issue open to change the name, but it's lower priority right now #34202.

Where things would get really interesting is if we needed to deploy both runtimes in the shared framework :-)

@tmds
Copy link
Member Author

tmds commented Jan 25, 2022

My 2 cents.

The existing flag runtimeFlavor allows to choose between different runtimes.
Currently what gets built is tied to the runtimeFlavor.
I think we're trying to decouple it. PrimaryRuntimeFlavor doesn't do that.

@tmds
Copy link
Member Author

tmds commented Feb 1, 2022

I'd rather get the feature than get stuck on the name.

Is PrimaryRuntimeFlavor considered the way to go?

@omajid
Copy link
Member

omajid commented Mar 14, 2022

Is something like this all that's needed if we are sticking to PrimaryRuntimeFlavor ?

diff --git a/eng/Subsets.props b/eng/Subsets.props
index 1213b5407e9..b07d643ee0f 100644
--- a/eng/Subsets.props
+++ b/eng/Subsets.props
@@ -26,8 +26,8 @@
        platforms (like s390x) where only Mono is supported. The primary runtime
        flavor is used to decide when to build the hosts and installers. -->
   <PropertyGroup>
-    <PrimaryRuntimeFlavor>CoreCLR</PrimaryRuntimeFlavor>
-    <PrimaryRuntimeFlavor Condition="'$(TargetArchitecture)' == 's390x' or '$(TargetArchitecture)' == 'armv6'">Mono</PrimaryRuntimeFlavor>
+    <PrimaryRuntimeFlavor Condition="'$(PrimaryRuntimeFlavor)' == '' and ('$(TargetArchitecture)' == 's390x' or '$(TargetArchitecture)' == 'armv6')">Mono</PrimaryRuntimeFlavor>
+    <PrimaryRuntimeFlavor Condition="'$(PrimaryRuntimeFlavor)' == ''">CoreCLR</PrimaryRuntimeFlavor>
   </PropertyGroup>
 
   <PropertyGroup>

@crummel
Copy link
Contributor

crummel commented Mar 17, 2022

@ericstj @safern @steveisok Looks like we just have to settle on a name and then @omajid will have the change ready. Can we settle on that soon?

@tmds
Copy link
Member Author

tmds commented Mar 17, 2022

Is something like this all that's needed if we are sticking to PrimaryRuntimeFlavor ?

Something like that, and it would be nice if it was an argument on the top-level build.sh script.

@ericstj
Copy link
Member

ericstj commented Mar 17, 2022

@safern is not working on this anymore and I'm not sure I have a voting stake here as a libraries lead. As I mentioned before I'd really like someone from the runtime team to drive this as they're more likely to have an opinion. @agocke @jkoritzinsky @hoyosjs @trylek

@crummel
Copy link
Contributor

crummel commented Apr 7, 2022

@agocke @jkoritzinsky @hoyosjs @trylek any of you want to volunteer to take a look?

@agocke agocke added area-Infrastructure-mono and removed area-Infrastructure untriaged New issue has not been triaged by the area owner labels Apr 14, 2022
@ghost
Copy link

ghost commented Apr 14, 2022

Tagging subscribers to this area: @directhex
See info in area-owners.md if you want to be subscribed.

Issue Details

I'd like to build this repo using mono as runtime similar to how it is built on s390x.

The build script doesn't provide a way to do that.

The s390x case is specially handled using PrimaryRuntimeFlavor:

<PropertyGroup>
<PrimaryRuntimeFlavor>CoreCLR</PrimaryRuntimeFlavor>
<PrimaryRuntimeFlavor Condition="'$(TargetArchitecture)' == 's390x'">Mono</PrimaryRuntimeFlavor>
</PropertyGroup>

Then subsequent checking occurs as:

'$(RuntimeFlavor)' == '$(PrimaryRuntimeFlavor)')" -> coreCLR or mono on s390x
'$(RuntimeFlavor)' != '$(PrimaryRuntimeFlavor)')" -> mono not on s390x

cc @ViktorHofer @uweigand @omajid @akoeplinger

Author: tmds
Assignees: -
Labels:

area-Infrastructure-mono

Milestone: -

@agocke
Copy link
Member

agocke commented Apr 14, 2022

This is Mono-specific so I think @directhex might be able to point you to someone who's more familiar that build.

@directhex
Copy link
Contributor

This discussion has been open since early December.

Omair's 2-line patch seems to work - if I set /p:PrimaryRuntimeFlavor=Mono I have artifacts/bin/coreclr/Linux.x64.Debug/ilasm and artifacts/bin/linux-x64.Debug/corehost/dotnet and artifacts/bin/mono/Linux.x64.Debug/libcoreclr.so. The bike shed colour is fine. The name is fine. We don't need months to agree on a name. @omajid if you PR #62440 (comment) I'll be happy to approve that in review.

@agocke
Copy link
Member

agocke commented Apr 22, 2022

Sounds great, no objections on my side either.

@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Apr 22, 2022
@tmds
Copy link
Member Author

tmds commented Apr 25, 2022

if I set /p:PrimaryRuntimeFlavor=Mono

aha...

When I created the issue I didn't realize appending /p:PrimaryRuntimeFlavor=Mono to ./build.sh already works, even without the '$(PrimaryRuntimeFlavor)' == '' condition.

@akoeplinger akoeplinger added this to the 8.0.0 milestone Aug 12, 2022
omajid added a commit to omajid/dotnet-runtime that referenced this issue Apr 11, 2023
We now have some architectures (eg, s390x) that produce a .NET runtime
that looks and feels like any other .NET runtime, except it's using
Mono instead of CoreCLR under the hood.

However, it's only possible to produce this Mono-based .NET runtime on a
select few architectures. That makes it hard to test this set up on
other architectures.  Issues that affect the mono build on all
architectures - such as dotnet#66594 become harder to fix and verify because
of this unnecessary architecture requirement.

Fix that by adding a flag to the top-level build.sh (and also to the
msbuild projects) to make it possible to produce a .NET runtime with
Mono on any platform.

The default configuration is unchanged: we still produced CoreCLR-based
.NET runtime on x64 and a Mono-based runtime on arm32,ppc64le,s390x.

Fixes: dotnet#62440
akoeplinger pushed a commit that referenced this issue May 3, 2023
We now have some architectures (eg, s390x and ppc64le) that produce a .NET runtime that looks and feels like any other .NET runtime, except it's using Mono instead of CoreCLR under the hood.

However, it's only possible to produce this Mono-based .NET runtime on s390x/ppc64le. That makes it hard to test this set up on other architectures. Issues that affect the mono build on all architectures - such as #66594 become harder to fix and verify because of this unnecessary architecture requirement.

Fix that by adding a flag to the top-level build.sh (and also to the msbuild projects) to make it possible to produce a .NET runtime with Mono on any platform.

The default configuration is unchanged: we still produced CoreCLR-based .NET runtime on x64 and a Mono-based runtime on s390x/ppc64le.

Fixes: #62440
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label May 3, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Jun 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.