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

Delete unused Crossgen2PackageRID property #66814

Closed
wants to merge 1 commit into from

Conversation

am11
Copy link
Member

@am11 am11 commented Mar 18, 2022

Unused after #65948

@ghost ghost added the community-contribution Indicates that the PR has been added by a community member label Mar 18, 2022
Copy link
Member

@mangod9 mangod9 left a comment

Choose a reason for hiding this comment

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

Thanks for fixing!

@am11
Copy link
Member Author

am11 commented Mar 18, 2022

Remaining CI failures seem unrelated infra issues.


Regarding second commit

cc @wfurt, @Thefrank

When building natively on FreeBSD, proper crossgen2 binary is produced (i.e. the one with FreeBSD compatible apphost).

However, when cross-compiling for FreeBSD from Linux, we had PublishReadyToRun disabled, but we were still producing shipping package with invalid CG2 binary (with linux apphost...). First commit uncovers that issue at package-restore-time and second commit prevents CG2 package generation in FreeBSD cross-compile mode.

Directory.Build.props Outdated Show resolved Hide resolved
@Thefrank
Copy link
Contributor

@am11 so crossbuilding FreeBSD under Linux was trying to restore a package that didn't exist (FreeBSD Crossgen2)? If yes, then I think your #66866 would be a good way of allowing community supported platforms to produce a "more complete" runtime or full SDK package via crossbuilding.

@am11
Copy link
Member Author

am11 commented Mar 19, 2022

@Thefrank, correct. The apphost package (microsoft.netcore.app.crossgen2.<platform>-<arch>.<version>.nupkg) restored for crossgen2 on the main branch is using linux-x64 when we pass -cross or -p:CrossBuild=true to ./build.sh. Which is incorrect when target is anything other than linux-x64. The crossgen2 tool used to cross build during the build is correct (i.e. when CrossBuild = true, it should use CG2 built with host compatible apphost, rather than that of target's).

The reason why we don't see any error on main branch is because of the wrong Crossgen2PackageRID value which is fixed by first commit in this PR. With that fix, the FreeBSD CI leg started to fail during apphost package restore like this:

.dotnet/sdk/6.0.100/Sdks/Microsoft.NET.Sdk/targets/Microsoft.NET.Sdk.FrameworkReferenceResolution.targets(129,5): error NETSDK1084: (NETCORE_ENGINEERING_TELEMETRY=Restore) There is no application host available for the specified RuntimeIdentifier 'freebsd-x64'.

The second commit disables crossgen2 project build on FreeBSD package generation in cross build mode. It was already partially disabled, but not properly excluded since the issue was hidden due to the said bug. When #66866 is resolved, we can enable it back.

@am11
Copy link
Member Author

am11 commented Apr 1, 2022

FreeBSD issue is handled by #65948 merge (we do not restore apphost for CG2 on unsupported platforms anymore). I have reverted the second commit and merged main to my PR branch.

Libs test failures are unrelated to this change.

@am11 am11 force-pushed the feature/build/cross branch 2 times, most recently from 4d8c509 to eb1b2e5 Compare April 1, 2022 20:35
@am11 am11 changed the title Fix crossgen2 RID in linux-musl cross build Delete unused Crossgen2PackageRID property Apr 1, 2022
@mangod9
Copy link
Member

mangod9 commented May 2, 2022

Hi @am11, assume this is ok to merge?

@am11
Copy link
Member Author

am11 commented May 2, 2022

This was folded into #67636.

@am11 am11 closed this May 2, 2022
@mangod9
Copy link
Member

mangod9 commented May 2, 2022

thanks for the update.

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

6 participants