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

Add rules_shell dep to rules_bazel_integration_test #368

Merged
merged 7 commits into from
Oct 18, 2024

Conversation

ted-xie
Copy link
Contributor

@ted-xie ted-xie commented Oct 18, 2024

native.{sh_test,sh_library} no longer exist in Bazel HEAD.

@katre katre self-requested a review October 18, 2024 19:14
@katre katre enabled auto-merge (squash) October 18, 2024 19:15
@katre
Copy link
Collaborator

katre commented Oct 18, 2024

Looks like the MODULE.bazel.lock file is stale. Upload a new version and I'll re-approve

auto-merge was automatically disabled October 18, 2024 19:22

Head branch was pushed to by a user without write access

@ted-xie
Copy link
Contributor Author

ted-xie commented Oct 18, 2024

@katre I updated the MODULE lockfile, I think -- I did rm MODULE.bazel.lock; bazelisk build ... --lockfile_mode=update.

@katre
Copy link
Collaborator

katre commented Oct 18, 2024

Now the error is generate the stardoc:

com.google.devtools.build.skydoc.SkydocMain$StarlarkEvaluationException: File /home/runner/.cache/bazel/_bazel_runner/331b13f68cad845627c8e44b927d4117/sandbox/linux-sandbox/35/execroot/_main/bazel-out/k8-opt-exec-ST-d57f47055a04/bin/doc/rules_and_macros_overview_stardoc.runfiles/_main/bazel_integration_test/private/bazel_integration_test.bzl imported '@rules_shell//shell:sh_test.bzl', yet /home/runner/.cache/bazel/_bazel_runner/331b13f68cad845627c8e44b927d4117/sandbox/linux-sandbox/35/execroot/_main/bazel-out/k8-opt-exec-ST-d57f47055a04/bin/doc/rules_and_macros_overview_stardoc.runfiles/rules_shell/shell/sh_test.bzl was not found.

I'm not sure how to resolve this, I assume you need to add the rules_shell dependency somewhere else but I don't know how the docs generation works.

I can take a look with you next week if that helps.

@ted-xie
Copy link
Contributor Author

ted-xie commented Oct 18, 2024

Turns out I just needed to add @rules_shell//shell:rules_bzl to the deps for bazel_integration_test.

@cgrindel
Copy link
Member

@ted-xie Do rules_sh work with Bazel 7.x?

@ted-xie
Copy link
Contributor Author

ted-xie commented Oct 18, 2024

@cgrindel Yes, we're currently using rules_shell + bazel 7.4 in rules_android.

@cgrindel
Copy link
Member

@ted-xie Can you just delete MODULE.bazel.lock? We really do not need this in a ruleset repo.

@ted-xie
Copy link
Contributor Author

ted-xie commented Oct 18, 2024

Happy to delete MODULE.bazel.lock in a follow-up PR, or in a follow-up commit in this current PR if the presubmit fails again.

@cgrindel
Copy link
Member

To make sure that everything is up-to-date, run bazel run //:tidy.

@katre
Copy link
Collaborator

katre commented Oct 18, 2024

You need to rebase onto the latest main anyway.

@ted-xie
Copy link
Contributor Author

ted-xie commented Oct 18, 2024

Rebased to latest main, deleted the lockfile, and ran tidy.

@cgrindel cgrindel enabled auto-merge (squash) October 18, 2024 20:45
@cgrindel
Copy link
Member

cgrindel commented Oct 18, 2024

Once we get this merged, I think that we should create a release unless there are other PRs that we want to wait on.

@katre @ted-xie Does that sound right to you?

.bazelrc Outdated Show resolved Hide resolved
@katre
Copy link
Collaborator

katre commented Oct 18, 2024

Cutting a release sounds good.

@katre
Copy link
Collaborator

katre commented Oct 18, 2024

Looks like the bazelrc change is causing problems, so go ahead and revert that.

@cgrindel
Copy link
Member

Give me a second, I'll pull it down and check.

@ted-xie
Copy link
Contributor Author

ted-xie commented Oct 18, 2024

It looks like the diff that got introduced was basically this:

Before:

custom_test_runner/Sources/CustomTestRunner,examples

After:

custom_test_runner/integration_tests,examples

I'm not familiar with this repo and its history, so I'll leave that to @cgrindel to ponder, and in the mean time I'll revert my change to the bazelrc.

auto-merge was automatically disabled October 18, 2024 20:55

Head branch was pushed to by a user without write access

@cgrindel
Copy link
Member

@ted-xie Don't make any changes. I'll just push to your branch.

@cgrindel
Copy link
Member

Nevermind. 🙂

@katre
Copy link
Collaborator

katre commented Oct 18, 2024

I'm heading out to the airport soon so I'll leave the rest of the review to @cgrindel.

@ted-xie
Copy link
Contributor Author

ted-xie commented Oct 18, 2024

I deleted the commit where I un-reverted the bazelrc change (whew!). If you have a more complete solution, feel free to push to my branch :)

@cgrindel cgrindel enabled auto-merge (squash) October 18, 2024 21:05
@cgrindel cgrindel merged commit 0b8c5ca into bazel-contrib:main Oct 18, 2024
5 checks passed
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 this pull request may close these issues.

3 participants