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

Enable portable cross-arch source-building to unknown architectures. #54031

Merged
merged 1 commit into from
Feb 14, 2024

Conversation

tmds
Copy link
Member

@tmds tmds commented Feb 14, 2024

The repository supports building for unknown rids to enable building for non-portable rids. This build configuration is under the '$(PortableBuild)' == 'false' condition.

This changes those conditions to '$(DotNetBuildSourceOnly)' == 'true' to enable building a portable configuration for an unknown portable rid. This configuration is used when cross-building for an architecture that is not yet known to the SDK, like linux-ppc64le or linux-s390x.

Fixes dotnet/source-build#3797.

@directhex @ViktorHofer @MichaelSimons ptal.

cc @Swapnali911 @omajid

The repository supports building for unknown rids to enable building for
non-portable rids. This build configuration is under the
'$(PortableBuild)' == 'false' condition.

This changes those conditions to '$(DotNetBuildSourceOnly)' == 'true'
to enable building a portable configuration for an unknown portable rid.
This configuration is used when cross-building for an architecture
that is not yet known to the SDK, like linux-ppc64le or linux-s390x.
@tmds tmds requested review from wtgodbe and a team as code owners February 14, 2024 10:29
@dotnet-issue-labeler dotnet-issue-labeler bot added the area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework label Feb 14, 2024
@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Feb 14, 2024
Copy link
Member

@wtgodbe wtgodbe left a comment

Choose a reason for hiding this comment

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

Assuming this was from a complete find/replace, LGTM

@directhex
Copy link
Member

directhex commented Feb 14, 2024 via email

@tmds
Copy link
Member Author

tmds commented Feb 14, 2024

Assuming this was from a complete find/replace

Yes, it is.

@wtgodbe wtgodbe merged commit f9d0d6b into dotnet:main Feb 14, 2024
26 checks passed
@corngood
Copy link

corngood commented Apr 1, 2024

This seems to have caused a regression of dotnet/source-build#3131

Previously I was able to build a source build with /p:PortableBuild=true, which caused aspnetcore to be built with /p:DotNetBuildSourceOnly=true /p:PortableBuild=true /p:TargetRuntimeIdentifier=linux-x64.

see dotnet/source-build#3131 (comment)

@tmds
Copy link
Member Author

tmds commented Apr 2, 2024

This seems to have caused a regression of dotnet/source-build#3131

I think #53528 fixed this. Do you see the issue when building the main branch?

@corngood
Copy link

corngood commented Apr 2, 2024

I haven't tested main, but that does look like it might fix it. I'm currently trying to get 9.0.0-preview.2 building in nixpkgs. For now I've patched it to Condition=" '$(DotNetBuildSourceOnly)' == 'true' and '$(PortableBuild)' == 'false' ".

I'll test again when that change gets released. Thanks!

@tmds
Copy link
Member Author

tmds commented Apr 2, 2024

I'm curious why you need to do a portable build when building for nixpkgs?

@corngood
Copy link

corngood commented Apr 2, 2024

It seemed to be the path of least resistance to get 8.0 working. I'll probably have to take another look at non-portable builds at some point, but:

  • nixpkgs isn't an OS, so we'd need to change or bypass the /etc/os-release logic
  • we need to support at least linux and darwin
  • existing 'portable' packages won't actually work out of the box on nixos, and may not on other linux/darwin machines in the nix sandbox

@tmds
Copy link
Member Author

tmds commented Apr 2, 2024

I assume this means you want to provide pre-built binaries for install.

For that, you may want to set PortableBuild=true so they work across a large range of distros.
You also need to take care of the native dependencies, or the effective range of distros might be small.
For their portable builds Microsoft does it by building on an older distro.

For the nixpkgs "portable" binaries, you should still use a non-portable TargetRuntimeIdentifier that is distinct from Microsoft's.
This enables the SDK to distinguish between the nixpkgs artifacts and portable artifacts Microsoft provides through nuget.org.
That will also fix the .NET 9 previous 2 build issue you are seeing.

@corngood
Copy link

corngood commented Apr 3, 2024

For the nixpkgs "portable" binaries, you should still use a non-portable TargetRuntimeIdentifier that is distinct from Microsoft's.

I think this might be tricky because we'd need variants for both linux and osx (and maybe bsd/windows in the future). I'll make a note to run some tests and open a more appropriate issue if I have trouble.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework 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.

[VMR] Issue with cross built tarball for Ppc64le
5 participants