Skip to content

Conversation

@gregestren
Copy link
Contributor

@gregestren gregestren commented Sep 22, 2025

--incompatible_remove_ctx_py_fragment removes ctx.fragments.py.

--incompatible_remove_ctx_bazel_py_fragment removes ctx.fragments.bazel_py.

rules_python can use this to determine if it should read Python flags from Starlark definitions or Bazel-native:

if not hasattr(ctx.fragments, "py"):
  print("I should read Python flags from Starlark.")

For bazel-contrib/rules_python#3252.

Caveats:
This approach has the downside that it's all-or-nothing: removing an entire fragment doesn't permit partial Starlarkification. Which is hypothetically an issue if we have to keep certain flags native because of strange inner Bazel dependencies.

I think for Python this isn't a huge risk. Worst-case we can redefine the flags to restrict fragment access instead of completely removing it.

@gregestren gregestren requested a review from mai93 September 22, 2025 20:04
@github-actions github-actions bot added team-Rules-Python Native rules for Python awaiting-review PR is awaiting review from an assigned reviewer labels Sep 22, 2025
@gregestren gregestren added the team-Configurability platforms, toolchains, cquery, select(), config transitions label Sep 22, 2025
@gregestren gregestren changed the title New Incompatible flags disable ctx.fragments.[py|bazel_py] New incompatible flags disable ctx.fragments.py, ctx.fragments.bazel_py Sep 22, 2025
@rickeylev
Copy link
Contributor

Checking ctx.fragments for an attribute SGTM in general, but what about configuration_field()? These are used during the loading phase when rules are defined. IIRC, if it references a fragment name that doesn't exist, it's an error. Similar question for the fragments = ... arg of rules.

@gregestren
Copy link
Contributor Author

Checking ctx.fragments for an attribute SGTM in general, but what about configuration_field()? These are used during the loading phase when rules are defined. IIRC, if it references a fragment name that doesn't exist, it's an error. Similar question for the fragments = ... arg of rules.

Good call.

Where does rules_python set configuration_field?

I just see one (unaffected) ctx.fragments.coverage call and some attr_builders references I don't fully follow?

@gregestren
Copy link
Contributor Author

Where does rules_python set configuration_field?

I just see one (unaffected) ctx.fragments.coverage call and some attr_builders references I don't fully follow?

For example, https://github.com/bazel-contrib/rules_python/pull/3280/files removed a bazel_py configuration_field reference.

@rickeylev
Copy link
Contributor

Oh nice, there was just that one configuration_field() call that you removed. So that's taken care of.

What about fragments = [py|bazel_py] ? Will passing that still work? py_library.bzl, py_executable.bzl, py_runtime_pair_rule.bzl, and py_runtime_rule.bzl all specify that.

The attrs_builders and rule_builders code are just wrappers around the attr.XXX functions. e.g. attrb.LabelList is a wrapper around attr.label_list.

github-merge-queue bot pushed a commit to bazel-contrib/rules_python that referenced this pull request Sep 23, 2025
…unction (#3290)

This lets Python logic support either Starlark- or native-defined flags,
based on Bazel support:

- Pre-Bazel 9.0: always use native flags
- Bazel 9.0+: use Starlark flags if [incompatible
change](bazelbuild/bazel#27056) that disables
`ctx.fragments.py` and `ctx.fragments.bazel_py` is set
- Include a developer override to trigger Starlark versions while
testing Starlarkification. Developers enable this by updating .bzl code
in their local workspace. This can be removed as soon as
Starlarkification is complete

Also check in a Starlark version of
`--experimental_python_import_all_repositories` for testing.

**This is a no-op. New flag sources of truth don't take effect without a
supporting bazel version that sets appropriate incompatible flags.**

**Caveats:**

- May not work with `configuration_field` as implemented. We can loosen
the incompatible flag lockdown if that's an issue.

For #3252

TODO list

* [ ] Add docs to docs/api/rules_python/python/config_settings/index.md
for starlarkified flags
@gregestren
Copy link
Contributor Author

What about fragments = [py|bazel_py] ? Will passing that still work? py_library.bzl, py_executable.bzl, py_runtime_pair_rule.bzl, and py_runtime_rule.bzl all specify that.

That's fine. I checked the Bazel code and also did some manual testing.

fragments = ["py"] just holds an unresolved string until rules try to read the fragment.

So as long as ctx.fragments.py.foo isn't called it's all good.

@gregestren gregestren added awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally and removed awaiting-review PR is awaiting review from an assigned reviewer labels Sep 23, 2025
@github-actions github-actions bot removed the awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally label Sep 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

team-Configurability platforms, toolchains, cquery, select(), config transitions team-Rules-Python Native rules for Python

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants