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

native.sh_binary reference in def.bzl blocks Bazel 8 integration #1959

Closed
ted-xie opened this issue Oct 18, 2024 · 10 comments · Fixed by #1960
Closed

native.sh_binary reference in def.bzl blocks Bazel 8 integration #1959

ted-xie opened this issue Oct 18, 2024 · 10 comments · Fixed by #1960

Comments

@ted-xie
Copy link
Contributor

ted-xie commented Oct 18, 2024

What version of gazelle are you using?

0.39.1

What version of rules_go are you using?

0.50.1

What version of Bazel are you using?

last_green (sort of 8.0)

Does this issue reproduce with the latest releases of all the above?

Yes

What operating system and processor architecture are you using?

Debian-like Linux, x86_64.

What did you do?

Build any dep with a gazelle reference using Bazel 8/last_green.

What did you expect to see?

Analysis success.

What did you see instead?

Bazel complains about missing native.sh_binary.

ted-xie added a commit to ted-xie/bazel-gazelle that referenced this issue Oct 18, 2024
ted-xie added a commit to ted-xie/bazel-gazelle that referenced this issue Oct 18, 2024
@fmeum
Copy link
Member

fmeum commented Oct 19, 2024

Happy to merge the PR, but the breakage is unexpected and certainly not intended for Bazel 8.

Are you perhaps using gazelle from a repo that happens to provide auto loaded starlark rules, e.g. rules_android? That would explain this.

@ted-xie
Copy link
Contributor Author

ted-xie commented Oct 21, 2024

I am building rules_android, but I saw this error even without enabling the autoloaded rules -- it seems that simply using the Bazel 8.0.0rc1 with a repo that does something like load("@bazel_gazelle//:deps.bzl", "gazelle_dependencies", "go_repository") is enough to trigger this behavior.

@fmeum
Copy link
Member

fmeum commented Oct 21, 2024

Could you check whether this repros from other repos (not called "rules_android") too?

@tetromino
Copy link

I am seeing this in tests for bazel_skylib_gazelle_plugin: https://buildkite.com/bazel/bazel-skylib/builds/3522#0192b626-0c02-41d7-b2cf-ecd98915abe1

@fmeum
Copy link
Member

fmeum commented Oct 24, 2024

I tried to reproduce this in a project that isn't special-cased by Bazel, but failed to do so. Could you share a reproducer?

I certainly agree that we should migrate all usages of native to loading Starlark rules, but if Gazelle breaks to due this, Bazel 8 contains an unintended breaking change and we should make sure that's fixed.

@tetromino
Copy link

tetromino commented Oct 28, 2024

