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

Use in-tree ilasm to run test suite natively on s390x #71326

Merged
merged 1 commit into from
Jul 6, 2022
Merged

Use in-tree ilasm to run test suite natively on s390x #71326

merged 1 commit into from
Jul 6, 2022

Conversation

uweigand
Copy link
Contributor

  • There are no linux-s390x packages on nuget.org, so use in-tree
    built ilasm when running the test suite natively on s390x

In #58952, @akoeplinger introduced code to allow building the test suite targeting linux-s390x, even through there are no linux-s390x packages currently available on nuget.org, by using the in-tree live-built versions of the host packages instead.

However, there is still one remaining obstacle to actually running the test suite natively on linux-s390x, and that is the availability of the ilasm package. This PR sets the ILAsmToolPath property to use the in-tree live-built ilasm binary instead.

With this PR applied, I'm able to successfully build and run the runtime libs.tests test suite natively on linux-s390x without any changes applied to the upstream repository.

* There are no linux-s390x packages on nuget.org, so use in-tree
  built ilasm when running the test suite natively on s390x
@ghost ghost added the community-contribution Indicates that the PR has been added by a community member label Jun 27, 2022
@ghost
Copy link

ghost commented Jun 27, 2022

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

Issue Details
  • There are no linux-s390x packages on nuget.org, so use in-tree
    built ilasm when running the test suite natively on s390x

In #58952, @akoeplinger introduced code to allow building the test suite targeting linux-s390x, even through there are no linux-s390x packages currently available on nuget.org, by using the in-tree live-built versions of the host packages instead.

However, there is still one remaining obstacle to actually running the test suite natively on linux-s390x, and that is the availability of the ilasm package. This PR sets the ILAsmToolPath property to use the in-tree live-built ilasm binary instead.

With this PR applied, I'm able to successfully build and run the runtime libs.tests test suite natively on linux-s390x without any changes applied to the upstream repository.

Author: uweigand
Assignees: -
Labels:

area-Infrastructure-libraries

Milestone: -

Copy link
Member

@ViktorHofer ViktorHofer left a comment

Choose a reason for hiding this comment

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

Looks good. Thank you.

@uweigand
Copy link
Contributor Author

uweigand commented Jul 5, 2022

Can this be merged now or is there anything else I should be doing? Thanks!

@ViktorHofer ViktorHofer merged commit d26ad8e into dotnet:main Jul 6, 2022
@akoeplinger
Copy link
Member

We probably need the same change for ppc64le, /cc @Sapana-Khemkar

@Sapana-Khemkar
Copy link
Contributor

@akoeplinger Thanks for highlighting. Yes we will take this for ppc64le.

@ViktorHofer
Copy link
Member

ViktorHofer commented Jul 12, 2022

Ideally we would just always use the live built version. We don't use ILAsm/ILDasm anymore in the product build as we removed CompilerServices.Unsafe.ilproj. The projects (I think only test projects) in the repo that continue to use the .ilproj extension could just import the props and targets file automatically instead of being an reference. The targets file could make sure that the toolchain is (incrementally) built. The package would continue to be available for consumers outside the repo, like roslyn.

That would remove the existing self dependencies for the Microsoft.NET.Sdk.IL and we could eventually remove the self referencing node in the build graph: runtime -> runtime.

image

from https://github.com//pull/72371

EDIT: As pointed out in #58109 (comment), a good first step to approach this might be removing the prebuilts of ILAsm and ILDasm by setting the ILAsmToolPath property unconditionally. That would always use the live built version of the tools themselves and with some more work I'm sure we will also be able to avoid the IL.SDK package prebuilt.

@ghost ghost locked as resolved and limited conversation to collaborators Aug 21, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-Infrastructure-libraries 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.

None yet

4 participants