Skip to content

Conversation

@mangod9
Copy link
Member

@mangod9 mangod9 commented Jun 7, 2021

Updating sharedfx.targets to use crossgen2 by default for ReadyToRun, similar change has been done within the sdk but that doesnt seem to take effect while building with sharedFx.targets.

With active work to decomm crossgen1 need to get this propagated to all repos so we could resolve any outstanding issues.

I have validated that WindowsDesktop repo does use crossgen2 if I manually update its sharedFx.targets with this change.

Copy link
Member

@trylek trylek left a comment

Choose a reason for hiding this comment

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

Awesome, thank you!

Copy link
Member

@AntonLapounov AntonLapounov left a comment

Choose a reason for hiding this comment

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

I think that setting PublishReadyToRun should be moved to sharedfx.props. That way SDK will set PublishReadyToRunUseCrossgen2 to true.

@jkoritzinsky
Copy link
Member

We can't move it there because the PlatformPackageType property is set in the user's project file, and the sharedfx.props file gets imported before properties there are read.

@mangod9
Copy link
Member Author

mangod9 commented Jun 7, 2021

yeah tested by moving it to props, but PublishReadyToRun is not set if moved.

@AntonLapounov
Copy link
Member

That is unfortunate. We should probably remove the '$(PublishReadyToRun)' == 'true' condition from this SDK line: https://github.com/dotnet/sdk/blob/751fb14a10203d201b18ec8557f8422e13afe998/src/Tasks/Microsoft.NET.Build.Tasks/targets/Microsoft.NET.CrossGen.targets#L14

@mangod9
Copy link
Member Author

mangod9 commented Jun 8, 2021

We should probably remove the '$(PublishReadyToRun)' == 'true' condition from this SDK

yeah might make sense to do that change. Let me merge this and we can undo once the SDK change makes it through.

@mangod9 mangod9 merged commit e13fddd into main Jun 8, 2021
@mangod9 mangod9 deleted the enablecg2 branch June 8, 2021 00:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants