Bazel integration tests get default repos from the actual WORKSPACE file.

[aiuto]

We reference some repositories in WORKSPACE then we replace the reference in test harnesses. This is brittle and can lead to tests which do not match the repo version we actually use. Below is the case for rules_cc and shell test. The problem exists for Java and Python tests too. This will get worse as we move more code out of native and //tools and into external repos.

In WORKSPACE:

http_archive(
    name = "rules_cc",
    sha256 = "36fa66d4d49debd71d05fba55c1353b522e8caef4a20f8080a3d17cdda001d89",
    strip_prefix = "rules_cc-0d5f3f2768c6ca2faca0079a997a97ce22997a0c",
    urls = [
        "https://mirror.bazel.build/github.com/bazelbuild/rules_cc/archive/0d5f3f2768c6ca2faca0079a997a97ce22997a0c.zip",
        "https://github.com/bazelbuild/rules_cc/archive/0d5f3f2768c6ca2faca0079a997a97ce22997a0c.zip",
    ],
)

bazel/src/test/shell/testenv.sh

Line 452 in 64239e0

function add_rules_cc_to_workspace() {

function add_rules_cc_to_workspace() {
  cat >> "$1"<<EOF
load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")
http_archive(
    name = "rules_cc",
    sha256 = "36fa66d4d49debd71d05fba55c1353b522e8caef4a20f8080a3d17cdda001d89",
    strip_prefix = "rules_cc-0d5f3f2768c6ca2faca0079a997a97ce22997a0c",
    urls = [
        "https://mirror.bazel.build/github.com/bazelbuild/rules_cc/archive/0d5f3f2768c6ca2faca0079a997a97ce22997a0c.zip",
        "https://github.com/bazelbuild/rules_cc/archive/0d5f3f2768c6ca2faca0079a997a97ce22997a0c.zip",
    ],
)
EOF
}

It is more than just one file.

grep -r 36fa66d4d49debd71d05fba55c1353b522e8caef4a20f8080a3d17cdda001d89 .
./src/main/java/com/google/devtools/build/lib/bazel/rules/java/jdk.WORKSPACE:    sha256 = "36fa66d4d49debd71d05fba55c1353b522e8caef4a20f8080a3d17cdda001d89",
./src/test/shell/testenv.sh:    sha256 = "36fa66d4d49debd71d05fba55c1353b522e8caef4a20f8080a3d17cdda001d89",
./src/test/java/com/google/devtools/build/lib/blackbox/framework/BlackBoxTestEnvironment.java:            "    sha256 = '36fa66d4d49debd71d05fba55c1353b522e8caef4a20f8080a3d17cdda001d89',",
./src/test/py/bazel/test_base.py:        '"36fa66d4d49debd71d05fba55c1353b522e8caef4a20f8080a3d17cdda001d89",',
./src/test/py/bazel/testdata/runfiles_test/WORKSPACE.mock:  sha256 = "36fa66d4d49debd71d05fba55c1353b522e8caef4a20f8080a3d17cdda001d89",
./WORKSPACE:        "0d5f3f2768c6ca2faca0079a997a97ce22997a0c.zip": "36fa66d4d49debd71d05fba55c1353b522e8caef4a20f8080a3d17cdda001d89",
./WORKSPACE:    sha256 = "36fa66d4d49debd71d05fba55c1353b522e8caef4a20f8080a3d17cdda001d89",
./WORKSPACE:        "0d5f3f2768c6ca2faca0079a997a97ce22997a0c.zip": "36fa66d4d49debd71d05fba55c1353b522e8caef4a20f8080a3d17cdda001d89",

[sergiocampama]

I believe rules_apple uses this on purpose to avoid downloading all WORKSPACE dependencies every time we run bazel tests. so as things start migrating into different repos, it might be worthwhile to rethink how shell tests work in order to avoid these extra downloads on each run, or risk tests being flaky on network calls

[aiuto]

Yes. I did not intend that we use the entire WORKSPACE. What we require is a scheme where the repos used in WORKSPACE can be marked in a particular way so that any test may do something like
add_to_workspace WORKSPACE rules_cc
Which will

• extract the rules_cc stanza from the overall WORKSPACE
• see if it is http_archive or git_repository (or maven ....) and add a loadI() for that to the top of WORKSPACE if not already there
• append the desired stanza to the end.

And then the tests should be respoonsible for saying what they really need, rather than relying on the "default" WORKSPACE, which happens to have rules_cc in it today.

[aiuto]

Straw man proposal for external repositories we depend on by reference.

Example with rules_pkg

• create 'deps/WORKSPACE.rules_pkg
• create a test to make sure that WORKSPACE containts the same content as the deps. It would validate that
  for dep in deps/WORKSPACE.*
  • /WORKSPACE contains the text

  #### begin dep
    ... content of dep
  #### end dep
  

• enhance .../devtools/build/lib/blackbox/framework/BlackBoxTestEnvironment.java to allow reference to rules_pkg. That would include the content of //deps/WORKSPACE.rules_pkg in the WORKSPACE file it is constructing.
• same or use the same utility for that .../devtools/build/lib/blackbox/tests/workspace/RepoWithRuleWritingTextGenerator.java

Extended proposal:

• deps/WORKSPACE.foo is generated
• input to rule which generates WORKSPACE stanza is type, name, urls, sha, deps function and initialization function
• integration tests depend on generated content, but that just works
• WORKSPACE sanity checks need a little tweaking because it can not glob deps/WORKSPACE.*
• BONUS: //:additional_distfiles can extract the repo name, urls and sha from the deps/ rules so that we do not have to repeat them multiple places in //WORKSPACE.

@philwo @aehlig WDYT?

[aiuto]

This is not rules server. It is a 'how do we build bazel problem'. Switching to ProductExcellence for now.

[philwo]

I think I understand the problem, but maybe not enough to give advice here. The proposal seems rather complex to me, but maybe I'm lacking context here. Why is it a problem if the integration tests just use the global WORKSPACE file? Bazel shouldn't download anything that's not used, even when it's present in the WORKSPACE, so why do we want to minimize the things in each test's WORKSPACE file?

Assigning to Florian to think about it a bit.

[aiuto]

Why is it a problem if the integration tests just use the global WORKSPACE file?

That is an important question. Our current tests do not. They carefully craft just what they need. But I didn't write any of them and they all predate me, so I don't know why we got here.

I do know there are several Java integration tests which create a WORKSPACE and BUILD. They could just as easily copy the top level WORKSPACE. It would get rid of a bunch of test scaffolding.

[aiuto]

I finally fixed the blackbox tests which lead to this issue. There are a bunch of tests that

• create a "repo writing rule" suite with a WORKSPACE, BUILD and repo.bzl file that generates new rules and then tar's it all up.
• and now creates a second WORKSPACE that loads the output of the first as an http_archive.

But I am not sure if the tests actually need that complexity. They are underdocumented w.r.t. to intent.

[aiuto]

rules_cc update bites another user.
https://github.com/greenhouse-org/bazel/commit/e673a9bd05b3d5a07eb11bc624c76d3de68ac811

[fweikert]

Unfortunately the link is dead. Is this still a problem, given the planned revamp of the whole external deps mechanism?

[github-actions]

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 2+ years. It will be closed in the next 14 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.

[aiuto]

I did a quick look and there are a few places left where tests hard code their own version of a repository.

• src/test/py/bazel/testdata/runfiles_test/WORKSPACE.mock
  rules_cc version 0.0.6
• src/test/py/bazel/bazel_external_repository_test.py
  does something bizarre with six-1.10. It can be fixed in an afternoon. bazel_external_repository_test.py has a wacky dependency on six-1.10.0.tar.gz #18272