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

Build support for s390x: corehost #53949

Merged
merged 1 commit into from
Jul 28, 2021
Merged

Build support for s390x: corehost #53949

merged 1 commit into from
Jul 28, 2021

Conversation

uweigand
Copy link
Contributor

@uweigand uweigand commented Jun 9, 2021

  • Build the (non-Windows) host binaries with either runtime flavor

  • s390x architecture support in get_arch utility

This allows building the host binaries including "dotnet" on s390x, where we only support the Mono runtime. These hosts all build fine with either runtime, it seems they're just not build currently because there's no need to build them twice.

(I'm not sure the Windows hosts need to be under the RUNTIME_FLAVOR condition either, but I left them alone since I cannot test whether they actually build with Mono.)

@ghost
Copy link

ghost commented Jun 9, 2021

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

Issue Details
  • Build the (non-Windows) host binaries with either runtime flavor

  • s390x architecture support in get_arch utility

This allows building the host binaries including "dotnet" on s390x, where we only support the Mono runtime. These hosts all build fine with either runtime, it seems they're just not build currently because there's no need to build them twice.

(I'm not sure the Windows hosts need to be under the RUNTIME_FLAVOR condition either, but I left them alone since I cannot test whether they actually build with Mono.)

Author: uweigand
Assignees: -
Labels:

area-Host

Milestone: -

* Build the (non-Windows) host binaries with either runtime flavor

* s390x architecture support in get_arch utility
@ericstj
Copy link
Member

ericstj commented Jul 14, 2021

Can some host owners please review and sign off on this change?

@terrajobst terrajobst added the community-contribution Indicates that the PR has been added by a community member label Jul 19, 2021
Copy link
Member

@vitek-karas vitek-karas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for such a late reply...
Looks good to me. @VSadov could you please take a look as well (if there are potential single-file related issues).

@uweigand
Copy link
Contributor Author

As to the singlefilehost, this is no longer built ouf of the corehost directory, but out of coreclr. This means it is not available on Mono -- this patch has no effect on that. (If we wanted to support a Mono-based singlefilehost, that would require a much larger change, most likely mirroring the coreclr build process inside the mono directory. That would in any case be an independent change.)

@uweigand
Copy link
Contributor Author

Hi @vitek-karas @VSadov I was wondering if there is any news on this? This PR is now the last missing piece blocking a successful build of the runtime on s390x, so if we could make progress here it would be much appreciated. Thanks!

Copy link
Member

@agocke agocke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. With the refactoring moving the singlefilehost to coreclr I don't think this should run into any problems

@agocke agocke merged commit db1b302 into dotnet:main Jul 28, 2021
@uweigand uweigand deleted the s390x-build-hosts branch July 29, 2021 07:04
@ghost ghost locked as resolved and limited conversation to collaborators Aug 28, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-Host community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants