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

[mono] Clean up internal usage of RuntimeComponents to always be treated as MSBuild Item #91800

Merged
merged 5 commits into from
Oct 18, 2023

Conversation

ivanpovazan
Copy link
Member

@ivanpovazan ivanpovazan commented Sep 8, 2023

This PR cleans up the internal usage of RuntimeComponents where in some places it is used as a MSBuild Property and in others as an Item, which can lead to unexpected behaviours during app builds for both Android and iOS-like platforms and additionally imposes unnecessary complication of treating them dually in different places.

This PR:

  • Removes the possibility of using a wildcard * which previously meant to include all supported components (this was probably the reason to use a Property in the first place).
    • This can be consider a BREAKING CHANGE - however I did not find any internal usage of said feature (I might be wrong though).
  • Introduces: UseAllRuntimeComponents MSBuild property which when set to true includes all supported components by parsing the RuntimeComponentManifest.json file and populating RuntimeComponents Item in _InitializeCommonProperties target (in both: AppleBuild.targets and AndroidBuild.targets)
  • Cleans up all references of RuntimeComponents to be treated as MSBuild Item
  • Simplifies usage of runtime components in various places
  • Updating the doc on diagnostic tracing
  • Reuses RuntimeComponentManifest.targets for local builds and testing to retrieve available runtime components

Follow-up tasks:

  • Switch the AndroidSampleApp to use AndroidBuild.targets directly instead of manually inferring build tasks and targets
  • Further improve managed debugging of libraries, functional tests and sample applications on both Android and iOS-like platforms

@ghost
Copy link

ghost commented Sep 8, 2023

Tagging subscribers to this area: @directhex
See info in area-owners.md if you want to be subscribed.

Issue Details

This PR cleans up the internal usage of RuntimeComponents where in some places it is used as a MSBuild Property and in others as an Item, which can lead to unexpected behaviours during app builds for both Android and iOS-like platforms and additionally imposes unnecessary complication of treating them dually in different places.

This PR:

  • Removes the possibility of using a wildcard * which previously meant to include all supported components (this was probably the reason to use a Property in the first place).
    • This can be consider a BREAKING CHANGE - however I did not find any internal usage of said feature (I might be wrong though).
  • Introduces: UseAllRuntimeComponents MSBuild property which when set to true includes all supported components by parsing the RuntimeComponentManifest.json file and populating RuntimeComponents Item in _InitializeCommonProperties target (in both: AppleBuild.targets and AndroidBuild.targets)
  • Cleans up all references of RuntimeComponents to be treated as MSBuild Item
  • Simplifies usage of runtime components in various places
  • Updating the doc on diagnostic tracing

Follow-up tasks:

  • Switch the AndroidSampleApp to use AndroidBuild.targets directly instead of manually inferring build tasks and targets
  • Further improve managed debugging of libraries, functional tests and sample applications on both Android and iOS-like platforms
Author: ivanpovazan
Assignees: -
Labels:

area-Infrastructure-mono

Milestone: -

@ivanpovazan
Copy link
Member Author

Copy link
Member

@jandupej jandupej left a comment

Choose a reason for hiding this comment

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

This is a welcome simplification. Thanks!

@ivanpovazan
Copy link
Member Author

/azp run runtime-extra-platforms

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@ivanpovazan
Copy link
Member Author

/azp run runtime-extra-platforms

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@ivanpovazan
Copy link
Member Author

/azp run runtime-extra-platforms

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Member

@steveisok steveisok left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for refactoring and making this more straightforward.

@ivanpovazan ivanpovazan requested a review from radical October 17, 2023 13:16
ivanpovazan and others added 2 commits October 17, 2023 18:17
Co-authored-by: Ankit Jain <[email protected]>
Co-authored-by: Ankit Jain <[email protected]>
@ivanpovazan
Copy link
Member Author

/azp run runtime-extra-platforms

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@ivanpovazan
Copy link
Member Author

Failures don't seem to be related

@ghost ghost locked as resolved and limited conversation to collaborators Nov 17, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants