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

No way to run tests in bzlmod dependency if they depend on dev_dependency #22187

Open
davidben opened this issue Apr 29, 2024 · 4 comments
Open
Labels
area-Bzlmod Bzlmod-specific PRs, issues, and feature requests help wanted Someone outside the Bazel team could own this P3 We're not considering working on this, but happy to review a PR. (No assignee) team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. type: feature request

Comments

@davidben
Copy link

Description of the bug:

Based on bazelbuild/bazel-central-registry#1833 (comment), I gather best practice for bzlmod is to mark your tests' dependencies as a dev_dependency. We've thus tentatively set googletest as a dev_dependency in BoringSSL.

Now, suppose someone consuming BoringSSL wants to run the tests anyway. For example, they may have some issue and we might ask them to run the tests and confirm it's not a toolchain problem. This then fails because the dependency is missing:

Starting local Bazel server and connecting to it...
ERROR: no such package '@@[unknown repo 'googletest' requested from @@boringssl~]//': The repository '@@[unknown repo 'googletest' requested from @@boringssl~]' could not be resolved: No repository visible as '@googletest' from repository '@@boringssl~'
ERROR: /usr/local/google/home/davidben/.cache/bazel/_bazel_davidben/11e4cdfc071b85e46241e062520598c0/external/boringssl~/BUILD.bazel:93:13: no such package '@@[unknown repo 'googletest' requested from @@boringssl~]//': The repository '@@[unknown repo 'googletest' requested from @@boringssl~]' could not be resolved: No repository visible as '@googletest' from repository '@@boringssl~' and referenced by '@@boringssl~//:crypto_test'
ERROR: Analysis of target '@@boringssl~//:crypto_test' failed; build aborted: Analysis failed
INFO: Elapsed time: 4.755s, Critical Path: 0.03s
INFO: 1 process: 1 internal.
ERROR: Build did NOT complete successfully
ERROR: No test targets were found, yet testing was requested
FAILED: 
    Fetching repository @@bazel_tools~cc_configure_extension~local_config_cc; starting

The same error occurs if I explicitly pull in googletest from the downstream module. As far as I can tell, there is no way to go back and tell boringssl that I do indeed have a googletest for you.

Which category does this issue belong to?

External Dependency

What's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

git clone https://boringssl.googlesource.com/boringssl
cd boringssl
git checkout 70d05d5a34f6366116e2b0a530ea8d0186bb2a8e
cd util/bazel-example
bazelisk test @boringssl//:crypto_test
echo 'bazel_dep(name = "googletest", version = "1.14.0.bcr.1")' >> MODULE.bazel
bazelisk test @boringssl//:crypto_test

Which operating system are you running Bazel on?

Linux

What is the output of bazel info release?

release 7.1.1

If bazel info release returns development version or (@non-git), tell us how you built Bazel.

No response

What's the output of git remote get-url origin; git rev-parse HEAD ?

https://boringssl.googlesource.com/boringssl
70d05d5a34f6366116e2b0a530ea8d0186bb2a8e

Is this a regression? If yes, please try to identify the Bazel commit where the bug was introduced.

No response

Have you found anything relevant by searching the web?

No response

Any other information, logs, or outputs that you want to share?

No response

@Wyverald
Copy link
Member

This is WAI; downstream users aren't expected to be able to run your tests.

We don't want to let boringssl see a googletest pulled in by downstream modules, because that would be akin to circumventing strict deps checks -- behavior could silently and unexpectedly change depending on whether downstream has pulled in googletest.

@Wyverald Wyverald closed this as not planned Won't fix, can't repro, duplicate, stale Apr 29, 2024
@davidben
Copy link
Author

I see. Running the tests in BCR is not as critical, but being able to run our tests with the downstream toolchain is quite important for triaging issues. Without this, we'll probably need to establish a policy of not accepting bug reports from bzlmod-based projects until they can reproduce it standalone.

@davidben
Copy link
Author

I suppose the other option is to make googletest a full dependency, and not dev_dependency. That's probably the best option given the lack of this mechanism.

Note I am not specifically requesting that boringssl pick up the outer googletest. An option like include_dev_dependencies to bazel_dep would also suffice.

davidben added a commit to google/boringssl that referenced this issue Apr 30, 2024
In principle, googletest is only needed for tests and could be a
dev_dependency. But it would then be impossible for downstream modules
to run BoringSSL's tests. The downstream module provides the toolchain,
so we may need to request they run tests when triaging issues. If
bazelbuild/bazel#22187 is ever fixed, we can
change this.

With this change, the following works:

  cd util/bazel-example
  bazelisk test @boringssl//:crypto_test

Change-Id: Ied2276047de134883d6b61b0789f3c7bfcaad669
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/68127
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>
@Wyverald Wyverald reopened this Apr 30, 2024
@Wyverald
Copy link
Member

Note I am not specifically requesting that boringssl pick up the outer googletest. An option like include_dev_dependencies to bazel_dep would also suffice.

This seems like a reasonable enough FR. Or maybe a flag --include_transitive_dev_dependencies that is the inverse of --ignore_dev_dependency, which would allow the end user to run tests of all transitive deps.

@Wyverald Wyverald added type: feature request P3 We're not considering working on this, but happy to review a PR. (No assignee) help wanted Someone outside the Bazel team could own this area-Bzlmod Bzlmod-specific PRs, issues, and feature requests and removed type: bug untriaged labels Apr 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-Bzlmod Bzlmod-specific PRs, issues, and feature requests help wanted Someone outside the Bazel team could own this P3 We're not considering working on this, but happy to review a PR. (No assignee) team-ExternalDeps External dependency handling, remote repositiories, WORKSPACE file. type: feature request
Projects
None yet
Development

No branches or pull requests

5 participants