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

Select different toolchain versions #7572

Closed
Globegitter opened this issue Feb 27, 2019 · 11 comments
Closed

Select different toolchain versions #7572

Globegitter opened this issue Feb 27, 2019 · 11 comments
Assignees
Labels
P3 We're not considering working on this, but happy to review a PR. (No assignee) team-Configurability platforms, toolchains, cquery, select(), config transitions

Comments

@Globegitter
Copy link

Globegitter commented Feb 27, 2019

Description of the problem / feature request:

As briefly discussed here: #7375 (comment) there are use-cases where it would be nice to be able to define multiple toolchain versions (for the same platform), so that one could have e.g. multiple versions of python, or scala or nodejs in the same build but for different targets.

In usage it would be nice if on a toolchain I could define an attribute, e.g.

node_toolchain(
  ...
  node_version = x.y
)
node_toolchain(
   ...
  node_version = a.b
)

and then on a target that uses this toolchain I can filter the specific toolchain with a special attribute, probably defined on the rule so:

nodejs_binary(
  ...
  node_version = x.y
)

Would select the toolchain with the right version.

Feature requests: what underlying problem are you trying to solve with this feature?

I want to be able to build different targets in different versions of a toolchain, that could be for different python versions or different nodejs versions etc.

Have you found anything relevant by searching the web?

Some discussion in #7375

#6517 seems possibly somewhat related.

cc @katre

@irengrig
Copy link
Contributor

irengrig commented Mar 1, 2019

Hello @Globegitter,

how about splitting this in two parts: platform-dependent and platform-independent?

  • platform dependent part (toolchain) returns whatever operating system constraints that help to locate the correct node artifact
  • platform independent part (rule) contains the actual node_toolchain rule code, it accepts the needed version number, and it accepts platform-detected part's information (i e toolchain) about the platform and creates the actual artifacts(s).

@irengrig irengrig added team-Configurability platforms, toolchains, cquery, select(), config transitions untriaged labels Mar 1, 2019
@gregestren gregestren added P3 We're not considering working on this, but happy to review a PR. (No assignee) and removed untriaged labels Mar 6, 2019
@Globegitter
Copy link
Author

@irengrig sorry for just responding now I thought I already did. Anyway, I think the overall idea sounds good and seems to make sense, but it is still a bit too abstract to me to be able to see how that would actually look like in a concrete example.

@iirina
Copy link
Contributor

iirina commented Jul 8, 2019

@katre can this be closed as a duplicate of #6519?

@katre
Copy link
Member

katre commented Jul 8, 2019

@iirina No, this is a separate issue, and won't be solved by flagless multiplatform builds.

This needs to be handled by the node rules, not generically by Bazel, because Bazel doesn't have enough information about what node considers different toolchains.

Once the Starlark build settings work is done, it will be possible for rules_node to define a setting for the node version, and then the rules can have a node_version attribute to transition that setting. The toolchain could then check the setting before deciding how to invoke node.

@ittaiz
Copy link
Member

ittaiz commented Jan 20, 2020

@katre shouldn’t this be closed then in favor of issues in the relevant rule sets?

@katre
Copy link
Member

katre commented Jan 21, 2020

@ittaiz You are correct, so I will close this now.

@katre katre closed this as completed Jan 21, 2020
@pauldraper
Copy link
Contributor

I'm confused....why was this closed?

@katre
Copy link
Member

katre commented Nov 16, 2020

@pauldraper What part of the history is unclear? This needs to be handled differently by the different toolchains for each rule, and so any issues need to be opened for specific rules.

I will point out that @comius is implementing a proposal for using config_setting to influence toolchain resolution, which may also be helpful for rules wanting to handle this type of functionality.

@pauldraper
Copy link
Contributor

pauldraper commented Nov 16, 2020

@katre the part that is unclear is how rules should cooperate with toolchains to make this happen.

To use the original example,

nodejs_binary(
  ...
  node_version = x.y
)

nodejs_binary(
  ...
  node_version = a.b
)

It's not clear how nodejs_binary should implement that support.

@katre
Copy link
Member

katre commented Nov 16, 2020

Here's a sketch of a possible solution. This would need some work to accomplish and I'm not aware of any existing examples to borrow from. It also assumes that @comius's work is complete and in Bazel.

  1. Define a new build setting for the Node version. I'll call it @rules_nodejs//:node_version here, but the actual location is up to you.
  2. Each nodejs toolchain then needs three targets: the actual nodejs toolchain target, a config_setting that checks that @rules_nodejs//:node_version is set to the proper value, and a toolchain target that declares the toolchain, toolchain type, exec and target constraints, and adds a target_setting attribute pointing to the new config_setting.
    1. Also you'll need to register the new toolchains, just like you do today.
  3. At this point, users can do builds like bazel build --@rules_nodejs//:node_version=x.y //:foo and the correct toolchain would be selected.
  4. You can also add a transition on the nodejs_binary rule, which reads a new node_version attribute and sets the @rules_nodejs//:node_version build setting. This will allow individual nodejs_binary targets to set the version (and thus to select the correct toolchain) for themselves and all dependencies.

@pauldraper
Copy link
Contributor

pauldraper commented Jan 21, 2022

@katre that only works for one global Node.js version in a build. If one tool require Node.js 14 and another tool requires Node.js 16, they can't run in the same build invocation.


As a reminder, this commonly appears in situations like targets requiring a certain compiler version or runtime version. Often this isn't a feature of the target platform or chosen by the end-user. Rather, it's a fundamental requirement imposed by the source files themselves, and should be expressed in the build configuration.

I believe there are two solutions that rule makers can implement:

1. Rule factory and transition.

Create a "rule factory" that adds a transition so that the tool matching the version is selected.

def nodejs_binary_rule(version):
  return rule(
    # ...
  )

The rule factory is then instantiated and used.

nodejs_binary_14 = nodejs_binary(version_14)

2. Attribute and target

More straightforwardly, just have an attribute.

nodejs_binary(
  node = "//:node14"
)

This target ("//:node14") can select the actual artifacts based on toolchains, host platform, target platform, etc. But there can multiple possible targets ("//:node14", "//:node16", etc) available to be chosen by the calling rule.

This latter approach is one that I recommend rule author adopt.


Generally speaking, rule authors have not cared to solve this very much. AFAIK python for example can't have targets with multiple versions, Java can't have targets with different versions, etc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P3 We're not considering working on this, but happy to review a PR. (No assignee) team-Configurability platforms, toolchains, cquery, select(), config transitions
Projects
None yet
Development

No branches or pull requests

7 participants