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

config_settings() needs a way to discriminate based on platform/OS in addition to CPU #350

Closed
steven-johnson opened this issue Jul 30, 2015 · 23 comments
Assignees
Labels
P2 We'll consider working on this in future. (Assignee optional) team-Configurability platforms, toolchains, cquery, select(), config transitions type: feature request
Milestone

Comments

@steven-johnson
Copy link
Contributor

At present, the apparently suggested solution is to examine 'cpu', but this is insufficient; it appears that the only possibilities currently are for linux (either x86-32 or x86-64), darwin (cpu type and size unspecified), or armeabi (which apparently can refer to either android or ios); there doesn't appear to be a way to specify android-x86 or arm64-anything.

To use Bazel effectively, we need to be able to discriminate based on the target platform as well as the CPU type and bit-width; this is absolutely essential for working with prebuilt non-Bazel libraries (e.g. pulled via new_http_archive).

@gregestren gregestren self-assigned this Jul 30, 2015
@gregestren gregestren added the P2 We'll consider working on this in future. (Assignee optional) label Jul 30, 2015
@gregestren
Copy link
Contributor

Agreed. We need a stronger concept of "platform" than exists today - cpu is obviously inadequate. The challenge is to make sure we can properly encapsulate everyone's concept of "platform", in all its manifestations.

To be clear, which platforms specifically are important to you?

@steven-johnson
Copy link
Contributor Author

which platforms specifically are important to you?

OSX (x86-32, x86-64)
Windows (x86-32, x86-64)
Linux (x86-32, x86-64)
Android (arm32, arm64, x86-32, x86-64 -- we can live without MIPS for now)
iOS (arm32, arm64)

@gregestren
Copy link
Contributor

Work is ensuing over this summer to define a stronger notion of "platform" that I think should cover practical nuances.

@steven-johnson
Copy link
Contributor Author

Is there any update on this?

@steven-johnson
Copy link
Contributor Author

No activity on this for a few months. Is it still on the radar?

@kchodorow
Copy link
Contributor

Yes! Sorry about the lack of updates, there is actually a bunch of activity on this front. Let me find someone who knows more about this area to chime in.

@katre katre assigned katre and unassigned gregestren Feb 9, 2017
@katre
Copy link
Member

katre commented Feb 9, 2017

Work is in progress on this. We've done some planning and I will start implementing soon. The public design doc is at https://docs.google.com/document/d/1-G-VPRLEj9VyfC6VrQBiR8To-dZjnBSQS66Y4nargGM/edit#heading=h.al54v2ddwqzv

@katre
Copy link
Member

katre commented Aug 9, 2017

Platforms now exist in bazel, using the platform() rule. The next step to solve this problem is to use platform and the constraint_setting/constraint_value rules in select() calls, which is a feature we plan to add in the near future.

bazel-io pushed a commit that referenced this issue Sep 21, 2017
… the configsetting rule class. This is a step in the process of integrating platforms with selects and works towards fixing #350.

Also allow the host platform to be empty/nullable to support clients without access to its default settings.

PiperOrigin-RevId: 169436155
juliexxia referenced this issue Sep 21, 2017
…orm. This is on the way to making select() work with constraint_values re: #305.

PiperOrigin-RevId: 169454982
bazel-io pushed a commit that referenced this issue Sep 22, 2017
…s multiple constraint_values from the same constraint_setting (re: #350)

PiperOrigin-RevId: 169577576
@katre
Copy link
Member

katre commented Dec 11, 2017

This functionality is now present, marking as closed.

@katre katre closed this as completed Dec 11, 2017
@steeve
Copy link
Contributor

steeve commented Jan 30, 2018

Is there any way to use the constraint system to match on Android or iOS target ? It seems matching on @bazel_tools//platforms: is insufficient.
I mean in for instance when building apple_* targets with for instance --ios_multi_cpu.

It seems at the moment the only way is to use config_setting by matching on ios_{armv7,arm64} or crosstool: external/android....

@katre
Copy link
Member

katre commented Jan 30, 2018

Currently, the ios (and android) rules don't work with the platforms system. We'll address this in the future when those rules move to using platforms and toolchains directly.

@sergiocampama
Copy link
Contributor

Do you have any more information on how the Apple rules should interface with the platform rule?

@steeve
Copy link
Contributor

steeve commented Jan 30, 2018

So right now there is no real way to define a toolchain (or constraint) that could be, say compatible with iOS?
I'm asking because we've been discussing it in bazel-contrib/rules_go#1264 as the rules_go toolchain use constraints heavily and I'm doing very dirty hack with config_setting to sort of work around this.

@katre
Copy link
Member

katre commented Jan 30, 2018

@steeve You can writ a platform definition to describe an iOS target, by defining new constraint_setting targets to describe the various features you need. If that platform is used with the --platforms flag, then config_setting targets can match against it. However, it won't change what iOS toolchain is used, because the iOS rules are not currently platform-aware.

@katre
Copy link
Member

katre commented Jan 30, 2018

@sergiocampama Take a look at the toolchains documentation, but the short way to add toolchain (and platform) support is to:

  1. Define a new new toolchain_type target.
  2. Define a new ios_toolchain rule (or re-use one that exists, if it exists) to also create a platform_common.ToolchainInfo provider with all platform-specific data that a rule would need.
  3. Register instances of the ios_toolchain rule using toolchain and register_toolchain
  4. Add toolchains = ['label of toolchain_type'] to your rules that need to access the toolchains, and access them by using ctx.toolchain['label of toolchain type']

The major work will be defining your constraint settings so users can define meaningful platforms for different types of iOS device they would like to target builds for.

@steeve
Copy link
Contributor

steeve commented Jan 30, 2018

@katre looks good, but the issue i have is that iOS and Android targets are builting for multiple architectures (fat binaries or apk) via a single target (by propagating the arch change to deps, it seems).
Would there be a way to handle that?

@katre
Copy link
Member

katre commented Jan 30, 2018

Yes, part of the work will be handling fat binaries by using the correct config split transition.

@sergiocampama
Copy link
Contributor

how is the toolchain support different from the CROSSTOOL?

@steeve
Copy link
Contributor

steeve commented Jan 30, 2018

Is there a way do that already?
To be clear, I'm trying to get rules_go to select the proper toolchain (which is constraint based) based on the current architecture building.
Basically rules_go builds a simple arch .a file, which will would get merged into a multi-arch fat binary via the ios_ rule.

EDIT: Also, this is already the case for all the @bazel_tools cpu and os (which does not define ios or arm64, however https://github.com/bazelbuild/rules_go/blob/master/go/platform/list.bzl#L15)

Something like this:

                +-------------+
                |             |
                |             |
          +----^+   IOS       +^------+
          |     |             |       |
          |     +-------------+       |
          |                           |
          |                           |
          |                           |
 +----------------+           +----------------+
 |                |           |                |
 |                |           |                |
 |   GO ARMV7     |           |   GO ARM64     |
 |                |           |                |
 |                |           |                |
 +----------------+           +----------------+

@katre
Copy link
Member

katre commented Jan 30, 2018

Please take a look at the docs, including the original design doc. The short version is that toolchains works for all rule types, not just CC rules, and has a better idea of what makes different platforms distinct from each other.

@katre
Copy link
Member

katre commented Jan 30, 2018

@steeve There's code to automatically convert from the --cpu flag to the auto-detected target platform (@bazel_tools//platforms:target_platform). This should work properly with split transitions like this, but I've never tested it, so if you're seeing errors there's a bug somewhere.

I'd like to encourage you to file this as a separate issue and I'll put it on my backlog, but it's unlikely to be something I can get to in the near future.

@steeve
Copy link
Contributor

steeve commented Jan 30, 2018

@katre sure thing, thank you for the quick answers
However, it seems the ios_* cpus are defined by the CROSSTOOL, so I'm not sure it will work.
Also, ios and arm64 are not considered OS or CPU at the moment (perhaps that would be the fastest route to take, however).

Can you point me to the code? I might take a look.

@katre
Copy link
Member

katre commented Jan 30, 2018

The auto-configuration for platforms happens here: https://source.bazel.build/bazel/+/master:src/main/java/com/google/devtools/build/lib/rules/platform/Platform.java;l=55

And is based on the set of cpus and oses available for autoconfig, set here: https://source.bazel.build/bazel/+/master:tools/platforms/platforms.BUILD;l=81?q=tools%2Fplatforms

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P2 We'll consider working on this in future. (Assignee optional) team-Configurability platforms, toolchains, cquery, select(), config transitions type: feature request
Projects
None yet
Development

No branches or pull requests

9 participants