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.

Sign up for GitHub

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

Jump to bottom

Starlark: provide a way to probe builtins availability #12735

Closed
divanorama opened this issue Dec 19, 2020 · 5 comments
Closed

Starlark: provide a way to probe builtins availability #12735

divanorama opened this issue Dec 19, 2020 · 5 comments
Labels
P4 This is either out of scope or we don't have bandwidth to review a PR. (No assignee) stale Issues or PRs that are stale (no activity for 30 days) team-Starlark-Integration Issues involving Bazel's integration with Starlark, excluding builtin symbols type: feature request

Comments

@divanorama
Copy link
Contributor

Description of the problem / feature request:

It seems that it's not possible to test whether some builtin is available or not from within Starlark

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

It is nice to have new built-in modules, like json (#3732), but it's hard to use them in Starlark libraries and/or repo rules in a way compatible with older bazel versions.

Example

some.bzl

load("@bazel_skylib//:lib.bzl", "versions")
load("//third_party/bazel_json/lib:json_parser.bzl", "json_parse")
def foo(s):
  if versions.is_at_least("4.0.0", versions.get()):
    return json.decode(s)
  return json_parse(s)

That would almost work if there was a way to not fail if "json" isn't a global name. Ideally though 2nd load() should also be conditional (maybe on workspace level somehow?).
@divanorama
Copy link
Contributor Author

divanorama commented Dec 19, 2020 •
edited
Loading

Maybe it's a candidate for skylib:

  • add a repository rule that knows builtins based on bazel version
  • it re-exports them if they exist in current version
  • if they don't exist, export stubs like: "def f(): fail()", "x = None"
  • exports list of builtin names for given bazel version to allow for fallbacks on caller side
  • additionally may add json_compat layer that'd add bazel_json repository and re-export its methods instead of "def f(): fail()" ones

That'd work for old versions, but for newer additions would be nice to have a native way to do that as well

@aiuto
Copy link
Contributor

aiuto commented Jan 6, 2021

We've talked about this a lot. It almost seems useful, but you soon want to do something different enough so that you hit the wall you hinted at about conditional load. One of the things we are thinking of is that rule sets versions should be explicitly tied to bazel versions. Depending on bazel version we swap the entire WORKSPACE to get a compatible suite. Of course, home grown rules would also have to be versioned, which could be a burden for monorepos.

@brandjon
Copy link
Member

See also related issues #8305 and #10907.

@brandjon brandjon added P4 This is either out of scope or we don't have bandwidth to review a PR. (No assignee) team-Build-Language type: feature request and removed team-Starlark untriaged labels Feb 19, 2021
shs96c added a commit to shs96c/rules_jvm_external that referenced this issue Jan 19, 2022
@shs96c
Use bazel's native json decode method
84cec76 
This means that `rules_jvm_external` is only compatible with
Bazel 4 and later. That shipped on 2021-01-21, and so should
be widely adopted already. Older versions of this ruleset
will continue to work for repos using older versions of Bazel
so it's not removing any existing functionality, and the new
Bazel 5 release should have already shipped.

The main reason for doing this is that the native json
decoding is significantly faster than the starlark-based one.

The following options were explored as alternatives:

1. Making the `json_parse` method check the bazel version and
   optionally use the `json` module if it was present. This
   doesn't work because of #12735

2. Create a `json` repository rule that creates a repo that
   exposes a `json.bzl` file that delegates to the native
   `json` module if present, or the starlark one if not
   (as determined by looking at the major version number of
   bazel). This doesn't work because `maven_install` is used
   by RJE's own `repositories.bzl` file, so there's no chance
   to load the repository rule before it's needed.

Links:

12735: bazelbuild/bazel#12735
@shs96c shs96c mentioned this issue Jan 19, 2022
Merged
shs96c added a commit to shs96c/rules_jvm_external that referenced this issue Jan 27, 2022
@shs96c
Use bazel's native json decode method
e9681e6 
This means that `rules_jvm_external` is only compatible with
Bazel 4 and later. That shipped on 2021-01-21, and so should
be widely adopted already. Older versions of this ruleset
will continue to work for repos using older versions of Bazel
so it's not removing any existing functionality, and the new
Bazel 5 release should have already shipped.

The main reason for doing this is that the native json
decoding is significantly faster than the starlark-based one.

The following options were explored as alternatives:

1. Making the `json_parse` method check the bazel version and
   optionally use the `json` module if it was present. This
   doesn't work because of #12735

2. Create a `json` repository rule that creates a repo that
   exposes a `json.bzl` file that delegates to the native
   `json` module if present, or the starlark one if not
   (as determined by looking at the major version number of
   bazel). This doesn't work because `maven_install` is used
   by RJE's own `repositories.bzl` file, so there's no chance
   to load the repository rule before it's needed.

Links:

12735: bazelbuild/bazel#12735
shs96c added a commit to shs96c/rules_jvm_external that referenced this issue Jan 28, 2022
@shs96c
Use bazel's native json decode method
3e10de6 
This means that `rules_jvm_external` is only compatible with
Bazel 4 and later. That shipped on 2021-01-21, and so should
be widely adopted already. Older versions of this ruleset
will continue to work for repos using older versions of Bazel
so it's not removing any existing functionality, and the new
Bazel 5 release should have already shipped.

The main reason for doing this is that the native json
decoding is significantly faster than the starlark-based one.

The following options were explored as alternatives:

1. Making the `json_parse` method check the bazel version and
   optionally use the `json` module if it was present. This
   doesn't work because of #12735

2. Create a `json` repository rule that creates a repo that
   exposes a `json.bzl` file that delegates to the native
   `json` module if present, or the starlark one if not
   (as determined by looking at the major version number of
   bazel). This doesn't work because `maven_install` is used
   by RJE's own `repositories.bzl` file, so there's no chance
   to load the repository rule before it's needed.

Links:

12735: bazelbuild/bazel#12735
shs96c added a commit to shs96c/rules_jvm_external that referenced this issue Feb 18, 2022
@shs96c
Use bazel's native json decode method
bd79ff4 
This means that `rules_jvm_external` is only compatible with
Bazel 4 and later. That shipped on 2021-01-21, and so should
be widely adopted already. Older versions of this ruleset
will continue to work for repos using older versions of Bazel
so it's not removing any existing functionality, and the new
Bazel 5 release should have already shipped.

The main reason for doing this is that the native json
decoding is significantly faster than the starlark-based one.

The following options were explored as alternatives:

1. Making the `json_parse` method check the bazel version and
   optionally use the `json` module if it was present. This
   doesn't work because of #12735

2. Create a `json` repository rule that creates a repo that
   exposes a `json.bzl` file that delegates to the native
   `json` module if present, or the starlark one if not
   (as determined by looking at the major version number of
   bazel). This doesn't work because `maven_install` is used
   by RJE's own `repositories.bzl` file, so there's no chance
   to load the repository rule before it's needed.

Links:

12735: bazelbuild/bazel#12735
shs96c added a commit to bazel-contrib/rules_jvm_external that referenced this issue Mar 24, 2022
@shs96c
Use bazel's native json decode method (#659)
156ba5b 
This means that `rules_jvm_external` is only compatible with
Bazel 4 and later. That shipped on 2021-01-21, and so should
be widely adopted already. Older versions of this ruleset
will continue to work for repos using older versions of Bazel
so it's not removing any existing functionality, and the new
Bazel 5 release should have already shipped.

The main reason for doing this is that the native json
decoding is significantly faster than the starlark-based one.

The following options were explored as alternatives:

1. Making the `json_parse` method check the bazel version and
   optionally use the `json` module if it was present. This
   doesn't work because of #12735

2. Create a `json` repository rule that creates a repo that
   exposes a `json.bzl` file that delegates to the native
   `json` module if present, or the starlark one if not
   (as determined by looking at the major version number of
   bazel). This doesn't work because `maven_install` is used
   by RJE's own `repositories.bzl` file, so there's no chance
   to load the repository rule before it's needed.

Links:

12735: bazelbuild/bazel#12735
@brandjon brandjon added team-Starlark-Integration Issues involving Bazel's integration with Starlark, excluding builtin symbols and removed team-Build-Language labels Nov 4, 2022
@github-actions GitHub Actions
Copy link

github-actions bot commented Jan 9, 2024

Thank you for contributing to the Bazel repository! This issue has been marked as stale since it has not had any activity in the last 1+ years. It will be closed in the next 90 days unless any other activity occurs or one of the following labels is added: "not stale", "awaiting-bazeler". Please reach out to the triage team (@bazelbuild/triage) if you think this issue is still relevant or you are interested in getting the issue resolved.

@github-actions github-actions bot added the stale Issues or PRs that are stale (no activity for 30 days) label Jan 9, 2024
@fmeum
Copy link
Collaborator

fmeum commented Jan 9, 2024

#12735 (comment) now has an official implementation at https://github.com/bazel-contrib/bazel_features.

@fmeum fmeum closed this as completed Jan 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
P4 This is either out of scope or we don't have bandwidth to review a PR. (No assignee) stale Issues or PRs that are stale (no activity for 30 days) team-Starlark-Integration Issues involving Bazel's integration with Starlark, excluding builtin symbols type: feature request
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@divanorama @brandjon @aiuto @fmeum