Skip to content

Conversation

@aiuto
Copy link

@aiuto aiuto commented Aug 13, 2019

I am just moving the code over in this PR. I still have to figure out where to place the index from https://docs.google.com/document/d/1UZaVcL08wePB41ATZHcxQV4Pu1YfA1RvvWm8FbZHuW8/edit#heading=h.r33iyda45sjm into the site (or a different site?).

@aiuto aiuto requested review from agoulti and hlopko and removed request for tualeron August 13, 2019 17:42
@hlopko
Copy link
Member

hlopko commented Aug 21, 2019

LGTM. Care to also migrate Julie's hlopko/bazel_platforms_examples#1?

Copy link

@agoulti agoulti left a comment

Choose a reason for hiding this comment

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

Sorry, just realized the whole thread was hidden.

I'd like to rework the examples a little bit before exposing them in a central place, since I'm afraid that they are occasionally misleading.

In the first example, HOST_CONSTRAINTS is used - and I don't think they'll do what it looks like they will do.
It creates a toolchain that would match a platform compatible with the host platform, but it doesn't guarantee that it selects the host platform.
For example, if you set linux_platform in the list of execution platforms, it will select that.
The toolchain, however, prints out pre-specified values and the example would seem to indicate that the host platform was still chosen.

  • I'm not sure if this is a good example for using HOST_CONSTRAINTS (I don't actually know of a good case to ever use them in your platform).
  • The fact that the examples print the strings hardcoded in the toolchain and not the actual platforms make this dangerous if people play with them in unexpected ways. I wonder if we could instead use the experimental Skylark API to print which platforms are actually selected.

## Structure

```
├── .bazelrc # Setting defaults until https://github.com/bazelbuild/bazel/issues/7081 is fixed.
Copy link

Choose a reason for hiding this comment

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

I can't find this file, am I looking in the wrong place?


# Tell Bazel that //:linux_platform is allowed execution platform - that our
# host system or remote execution service can handle that platform.
register_execution_platforms("//:linux_platform")
Copy link

Choose a reason for hiding this comment

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

Is there a plan to handle examples that don't want to register this platform?

@rockwotj
Copy link

Is there work planned to merge this? These examples are super helpful but where very hard to find.

@agoulti
Copy link

agoulti commented Feb 26, 2020

Is there work planned to merge this? These examples are super helpful but where very hard to find.

See comments above. There's some issues with these examples.

  • yolo_library is not examining the platform that got selected, but only prints what the toolchain gave it. So executing_on_cpu etc are the toolchain's "wishful thinking" and may or may not match the platform that's actually selected. This could lead to misunderstandings.

  • the use of HOST_CONSTRAINTS is wrong. It seems to think that this would pick the host platform, where in reality it would pick any platform with the same constraints as host.

I don't think there's anyone actively working on this at the moment.

@aiuto
Copy link
Author

aiuto commented Feb 27, 2020 via email

@hlopko hlopko removed their request for review October 12, 2020 09:06
@aiuto aiuto closed this Oct 7, 2021
@aiuto aiuto deleted the hlopko branch October 7, 2021 04:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants