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 Apple visionos. #71

Merged
merged 1 commit into from
Jul 28, 2023
Merged

Add Apple visionos. #71

merged 1 commit into from
Jul 28, 2023

Conversation

aiuto
Copy link
Contributor

@aiuto aiuto commented Jun 26, 2023

I'm not really satisified with this PR. The number of Apple specific platforms has grown to the point where we may want to refactor them. The question I would focus on is how various toolchain matches and select clauses go.

Do we see things like:

foo = select({
  ".../os:linux": A,
  ".../os:windows": B,
  ".../os:macos": C,
  ".../os:watchos": C,
  ".../os:visionos": C,
})

Where C is the same for all the apple platforms?

Or, do we see real distinctions across the various per-device OSes. Or a mix of both? And, do we see the fanout of the Apple OSes done with a select_or wrapper, so users end up seeing the simple selection of just apple, linux, or windows, but we buried complexity elsewhere?

I'm not really satisified with this PR. The number of Apple specific platforms has grown to the point where we may want to refactor them. The question I would focus on is how various toolchain matches and select clauses go.

Do we see things like:

```
foo = select({
  ".../os:linux": A,
  ".../os:windows": B,
  ".../os:macos": C,
  ".../os:watchos": C,
  ".../os:visionos": C,
})
```

Where C is the same for all the apple platforms?

Or, do we see real distinctions across the various per-device OSes. Or a mix of both?
And, do we see the fanout of the Apple OSes done with a select_or wrapper, so users end up seeing the simple selection of just apple, linux, or windows, but we buried complexity elsewhere?
@aiuto aiuto requested a review from katre June 26, 2023 15:03
@keith
Copy link
Member

keith commented Jun 26, 2023

I think in general we see both "all apple" selects and specific OS version selects. In apple_support we provide a config_setting for "all apple" here.

Copy link
Member

@keith keith left a comment

Choose a reason for hiding this comment

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

I just submitted bazelbuild/bazel#18905 which is also required for this new support, it would be nice if this one could land as well

keith added a commit to bazelbuild/apple_support that referenced this pull request Jul 11, 2023
keith added a commit to bazelbuild/apple_support that referenced this pull request Jul 11, 2023
keith added a commit to bazelbuild/apple_support that referenced this pull request Jul 14, 2023
@keith
Copy link
Member

keith commented Jul 27, 2023

@aiuto friendly ping

@katre katre merged commit 1e83c41 into bazelbuild:main Jul 28, 2023
@brentleyjones
Copy link

Thanks @meteorcloudy @katre!

@keith
Copy link
Member

keith commented Jul 28, 2023

Thanks folks, could you also push a 0.0.7 with this?

@aiuto aiuto deleted the vos branch July 28, 2023 18:11
@aiuto
Copy link
Contributor Author

aiuto commented Jul 28, 2023

I'll get to that today. Have to run a short errand.

keith added a commit to bazelbuild/apple_support that referenced this pull request Aug 10, 2023
keith added a commit to bazelbuild/apple_support that referenced this pull request Aug 10, 2023
keith added a commit to bazelbuild/apple_support that referenced this pull request Aug 12, 2023
keith added a commit to bazelbuild/apple_support that referenced this pull request Aug 12, 2023
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.

5 participants