@fmeum - I suspect that the failure occurs only if the module is a transitive dep of rules_shell itself (Bazel 8 can't insert a definition of native.sh_binary from rules_shell whilst in the process of loading rules_shell). The example that concerns me is bazel_skylib - it's a dependency of rules_shell, and in turn has (when building from a git checkout, as opposed to a stable release from bcr) a dev-only transitive dep on gazelle.

To reproduce:

git clone https://github.com/bazelbuild/bazel-skylib.git
cd bazel-skylib
git checkout 6edf03b1954900034ca2d85d6bf381da6663a575  # current main HEAD
USE_BAZEL_VERSION=last_green bazelisk build --nobuild //...

Expected:

WARNING: For repository 'platforms', the root module requires module version [email protected], but got [email protected] in the resolved dependency graph.
WARNING: /Users/arostovtsev/x/bazel-skylib/BUILD:37:12: target '//:lib' is deprecated: lib.bzl will go away in the future, please directly depend on the module(s) needed as it is more efficient.
INFO: Analyzed 357 targets (123 packages loaded, 2716 targets configured).
INFO: Found 357 targets...
INFO: Elapsed time: 12.154s, Critical Path: 0.00s
INFO: 0 processes.
INFO: Build completed successfully, 0 total actions

Actual result:

2024/10/28 13:00:37 Using unreleased version at commit 78abca3b5fae72c06cb2a1be2e18ea7107b5ad81
ERROR: Traceback (most recent call last):
	File "/private/var/tmp/_bazel_arostovtsev/92a2642c38066dd6b1c5d2f9db209f70/external/bazel_skylib_gazelle_plugin+/bzl/BUILD", line 55, column 8, in <toplevel>
		gazelle(
	File "/private/var/tmp/_bazel_arostovtsev/92a2642c38066dd6b1c5d2f9db209f70/external/gazelle+/def.bzl", line 184, column 11, in gazelle
		native.sh_binary(
Error: no native function or rule 'sh_binary'
Available attributes: action_listener, alias, available_xcodes, cc_binary, cc_import, cc_libc_top_alias, cc_library, cc_proto_library, cc_shared_library, cc_static_library, cc_test, cc_toolchain, cc_toolchain_alias, cc_toolchain_suite, config_feature_flag, config_setting, constraint_setting, constraint_value, environment, existing_rule, existing_rules, exports_files, extra_action, fdo_prefetch_hints, fdo_profile, filegroup, genquery, genrule, glob, java_binary, java_import, java_library, java_lite_proto_library, java_package_configuration, java_plugin, java_plugins_flag_alias, java_proto_library, java_runtime, java_test, java_toolchain, label_flag, label_setting, legacy_globals, memprof_profile, module_name, module_version, objc_import, objc_library, package, package_group, package_name, package_relative_label, platform, propeller_optimize, proto_lang_toolchain, proto_library, repo_name, repository_name, starlark_doc_extract, subpackages, test_suite, toolchain, toolchain_type, xcode_config, xcode_config_alias, xcode_version
ERROR: /private/var/tmp/_bazel_arostovtsev/92a2642c38066dd6b1c5d2f9db209f70/external/bazel_skylib_gazelle_plugin+/bzl/BUILD: no such target '@@bazel_skylib_gazelle_plugin+//bzl:distribution': target 'distribution' not declared in package 'bzl' defined by /private/var/tmp/_bazel_arostovtsev/92a2642c38066dd6b1c5d2f9db209f70/external/bazel_skylib_gazelle_plugin+/bzl/BUILD

As a workaround, we could cut the dev-only dependency - at the expense of inconveniencing skylib maintainers and making it impossible to test dev versions of bazel_skylib_gazelle_plugin and bazel_skylib against each other...

@ted-xie
Copy link
Contributor Author

ted-xie commented Oct 28, 2024

@fmeum I finally got around to making a reproducer. It's not specifically for Gazelle; it's for rules_java. However, it illustrates the issues of autoloads with Bazel 8 inside of special-cased main modules (for instance, rules_android).

https://github.com/ted-xie/autoloads_with_dummy_rules_android

@rickeylev
Copy link

I ran into this in rules_python with Bazel 8, too.

@fmeum
Copy link
Member

fmeum commented Nov 6, 2024

I'm working on a release that will fix this, but there are some unrelated issues that make this a bit more difficult.

@rickeylev
Copy link

FYI: I patched in the core changes from df259ce and rules_python seems to be happy.

fmeum pushed a commit to ted-xie/bazel-gazelle that referenced this issue Nov 11, 2024
fmeum added a commit that referenced this issue Nov 11, 2024
**What type of PR is this?**

 Bug fix

**What package or component does this PR mostly affect?**

all

**What does this PR do? Why is it needed?**

Adds an explicit load (and dep) for sh_binary, which is no longer
packaged inside Bazel 8. This is backwards-compatible with Bazel 7.

**Which issues(s) does this PR fix?**

Fixes #1959

**Other notes for review**

---------

Co-authored-by: Fabian Meumertzheim <[email protected]>
github-merge-queue bot pushed a commit to bazelbuild/rules_python that referenced this issue Nov 12, 2024
Various changes to support Bazel 8. An important note is dependencies
have forced
us to change the versions of Bazel we support.

Summary of changes:
* rules_cc 0.0.14: Releases after 0.0.9 have some Bazel 8 fixes, but
also broke
  some things. Things seemed to have settled by 0.0.14.
* protobuf 29.0-rc1: Technically 28.0 works, however:
  1. 29.0-rc1 is coming via a transitive dependency anyways, and
2. In protobuf 28.0, compile warnings are treated as errors, which our
Debian CI
     respects (and thus fails), while other platforms ignore.
* stardoc 0.7.1: Fixes an issue with Bazel 8 and stardoc using empty
globs.
* Bazel 7.4 is now the minimum supported Bazel version. This
requirements comes via
  dependencies.
* Drop Bazel 6 bzlmod support. This requirement comes via dependencies.
* Add a presubmit job for `last_rc` Bazel (currently the 8.x RC).
* Use a local patch so Gazelle works with Bazel 8. This can be removed
once bazel-contrib/bazel-gazelle#1959 is fixed
and released.
* Fix a `$(rpathlocation)` call in bootstrap tests.
* Update bzl_library deps after upgrading deps: the set of targets that
provide
  bzl sources changed in rules_cc and protobuf in these newer versions.

Sorting this all out and finding the right combination of dependency
versions was
fairly involved. The details of that are in
#2378.

Work towards #2378,
#2387
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants