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

Add visionOS unit tests, unit test targets, test app, testing host app, and xcconfigs #1991

Closed
wants to merge 6 commits into from

Conversation

mipetriu
Copy link
Contributor

@mipetriu mipetriu commented Feb 1, 2024

Proposed changes

This PR adds a visionOS test app in MSAL mostly taken from the publicly available iOS Graph API sample. Keeping these files separate should allow for more simple review of the overall visionOS changes needed in MSAL: #1990

Type of change

  • Feature work
  • Bug fix
  • Documentation
  • Engineering change
  • Test
  • Logging/Telemetry

Risk

  • High – Errors could cause MAJOR regression of many scenarios. (Example: new large features or high level infrastructure changes)
  • Medium – Errors could cause regression of 1 or more scenarios. (Example: somewhat complex bug fixes, small new features)
  • Small – No issues are expected. (Example: Very small bug fixes, string changes, or configuration settings changes)

Additional information

@mipetriu mipetriu requested review from a team as code owners February 1, 2024 20:12
@mipetriu mipetriu requested review from swasti29 and hieunguyenmsft and removed request for a team February 1, 2024 20:12
@@ -1 +1 @@
Subproject commit 03ac64f7af61323b5ff970781cc50c2be16e83b8

Choose a reason for hiding this comment

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

This pull request does not update CHANGELOG.md.

Please consider if this change would be noticeable to a partner or user and either update CHANGELOG.md or resolve this conversation.

@frogcjn
Copy link

frogcjn commented Feb 27, 2024

I have improved the xcconfig parts for visionOS and building XCFramework process which developer could build themselves swift package version of MSAL, you could check the pull request at #2065

@mipetriu mipetriu changed the title visionOS test app in MSAL Add visionOS unit tests, unit test targets, test app, testing host app, and xcconfigs Apr 23, 2024
@mipetriu mipetriu closed this May 23, 2024
GENERATE_MASTER_OBJECT_FILE = YES

// Add armv7s and arm64e to standard ARCHs.
ARCHS = $(ARCHS_STANDARD) armv7s arm64e

Choose a reason for hiding this comment

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

I don't think you need to add any architectures to ARCHS_STANDARD for iOS or visionOS

ARCHS[sdk=xros*] = $(ARCHS_STANDARD) armv7s arm64e

// Activate full bitcode on release configuration for real devices.
OTHER_CFLAGS[config=Release][sdk=xros*] = $(OTHER_CFLAGS) -fembed-bitcode

Choose a reason for hiding this comment

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

bitcode is long deprecated for iOS and is not available for visionOS

Choose a reason for hiding this comment

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

Per our conversation it would be much easier just to add the following to the global/shared xcconfig and not create a separate target for visionOS:

SUPPORTED_PLATFORMS = iphoneos iphonesimulator xros xrsimulator
// 1: iPhone, 2: iPad, 3: appleTV, 7: vision
// when left empty, uses one default device for given platform
TARGETED_DEVICE_FAMILY[sdk=iphone*] = 1,2

The differences between iOS and visionOS in your code are minor and can easily be handled with compile time #ifs in the sources. This will simplify usage of your library in clients building for different iOS-derived platforms.

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.

3 participants