From d9ed9c978dccc28ca13e1ba6593f16604907f017 Mon Sep 17 00:00:00 2001 From: Zhongpeng Lin Date: Wed, 8 Feb 2023 19:14:35 -0800 Subject: [PATCH 01/21] Making exclusions more strict (#1054) --- gazelle/python/configure.go | 2 +- gazelle/python/generate.go | 9 ++++--- gazelle/python/python_test.go | 26 ++++++-------------- gazelle/python/testdata/monorepo/a/BUILD.in | 1 + gazelle/python/testdata/monorepo/a/BUILD.out | 1 + gazelle/python/testdata/monorepo/a/README.md | 3 +++ 6 files changed, 19 insertions(+), 23 deletions(-) create mode 100644 gazelle/python/testdata/monorepo/a/BUILD.in create mode 100644 gazelle/python/testdata/monorepo/a/BUILD.out create mode 100644 gazelle/python/testdata/monorepo/a/README.md diff --git a/gazelle/python/configure.go b/gazelle/python/configure.go index 901c226782..32f9ab0a11 100644 --- a/gazelle/python/configure.go +++ b/gazelle/python/configure.go @@ -103,7 +103,7 @@ func (py *Configurer) Configure(c *config.Config, rel string, f *rule.File) { case "exclude": // We record the exclude directive for coarse-grained packages // since we do manual tree traversal in this mode. - config.AddExcludedPattern(strings.TrimSpace(d.Value)) + config.AddExcludedPattern(filepath.Join(rel, strings.TrimSpace(d.Value))) case pythonconfig.PythonExtensionDirective: switch d.Value { case "enabled": diff --git a/gazelle/python/generate.go b/gazelle/python/generate.go index 3d63124028..26ffedaca2 100644 --- a/gazelle/python/generate.go +++ b/gazelle/python/generate.go @@ -161,13 +161,14 @@ func (py *Python) GenerateRules(args language.GenerateArgs) language.GenerateRes } if filepath.Ext(path) == ".py" { if cfg.CoarseGrainedGeneration() || !isEntrypointFile(path) { - f, _ := filepath.Rel(args.Dir, path) + srcPath, _ := filepath.Rel(args.Dir, path) + repoPath := filepath.Join(args.Rel, srcPath) excludedPatterns := cfg.ExcludedPatterns() if excludedPatterns != nil { it := excludedPatterns.Iterator() for it.Next() { excludedPattern := it.Value().(string) - isExcluded, err := doublestar.Match(excludedPattern, f) + isExcluded, err := doublestar.Match(excludedPattern, repoPath) if err != nil { return err } @@ -178,9 +179,9 @@ func (py *Python) GenerateRules(args language.GenerateArgs) language.GenerateRes } baseName := filepath.Base(path) if strings.HasSuffix(baseName, "_test.py") || strings.HasPrefix(baseName, "test_") { - pyTestFilenames.Add(f) + pyTestFilenames.Add(srcPath) } else { - pyLibraryFilenames.Add(f) + pyLibraryFilenames.Add(srcPath) } } } diff --git a/gazelle/python/python_test.go b/gazelle/python/python_test.go index e8edf89275..51e0101df1 100644 --- a/gazelle/python/python_test.go +++ b/gazelle/python/python_test.go @@ -23,7 +23,6 @@ import ( "bytes" "context" "errors" - "fmt" "os" "os/exec" "path/filepath" @@ -33,7 +32,6 @@ import ( "github.com/bazelbuild/bazel-gazelle/testtools" "github.com/bazelbuild/rules_go/go/tools/bazel" - "github.com/emirpasic/gods/lists/singlylinkedlist" "github.com/ghodss/yaml" ) @@ -161,31 +159,23 @@ func testPath(t *testing.T, name string, files []bazel.RunfileEntry) { t.Fatal(err) } } - errs := singlylinkedlist.New() + actualExitCode := cmd.ProcessState.ExitCode() if config.Expect.ExitCode != actualExitCode { - errs.Add(fmt.Errorf("expected gazelle exit code: %d\ngot: %d", - config.Expect.ExitCode, actualExitCode, - )) + t.Errorf("expected gazelle exit code: %d\ngot: %d", + config.Expect.ExitCode, actualExitCode) } actualStdout := stdout.String() if strings.TrimSpace(config.Expect.Stdout) != strings.TrimSpace(actualStdout) { - errs.Add(fmt.Errorf("expected gazelle stdout: %s\ngot: %s", - config.Expect.Stdout, actualStdout, - )) + t.Errorf("expected gazelle stdout: %s\ngot: %s", + config.Expect.Stdout, actualStdout) } actualStderr := stderr.String() if strings.TrimSpace(config.Expect.Stderr) != strings.TrimSpace(actualStderr) { - errs.Add(fmt.Errorf("expected gazelle stderr: %s\ngot: %s", - config.Expect.Stderr, actualStderr, - )) + t.Errorf("expected gazelle stderr: %s\ngot: %s", + config.Expect.Stderr, actualStderr) } - if !errs.Empty() { - errsIt := errs.Iterator() - for errsIt.Next() { - err := errsIt.Value().(error) - t.Log(err) - } + if t.Failed() { t.FailNow() } diff --git a/gazelle/python/testdata/monorepo/a/BUILD.in b/gazelle/python/testdata/monorepo/a/BUILD.in new file mode 100644 index 0000000000..265129ea56 --- /dev/null +++ b/gazelle/python/testdata/monorepo/a/BUILD.in @@ -0,0 +1 @@ +# gazelle:exclude bar/baz/hue.py \ No newline at end of file diff --git a/gazelle/python/testdata/monorepo/a/BUILD.out b/gazelle/python/testdata/monorepo/a/BUILD.out new file mode 100644 index 0000000000..265129ea56 --- /dev/null +++ b/gazelle/python/testdata/monorepo/a/BUILD.out @@ -0,0 +1 @@ +# gazelle:exclude bar/baz/hue.py \ No newline at end of file diff --git a/gazelle/python/testdata/monorepo/a/README.md b/gazelle/python/testdata/monorepo/a/README.md new file mode 100644 index 0000000000..84d3bff052 --- /dev/null +++ b/gazelle/python/testdata/monorepo/a/README.md @@ -0,0 +1,3 @@ +# Exclusions +* Intentionally make the directory "a" so Gazelle visit this before "coarse_grained" +* Making sure that the exclusion here doesn't affect coarse_grained/bar/baz/hue.py \ No newline at end of file From 6bcee35fd72af6c7374318ab24aa71367e5100be Mon Sep 17 00:00:00 2001 From: Alex Eagle Date: Thu, 9 Feb 2023 00:49:40 -0600 Subject: [PATCH 02/21] docs: fix requirement line for runfiles example (#1052) The PyPI name is "bazel-runfiles", which is what should be used in `requirement()`; "runfiles" is the import name. --- python/runfiles/README.rst | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/python/runfiles/README.rst b/python/runfiles/README.rst index 59a3852c5f..ac61d2dd80 100644 --- a/python/runfiles/README.rst +++ b/python/runfiles/README.rst @@ -3,10 +3,13 @@ bazel-runfiles library This is a Bazel Runfiles lookup library for Bazel-built Python binaries and tests. +Learn about runfiles: read `Runfiles guide `_ +or watch `Fabian's BazelCon talk `_. + Typical Usage ------------- -1. Add the 'runfiles' dependency along with other third-party dependencies, for example in your +1. Add the 'bazel-runfiles' dependency along with other third-party dependencies, for example in your ``requirements.txt`` file. 2. Depend on this runfiles library from your build rule, like you would other third-party libraries:: @@ -14,7 +17,7 @@ Typical Usage py_binary( name = "my_binary", ... - deps = [requirement("runfiles")], + deps = [requirement("bazel-runfiles")], ) 3. Import the runfiles library:: From 6905e63ee9e63e210db7d1a2b65ebbae12d691d8 Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius Date: Sat, 11 Feb 2023 14:02:33 +0900 Subject: [PATCH 03/21] fix: make py_proto_library respect PyInfo imports (#1046) py_proto_library has an implicitly dependency on the protobuf client runtime, and it was ending up in runfiles, but it wasn't imported because the `imports` value it was setting wasn't be propagated. To fix, make py_proto_library properly propagate the imports attribute the protobuf client runtime so that the libraries are added to sys.path correct. Also adds an example for bzlmod and old way of using the py_proto_library as a test. --------- Co-authored-by: Richard Levasseur --- .bazelci/presubmit.yml | 52 +++++++++++++++++++ .bazelrc | 4 +- examples/BUILD.bazel | 12 +++++ examples/py_proto_library/.bazelrc | 0 examples/py_proto_library/.gitignore | 4 ++ examples/py_proto_library/BUILD.bazel | 22 ++++++++ examples/py_proto_library/MODULE.bazel | 28 ++++++++++ examples/py_proto_library/WORKSPACE | 48 +++++++++++++++++ examples/py_proto_library/WORKSPACE.bzlmod | 0 examples/py_proto_library/pricetag.proto | 8 +++ examples/py_proto_library/test.py | 17 ++++++ python/private/proto/py_proto_library.bzl | 7 +++ .../bazel_integration_test.bzl | 5 +- 13 files changed, 203 insertions(+), 4 deletions(-) create mode 100644 examples/py_proto_library/.bazelrc create mode 100644 examples/py_proto_library/.gitignore create mode 100644 examples/py_proto_library/BUILD.bazel create mode 100644 examples/py_proto_library/MODULE.bazel create mode 100644 examples/py_proto_library/WORKSPACE create mode 100644 examples/py_proto_library/WORKSPACE.bzlmod create mode 100644 examples/py_proto_library/pricetag.proto create mode 100644 examples/py_proto_library/test.py diff --git a/.bazelci/presubmit.yml b/.bazelci/presubmit.yml index 76f9d8b5aa..706c655f73 100644 --- a/.bazelci/presubmit.yml +++ b/.bazelci/presubmit.yml @@ -31,6 +31,11 @@ buildifier: - "..." test_flags: - "--test_tag_filters=-integration-test" +.common_bzlmod_flags: &common_bzlmod_flags + test_flags: + - "--experimental_enable_bzlmod" + build_flags: + - "--experimental_enable_bzlmod" .reusable_build_test_all: &reusable_build_test_all build_targets: ["..."] test_targets: ["..."] @@ -211,6 +216,53 @@ tasks: # We don't run pip_parse_vendored under Windows as the file checked in is # generated from a repository rule containing OS-specific rendered paths. + integration_test_py_proto_library_ubuntu: + <<: *reusable_build_test_all + name: py_proto_library integration tests on Ubuntu + working_directory: examples/py_proto_library + platform: ubuntu2004 + integration_test_py_proto_library_debian: + <<: *reusable_build_test_all + name: py_proto_library integration tests on Debian + working_directory: examples/py_proto_library + platform: debian11 + integration_test_py_proto_library_macos: + <<: *reusable_build_test_all + name: py_proto_library integration tests on macOS + working_directory: examples/py_proto_library + platform: macos + integration_test_py_proto_library_windows: + <<: *reusable_build_test_all + name: py_proto_library integration tests on Windows + working_directory: examples/py_proto_library + platform: windows + + # Check the same using bzlmod as well + integration_test_py_proto_library_bzlmod_ubuntu: + <<: *reusable_build_test_all + <<: *common_bzlmod_flags + name: py_proto_library bzlmod integration tests on Ubuntu + working_directory: examples/py_proto_library + platform: ubuntu2004 + integration_test_py_proto_library_bzlmod_debian: + <<: *reusable_build_test_all + <<: *common_bzlmod_flags + name: py_proto_library bzlmod integration tests on Debian + working_directory: examples/py_proto_library + platform: debian11 + integration_test_py_proto_library_bzlmod_macos: + <<: *reusable_build_test_all + <<: *common_bzlmod_flags + name: py_proto_library bzlmod integration tests on macOS + working_directory: examples/py_proto_library + platform: macos + integration_test_py_proto_library_bzlmod_windows: + <<: *reusable_build_test_all + <<: *common_bzlmod_flags + name: py_proto_library bzlmod integration tests on Windows + working_directory: examples/py_proto_library + platform: windows + integration_test_pip_repository_annotations_ubuntu: <<: *reusable_build_test_all name: pip_repository_annotations integration tests on Ubuntu diff --git a/.bazelrc b/.bazelrc index 2dc32594d4..d607cdd9b7 100644 --- a/.bazelrc +++ b/.bazelrc @@ -3,8 +3,8 @@ # This lets us glob() up all the files inside the examples to make them inputs to tests # (Note, we cannot use `common --deleted_packages` because the bazel version command doesn't support it) # To update these lines, run tools/bazel_integration_test/update_deleted_packages.sh -build --deleted_packages=examples/build_file_generation,examples/build_file_generation/get_url,examples/bzlmod,examples/bzlmod/other_module/other_module/pkg,examples/bzlmod/runfiles,examples/multi_python_versions,examples/multi_python_versions/libs/my_lib,examples/multi_python_versions/requirements,examples/multi_python_versions/tests,examples/pip_install,examples/pip_parse,examples/pip_parse_vendored,examples/pip_repository_annotations,examples/py_import,examples/relative_requirements,tests/compile_pip_requirements,tests/pip_repository_entry_points,tests/pip_deps -query --deleted_packages=examples/build_file_generation,examples/build_file_generation/get_url,examples/bzlmod,examples/bzlmod/other_module/other_module/pkg,examples/bzlmod/runfiles,examples/multi_python_versions,examples/multi_python_versions/libs/my_lib,examples/multi_python_versions/requirements,examples/multi_python_versions/tests,examples/pip_install,examples/pip_parse,examples/pip_parse_vendored,examples/pip_repository_annotations,examples/py_import,examples/relative_requirements,tests/compile_pip_requirements,tests/pip_repository_entry_points,tests/pip_deps +build --deleted_packages=examples/build_file_generation,examples/build_file_generation/get_url,examples/bzlmod,examples/bzlmod/other_module/other_module/pkg,examples/bzlmod/runfiles,examples/multi_python_versions,examples/multi_python_versions/libs/my_lib,examples/multi_python_versions/requirements,examples/multi_python_versions/tests,examples/pip_install,examples/pip_parse,examples/pip_parse_vendored,examples/pip_repository_annotations,examples/py_import,examples/py_proto_library,examples/relative_requirements,tests/compile_pip_requirements,tests/pip_repository_entry_points,tests/pip_deps +query --deleted_packages=examples/build_file_generation,examples/build_file_generation/get_url,examples/bzlmod,examples/bzlmod/other_module/other_module/pkg,examples/bzlmod/runfiles,examples/multi_python_versions,examples/multi_python_versions/libs/my_lib,examples/multi_python_versions/requirements,examples/multi_python_versions/tests,examples/pip_install,examples/pip_parse,examples/pip_parse_vendored,examples/pip_repository_annotations,examples/py_import,examples/py_proto_library,examples/relative_requirements,tests/compile_pip_requirements,tests/pip_repository_entry_points,tests/pip_deps test --test_output=errors diff --git a/examples/BUILD.bazel b/examples/BUILD.bazel index e0a7e5a72d..3ef89054c9 100644 --- a/examples/BUILD.bazel +++ b/examples/BUILD.bazel @@ -43,6 +43,18 @@ bazel_integration_test( timeout = "long", ) +bazel_integration_test( + name = "py_proto_library_example", + timeout = "long", +) + +bazel_integration_test( + name = "py_proto_library_example_bzlmod", + timeout = "long", + bzlmod = True, + dirname = "py_proto_library", +) + bazel_integration_test( name = "multi_python_versions_example", timeout = "long", diff --git a/examples/py_proto_library/.bazelrc b/examples/py_proto_library/.bazelrc new file mode 100644 index 0000000000..e69de29bb2 diff --git a/examples/py_proto_library/.gitignore b/examples/py_proto_library/.gitignore new file mode 100644 index 0000000000..e5ae073b3c --- /dev/null +++ b/examples/py_proto_library/.gitignore @@ -0,0 +1,4 @@ +# git ignore patterns + +/bazel-* +user.bazelrc diff --git a/examples/py_proto_library/BUILD.bazel b/examples/py_proto_library/BUILD.bazel new file mode 100644 index 0000000000..7a18a5e4e1 --- /dev/null +++ b/examples/py_proto_library/BUILD.bazel @@ -0,0 +1,22 @@ +load("@rules_proto//proto:defs.bzl", "proto_library") +load("@rules_python//python:defs.bzl", "py_test") +load("@rules_python//python:proto.bzl", "py_proto_library") + +py_proto_library( + name = "pricetag_proto_py_pb2", + deps = [":pricetag_proto"], +) + +proto_library( + name = "pricetag_proto", + srcs = ["pricetag.proto"], +) + +py_test( + name = "pricetag_test", + srcs = ["test.py"], + main = "test.py", + deps = [ + ":pricetag_proto_py_pb2", + ], +) diff --git a/examples/py_proto_library/MODULE.bazel b/examples/py_proto_library/MODULE.bazel new file mode 100644 index 0000000000..5ce0924a99 --- /dev/null +++ b/examples/py_proto_library/MODULE.bazel @@ -0,0 +1,28 @@ +module( + name = "rules_python_py_proto_library_example", + version = "0.0.0", + compatibility_level = 1, +) + +bazel_dep(name = "rules_python", version = "0.17.3") + +# The following local_path_override is only needed to run this example as part of our CI. +local_path_override( + module_name = "rules_python", + path = "../..", +) + +python = use_extension("@rules_python//python:extensions.bzl", "python") +python.toolchain( + name = "python3_9", + configure_coverage_tool = True, + python_version = "3.9", +) +use_repo(python, "python3_9_toolchains") + +register_toolchains( + "@python3_9_toolchains//:all", +) + +# We are using rules_proto to define rules_proto targets to be consumed by py_proto_library. +bazel_dep(name = "rules_proto", version = "5.3.0-21.7") diff --git a/examples/py_proto_library/WORKSPACE b/examples/py_proto_library/WORKSPACE new file mode 100644 index 0000000000..bf38112f98 --- /dev/null +++ b/examples/py_proto_library/WORKSPACE @@ -0,0 +1,48 @@ +workspace(name = "rules_python_py_proto_library_example") + +# The following local_path_override is only needed to run this example as part of our CI. +local_repository( + name = "rules_python", + path = "../..", +) + +# When not using this example in the rules_python git repo you would load the python +# rules using http_archive(), as documented in the release notes. + +load("@rules_python//python:repositories.bzl", "py_repositories", "python_register_toolchains") + +# We install the rules_python dependencies using the function below. +py_repositories() + +python_register_toolchains( + name = "python39", + python_version = "3.9", +) + +# Then we need to setup dependencies in order to use py_proto_library +load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive") + +http_archive( + name = "rules_proto", + sha256 = "dc3fb206a2cb3441b485eb1e423165b231235a1ea9b031b4433cf7bc1fa460dd", + strip_prefix = "rules_proto-5.3.0-21.7", + urls = [ + "https://github.com/bazelbuild/rules_proto/archive/refs/tags/5.3.0-21.7.tar.gz", + ], +) + +http_archive( + name = "com_google_protobuf", + sha256 = "75be42bd736f4df6d702a0e4e4d30de9ee40eac024c4b845d17ae4cc831fe4ae", + strip_prefix = "protobuf-21.7", + urls = [ + "https://mirror.bazel.build/github.com/protocolbuffers/protobuf/archive/v21.7.tar.gz", + "https://github.com/protocolbuffers/protobuf/archive/v21.7.tar.gz", + ], +) + +load("@rules_proto//proto:repositories.bzl", "rules_proto_dependencies", "rules_proto_toolchains") + +rules_proto_dependencies() + +rules_proto_toolchains() diff --git a/examples/py_proto_library/WORKSPACE.bzlmod b/examples/py_proto_library/WORKSPACE.bzlmod new file mode 100644 index 0000000000..e69de29bb2 diff --git a/examples/py_proto_library/pricetag.proto b/examples/py_proto_library/pricetag.proto new file mode 100644 index 0000000000..c952248846 --- /dev/null +++ b/examples/py_proto_library/pricetag.proto @@ -0,0 +1,8 @@ +syntax = "proto3"; + +package rules_python; + +message PriceTag { + string name = 2; + double cost = 1; +} diff --git a/examples/py_proto_library/test.py b/examples/py_proto_library/test.py new file mode 100644 index 0000000000..9f09702f8c --- /dev/null +++ b/examples/py_proto_library/test.py @@ -0,0 +1,17 @@ +import sys +import unittest + +import pricetag_pb2 + + +class TestCase(unittest.TestCase): + def test_pricetag(self): + got = pricetag_pb2.PriceTag( + name="dollar", + cost=5.00, + ) + self.assertIsNotNone(got) + + +if __name__ == "__main__": + unittest.main() diff --git a/python/private/proto/py_proto_library.bzl b/python/private/proto/py_proto_library.bzl index ef5f2cae70..988558500d 100644 --- a/python/private/proto/py_proto_library.bzl +++ b/python/private/proto/py_proto_library.bzl @@ -22,6 +22,9 @@ ProtoLangToolchainInfo = proto_common.ProtoLangToolchainInfo _PyProtoInfo = provider( doc = "Encapsulates information needed by the Python proto rules.", fields = { + "imports": """ + (depset[str]) The field forwarding PyInfo.imports coming from + the proto language runtime dependency.""", "runfiles_from_proto_deps": """ (depset[File]) Files from the transitive closure implicit proto dependencies""", @@ -95,6 +98,9 @@ def _py_proto_aspect_impl(target, ctx): return [ _PyProtoInfo( + imports = depset( + transitive = [dep[PyInfo].imports for dep in api_deps], + ), runfiles_from_proto_deps = runfiles_from_proto_deps, transitive_sources = transitive_sources, ), @@ -142,6 +148,7 @@ def _py_proto_library_rule(ctx): ), PyInfo( transitive_sources = default_outputs, + imports = depset(transitive = [info.imports for info in pyproto_infos]), # Proto always produces 2- and 3- compatible source files has_py2_only_sources = False, has_py3_only_sources = False, diff --git a/tools/bazel_integration_test/bazel_integration_test.bzl b/tools/bazel_integration_test/bazel_integration_test.bzl index 66e0cbded1..c016551319 100644 --- a/tools/bazel_integration_test/bazel_integration_test.bzl +++ b/tools/bazel_integration_test/bazel_integration_test.bzl @@ -84,18 +84,19 @@ _config = rule( attrs = _ATTRS, ) -def bazel_integration_test(name, override_bazel_version = None, bzlmod = False, **kwargs): +def bazel_integration_test(name, override_bazel_version = None, bzlmod = False, dirname = None, **kwargs): """Wrapper macro to set default srcs and run a py_test with config Args: name: name of the resulting py_test override_bazel_version: bazel version to use in test bzlmod: whether the test uses bzlmod + dirname: the directory name of the test. Defaults to value of `name` after trimming the `_example` suffix. **kwargs: additional attributes like timeout and visibility """ # By default, we assume sources for "pip_example" are in examples/pip/**/* - dirname = name[:-len("_example")] + dirname = dirname or name[:-len("_example")] native.filegroup( name = "_%s_sources" % name, srcs = native.glob( From fab77f7781c21e799bedc4a7e2eccb8fd0191916 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Sat, 11 Feb 2023 08:13:34 -0800 Subject: [PATCH 04/21] Make toolchain acceptance tests work with latest Bazel build CI pipeline (#1062) --- .../toolchains/run_acceptance_test.py.tmpl | 20 +++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/python/tests/toolchains/run_acceptance_test.py.tmpl b/python/tests/toolchains/run_acceptance_test.py.tmpl index b3071a7b3c..150e1a99df 100644 --- a/python/tests/toolchains/run_acceptance_test.py.tmpl +++ b/python/tests/toolchains/run_acceptance_test.py.tmpl @@ -23,9 +23,8 @@ class TestPythonVersion(unittest.TestCase): os.chdir("%test_location%") rules_python_path = os.path.join(os.environ["TEST_SRCDIR"], "rules_python") + test_tmpdir = os.environ["TEST_TMPDIR"] if %is_windows%: - test_tmpdir = os.environ["TEST_TMPDIR"] - home = os.path.join(test_tmpdir, "HOME") os.mkdir(home) os.environ["HOME"] = home @@ -34,6 +33,16 @@ class TestPythonVersion(unittest.TestCase): os.mkdir(local_app_data) os.environ["LocalAppData"] = local_app_data + # Bazelisk requires a cache directory be set + os.environ["XDG_CACHE_HOME"] = os.path.join(test_tmpdir, "xdg-cache-home") + + # Unset this so this works when called by Bazel's latest Bazel build + # pipeline. It sets the following combination, which interfere with each other: + # * --sandbox_tmpfs_path=/tmp + # * --test_env=USE_BAZEL_VERSION + # * USE_BAZEL_VERSION=/tmp/ + os.environ.pop("USE_BAZEL_VERSION", None) + with open(".bazelrc", "w") as bazelrc: bazelrc.write( os.linesep.join( @@ -47,8 +56,11 @@ class TestPythonVersion(unittest.TestCase): ) def test_match_toolchain(self): - stream = os.popen("bazel run @python//:python3 -- --version") - output = stream.read().strip() + output = subprocess.check_output( + f"bazel run @python//:python3 -- --version", + shell = True, # Shell needed to look up via PATH + text=True, + ).strip() self.assertEqual(output, "Python %python_version%") subprocess.run("bazel test //...", shell=True, check=True) From 4f8ca60cce4ca74b56b5b713bc445d87c844a673 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Sat, 11 Feb 2023 09:13:18 -0800 Subject: [PATCH 05/21] Only set `py_runtime.coverage_tool` for Bazel 6 and higher. (#1061) Only set `py_runtime.coverage_tool` for Bazel 6 and higher. Avoid setting it in earlier version by checking `native.bazel_version` in the repository rule and disabling it if less than Bazel 6 is detected. A warning is also printed if coverage was requested, but the Bazel version check is ignoring it. Fixes #1056 --- python/repositories.bzl | 37 ++++++++++++++++++++++++++++++++----- 1 file changed, 32 insertions(+), 5 deletions(-) diff --git a/python/repositories.bzl b/python/repositories.bzl index e61b057d22..df72497d91 100644 --- a/python/repositories.bzl +++ b/python/repositories.bzl @@ -243,6 +243,21 @@ def _python_repository_impl(rctx): "share/**", ] + if rctx.attr.coverage_tool: + if "windows" in rctx.os.name: + coverage_tool = None + else: + coverage_tool = '"{}"'.format(rctx.attr.coverage_tool) + + coverage_attr_text = """\ + coverage_tool = select({{ + ":coverage_enabled": {coverage_tool}, + "//conditions:default": None + }}), +""".format(coverage_tool = coverage_tool) + else: + coverage_attr_text = " # coverage_tool attribute not supported by this Bazel version" + build_content = """\ # Generated by python/repositories.bzl @@ -308,10 +323,7 @@ config_setting( py_runtime( name = "py3_runtime", files = [":files"], - coverage_tool = select({{ - ":coverage_enabled": {coverage_tool}, - "//conditions:default": None, - }}), +{coverage_attr} interpreter = "{python_path}", python_version = "PY3", ) @@ -327,7 +339,7 @@ py_runtime_pair( python_path = python_bin, python_version = python_short_version, python_version_nodot = python_short_version.replace(".", ""), - coverage_tool = rctx.attr.coverage_tool if rctx.attr.coverage_tool == None or "windows" in rctx.os.name else "\"{}\"".format(rctx.attr.coverage_tool), + coverage_attr = coverage_attr_text, ) rctx.delete("python") rctx.symlink(python_bin, "python") @@ -459,6 +471,8 @@ def python_register_toolchains( distutils_content: see the distutils_content attribute in the python_repository repository rule. register_toolchains: Whether or not to register the downloaded toolchains. register_coverage_tool: Whether or not to register the downloaded coverage tool to the toolchains. + NOTE: Coverage support using the toolchain is only supported in Bazel 6 and higher. + set_python_version_constraint: When set to true, target_compatible_with for the toolchains will include a version constraint. tool_versions: a dict containing a mapping of version with SHASUM and platform info. If not supplied, the defaults in python/versions.bzl will be used. @@ -472,6 +486,19 @@ def python_register_toolchains( toolchain_repo_name = "{name}_toolchains".format(name = name) + bazel_major = int(native.bazel_version.split(".")[0]) + if bazel_major < 6: + if register_coverage_tool: + # buildifier: disable=print + print(( + "WARNING: ignoring register_coverage_tool=True when " + + "registering @{name}: Bazel 6+ required, got {version}" + ).format( + name = name, + version = native.bazel_version, + )) + register_coverage_tool = False + for platform in PLATFORMS.keys(): sha256 = tool_versions[python_version]["sha256"].get(platform, None) if not sha256: From aef1abfacde34bcff22b4b6481998866a346917b Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Sat, 11 Feb 2023 11:24:07 -0800 Subject: [PATCH 06/21] Allow building with unreleased Bazel versions. (#1063) --- python/repositories.bzl | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/python/repositories.bzl b/python/repositories.bzl index df72497d91..f676610ae2 100644 --- a/python/repositories.bzl +++ b/python/repositories.bzl @@ -486,18 +486,20 @@ def python_register_toolchains( toolchain_repo_name = "{name}_toolchains".format(name = name) - bazel_major = int(native.bazel_version.split(".")[0]) - if bazel_major < 6: - if register_coverage_tool: - # buildifier: disable=print - print(( - "WARNING: ignoring register_coverage_tool=True when " + - "registering @{name}: Bazel 6+ required, got {version}" - ).format( - name = name, - version = native.bazel_version, - )) - register_coverage_tool = False + # When using unreleased Bazel versions, the version is an empty string + if native.bazel_version: + bazel_major = int(native.bazel_version.split(".")[0]) + if bazel_major < 6: + if register_coverage_tool: + # buildifier: disable=print + print(( + "WARNING: ignoring register_coverage_tool=True when " + + "registering @{name}: Bazel 6+ required, got {version}" + ).format( + name = name, + version = native.bazel_version, + )) + register_coverage_tool = False for platform in PLATFORMS.keys(): sha256 = tool_versions[python_version]["sha256"].get(platform, None) From 2f29f1243ca6dd65688fc2053cf6efc46a3e2c01 Mon Sep 17 00:00:00 2001 From: Zhongpeng Lin Date: Mon, 13 Feb 2023 12:35:50 -0800 Subject: [PATCH 07/21] Extending server process timeout (#1060) --- gazelle/python/parser.go | 2 +- gazelle/python/std_modules.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/gazelle/python/parser.go b/gazelle/python/parser.go index 3809a461cd..33eb6f4b33 100644 --- a/gazelle/python/parser.go +++ b/gazelle/python/parser.go @@ -46,7 +46,7 @@ func init() { } ctx := context.Background() - ctx, parserCancel := context.WithTimeout(ctx, time.Minute*5) + ctx, parserCancel := context.WithTimeout(ctx, time.Minute*10) cmd := exec.CommandContext(ctx, parseScriptRunfile) cmd.Stderr = os.Stderr diff --git a/gazelle/python/std_modules.go b/gazelle/python/std_modules.go index 17bc5263ae..94ef45666e 100644 --- a/gazelle/python/std_modules.go +++ b/gazelle/python/std_modules.go @@ -47,7 +47,7 @@ func init() { } ctx := context.Background() - ctx, stdModulesCancel := context.WithTimeout(ctx, time.Minute*5) + ctx, stdModulesCancel := context.WithTimeout(ctx, time.Minute*10) cmd := exec.CommandContext(ctx, stdModulesScriptRunfile) cmd.Stderr = os.Stderr From e35cd882386fad30e29354e738ce547ab2600d73 Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius Date: Tue, 14 Feb 2023 05:36:32 +0900 Subject: [PATCH 08/21] chore: regenerate gazelle_python.yaml manifest (#1066) --- examples/build_file_generation/gazelle_python.yaml | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/examples/build_file_generation/gazelle_python.yaml b/examples/build_file_generation/gazelle_python.yaml index 9953ad3936..847d1ecc55 100644 --- a/examples/build_file_generation/gazelle_python.yaml +++ b/examples/build_file_generation/gazelle_python.yaml @@ -1,17 +1,3 @@ -# Copyright 2023 The Bazel Authors. All rights reserved. -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - # GENERATED FILE - DO NOT EDIT! # # To update this file, run: From 00513936230ddb0d3a77728584c82ce9dde2ae89 Mon Sep 17 00:00:00 2001 From: Alex Eagle Date: Mon, 13 Feb 2023 14:17:05 -0800 Subject: [PATCH 09/21] feat: wheel publishing (#1015) feat: add a .publish target to py_wheel macro --- docs/packaging.md | 28 +++++++++++++++++++- python/packaging.bzl | 51 +++++++++++++++++++++++++++++++++++-- python/runfiles/BUILD.bazel | 21 ++------------- 3 files changed, 78 insertions(+), 22 deletions(-) diff --git a/docs/packaging.md b/docs/packaging.md index a7a65ab7f8..b244b42767 100755 --- a/docs/packaging.md +++ b/docs/packaging.md @@ -121,7 +121,7 @@ Information about a wheel produced by `py_wheel` ## py_wheel 
-py_wheel(name, kwargs)
+py_wheel(name, twine, kwargs)
Builds a Python Wheel. @@ -168,6 +168,31 @@ py_wheel( ) ``` +To publish the wheel to Pypi, the twine package is required. +rules_python doesn't provide twine itself, see https://github.com/bazelbuild/rules_python/issues/1016 +However you can install it with pip_parse, just like we do in the WORKSPACE file in rules_python. + +Once you've installed twine, you can pass its label to the `twine` attribute of this macro, +to get a "[name].publish" target. + +Example: + +```python +py_wheel( + name = "my_wheel", + twine = "@publish_deps_twine//:pkg", + ... +) +``` + +Now you can run a command like the following, which publishes to https://test.pypi.org/ + +```sh +% TWINE_USERNAME=__token__ TWINE_PASSWORD=pypi-*** \ + bazel run --stamp --embed_label=1.2.4 -- \ + //path/to:my_wheel.publish --repository testpypi +``` + **PARAMETERS** @@ -175,6 +200,7 @@ py_wheel( | Name | Description | Default Value | | :------------- | :------------- | :------------- | | name | A unique name for this target. | none | +| twine | A label of the external location of the py_library target for twine | None | | kwargs | other named parameters passed to the underlying [py_wheel rule](#py_wheel_rule) | none | diff --git a/python/packaging.bzl b/python/packaging.bzl index 92745792a5..1984eb7b4b 100644 --- a/python/packaging.bzl +++ b/python/packaging.bzl @@ -68,7 +68,7 @@ This also has the advantage that stamping information is included in the wheel's }, ) -def py_wheel(name, **kwargs): +def py_wheel(name, twine = None, **kwargs): """Builds a Python Wheel. Wheels are Python distribution format defined in https://www.python.org/dev/peps/pep-0427/. @@ -113,16 +113,63 @@ def py_wheel(name, **kwargs): ) ``` + To publish the wheel to Pypi, the twine package is required. + rules_python doesn't provide twine itself, see https://github.com/bazelbuild/rules_python/issues/1016 + However you can install it with pip_parse, just like we do in the WORKSPACE file in rules_python. + + Once you've installed twine, you can pass its label to the `twine` attribute of this macro, + to get a "[name].publish" target. + + Example: + + ```python + py_wheel( + name = "my_wheel", + twine = "@publish_deps_twine//:pkg", + ... + ) + ``` + + Now you can run a command like the following, which publishes to https://test.pypi.org/ + + ```sh + % TWINE_USERNAME=__token__ TWINE_PASSWORD=pypi-*** \\ + bazel run --stamp --embed_label=1.2.4 -- \\ + //path/to:my_wheel.publish --repository testpypi + ``` + Args: name: A unique name for this target. + twine: A label of the external location of the py_library target for twine **kwargs: other named parameters passed to the underlying [py_wheel rule](#py_wheel_rule) """ + _dist_target = "{}.dist".format(name) py_wheel_dist( - name = "{}.dist".format(name), + name = _dist_target, wheel = name, out = kwargs.pop("dist_folder", "{}_dist".format(name)), ) _py_wheel(name = name, **kwargs) + if twine: + if not twine.endswith(":pkg"): + fail("twine label should look like @my_twine_repo//:pkg") + twine_main = twine.replace(":pkg", ":rules_python_wheel_entry_point_twine.py") + + # TODO: use py_binary from //python:defs.bzl after our stardoc setup is less brittle + # buildifier: disable=native-py + native.py_binary( + name = "{}.publish".format(name), + srcs = [twine_main], + args = [ + "upload", + "$(rootpath :{})/*".format(_dist_target), + ], + data = [_dist_target], + imports = ["."], + main = twine_main, + deps = [twine], + ) + py_wheel_rule = _py_wheel diff --git a/python/runfiles/BUILD.bazel b/python/runfiles/BUILD.bazel index 19d7804c06..3a93d40f32 100644 --- a/python/runfiles/BUILD.bazel +++ b/python/runfiles/BUILD.bazel @@ -12,7 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. -load("//python:defs.bzl", "py_binary", "py_library") +load("//python:defs.bzl", "py_library") load("//python:packaging.bzl", "py_wheel") filegroup( @@ -45,26 +45,9 @@ py_wheel( distribution = "bazel_runfiles", homepage = "https://github.com/bazelbuild/rules_python", strip_path_prefixes = ["python"], + twine = "@publish_deps_twine//:pkg", # this can be replaced by building with --stamp --embed_label=1.2.3 version = "{BUILD_EMBED_LABEL}", visibility = ["//visibility:public"], deps = [":runfiles"], ) - -# TODO(alexeagle): carry forward #1015 to make this part of the py_wheel macro -# Typical command-line to run this: -# TWINE_USERNAME=__token__ TWINE_PASSWORD=pypi-*** \ -# bazel run --stamp --embed_label=1.2.4 -- \ -# //python/runfiles:wheel.publish --repository testpypi -py_binary( - name = "wheel.publish", - srcs = ["@publish_deps_twine//:rules_python_wheel_entry_point_twine.py"], - args = [ - "upload", - "$(rootpath :wheel.dist)/*", - ], - data = [":wheel.dist"], - imports = ["."], - main = "@publish_deps_twine//:rules_python_wheel_entry_point_twine.py", - deps = ["@publish_deps_twine//:pkg"], -) From 767b050e45c49e63cfe103e6b97f7e2fd9be5e2e Mon Sep 17 00:00:00 2001 From: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com> Date: Mon, 13 Feb 2023 14:58:53 -0800 Subject: [PATCH 10/21] fix: checked-in requirements imports generated requirements (#1053) * fix: checked-in requirements imports generated requirements Signed-off-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com> * fix: shutil.copy instead of shutil.copyfile This allows copying from one filesystem to another, as the `os.rename` (used by copyfile) doesn't work with multiple filesystems. Signed-off-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com> * fix: patch os.replace to use shutil.copy Same as the previous commit. Signed-off-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com> * fix: runfiles.Rlocation requires paths to be normalized Signed-off-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com> * fix: drop rules_python from import This is not compatible with bzlmod. Importing python.runfiles works for both ways. Signed-off-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com> * fix: remove unnecessary runfiles Signed-off-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com> * doc: why os.replace = shutil.copy Signed-off-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com> * fix: allow the test to still be remote cacheable Signed-off-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com> * doc: why shutil.copy Signed-off-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com> * doc: add missing punctuation Signed-off-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com> * fix: remove unnecessary _fix_up_requirements_in_path Signed-off-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com> * test: make sure the locked requirements is updated Signed-off-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com> * fix: copy requirements back into src tree if needed Signed-off-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com> * fix: make sure windows uses forward slashes Signed-off-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com> --------- Signed-off-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com> --- .bazelci/presubmit.yml | 28 ++++++ python/pip_install/requirements.bzl | 2 + .../dependency_resolver.py | 89 +++++++++---------- tests/compile_pip_requirements/BUILD.bazel | 3 +- .../compile_pip_requirements/requirements.txt | 1 + tools/publish/BUILD.bazel | 8 -- 6 files changed, 76 insertions(+), 55 deletions(-) create mode 100644 tests/compile_pip_requirements/requirements.txt diff --git a/.bazelci/presubmit.yml b/.bazelci/presubmit.yml index 706c655f73..5f92fbff04 100644 --- a/.bazelci/presubmit.yml +++ b/.bazelci/presubmit.yml @@ -289,21 +289,49 @@ tasks: name: compile_pip_requirements integration tests on Ubuntu working_directory: tests/compile_pip_requirements platform: ubuntu2004 + shell_commands: + # Make a change to the locked requirements and then assert that //:requirements.update does the + # right thing. + - "echo '' > requirements_lock.txt" + - "! git diff --exit-code" + - "bazel run //:requirements.update" + - "git diff --exit-code" integration_test_compile_pip_requirements_debian: <<: *reusable_build_test_all name: compile_pip_requirements integration tests on Debian working_directory: tests/compile_pip_requirements platform: debian11 + shell_commands: + # Make a change to the locked requirements and then assert that //:requirements.update does the + # right thing. + - "echo '' > requirements_lock.txt" + - "! git diff --exit-code" + - "bazel run //:requirements.update" + - "git diff --exit-code" integration_test_compile_pip_requirements_macos: <<: *reusable_build_test_all name: compile_pip_requirements integration tests on macOS working_directory: tests/compile_pip_requirements platform: macos + shell_commands: + # Make a change to the locked requirements and then assert that //:requirements.update does the + # right thing. + - "echo '' > requirements_lock.txt" + - "! git diff --exit-code" + - "bazel run //:requirements.update" + - "git diff --exit-code" integration_test_compile_pip_requirements_windows: <<: *reusable_build_test_all name: compile_pip_requirements integration tests on Windows working_directory: tests/compile_pip_requirements platform: windows + shell_commands: + # Make a change to the locked requirements and then assert that //:requirements.update does the + # right thing. + - "echo '' > requirements_lock.txt" + - "! git diff --exit-code" + - "bazel run //:requirements.update" + - "git diff --exit-code" integration_test_pip_repository_entry_points_ubuntu: <<: *reusable_build_test_all diff --git a/python/pip_install/requirements.bzl b/python/pip_install/requirements.bzl index 51e34a2246..af3c194d18 100644 --- a/python/pip_install/requirements.bzl +++ b/python/pip_install/requirements.bzl @@ -103,6 +103,8 @@ def compile_pip_requirements( tags = tags or [] tags.append("requires-network") + tags.append("no-remote-exec") + tags.append("no-sandbox") attrs = { "args": args, "data": data, diff --git a/python/pip_install/tools/dependency_resolver/dependency_resolver.py b/python/pip_install/tools/dependency_resolver/dependency_resolver.py index db84977a0d..e636febd93 100644 --- a/python/pip_install/tools/dependency_resolver/dependency_resolver.py +++ b/python/pip_install/tools/dependency_resolver/dependency_resolver.py @@ -14,14 +14,39 @@ "Set defaults for the pip-compile command to run it under Bazel" +import atexit import os -import re +import shutil import sys from pathlib import Path -from shutil import copyfile +import piptools.writer as piptools_writer from piptools.scripts.compile import cli +# Replace the os.replace function with shutil.copy to work around os.replace not being able to +# replace or move files across filesystems. +os.replace = shutil.copy + +# Next, we override the annotation_style_split and annotation_style_line functions to replace the +# backslashes in the paths with forward slashes. This is so that we can have the same requirements +# file on Windows and Unix-like. +original_annotation_style_split = piptools_writer.annotation_style_split +original_annotation_style_line = piptools_writer.annotation_style_line + + +def annotation_style_split(required_by) -> str: + required_by = set([v.replace("\\", "/") for v in required_by]) + return original_annotation_style_split(required_by) + + +def annotation_style_line(required_by) -> str: + required_by = set([v.replace("\\", "/") for v in required_by]) + return original_annotation_style_line(required_by) + + +piptools_writer.annotation_style_split = annotation_style_split +piptools_writer.annotation_style_line = annotation_style_line + def _select_golden_requirements_file( requirements_txt, requirements_linux, requirements_darwin, requirements_windows @@ -41,19 +66,6 @@ def _select_golden_requirements_file( return requirements_txt -def _fix_up_requirements_in_path(absolute_prefix, output_file): - """Fix up references to the input file inside of the generated requirements file. - - We don't want fully resolved, absolute paths in the generated requirements file. - The paths could differ for every invocation. Replace them with a predictable path. - """ - output_file = Path(output_file) - contents = output_file.read_text() - contents = contents.replace(absolute_prefix, "") - contents = re.sub(r"\\(?!(\n|\r\n))", "/", contents) - output_file.write_text(contents) - - if __name__ == "__main__": if len(sys.argv) < 4: print( @@ -75,7 +87,6 @@ def _fix_up_requirements_in_path(absolute_prefix, output_file): # absolute prefixes in the locked requirements output file. requirements_in_path = Path(requirements_in) resolved_requirements_in = str(requirements_in_path.resolve()) - absolute_prefix = resolved_requirements_in[: -len(str(requirements_in_path))] # Before loading click, set the locale for its parser. # If it leaks through to the system setting, it may fail: @@ -86,7 +97,7 @@ def _fix_up_requirements_in_path(absolute_prefix, output_file): os.environ["LANG"] = "C.UTF-8" UPDATE = True - # Detect if we are running under `bazel test` + # Detect if we are running under `bazel test`. if "TEST_TMPDIR" in os.environ: UPDATE = False # pip-compile wants the cache files to be writeable, but if we point @@ -95,31 +106,13 @@ def _fix_up_requirements_in_path(absolute_prefix, output_file): # In theory this makes the test more hermetic as well. sys.argv.append("--cache-dir") sys.argv.append(os.environ["TEST_TMPDIR"]) - # Make a copy for pip-compile to read and mutate + # Make a copy for pip-compile to read and mutate. requirements_out = os.path.join( os.environ["TEST_TMPDIR"], os.path.basename(requirements_txt) + ".out" ) - copyfile(requirements_txt, requirements_out) - - elif "BUILD_WORKSPACE_DIRECTORY" in os.environ: - # This value, populated when running under `bazel run`, is a path to the - # "root of the workspace where the build was run." - # This matches up with the values passed in via the macro using the 'rootpath' Make variable, - # which for source files provides a path "relative to your workspace root." - # - # Changing to the WORKSPACE root avoids 'file not found' errors when the `.update` target is run - # from different directories within the WORKSPACE. - os.chdir(os.environ["BUILD_WORKSPACE_DIRECTORY"]) - else: - err_msg = ( - "Expected to find BUILD_WORKSPACE_DIRECTORY (running under `bazel run`) or " - "TEST_TMPDIR (running under `bazel test`) in environment." - ) - print( - err_msg, - file=sys.stderr, - ) - sys.exit(1) + # Those two files won't necessarily be on the same filesystem, so we can't use os.replace + # or shutil.copyfile, as they will fail with OSError: [Errno 18] Invalid cross-device link. + shutil.copy(requirements_txt, requirements_out) update_command = os.getenv("CUSTOM_COMPILE_COMMAND") or "bazel run %s" % ( update_target_label, @@ -137,12 +130,17 @@ def _fix_up_requirements_in_path(absolute_prefix, output_file): if UPDATE: print("Updating " + requirements_txt) - try: - cli() - except SystemExit as e: - if e.code == 0: - _fix_up_requirements_in_path(absolute_prefix, requirements_txt) - raise + if "BUILD_WORKSPACE_DIRECTORY" in os.environ: + workspace = os.environ["BUILD_WORKSPACE_DIRECTORY"] + requirements_txt_tree = os.path.join(workspace, requirements_txt) + # In most cases, requirements_txt will be a symlink to the real file in the source tree. + # If symlinks are not enabled (e.g. on Windows), then requirements_txt will be a copy, + # and we should copy the updated requirements back to the source tree. + if not os.path.samefile(requirements_txt, requirements_txt_tree): + atexit.register( + lambda: shutil.copy(requirements_txt, requirements_txt_tree) + ) + cli() else: # cli will exit(0) on success try: @@ -160,7 +158,6 @@ def _fix_up_requirements_in_path(absolute_prefix, output_file): ) sys.exit(1) elif e.code == 0: - _fix_up_requirements_in_path(absolute_prefix, requirements_out) golden_filename = _select_golden_requirements_file( requirements_txt, requirements_linux, diff --git a/tests/compile_pip_requirements/BUILD.bazel b/tests/compile_pip_requirements/BUILD.bazel index 3a67dcca47..d6ac0086ab 100644 --- a/tests/compile_pip_requirements/BUILD.bazel +++ b/tests/compile_pip_requirements/BUILD.bazel @@ -22,12 +22,13 @@ EOF compile_pip_requirements( name = "requirements", data = [ + "requirements.in", "requirements_extra.in", ], extra_args = [ "--allow-unsafe", "--resolver=backtracking", ], - requirements_in = "requirements.in", + requirements_in = "requirements.txt", requirements_txt = "requirements_lock.txt", ) diff --git a/tests/compile_pip_requirements/requirements.txt b/tests/compile_pip_requirements/requirements.txt new file mode 100644 index 0000000000..4826399f01 --- /dev/null +++ b/tests/compile_pip_requirements/requirements.txt @@ -0,0 +1 @@ +-r requirements.in diff --git a/tools/publish/BUILD.bazel b/tools/publish/BUILD.bazel index 8c4b3ab4a2..065e56bd69 100644 --- a/tools/publish/BUILD.bazel +++ b/tools/publish/BUILD.bazel @@ -4,12 +4,4 @@ compile_pip_requirements( name = "requirements", requirements_darwin = "requirements_darwin.txt", requirements_windows = "requirements_windows.txt", - # This fails on RBE right now, and we don't need coverage there: - # WARNING: Retrying (Retry(total=0, connect=None, read=None, redirect=None, status=None)) - # after connection broken by 'NewConnectionError(': - # Failed to establish a new connection: [Errno -3] Temporary failure in name resolution')': /simple/twine/ - # - # ERROR: Could not find a version that satisfies the requirement twine==4.0.2 - # (from -r tools/publish/requirements.in (line 1)) (from versions: none) - tags = ["no-remote-exec"], ) From 2893d858262fd1dd4fd5d011b7f4a6d50ba5d88d Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Wed, 15 Feb 2023 20:08:22 -0800 Subject: [PATCH 11/21] fix: Propagate testonly et al for wheel `.dist` targets (#1064) * Propagate testonly et al for wheel `.dist` targets The `.dist` target depends on the wheel, so it must copy the `testonly` setting as well as some others. * Also adds a utility function to do this, since the multi-version rules also do this copying. Fixes #1057 * fixup! Allow building with unreleased Bazel versions. (#1063) --- docs/BUILD.bazel | 1 + examples/wheel/BUILD.bazel | 7 +++++++ python/packaging.bzl | 2 ++ python/private/BUILD.bazel | 1 + python/private/util.bzl | 31 +++++++++++++++++++++++++++++++ 5 files changed, 42 insertions(+) create mode 100644 python/private/util.bzl diff --git a/docs/BUILD.bazel b/docs/BUILD.bazel index 3abff033ae..2afd0ad7c2 100644 --- a/docs/BUILD.bazel +++ b/docs/BUILD.bazel @@ -84,6 +84,7 @@ bzl_library( "//python/private:py_package.bzl", "//python/private:py_wheel.bzl", "//python/private:stamp.bzl", + "//python/private:util.bzl", ], ) diff --git a/examples/wheel/BUILD.bazel b/examples/wheel/BUILD.bazel index c3dec29c01..4124a826d1 100644 --- a/examples/wheel/BUILD.bazel +++ b/examples/wheel/BUILD.bazel @@ -12,6 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. +load("@bazel_skylib//rules:build_test.bzl", "build_test") load("//examples/wheel/private:wheel_utils.bzl", "directory_writer") load("//python:defs.bzl", "py_library", "py_test") load("//python:packaging.bzl", "py_package", "py_wheel") @@ -50,6 +51,7 @@ directory_writer( # Package just a specific py_libraries, without their dependencies py_wheel( name = "minimal_with_py_library", + testonly = True, # Set this to verify the generated .dist target doesn't break things # Package data. We're building "example_minimal_library-0.0.1-py3-none-any.whl" distribution = "example_minimal_library", python_tag = "py3", @@ -60,6 +62,11 @@ py_wheel( ], ) +build_test( + name = "dist_build_tests", + targets = [":minimal_with_py_library.dist"], +) + # Package just a specific py_libraries, without their dependencies py_wheel( name = "minimal_with_py_library_with_stamp", diff --git a/python/packaging.bzl b/python/packaging.bzl index 1984eb7b4b..fffd239c15 100644 --- a/python/packaging.bzl +++ b/python/packaging.bzl @@ -16,6 +16,7 @@ load("//python/private:py_package.bzl", "py_package_lib") load("//python/private:py_wheel.bzl", _PyWheelInfo = "PyWheelInfo", _py_wheel = "py_wheel") +load("//python/private:util.bzl", "copy_propagating_kwargs") # Re-export as public API PyWheelInfo = _PyWheelInfo @@ -148,6 +149,7 @@ def py_wheel(name, twine = None, **kwargs): name = _dist_target, wheel = name, out = kwargs.pop("dist_folder", "{}_dist".format(name)), + **copy_propagating_kwargs(kwargs) ) _py_wheel(name = name, **kwargs) diff --git a/python/private/BUILD.bazel b/python/private/BUILD.bazel index f2e1c9b8bc..d56d31b27a 100644 --- a/python/private/BUILD.bazel +++ b/python/private/BUILD.bazel @@ -41,6 +41,7 @@ exports_files( "py_wheel.bzl", "reexports.bzl", "stamp.bzl", + "util.bzl", ], visibility = ["//docs:__pkg__"], ) diff --git a/python/private/util.bzl b/python/private/util.bzl new file mode 100644 index 0000000000..8ea1f493f5 --- /dev/null +++ b/python/private/util.bzl @@ -0,0 +1,31 @@ +"""Functionality shared by multiple pieces of code.""" + +def copy_propagating_kwargs(from_kwargs, into_kwargs = None): + """Copies args that must be compatible between two targets with a dependency relationship. + + This is intended for when one target depends on another, so they must have + compatible settings such as `testonly` and `compatible_with`. This usually + happens when a macro generates multiple targets, some of which depend + on one another, so their settings must be compatible. + + Args: + from_kwargs: keyword args dict whose common kwarg will be copied. + into_kwargs: optional keyword args dict that the values from `from_kwargs` + will be copied into. The values in this dict will take precedence + over the ones in `from_kwargs` (i.e., if this has `testonly` already + set, then it won't be overwritten). + NOTE: THIS WILL BE MODIFIED IN-PLACE. + + Returns: + Keyword args to use for the depender target derived from the dependency + target. If `into_kwargs` was passed in, then that same object is + returned; this is to facilitate easy `**` expansion. + """ + if into_kwargs == None: + into_kwargs = {} + + # Include tags because people generally expect tags to propagate. + for attr in ("testonly", "tags", "compatible_with", "restricted_to"): + if attr in from_kwargs and attr not in into_kwargs: + into_kwargs[attr] = from_kwargs[attr] + return into_kwargs From 64d9d6f59f645ae2c7870623e82ac8b1a9534e44 Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius Date: Thu, 16 Feb 2023 13:11:23 +0900 Subject: [PATCH 12/21] fix: correctly advertise minimum supported version (#1065) * feat: bump the latest supported version to 5.4.0 * feat: add gazelle and RBE minimum supported version tests * feat: pip_parse_vendored example is now 5.4.0 compatible --- .bazelci/presubmit.yml | 100 +++++++++++++++++++ BUILD.bazel | 1 + docs/BUILD.bazel | 1 + examples/pip_parse_vendored/BUILD.bazel | 3 + examples/pip_parse_vendored/requirements.bzl | 2 +- python/pip_install/repositories.bzl | 3 +- version.bzl | 8 +- 7 files changed, 115 insertions(+), 3 deletions(-) diff --git a/.bazelci/presubmit.yml b/.bazelci/presubmit.yml index 5f92fbff04..a1b16bbc66 100644 --- a/.bazelci/presubmit.yml +++ b/.bazelci/presubmit.yml @@ -17,6 +17,12 @@ buildifier: version: latest # keep this argument in sync with .pre-commit-config.yaml warnings: "all" +.minimum_supported_version: &minimum_supported_version + # For testing minimum supported version. + # NOTE: Keep in sync with //:version.bzl + bazel: 5.4.0 +.minimum_supported_bzlmod_version: &minimum_supported_bzlmod_version + bazel: 6.0.0 # test minimum supported version of bazel for bzlmod tests .reusable_config: &reusable_config build_targets: - "--" @@ -54,12 +60,24 @@ buildifier: - //tests:version_3_9_test - //tests:version_default_test tasks: + gazelle_extension_min: + <<: *minimum_supported_version + name: Test the Gazelle extension using minimum supported Bazel version + platform: ubuntu2004 + build_targets: ["//..."] + test_targets: ["//..."] + working_directory: gazelle gazelle_extension: name: Test the Gazelle extension platform: ubuntu2004 build_targets: ["//..."] test_targets: ["//..."] working_directory: gazelle + ubuntu_min: + <<: *minimum_supported_version + <<: *reusable_config + name: Default test on Ubuntu using minimum supported Bazel version + platform: ubuntu2004 ubuntu: <<: *reusable_config name: Default test on Ubuntu @@ -78,6 +96,14 @@ tasks: platform: windows test_flags: - "--test_tag_filters=-integration-test,-fix-windows" + + rbe_min: + <<: *minimum_supported_version + <<: *reusable_config + name: Test on RBE using minimum supported Bazel version + platform: rbe_ubuntu1604 + test_flags: + - "--test_tag_filters=-integration-test,-acceptance-test" rbe: <<: *reusable_config name: Test on RBE @@ -85,6 +111,12 @@ tasks: test_flags: - "--test_tag_filters=-integration-test,-acceptance-test" + integration_test_build_file_generation_ubuntu_minimum_supported: + <<: *minimum_supported_version + <<: *reusable_build_test_all + name: build_file_generation integration tests on Ubuntu using minimum supported Bazel version + working_directory: examples/build_file_generation + platform: ubuntu2004 integration_test_build_file_generation_ubuntu: <<: *reusable_build_test_all name: build_file_generation integration tests on Ubuntu @@ -106,6 +138,13 @@ tasks: working_directory: examples/build_file_generation platform: windows + integration_test_bzlmod_ubuntu_min: + <<: *minimum_supported_bzlmod_version + <<: *reusable_build_test_all + <<: *coverage_targets_example_bzlmod + name: bzlmod integration tests on Ubuntu using minimum supported Bazel version + working_directory: examples/bzlmod + platform: ubuntu2004 integration_test_bzlmod_ubuntu: <<: *reusable_build_test_all <<: *coverage_targets_example_bzlmod @@ -131,6 +170,12 @@ tasks: working_directory: examples/bzlmod platform: windows + integration_test_multi_python_versions_ubuntu_min: + <<: *minimum_supported_version + <<: *reusable_build_test_all + name: multi_python_versions integration tests on Ubuntu using minimum supported Bazel version + working_directory: examples/multi_python_versions + platform: ubuntu2004 integration_test_multi_python_versions_ubuntu: <<: *reusable_build_test_all <<: *coverage_targets_example_multi_python @@ -156,6 +201,12 @@ tasks: working_directory: examples/multi_python_versions platform: windows + integration_test_pip_install_ubuntu_min: + <<: *minimum_supported_version + <<: *reusable_build_test_all + name: pip_install integration tests on Ubuntu using minimum supported Bazel version + working_directory: examples/pip_install + platform: ubuntu2004 integration_test_pip_install_ubuntu: <<: *reusable_build_test_all name: pip_install integration tests on Ubuntu @@ -177,6 +228,12 @@ tasks: working_directory: examples/pip_install platform: windows + integration_test_pip_parse_ubuntu_min: + <<: *minimum_supported_version + <<: *reusable_build_test_all + name: pip_parse integration tests on Ubuntu using minimum supported Bazel version + working_directory: examples/pip_parse + platform: ubuntu2004 integration_test_pip_parse_ubuntu: <<: *reusable_build_test_all name: pip_parse integration tests on Ubuntu @@ -198,6 +255,12 @@ tasks: working_directory: examples/pip_parse platform: windows + integration_test_pip_parse_vendored_ubuntu_min: + <<: *minimum_supported_version + <<: *reusable_build_test_all + name: pip_parse_vendored integration tests on Ubuntu using minimum supported Bazel version + working_directory: examples/pip_parse_vendored + platform: ubuntu2004 integration_test_pip_parse_vendored_ubuntu: <<: *reusable_build_test_all name: pip_parse_vendored integration tests on Ubuntu @@ -216,6 +279,12 @@ tasks: # We don't run pip_parse_vendored under Windows as the file checked in is # generated from a repository rule containing OS-specific rendered paths. + integration_test_py_proto_library_ubuntu_min: + <<: *minimum_supported_version + <<: *reusable_build_test_all + name: py_proto_library integration tests on Ubuntu using minimum supported Bazel version + working_directory: examples/py_proto_library + platform: ubuntu2004 integration_test_py_proto_library_ubuntu: <<: *reusable_build_test_all name: py_proto_library integration tests on Ubuntu @@ -238,6 +307,13 @@ tasks: platform: windows # Check the same using bzlmod as well + integration_test_py_proto_library_bzlmod_ubuntu_min: + <<: *minimum_supported_bzlmod_version + <<: *common_bzlmod_flags + <<: *reusable_build_test_all + name: py_proto_library bzlmod integration tests on Ubuntu using minimum supported Bazel version + working_directory: examples/py_proto_library + platform: ubuntu2004 integration_test_py_proto_library_bzlmod_ubuntu: <<: *reusable_build_test_all <<: *common_bzlmod_flags @@ -263,6 +339,12 @@ tasks: working_directory: examples/py_proto_library platform: windows + integration_test_pip_repository_annotations_ubuntu_min: + <<: *minimum_supported_version + <<: *reusable_build_test_all + name: pip_repository_annotations integration tests on Ubuntu using minimum supported Bazel version + working_directory: examples/pip_repository_annotations + platform: ubuntu2004 integration_test_pip_repository_annotations_ubuntu: <<: *reusable_build_test_all name: pip_repository_annotations integration tests on Ubuntu @@ -284,6 +366,12 @@ tasks: working_directory: examples/pip_repository_annotations platform: windows + integration_test_compile_pip_requirements_ubuntu_min: + <<: *minimum_supported_version + <<: *reusable_build_test_all + name: compile_pip_requirements integration tests on Ubuntu using minimum supported Bazel version + working_directory: tests/compile_pip_requirements + platform: ubuntu2004 integration_test_compile_pip_requirements_ubuntu: <<: *reusable_build_test_all name: compile_pip_requirements integration tests on Ubuntu @@ -333,6 +421,12 @@ tasks: - "bazel run //:requirements.update" - "git diff --exit-code" + integration_test_pip_repository_entry_points_ubuntu_min: + <<: *minimum_supported_version + <<: *reusable_build_test_all + name: pip_repository_entry_points integration tests on Ubuntu using minimum supported Bazel version + working_directory: tests/pip_repository_entry_points + platform: ubuntu2004 integration_test_pip_repository_entry_points_ubuntu: <<: *reusable_build_test_all name: pip_repository_entry_points integration tests on Ubuntu @@ -354,6 +448,12 @@ tasks: working_directory: tests/pip_repository_entry_points platform: windows + integration_test_ignore_root_user_error_ubuntu_min: + <<: *minimum_supported_version + <<: *reusable_build_test_all + name: ignore_root_user_error integration tests on Ubuntu using minimum supported Bazel version + working_directory: tests/ignore_root_user_error + platform: ubuntu2004 integration_test_ignore_root_user_error_ubuntu: <<: *reusable_build_test_all name: ignore_root_user_error integration tests on Ubuntu diff --git a/BUILD.bazel b/BUILD.bazel index fc95328a89..5b37fce29a 100644 --- a/BUILD.bazel +++ b/BUILD.bazel @@ -31,6 +31,7 @@ filegroup( "WORKSPACE", "internal_deps.bzl", "internal_setup.bzl", + "version.bzl", "//python:distribution", "//python/pip_install:distribution", "//tools:distribution", diff --git a/docs/BUILD.bazel b/docs/BUILD.bazel index 2afd0ad7c2..d2f0b04b56 100644 --- a/docs/BUILD.bazel +++ b/docs/BUILD.bazel @@ -67,6 +67,7 @@ bzl_library( ], deps = [ ":defs", + "//:version.bzl", ], ) diff --git a/examples/pip_parse_vendored/BUILD.bazel b/examples/pip_parse_vendored/BUILD.bazel index b5a85295e3..9585195f1b 100644 --- a/examples/pip_parse_vendored/BUILD.bazel +++ b/examples/pip_parse_vendored/BUILD.bazel @@ -17,6 +17,9 @@ genrule( "cat $<", # Insert our load statement after the existing one so we don't produce a file with buildifier warnings """sed -e '/^load.*/i\\'$$'\\n''load("@python39//:defs.bzl", "interpreter")'""", + # Replace the bazel 6.0.0 specific comment with something that bazel 5.4.0 would produce. + # This enables this example to be run as a test under bazel 5.4.0. + """sed -e 's#@//#//#'""", """tr "'" '"' """, """sed 's#"@python39_.*//:bin/python3"#interpreter#' >$@""", ]), diff --git a/examples/pip_parse_vendored/requirements.bzl b/examples/pip_parse_vendored/requirements.bzl index cc24aa63ca..e13503ac6c 100644 --- a/examples/pip_parse_vendored/requirements.bzl +++ b/examples/pip_parse_vendored/requirements.bzl @@ -1,7 +1,7 @@ """Starlark representation of locked requirements. @generated by rules_python pip_parse repository rule -from @//:requirements.txt +from //:requirements.txt """ load("@python39//:defs.bzl", "interpreter") diff --git a/python/pip_install/repositories.bzl b/python/pip_install/repositories.bzl index e5567c8c58..664556da12 100644 --- a/python/pip_install/repositories.bzl +++ b/python/pip_install/repositories.bzl @@ -17,6 +17,7 @@ load("@bazel_skylib//lib:versions.bzl", "versions") load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive") load("@bazel_tools//tools/build_defs/repo:utils.bzl", "maybe") +load("//:version.bzl", "MINIMUM_BAZEL_VERSION") _RULE_DEPS = [ ( @@ -133,7 +134,7 @@ def pip_install_dependencies(): # Give the user an obvious error to upgrade rather than some obscure missing symbol later. # It's not guaranteed that users call this function, but it's used by all the pip fetch # repository rules so it's likely that most users get the right error. - versions.check("4.0.0") + versions.check(MINIMUM_BAZEL_VERSION) for (name, url, sha256) in _RULE_DEPS: maybe( diff --git a/version.bzl b/version.bzl index 91125c21bf..8c7f01cd19 100644 --- a/version.bzl +++ b/version.bzl @@ -19,14 +19,20 @@ # in .bazelversion. BAZEL_VERSION = "6.0.0" +# NOTE: Keep in sync with .bazelci/presubmit.yml +# This is the minimum supported bazel version, that we have some tests for. +MINIMUM_BAZEL_VERSION = "5.4.0" + # Versions of Bazel which users should be able to use. # Ensures we don't break backwards-compatibility, # accidentally forcing users to update their LTS-supported bazel. # These are the versions used when testing nested workspaces with # bazel_integration_test. SUPPORTED_BAZEL_VERSIONS = [ - # TODO: add LTS versions of bazel like 1.0.0, 2.0.0 BAZEL_VERSION, + # TODO @aignas 2023-02-15: the integration tests currently support + # only a single element in this array. + #MINIMUM_BAZEL_VERSION, ] def bazel_version_to_binary_label(version): From 2ab842d36ce2b13fec3a12dd7cc7158c33e8f8be Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius Date: Thu, 16 Feb 2023 14:04:54 +0900 Subject: [PATCH 13/21] refactor: starlark reimplementation of pip_repository (#1043) Previously we were using pip_parse python scripts. This has a few drawbacks: * Requires system python to be present. * Usage of a Python script makes it harder to reason as there is an extra layer of abstraction. * Extending/reusing code between multi_pip_parse and pip_parse is hard. Now we use Starlark to parse the requirements.txt into requirements.bzl. --- docs/pip.md | 3 +- docs/pip_repository.md | 29 +- examples/pip_parse_vendored/BUILD.bazel | 1 - examples/pip_parse_vendored/requirements.bzl | 11 +- python/extensions.bzl | 14 +- python/pip.bzl | 11 +- python/pip_install/BUILD.bazel | 2 - python/pip_install/pip_repository.bzl | 214 +++++++---- .../pip_repository_requirements.bzl.tmpl | 52 +++ ...ip_repository_requirements_bzlmod.bzl.tmpl | 24 ++ python/pip_install/private/srcs.bzl | 2 - .../tools/lock_file_generator/BUILD.bazel | 50 --- .../tools/lock_file_generator/__init__.py | 14 - .../lock_file_generator.py | 336 ------------------ .../lock_file_generator_test.py | 163 --------- 15 files changed, 257 insertions(+), 669 deletions(-) create mode 100644 python/pip_install/pip_repository_requirements.bzl.tmpl create mode 100644 python/pip_install/pip_repository_requirements_bzlmod.bzl.tmpl delete mode 100644 python/pip_install/tools/lock_file_generator/BUILD.bazel delete mode 100644 python/pip_install/tools/lock_file_generator/__init__.py delete mode 100644 python/pip_install/tools/lock_file_generator/lock_file_generator.py delete mode 100644 python/pip_install/tools/lock_file_generator/lock_file_generator_test.py diff --git a/docs/pip.md b/docs/pip.md index 2f5b92ebf1..528abf737d 100644 --- a/docs/pip.md +++ b/docs/pip.md @@ -168,7 +168,7 @@ install_deps() ## pip_parse 
-pip_parse(requirements, requirements_lock, name, bzlmod, kwargs)
+pip_parse(requirements, requirements_lock, name, kwargs)
Accepts a locked/compiled requirements file and installs the dependencies listed within. @@ -264,7 +264,6 @@ See the example in rules_python/examples/pip_parse_vendored. | requirements | Deprecated. See requirements_lock. | None | | requirements_lock | A fully resolved 'requirements.txt' pip requirement file containing the transitive set of your dependencies. If this file is passed instead of 'requirements' no resolve will take place and pip_repository will create individual repositories for each of your dependencies so that wheels are fetched/built only for the targets specified by 'build/run/test'. Note that if your lockfile is platform-dependent, you can use the requirements_[platform] attributes. | None | | name | The name of the generated repository. The generated repositories containing each requirement will be of the form <name>_<requirement-name>. | "pip_parsed_deps" | -| bzlmod | Whether this rule is being run under a bzlmod module extension. | False | | kwargs | Additional arguments to the [pip_repository](./pip_repository.md) repository rule. | none | diff --git a/docs/pip_repository.md b/docs/pip_repository.md index 7abb503c78..2ccdc64854 100644 --- a/docs/pip_repository.md +++ b/docs/pip_repository.md @@ -7,8 +7,8 @@ ## pip_repository 
-pip_repository(name, annotations, bzlmod, download_only, enable_implicit_namespace_pkgs,
-               environment, extra_pip_args, isolated, pip_data_exclude, python_interpreter,
+pip_repository(name, annotations, download_only, enable_implicit_namespace_pkgs, environment,
+               extra_pip_args, isolated, pip_data_exclude, python_interpreter,
                python_interpreter_target, quiet, repo_mapping, repo_prefix, requirements_darwin,
                requirements_linux, requirements_lock, requirements_windows, timeout)
@@ -60,7 +60,6 @@ py_binary( | :------------- | :------------- | :------------- | :------------- | :------------- | | name | A unique name for this repository. | Name | required | | | annotations | Optional annotations to apply to packages | Dictionary: String -> String | optional | {} | -| bzlmod | Whether this repository rule is invoked under bzlmod, in which case we do not create the install_deps() macro. | Boolean | optional | False | | download_only | Whether to use "pip download" instead of "pip wheel". Disables building wheels from source, but allows use of --platform, --python-version, --implementation, and --abi in --extra_pip_args to download wheels for a different platform from the host platform. | Boolean | optional | False | | enable_implicit_namespace_pkgs | If true, disables conversion of native namespace packages into pkg-util style namespace packages. When set all py_binary and py_test targets must specify either legacy_create_init=False or the global Bazel option --incompatible_default_to_explicit_init_py to prevent __init__.py being automatically generated in every directory.

This option is required to support some packages which cannot handle the conversion to pkg-util style. | Boolean | optional | False | | environment | Environment variables to set in the pip subprocess. Can be used to set common variables such as http_proxy, https_proxy and no_proxy Note that pip is run with "--isolated" on the CLI so PIP_<VAR>_<NAME> style env vars are ignored, but env vars that control requests and urllib3 can be passed. | Dictionary: String -> String | optional | {} | @@ -79,6 +78,30 @@ py_binary( | timeout | Timeout (in seconds) on the rule's execution duration. | Integer | optional | 600 | + + +## pip_repository_bzlmod + +
+pip_repository_bzlmod(name, repo_mapping, requirements_darwin, requirements_linux,
+                      requirements_lock, requirements_windows)
+
+ +A rule for bzlmod pip_repository creation. Intended for private use only. + +**ATTRIBUTES** + + +| Name | Description | Type | Mandatory | Default | +| :------------- | :------------- | :------------- | :------------- | :------------- | +| name | A unique name for this repository. | Name | required | | +| repo_mapping | A dictionary from local repository name to global repository name. This allows controls over workspace dependency resolution for dependencies of this repository.<p>For example, an entry "@foo": "@bar" declares that, for any time this repository depends on @foo (such as a dependency on @foo//some:target, it should actually resolve that dependency within globally-declared @bar (@bar//some:target). | Dictionary: String -> String | required | | +| requirements_darwin | Override the requirements_lock attribute when the host platform is Mac OS | Label | optional | None | +| requirements_linux | Override the requirements_lock attribute when the host platform is Linux | Label | optional | None | +| requirements_lock | A fully resolved 'requirements.txt' pip requirement file containing the transitive set of your dependencies. If this file is passed instead of 'requirements' no resolve will take place and pip_repository will create individual repositories for each of your dependencies so that wheels are fetched/built only for the targets specified by 'build/run/test'. | Label | optional | None | +| requirements_windows | Override the requirements_lock attribute when the host platform is Windows | Label | optional | None | + + ## whl_library diff --git a/examples/pip_parse_vendored/BUILD.bazel b/examples/pip_parse_vendored/BUILD.bazel index 9585195f1b..56630e513d 100644 --- a/examples/pip_parse_vendored/BUILD.bazel +++ b/examples/pip_parse_vendored/BUILD.bazel @@ -20,7 +20,6 @@ genrule( # Replace the bazel 6.0.0 specific comment with something that bazel 5.4.0 would produce. # This enables this example to be run as a test under bazel 5.4.0. """sed -e 's#@//#//#'""", - """tr "'" '"' """, """sed 's#"@python39_.*//:bin/python3"#interpreter#' >$@""", ]), ) diff --git a/examples/pip_parse_vendored/requirements.bzl b/examples/pip_parse_vendored/requirements.bzl index e13503ac6c..015df9340a 100644 --- a/examples/pip_parse_vendored/requirements.bzl +++ b/examples/pip_parse_vendored/requirements.bzl @@ -14,29 +14,20 @@ all_whl_requirements = ["@pip_certifi//:whl", "@pip_charset_normalizer//:whl", " _packages = [("pip_certifi", "certifi==2022.12.7 --hash=sha256:35824b4c3a97115964b408844d64aa14db1cc518f6562e8d7261699d1350a9e3 --hash=sha256:4ad3232f5e926d6718ec31cfc1fcadfde020920e278684144551c91769c7bc18"), ("pip_charset_normalizer", "charset-normalizer==2.1.1 --hash=sha256:5a3d016c7c547f69d6f81fb0db9449ce888b418b5b9952cc5e6e66843e9dd845 --hash=sha256:83e9a75d1911279afd89352c68b45348559d1fc0506b054b346651b5e7fee29f"), ("pip_idna", "idna==3.4 --hash=sha256:814f528e8dead7d329833b91c5faa87d60bf71824cd12a7530b5526063d02cb4 --hash=sha256:90b77e79eaa3eba6de819a0c442c0b4ceefc341a7a2ab77d7562bf49f425c5c2"), ("pip_requests", "requests==2.28.1 --hash=sha256:7c5599b102feddaa661c826c56ab4fee28bfd17f5abca1ebbe3e7f19d7c97983 --hash=sha256:8fefa2a1a1365bf5520aac41836fbee479da67864514bdb821f31ce07ce65349"), ("pip_urllib3", "urllib3==1.26.13 --hash=sha256:47cc05d99aaa09c9e72ed5809b60e7ba354e64b59c9c173ac3018642d8bb41fc --hash=sha256:c083dd0dce68dbfbe1129d5271cb90f9447dea7d52097c6e0126120c521ddea8")] _config = {"download_only": False, "enable_implicit_namespace_pkgs": False, "environment": {}, "extra_pip_args": [], "isolated": True, "pip_data_exclude": [], "python_interpreter": "python3", "python_interpreter_target": interpreter, "quiet": True, "repo": "pip", "repo_prefix": "pip_", "timeout": 600} _annotations = {} -_bzlmod = False def _clean_name(name): return name.replace("-", "_").replace(".", "_").lower() def requirement(name): - if _bzlmod: - return "@@pip//:" + _clean_name(name) + "_pkg" return "@pip_" + _clean_name(name) + "//:pkg" def whl_requirement(name): - if _bzlmod: - return "@@pip//:" + _clean_name(name) + "_whl" return "@pip_" + _clean_name(name) + "//:whl" def data_requirement(name): - if _bzlmod: - return "@@pip//:" + _clean_name(name) + "_data" return "@pip_" + _clean_name(name) + "//:data" def dist_info_requirement(name): - if _bzlmod: - return "@@pip//:" + _clean_name(name) + "_dist_info" return "@pip_" + _clean_name(name) + "//:dist_info" def entry_point(pkg, script = None): @@ -46,7 +37,7 @@ def entry_point(pkg, script = None): def _get_annotation(requirement): # This expects to parse `setuptools==58.2.0 --hash=sha256:2551203ae6955b9876741a26ab3e767bb3242dafe86a32a749ea0d78b6792f11` - # down wo `setuptools`. + # down to `setuptools`. name = requirement.split(" ")[0].split("=")[0].split("[")[0] return _annotations.get(name) diff --git a/python/extensions.bzl b/python/extensions.bzl index bc0d570c52..01f731f14f 100644 --- a/python/extensions.bzl +++ b/python/extensions.bzl @@ -14,9 +14,8 @@ "Module extensions for use with bzlmod" -load("@rules_python//python:pip.bzl", "pip_parse") load("@rules_python//python:repositories.bzl", "python_register_toolchains") -load("@rules_python//python/pip_install:pip_repository.bzl", "locked_requirements_label", "pip_repository_attrs", "use_isolated", "whl_library") +load("@rules_python//python/pip_install:pip_repository.bzl", "locked_requirements_label", "pip_repository_attrs", "pip_repository_bzlmod", "use_isolated", "whl_library") load("@rules_python//python/pip_install:repositories.bzl", "pip_install_dependencies") load("@rules_python//python/pip_install:requirements_parser.bzl", parse_requirements = "parse") load("@rules_python//python/private:coverage_deps.bzl", "install_coverage_deps") @@ -68,7 +67,7 @@ def _pip_impl(module_ctx): # Parse the requirements file directly in starlark to get the information # needed for the whl_libary declarations below. This is needed to contain - # the pip_parse logic to a single module extension. + # the pip_repository logic to a single module extension. requirements_lock_content = module_ctx.read(requrements_lock) parse_result = parse_requirements(requirements_lock_content) requirements = parse_result.requirements @@ -76,14 +75,9 @@ def _pip_impl(module_ctx): # Create the repository where users load the `requirement` macro. Under bzlmod # this does not create the install_deps() macro. - pip_parse( + pip_repository_bzlmod( name = attr.name, requirements_lock = attr.requirements_lock, - bzlmod = True, - timeout = attr.timeout, - python_interpreter = attr.python_interpreter, - python_interpreter_target = attr.python_interpreter_target, - quiet = attr.quiet, ) for name, requirement_line in requirements: @@ -114,7 +108,7 @@ def _pip_parse_ext_attrs(): "name": attr.string(mandatory = True), }, **pip_repository_attrs) - # Like the pip_parse macro, we end up setting this manually so + # Like the pip_repository rule, we end up setting this manually so # don't allow users to override it. attrs.pop("repo_prefix") diff --git a/python/pip.bzl b/python/pip.bzl index 3d45aed61e..3c06301306 100644 --- a/python/pip.bzl +++ b/python/pip.bzl @@ -47,7 +47,7 @@ def pip_install(requirements = None, name = "pip", **kwargs): print("pip_install is deprecated. Please switch to pip_parse. pip_install will be removed in a future release.") pip_parse(requirements = requirements, name = name, **kwargs) -def pip_parse(requirements = None, requirements_lock = None, name = "pip_parsed_deps", bzlmod = False, **kwargs): +def pip_parse(requirements = None, requirements_lock = None, name = "pip_parsed_deps", **kwargs): """Accepts a locked/compiled requirements file and installs the dependencies listed within. Those dependencies become available in a generated `requirements.bzl` file. @@ -143,14 +143,9 @@ def pip_parse(requirements = None, requirements_lock = None, name = "pip_parsed_ requirements (Label): Deprecated. See requirements_lock. name (str, optional): The name of the generated repository. The generated repositories containing each requirement will be of the form `_`. - bzlmod (bool, optional): Whether this rule is being run under a bzlmod module extension. **kwargs (dict): Additional arguments to the [`pip_repository`](./pip_repository.md) repository rule. """ - - # Don't try to fetch dependencies under bzlmod because they are already fetched via the internal_deps - # module extention, and because the maybe-install pattern doesn't work under bzlmod. - if not bzlmod: - pip_install_dependencies() + pip_install_dependencies() # Temporary compatibility shim. # pip_install was previously document to use requirements while pip_parse was using requirements_lock. @@ -160,8 +155,6 @@ def pip_parse(requirements = None, requirements_lock = None, name = "pip_parsed_ pip_repository( name = name, requirements_lock = reqs_to_use, - repo_prefix = "{}_".format(name), - bzlmod = bzlmod, **kwargs ) diff --git a/python/pip_install/BUILD.bazel b/python/pip_install/BUILD.bazel index 451e7fab70..281ccba6a9 100644 --- a/python/pip_install/BUILD.bazel +++ b/python/pip_install/BUILD.bazel @@ -4,7 +4,6 @@ filegroup( "BUILD.bazel", "//python/pip_install/tools/dependency_resolver:distribution", "//python/pip_install/tools/lib:distribution", - "//python/pip_install/tools/lock_file_generator:distribution", "//python/pip_install/tools/wheel_installer:distribution", "//python/pip_install/private:distribution", ], @@ -24,7 +23,6 @@ filegroup( srcs = [ "//python/pip_install/tools/dependency_resolver:py_srcs", "//python/pip_install/tools/lib:py_srcs", - "//python/pip_install/tools/lock_file_generator:py_srcs", "//python/pip_install/tools/wheel_installer:py_srcs", ], visibility = ["//python/pip_install/private:__pkg__"], diff --git a/python/pip_install/pip_repository.bzl b/python/pip_install/pip_repository.bzl index d5d93f3e9d..982d8536ba 100644 --- a/python/pip_install/pip_repository.bzl +++ b/python/pip_install/pip_repository.bzl @@ -257,110 +257,177 @@ A requirements_lock attribute must be specified, or a platform-specific lockfile """) return requirements_txt -# Keep in sync with `_clean_name` in generated requirements.bzl +# Keep in sync with `_clean_pkg_name` in generated bzlmod requirements.bzl def _clean_pkg_name(name): return name.replace("-", "_").replace(".", "_").lower() -def _bzlmod_pkg_aliases(rctx, requirements_txt): +def _bzlmod_pkg_aliases(repo_name, bzl_packages): """Create alias declarations for each python dependency. - The aliases should be appended to the pip_parse repo's BUILD.bazel file. These aliases + The aliases should be appended to the pip_repository BUILD.bazel file. These aliases allow users to use requirement() without needed a corresponding `use_repo()` for each dep when using bzlmod. Args: - rctx: the repository context - requirements_txt: label to the requirements lock file + repo_name: the repository name of the parent that is visible to the users. + bzl_packages: the list of packages to setup. """ - requirements = parse_requirements(rctx.read(requirements_txt)).requirements - build_content = "" - for requirement in requirements: + for name in bzl_packages: build_content += """\ alias( name = "{name}_pkg", - actual = "@{repo_prefix}{dep}//:pkg", + actual = "@{repo_name}_{dep}//:pkg", ) alias( name = "{name}_whl", - actual = "@{repo_prefix}{dep}//:whl", + actual = "@{repo_name}_{dep}//:whl", ) alias( name = "{name}_data", - actual = "@{repo_prefix}{dep}//:data", + actual = "@{repo_name}_{dep}//:data", ) alias( name = "{name}_dist_info", - actual = "@{repo_prefix}{dep}//:dist_info", + actual = "@{repo_name}_{dep}//:dist_info", ) """.format( - name = _clean_pkg_name(requirement[0]), - repo_prefix = rctx.attr.repo_prefix, - dep = _clean_pkg_name(requirement[0]), + name = name, + repo_name = repo_name, + dep = name, ) return build_content -def _pip_repository_impl(rctx): - python_interpreter = _resolve_python_interpreter(rctx) +def _pip_repository_bzlmod_impl(rctx): + requirements_txt = locked_requirements_label(rctx, rctx.attr) + content = rctx.read(requirements_txt) + parsed_requirements_txt = parse_requirements(content) - # Write the annotations file to pass to the wheel maker - annotations = {package: json.decode(data) for (package, data) in rctx.attr.annotations.items()} - annotations_file = rctx.path("annotations.json") - rctx.file(annotations_file, json.encode_indent(annotations, indent = " " * 4)) + packages = [(_clean_pkg_name(name), requirement) for name, requirement in parsed_requirements_txt.requirements] - requirements_txt = locked_requirements_label(rctx, rctx.attr) - args = [ - python_interpreter, - "-m", - "python.pip_install.tools.lock_file_generator.lock_file_generator", - "--requirements_lock", - rctx.path(requirements_txt), - "--requirements_lock_label", - str(requirements_txt), - # pass quiet and timeout args through to child repos. - "--quiet", - str(rctx.attr.quiet), - "--timeout", - str(rctx.attr.timeout), - "--annotations", - annotations_file, - "--bzlmod", - str(rctx.attr.bzlmod).lower(), - ] + bzl_packages = sorted([name for name, _ in packages]) - args += ["--python_interpreter", _get_python_interpreter_attr(rctx)] - if rctx.attr.python_interpreter_target: - args += ["--python_interpreter_target", str(rctx.attr.python_interpreter_target)] - progress_message = "Parsing requirements to starlark" + repo_name = rctx.attr.name.split("~")[-1] - args += ["--repo", rctx.attr.name, "--repo-prefix", rctx.attr.repo_prefix] - args = _parse_optional_attrs(rctx, args) + build_contents = _BUILD_FILE_CONTENTS + _bzlmod_pkg_aliases(repo_name, bzl_packages) - rctx.report_progress(progress_message) + rctx.file("BUILD.bazel", build_contents) + rctx.template("requirements.bzl", rctx.attr._template, substitutions = { + "%%ALL_REQUIREMENTS%%": _format_repr_list([ + "@{}//:{}_pkg".format(repo_name, p) + for p in bzl_packages + ]), + "%%ALL_WHL_REQUIREMENTS%%": _format_repr_list([ + "@{}//:{}_whl".format(repo_name, p) + for p in bzl_packages + ]), + "%%NAME%%": rctx.attr.name, + "%%REQUIREMENTS_LOCK%%": str(requirements_txt), + }) - result = rctx.execute( - args, - # Manually construct the PYTHONPATH since we cannot use the toolchain here - environment = _create_repository_execution_environment(rctx), - timeout = rctx.attr.timeout, - quiet = rctx.attr.quiet, - ) +pip_repository_bzlmod_attrs = { + "requirements_darwin": attr.label( + allow_single_file = True, + doc = "Override the requirements_lock attribute when the host platform is Mac OS", + ), + "requirements_linux": attr.label( + allow_single_file = True, + doc = "Override the requirements_lock attribute when the host platform is Linux", + ), + "requirements_lock": attr.label( + allow_single_file = True, + doc = """ +A fully resolved 'requirements.txt' pip requirement file containing the transitive set of your dependencies. If this file is passed instead +of 'requirements' no resolve will take place and pip_repository will create individual repositories for each of your dependencies so that +wheels are fetched/built only for the targets specified by 'build/run/test'. +""", + ), + "requirements_windows": attr.label( + allow_single_file = True, + doc = "Override the requirements_lock attribute when the host platform is Windows", + ), + "_template": attr.label( + default = ":pip_repository_requirements_bzlmod.bzl.tmpl", + ), +} - if result.return_code: - fail("rules_python failed: %s (%s)" % (result.stdout, result.stderr)) +pip_repository_bzlmod = repository_rule( + attrs = pip_repository_bzlmod_attrs, + doc = """A rule for bzlmod pip_repository creation. Intended for private use only.""", + implementation = _pip_repository_bzlmod_impl, +) + +def _pip_repository_impl(rctx): + requirements_txt = locked_requirements_label(rctx, rctx.attr) + content = rctx.read(requirements_txt) + parsed_requirements_txt = parse_requirements(content) + + packages = [(_clean_pkg_name(name), requirement) for name, requirement in parsed_requirements_txt.requirements] - # We need a BUILD file to load the generated requirements.bzl - build_contents = _BUILD_FILE_CONTENTS + bzl_packages = sorted([name for name, _ in packages]) - if rctx.attr.bzlmod: - build_contents += _bzlmod_pkg_aliases(rctx, requirements_txt) + imports = [ + 'load("@rules_python//python/pip_install:pip_repository.bzl", "whl_library")', + ] + + annotations = {} + for pkg, annotation in rctx.attr.annotations.items(): + filename = "{}.annotation.json".format(_clean_pkg_name(pkg)) + rctx.file(filename, json.encode_indent(json.decode(annotation))) + annotations[pkg] = "@{name}//:{filename}".format(name = rctx.attr.name, filename = filename) + + tokenized_options = [] + for opt in parsed_requirements_txt.options: + for p in opt.split(" "): + tokenized_options.append(p) + + options = tokenized_options + rctx.attr.extra_pip_args + + config = { + "download_only": rctx.attr.download_only, + "enable_implicit_namespace_pkgs": rctx.attr.enable_implicit_namespace_pkgs, + "environment": rctx.attr.environment, + "extra_pip_args": options, + "isolated": use_isolated(rctx, rctx.attr), + "pip_data_exclude": rctx.attr.pip_data_exclude, + "python_interpreter": _get_python_interpreter_attr(rctx), + "quiet": rctx.attr.quiet, + "repo": rctx.attr.name, + "repo_prefix": "{}_".format(rctx.attr.name), + "timeout": rctx.attr.timeout, + } - rctx.file("BUILD.bazel", build_contents + "\n# The requirements.bzl file was generated by running:\n# " + " ".join([str(a) for a in args])) + if rctx.attr.python_interpreter_target: + config["python_interpreter_target"] = str(rctx.attr.python_interpreter_target) + + rctx.file("BUILD.bazel", _BUILD_FILE_CONTENTS) + rctx.template("requirements.bzl", rctx.attr._template, substitutions = { + "%%ALL_REQUIREMENTS%%": _format_repr_list([ + "@{}_{}//:pkg".format(rctx.attr.name, p) + for p in bzl_packages + ]), + "%%ALL_WHL_REQUIREMENTS%%": _format_repr_list([ + "@{}_{}//:whl".format(rctx.attr.name, p) + for p in bzl_packages + ]), + "%%ANNOTATIONS%%": _format_dict(_repr_dict(annotations)), + "%%CONFIG%%": _format_dict(_repr_dict(config)), + "%%EXTRA_PIP_ARGS%%": json.encode(options), + "%%IMPORTS%%": "\n".join(sorted(imports)), + "%%NAME%%": rctx.attr.name, + "%%PACKAGES%%": _format_repr_list( + [ + ("{}_{}".format(rctx.attr.name, p), r) + for p, r in packages + ], + ), + "%%REQUIREMENTS_LOCK%%": str(requirements_txt), + }) return @@ -453,12 +520,6 @@ pip_repository_attrs = { "annotations": attr.string_dict( doc = "Optional annotations to apply to packages", ), - "bzlmod": attr.bool( - default = False, - doc = """Whether this repository rule is invoked under bzlmod, in which case -we do not create the install_deps() macro. -""", - ), "requirements_darwin": attr.label( allow_single_file = True, doc = "Override the requirements_lock attribute when the host platform is Mac OS", @@ -479,6 +540,9 @@ wheels are fetched/built only for the targets specified by 'build/run/test'. allow_single_file = True, doc = "Override the requirements_lock attribute when the host platform is Windows", ), + "_template": attr.label( + default = ":pip_repository_requirements.bzl.tmpl", + ), } pip_repository_attrs.update(**common_attrs) @@ -625,3 +689,19 @@ def package_annotation( data_exclude_glob = data_exclude_glob, srcs_exclude_glob = srcs_exclude_glob, )) + +# pip_repository implementation + +def _format_list(items): + return "[{}]".format(", ".join(items)) + +def _format_repr_list(strings): + return _format_list( + [repr(s) for s in strings], + ) + +def _repr_dict(items): + return {k: repr(v) for k, v in items.items()} + +def _format_dict(items): + return "{{{}}}".format(", ".join(sorted(['"{}": {}'.format(k, v) for k, v in items.items()]))) diff --git a/python/pip_install/pip_repository_requirements.bzl.tmpl b/python/pip_install/pip_repository_requirements.bzl.tmpl new file mode 100644 index 0000000000..bf6a053622 --- /dev/null +++ b/python/pip_install/pip_repository_requirements.bzl.tmpl @@ -0,0 +1,52 @@ +"""Starlark representation of locked requirements. + +@generated by rules_python pip_parse repository rule +from %%REQUIREMENTS_LOCK%% +""" + +%%IMPORTS%% + +all_requirements = %%ALL_REQUIREMENTS%% + +all_whl_requirements = %%ALL_WHL_REQUIREMENTS%% + +_packages = %%PACKAGES%% +_config = %%CONFIG%% +_annotations = %%ANNOTATIONS%% + +def _clean_name(name): + return name.replace("-", "_").replace(".", "_").lower() + +def requirement(name): + return "@%%NAME%%_" + _clean_name(name) + "//:pkg" + +def whl_requirement(name): + return "@%%NAME%%_" + _clean_name(name) + "//:whl" + +def data_requirement(name): + return "@%%NAME%%_" + _clean_name(name) + "//:data" + +def dist_info_requirement(name): + return "@%%NAME%%_" + _clean_name(name) + "//:dist_info" + +def entry_point(pkg, script = None): + if not script: + script = pkg + return "@%%NAME%%_" + _clean_name(pkg) + "//:rules_python_wheel_entry_point_" + script + +def _get_annotation(requirement): + # This expects to parse `setuptools==58.2.0 --hash=sha256:2551203ae6955b9876741a26ab3e767bb3242dafe86a32a749ea0d78b6792f11` + # down to `setuptools`. + name = requirement.split(" ")[0].split("=")[0].split("[")[0] + return _annotations.get(name) + +def install_deps(**whl_library_kwargs): + whl_config = dict(_config) + whl_config.update(whl_library_kwargs) + for name, requirement in _packages: + whl_library( + name = name, + requirement = requirement, + annotation = _get_annotation(requirement), + **whl_config + ) diff --git a/python/pip_install/pip_repository_requirements_bzlmod.bzl.tmpl b/python/pip_install/pip_repository_requirements_bzlmod.bzl.tmpl new file mode 100644 index 0000000000..462829d074 --- /dev/null +++ b/python/pip_install/pip_repository_requirements_bzlmod.bzl.tmpl @@ -0,0 +1,24 @@ +"""Starlark representation of locked requirements. + +@generated by rules_python pip_parse repository rule +from %%REQUIREMENTS_LOCK%%. +""" + +all_requirements = %%ALL_REQUIREMENTS%% + +all_whl_requirements = %%ALL_WHL_REQUIREMENTS%% + +def _clean_name(name): + return name.replace("-", "_").replace(".", "_").lower() + +def requirement(name): + return "@@%%NAME%%//:" + _clean_name(name) + "_pkg" + +def whl_requirement(name): + return "@@%%NAME%%//:" + _clean_name(name) + "_whl" + +def data_requirement(name): + return "@@%%NAME%%//:" + _clean_name(name) + "_data" + +def dist_info_requirement(name): + return "@@%%NAME%%//:" + _clean_name(name) + "_dist_info" diff --git a/python/pip_install/private/srcs.bzl b/python/pip_install/private/srcs.bzl index 57644f612f..f3064a3aec 100644 --- a/python/pip_install/private/srcs.bzl +++ b/python/pip_install/private/srcs.bzl @@ -13,8 +13,6 @@ PIP_INSTALL_PY_SRCS = [ "@rules_python//python/pip_install/tools/lib:annotation.py", "@rules_python//python/pip_install/tools/lib:arguments.py", "@rules_python//python/pip_install/tools/lib:bazel.py", - "@rules_python//python/pip_install/tools/lock_file_generator:__init__.py", - "@rules_python//python/pip_install/tools/lock_file_generator:lock_file_generator.py", "@rules_python//python/pip_install/tools/wheel_installer:namespace_pkgs.py", "@rules_python//python/pip_install/tools/wheel_installer:wheel.py", "@rules_python//python/pip_install/tools/wheel_installer:wheel_installer.py", diff --git a/python/pip_install/tools/lock_file_generator/BUILD.bazel b/python/pip_install/tools/lock_file_generator/BUILD.bazel deleted file mode 100644 index 804f36a946..0000000000 --- a/python/pip_install/tools/lock_file_generator/BUILD.bazel +++ /dev/null @@ -1,50 +0,0 @@ -load("//python:defs.bzl", "py_binary", "py_library", "py_test") -load("//python/pip_install:repositories.bzl", "requirement") - -py_library( - name = "lib", - srcs = [ - "lock_file_generator.py", - ], - deps = [ - "//python/pip_install/tools/lib", - requirement("pip"), - ], -) - -py_binary( - name = "lock_file_generator", - srcs = [ - "lock_file_generator.py", - ], - deps = [":lib"], -) - -py_test( - name = "lock_file_generator_test", - size = "small", - srcs = [ - "lock_file_generator_test.py", - ], - deps = [ - ":lib", - ], -) - -filegroup( - name = "distribution", - srcs = glob( - ["*"], - exclude = ["*_test.py"], - ), - visibility = ["//python/pip_install:__subpackages__"], -) - -filegroup( - name = "py_srcs", - srcs = glob( - include = ["**/*.py"], - exclude = ["**/*_test.py"], - ), - visibility = ["//python/pip_install:__subpackages__"], -) diff --git a/python/pip_install/tools/lock_file_generator/__init__.py b/python/pip_install/tools/lock_file_generator/__init__.py deleted file mode 100644 index bbdfb4c588..0000000000 --- a/python/pip_install/tools/lock_file_generator/__init__.py +++ /dev/null @@ -1,14 +0,0 @@ -# Copyright 2023 The Bazel Authors. All rights reserved. -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - diff --git a/python/pip_install/tools/lock_file_generator/lock_file_generator.py b/python/pip_install/tools/lock_file_generator/lock_file_generator.py deleted file mode 100644 index ed1488dd45..0000000000 --- a/python/pip_install/tools/lock_file_generator/lock_file_generator.py +++ /dev/null @@ -1,336 +0,0 @@ -# Copyright 2023 The Bazel Authors. All rights reserved. -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -import argparse -import json -import shlex -import sys -import textwrap -from pathlib import Path -from typing import Any, Dict, List, TextIO, Tuple - -from pip._internal.network.session import PipSession -from pip._internal.req import constructors -from pip._internal.req.req_file import ( - RequirementsFileParser, - get_file_content, - get_line_parser, - preprocess, -) -from pip._internal.req.req_install import InstallRequirement - -from python.pip_install.tools.lib import annotation, arguments, bazel - - -def parse_install_requirements( - requirements_lock: str, extra_pip_args: List[str] -) -> List[Tuple[InstallRequirement, str]]: - ps = PipSession() - # This is roughly taken from pip._internal.req.req_file.parse_requirements - # (https://github.com/pypa/pip/blob/21.0.1/src/pip/_internal/req/req_file.py#L127) in order to keep - # the original line (sort-of, its preprocessed) from the requirements_lock file around, to pass to sub repos - # as the requirement. - line_parser = get_line_parser(finder=None) - parser = RequirementsFileParser(ps, line_parser) - install_req_and_lines: List[Tuple[InstallRequirement, str]] = [] - _, content = get_file_content(requirements_lock, ps) - unpinned_reqs = [] - for parsed_line, (_, line) in zip( - parser.parse(requirements_lock, constraint=False), preprocess(content) - ): - if parsed_line.is_requirement: - install_req = constructors.install_req_from_line(parsed_line.requirement) - if ( - # PEP-440 direct references are considered pinned - # See: https://peps.python.org/pep-0440/#direct-references and https://peps.python.org/pep-0508/ - not install_req.link - and not install_req.is_pinned - ): - unpinned_reqs.append(str(install_req)) - install_req_and_lines.append((install_req, line)) - - else: - extra_pip_args.extend(shlex.split(line)) - - if len(unpinned_reqs) > 0: - unpinned_reqs_str = "\n".join(unpinned_reqs) - raise RuntimeError( - f"""\ -The `requirements_lock` file must be fully pinned. See `compile_pip_requirements`. -Alternatively, use `pip-tools` or a similar mechanism to produce a pinned lockfile. - -The following requirements were not pinned: -{unpinned_reqs_str}""" - ) - - return install_req_and_lines - - -def repo_names_and_requirements( - install_reqs: List[Tuple[InstallRequirement, str]], repo_prefix: str -) -> List[Tuple[str, str]]: - return [ - ( - bazel.sanitise_name(ir.name, prefix=repo_prefix), - line, - ) - for ir, line in install_reqs - ] - - -def parse_whl_library_args(args: argparse.Namespace) -> Dict[str, Any]: - whl_library_args = dict(vars(args)) - whl_library_args = arguments.deserialize_structured_args(whl_library_args) - whl_library_args.setdefault("python_interpreter", sys.executable) - - # These arguments are not used by `whl_library` - for arg in ( - "requirements_lock", - "requirements_lock_label", - "annotations", - "bzlmod", - ): - if arg in whl_library_args: - whl_library_args.pop(arg) - - return whl_library_args - - -def generate_parsed_requirements_contents( - requirements_lock: Path, - repo: str, - repo_prefix: str, - whl_library_args: Dict[str, Any], - annotations: Dict[str, str] = dict(), - bzlmod: bool = False, -) -> str: - """ - Parse each requirement from the requirements_lock file, and prepare arguments for each - repository rule, which will represent the individual requirements. - - Generates a requirements.bzl file containing a macro (install_deps()) which instantiates - a repository rule for each requirement in the lock file. - """ - install_req_and_lines = parse_install_requirements( - requirements_lock, whl_library_args["extra_pip_args"] - ) - repo_names_and_reqs = repo_names_and_requirements( - install_req_and_lines, repo_prefix - ) - - all_requirements = ", ".join( - [ - bazel.sanitised_repo_library_label(ir.name, repo_prefix=repo_prefix) - for ir, _ in install_req_and_lines - ] - ) - all_whl_requirements = ", ".join( - [ - bazel.sanitised_repo_file_label(ir.name, repo_prefix=repo_prefix) - for ir, _ in install_req_and_lines - ] - ) - - install_deps_macro = """ - def install_deps(**whl_library_kwargs): - whl_config = dict(_config) - whl_config.update(whl_library_kwargs) - for name, requirement in _packages: - whl_library( - name = name, - requirement = requirement, - annotation = _get_annotation(requirement), - **whl_config - ) -""" - return textwrap.dedent( - ( - """\ - - load("@rules_python//python/pip_install:pip_repository.bzl", "whl_library") - - all_requirements = [{all_requirements}] - - all_whl_requirements = [{all_whl_requirements}] - - _packages = {repo_names_and_reqs} - _config = {args} - _annotations = {annotations} - _bzlmod = {bzlmod} - - def _clean_name(name): - return name.replace("-", "_").replace(".", "_").lower() - - def requirement(name): - if _bzlmod: - return "@@{repo}//:" + _clean_name(name) + "_{py_library_label}" - return "@{repo_prefix}" + _clean_name(name) + "//:{py_library_label}" - - def whl_requirement(name): - if _bzlmod: - return "@@{repo}//:" + _clean_name(name) + "_{wheel_file_label}" - return "@{repo_prefix}" + _clean_name(name) + "//:{wheel_file_label}" - - def data_requirement(name): - if _bzlmod: - return "@@{repo}//:" + _clean_name(name) + "_{data_label}" - return "@{repo_prefix}" + _clean_name(name) + "//:{data_label}" - - def dist_info_requirement(name): - if _bzlmod: - return "@@{repo}//:" + _clean_name(name) + "_{dist_info_label}" - return "@{repo_prefix}" + _clean_name(name) + "//:{dist_info_label}" - - def entry_point(pkg, script = None): - if not script: - script = pkg - return "@{repo_prefix}" + _clean_name(pkg) + "//:{entry_point_prefix}_" + script - - def _get_annotation(requirement): - # This expects to parse `setuptools==58.2.0 --hash=sha256:2551203ae6955b9876741a26ab3e767bb3242dafe86a32a749ea0d78b6792f11` - # down wo `setuptools`. - name = requirement.split(" ")[0].split("=")[0].split("[")[0] - return _annotations.get(name) -""" - + (install_deps_macro if not bzlmod else "") - ).format( - all_requirements=all_requirements, - all_whl_requirements=all_whl_requirements, - annotations=json.dumps(annotations), - args=dict(sorted(whl_library_args.items())), - data_label=bazel.DATA_LABEL, - dist_info_label=bazel.DIST_INFO_LABEL, - entry_point_prefix=bazel.WHEEL_ENTRY_POINT_PREFIX, - py_library_label=bazel.PY_LIBRARY_LABEL, - repo_names_and_reqs=repo_names_and_reqs, - repo=repo, - repo_prefix=repo_prefix, - wheel_file_label=bazel.WHEEL_FILE_LABEL, - bzlmod=bzlmod, - ) - ) - - -def coerce_to_bool(option): - return str(option).lower() == "true" - - -def main(output: TextIO) -> None: - """Args: - - output: where to write the resulting starlark, such as sys.stdout or an open file - """ - parser = argparse.ArgumentParser( - description="Create rules to incrementally fetch needed \ -dependencies from a fully resolved requirements lock file." - ) - parser.add_argument( - "--requirements_lock", - action="store", - required=True, - help="Path to fully resolved requirements.txt to use as the source of repos.", - ) - parser.add_argument( - "--requirements_lock_label", - help="Label used to declare the requirements.lock, included in comments in the file.", - ) - parser.add_argument( - "--python_interpreter", - help="The python interpreter that will be used to download and unpack the wheels.", - ) - parser.add_argument( - "--python_interpreter_target", - help="Bazel target of a python interpreter.\ -It will be used in repository rules so it must be an already built interpreter.\ -If set, it will take precedence over python_interpreter.", - ) - parser.add_argument( - "--quiet", - type=coerce_to_bool, - default=True, - required=True, - help="Whether to print stdout / stderr from child repos.", - ) - parser.add_argument( - "--timeout", - type=int, - action="store", - required=True, - help="timeout to use for pip operation.", - ) - parser.add_argument( - "--annotations", - type=annotation.annotations_map_from_str_path, - help="A json encoded file containing annotations for rendered packages.", - ) - parser.add_argument( - "--bzlmod", - type=coerce_to_bool, - default=False, - help="Whether this script is run under bzlmod. Under bzlmod we don't generate the install_deps() macro as it isn't needed.", - ) - arguments.parse_common_args(parser) - args = parser.parse_args() - - whl_library_args = parse_whl_library_args(args) - - # Check for any annotations which match packages in the locked requirements file - install_requirements = parse_install_requirements( - args.requirements_lock, whl_library_args["extra_pip_args"] - ) - req_names = sorted([req.name for req, _ in install_requirements]) - annotations = args.annotations.collect(req_names) if args.annotations else {} - - # Write all rendered annotation files and generate a list of the labels to write to the requirements file - annotated_requirements = dict() - for name, content in annotations.items(): - annotation_path = Path(name + ".annotation.json") - annotation_path.write_text(json.dumps(content, indent=4)) - annotated_requirements.update( - { - name: "@{}//:{}.annotation.json".format( - args.repo, name - ) - } - ) - - output.write( - textwrap.dedent( - """\ - \"\"\"Starlark representation of locked requirements. - - @generated by rules_python pip_parse repository rule - from {} - \"\"\" - """.format( - args.requirements_lock_label - ) - ) - ) - - output.write( - generate_parsed_requirements_contents( - requirements_lock=args.requirements_lock, - repo=args.repo, - repo_prefix=args.repo_prefix, - whl_library_args=whl_library_args, - annotations=annotated_requirements, - bzlmod=args.bzlmod, - ) - ) - - -if __name__ == "__main__": - with open("requirements.bzl", "w") as requirement_file: - main(requirement_file) diff --git a/python/pip_install/tools/lock_file_generator/lock_file_generator_test.py b/python/pip_install/tools/lock_file_generator/lock_file_generator_test.py deleted file mode 100644 index be244b1c07..0000000000 --- a/python/pip_install/tools/lock_file_generator/lock_file_generator_test.py +++ /dev/null @@ -1,163 +0,0 @@ -# Copyright 2023 The Bazel Authors. All rights reserved. -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -import argparse -import json -import tempfile -import unittest -from pathlib import Path -from textwrap import dedent - -from pip._internal.req.req_install import InstallRequirement - -from python.pip_install.tools.lock_file_generator import lock_file_generator - - -class TestParseRequirementsToBzl(unittest.TestCase): - maxDiff = None - - def test_generated_requirements_bzl(self) -> None: - with tempfile.TemporaryDirectory() as temp_dir: - requirements_lock = Path(temp_dir) / "requirements.txt" - comments_and_flags = "#comment\n--require-hashes True\n" - requirement_string = "foo==0.0.0 --hash=sha256:hashofFoowhl" - requirements_lock.write_bytes( - bytes(comments_and_flags + requirement_string, encoding="utf-8") - ) - args = argparse.Namespace() - args.requirements_lock = str(requirements_lock.resolve()) - args.repo = ("pip_parsed_deps_pypi__",) - args.repo_prefix = "pip_parsed_deps_pypi__" - extra_pip_args = ["--index-url=pypi.org/simple"] - pip_data_exclude = ["**.foo"] - args.extra_pip_args = json.dumps({"arg": extra_pip_args}) - args.pip_data_exclude = json.dumps({"arg": pip_data_exclude}) - args.python_interpreter = "/custom/python3" - args.python_interpreter_target = "@custom_python//:exec" - args.environment = json.dumps({"arg": {}}) - whl_library_args = lock_file_generator.parse_whl_library_args(args) - contents = lock_file_generator.generate_parsed_requirements_contents( - requirements_lock=args.requirements_lock, - repo=args.repo, - repo_prefix=args.repo_prefix, - whl_library_args=whl_library_args, - ) - library_target = "@pip_parsed_deps_pypi__foo//:pkg" - whl_target = "@pip_parsed_deps_pypi__foo//:whl" - all_requirements = 'all_requirements = ["{library_target}"]'.format( - library_target=library_target - ) - all_whl_requirements = 'all_whl_requirements = ["{whl_target}"]'.format( - whl_target=whl_target - ) - self.assertIn(all_requirements, contents, contents) - self.assertIn(all_whl_requirements, contents, contents) - self.assertIn(requirement_string, contents, contents) - all_flags = extra_pip_args + ["--require-hashes", "True"] - self.assertIn( - "'extra_pip_args': {}".format(repr(all_flags)), contents, contents - ) - self.assertIn( - "'pip_data_exclude': {}".format(repr(pip_data_exclude)), - contents, - contents, - ) - self.assertIn("'python_interpreter': '/custom/python3'", contents, contents) - self.assertIn( - "'python_interpreter_target': '@custom_python//:exec'", - contents, - contents, - ) - # Assert it gets set to an empty dict by default. - self.assertIn("'environment': {}", contents, contents) - - def test_parse_install_requirements_with_args(self): - # Test requirements files with varying arguments - for requirement_args in ("", "--index-url https://index.python.com"): - with tempfile.TemporaryDirectory() as temp_dir: - requirements_lock = Path(temp_dir) / "requirements.txt" - requirements_lock.write_text( - dedent( - """\ - {} - - wheel==0.37.1 \\ - --hash=sha256:4bdcd7d840138086126cd09254dc6195fb4fc6f01c050a1d7236f2630db1d22a \\ - --hash=sha256:e9a504e793efbca1b8e0e9cb979a249cf4a0a7b5b8c9e8b65a5e39d49529c1c4 - # via -r requirements.in - setuptools==58.2.0 \\ - --hash=sha256:2551203ae6955b9876741a26ab3e767bb3242dafe86a32a749ea0d78b6792f11 \ - --hash=sha256:2c55bdb85d5bb460bd2e3b12052b677879cffcf46c0c688f2e5bf51d36001145 - # via -r requirements.in - """.format( - requirement_args - ) - ) - ) - - install_req_and_lines = lock_file_generator.parse_install_requirements( - str(requirements_lock), ["-v"] - ) - - # There should only be two entries for the two requirements - self.assertEqual(len(install_req_and_lines), 2) - - # The first index in each tuple is expected to be an `InstallRequirement` object - self.assertIsInstance(install_req_and_lines[0][0], InstallRequirement) - self.assertIsInstance(install_req_and_lines[1][0], InstallRequirement) - - # Ensure the requirements text is correctly parsed with the trailing arguments - self.assertTupleEqual( - install_req_and_lines[0][1:], - ( - "wheel==0.37.1 --hash=sha256:4bdcd7d840138086126cd09254dc6195fb4fc6f01c050a1d7236f2630db1d22a --hash=sha256:e9a504e793efbca1b8e0e9cb979a249cf4a0a7b5b8c9e8b65a5e39d49529c1c4", - ), - ) - self.assertTupleEqual( - install_req_and_lines[1][1:], - ( - "setuptools==58.2.0 --hash=sha256:2551203ae6955b9876741a26ab3e767bb3242dafe86a32a749ea0d78b6792f11 --hash=sha256:2c55bdb85d5bb460bd2e3b12052b677879cffcf46c0c688f2e5bf51d36001145", - ), - ) - - def test_parse_install_requirements_pinned_direct_reference(self): - # Test PEP-440 direct references - with tempfile.TemporaryDirectory() as temp_dir: - requirements_lock = Path(temp_dir) / "requirements.txt" - requirements_lock.write_text( - dedent( - """\ - onnx @ https://files.pythonhosted.org/packages/24/93/f5b001dc0f5de84ce049a34ff382032cd9478e1080aa6ac48470fa810577/onnx-1.11.0-cp39-cp39-manylinux_2_12_x86_64.manylinux2010_x86_64.whl \ - --hash=sha256:67c6d2654c1c203e5c839a47900b51f588fd0de71bbd497fb193d30a0b3ec1e9 - """ - ) - ) - - install_req_and_lines = lock_file_generator.parse_install_requirements( - str(requirements_lock), ["-v"] - ) - - self.assertEqual(len(install_req_and_lines), 1) - self.assertEqual(install_req_and_lines[0][0].name, "onnx") - - self.assertTupleEqual( - install_req_and_lines[0][1:], - ( - "onnx @ https://files.pythonhosted.org/packages/24/93/f5b001dc0f5de84ce049a34ff382032cd9478e1080aa6ac48470fa810577/onnx-1.11.0-cp39-cp39-manylinux_2_12_x86_64.manylinux2010_x86_64.whl --hash=sha256:67c6d2654c1c203e5c839a47900b51f588fd0de71bbd497fb193d30a0b3ec1e9", - ), - ) - - -if __name__ == "__main__": - unittest.main() From 076d8746d970b05efc2199532564d9018b2c2313 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Thu, 16 Feb 2023 09:17:29 -0800 Subject: [PATCH 14/21] Add some docs about how to configure coverage. (#1074) * Add some docs about how to configure coverage. This is to replace the docs on bazel.build that talk about coverage support for Python. * Update docs/coverage.md --------- Co-authored-by: Thulio Ferraz Assis <3149049+f0rmiga@users.noreply.github.com> --- docs/coverage.md | 58 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) create mode 100644 docs/coverage.md diff --git a/docs/coverage.md b/docs/coverage.md new file mode 100644 index 0000000000..bc613f8295 --- /dev/null +++ b/docs/coverage.md @@ -0,0 +1,58 @@ +# Setting up coverage + +As of Bazel 6, the Python toolchains and bootstrap logic supports providing +coverage information using the `coverage` library. + +As of `rules_python` version `0.18.1`, builtin coverage support can be enabled +when configuring toolchains. + +## Enabling `rules_python` coverage support + +Enabling the coverage support bundled with `rules_python` just requires setting an +argument when registerting toolchains. + +For Bzlmod: + +```starlark +python.toolchain( + "@python3_9_toolchains//:all", + configure_coverage_tool = True, +) +``` + +For WORKSPACE configuration: + +```starlark +register_python_toolchains( + register_coverage_tool = True, +) +``` + +NOTE: This will implicitly add the version of `coverage` bundled with +`rules_python` to the dependencies of `py_test` rules when `bazel coverage` is +run. If a target already transitively depends on a different version of +`coverage`, then behavior is undefined -- it is undefined which version comes +first in the import path. If you find yourself in this situation, then you'll +need to manually configure coverage (see below). + +## Manually configuring coverage + +To manually configure coverage support, you'll need to set the +`py_runtime.coverage_tool` attribute. This attribute is a target that specifies +the coverage entry point file and, optionally, client libraries that are added +to `py_test` targets. Typically, this would be a `filegroup` that looked like: + +```starlark +filegroup( + name = "coverage", + srcs = ["coverage_main.py"], + data = ["coverage_lib1.py", ...] +) +``` + +Using `filegroup` isn't required, nor are including client libraries. The +important behaviors of the target are: + +* It provides a single output file OR it provides an executable output; this + output is treated as the coverage entry point. +* If it provides runfiles, then `runfiles.files` are included into `py_test`. From f4396956f16072c34748001baa17e9bcb89cad78 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Tue, 21 Feb 2023 16:11:28 -0800 Subject: [PATCH 15/21] Remove empty line between copyright and build file docstring. (#1084) This makes modifying it with buildozer easier. When the empty line is present, Buildozer gets confused and adds loads() before the intended doc string. --- python/BUILD.bazel | 1 - 1 file changed, 1 deletion(-) diff --git a/python/BUILD.bazel b/python/BUILD.bazel index dcdbee15af..2e275b6650 100644 --- a/python/BUILD.bazel +++ b/python/BUILD.bazel @@ -11,7 +11,6 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. - """This package contains two sets of rules: 1) the "core" Python rules, which were historically bundled with Bazel and From b9865470cc567eb780bf5c8f7823d0200d199e97 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Wed, 22 Feb 2023 19:21:18 -0800 Subject: [PATCH 16/21] cleanup: Remove license type comment; they're no longer required (#1078) cleanup: Remove license type comment; they're no longer required The `# License type` comments are no longer required. Removing it makes it easier to import the source into Google. --- BUILD.bazel | 2 +- python/private/BUILD.bazel | 2 +- tools/BUILD.bazel | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/BUILD.bazel b/BUILD.bazel index 5b37fce29a..dff608a1ca 100644 --- a/BUILD.bazel +++ b/BUILD.bazel @@ -16,7 +16,7 @@ load(":version.bzl", "BAZEL_VERSION") package(default_visibility = ["//visibility:public"]) -licenses(["notice"]) # Apache 2.0 +licenses(["notice"]) exports_files([ "LICENSE", diff --git a/python/private/BUILD.bazel b/python/private/BUILD.bazel index d56d31b27a..7d321ebbe7 100644 --- a/python/private/BUILD.bazel +++ b/python/private/BUILD.bazel @@ -15,7 +15,7 @@ load("//python:versions.bzl", "print_toolchains_checksums") load(":stamp.bzl", "stamp_build_setting") -licenses(["notice"]) # Apache 2.0 +licenses(["notice"]) filegroup( name = "distribution", diff --git a/tools/BUILD.bazel b/tools/BUILD.bazel index 7c9b492a3c..fd951d9086 100644 --- a/tools/BUILD.bazel +++ b/tools/BUILD.bazel @@ -15,7 +15,7 @@ load("//python:defs.bzl", "py_binary") package(default_visibility = ["//visibility:public"]) -licenses(["notice"]) # Apache 2.0 +licenses(["notice"]) # Implementation detail of py_wheel rule. py_binary( From 5419e235440f68c75756e6474d3fe41eda920575 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Fri, 24 Feb 2023 11:09:08 -0800 Subject: [PATCH 17/21] fix: Use GitHub download URL for BCR URL instead of archive URL. (#1093) This is to prevent the issue where the checksum of the auto-generated archive files may change due to GitHub internal changes. Fixes #1072 --- .bcr/source.template.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.bcr/source.template.json b/.bcr/source.template.json index a3bd62f161..c23b7652e7 100644 --- a/.bcr/source.template.json +++ b/.bcr/source.template.json @@ -1,5 +1,5 @@ { "integrity": "", "strip_prefix": "{REPO}-{VERSION}", - "url": "https://github.com/{OWNER}/{REPO}/archive/refs/tags/{TAG}.tar.gz" + "url": "https://github.com/{OWNER}/{REPO}/releases/download/{TAG}/rules_python-{TAG}.tar.gz" } From 797c2d031018cfe68fb1ac475c0a599671885c57 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Fri, 24 Feb 2023 13:45:42 -0800 Subject: [PATCH 18/21] Add a script to add missing license headers (#1094) Work towards #916 --- addlicense.sh | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) create mode 100755 addlicense.sh diff --git a/addlicense.sh b/addlicense.sh new file mode 100755 index 0000000000..8cc8fb33bc --- /dev/null +++ b/addlicense.sh @@ -0,0 +1,23 @@ +#!/bin/bash +# Copyright 2023 The Bazel Authors. All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + + +if ! command -v addlicense @>&1 >/dev/null; then + echo "ERROR: addlicense not installed." + echo "Install using https://github.com/google/addlicense#install" + exit 1 +fi + +addlicense -v -l apache -c 'The Bazel Authors. All rights reserved.' "$@" From bce3ccd0f151efa72182a4d5377d14f3d297087d Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Fri, 24 Feb 2023 23:03:07 -0800 Subject: [PATCH 19/21] fix: Update pre-commit dependency versions so isort works. (#1096) This resolve an issue where an older isort version don't work with a newer poetry version. I think this problem occurs if you've upgraded isort (which upgrades poetry, I think), but then downgrade isort (which doesn't downgrade poetry, too). --- .pre-commit-config.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index e4ae5d3e0a..9403dd5338 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -26,7 +26,7 @@ repos: - id: buildifier-lint args: *args - repo: https://github.com/pycqa/isort - rev: 5.10.1 + rev: 5.12.0 hooks: - id: isort name: isort (python) @@ -34,6 +34,6 @@ repos: - --profile - black - repo: https://github.com/psf/black - rev: 22.8.0 + rev: 23.1.0 hooks: - id: black From f97e00853666f1918ff58b7b2fd846791888a02d Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Sat, 25 Feb 2023 15:55:32 -0800 Subject: [PATCH 20/21] docs: doc that the Conventional Commit style should be used for merged commits and PRs (#1099) --- CONTRIBUTING.md | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 7b80037244..54ecfb01e5 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -114,6 +114,35 @@ information on using pull requests. [GitHub Help]: https://help.github.com/articles/about-pull-requests/ +### Commit messages + +Commit messages (upon merging) and PR messages should follow the [Conventional +Commits](https://www.conventionalcommits.org/) style: + +``` +type(scope)!: + + + +BREAKING CHANGE: +``` + +Where `(scope)` is optional, and `!` is only required if there is a breaking change. +If a breaking change is introduced, then `BREAKING CHANGE:` is required. + +Common `type`s: + +* `build:` means it affects the building or development workflow. +* `docs:` means only documentation is being added, updated, or fixed. +* `feat:` means a user-visible feature is being added. +* `fix:` means a user-visible behavior is being fixed. +* `refactor:` means some sort of code cleanup that doesn't change user-visible behavior. +* `revert:` means a prior change is being reverted in some way. +* `test:` means only tests are being added. + +For the full details of types, see +[Conventional Commits](https://www.conventionalcommits.org/). + ## Generated files Some checked-in files are generated and need to be updated when a new PR is From c504355672223144cefb2cbf3f69e2d38e7e2726 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Mon, 27 Feb 2023 15:38:55 -0800 Subject: [PATCH 21/21] test(core): Add analysis tests for base Python rules. (#1102) This is to provide some regression tests for the Starlark rewrite. These tests are approximately the same as Bazel's Java-implemented tests. Work towards #1069 --- internal_deps.bzl | 7 + tools/build_defs/python/BUILD.bazel | 13 + tools/build_defs/python/tests/BUILD.bazel | 79 +++++ tools/build_defs/python/tests/base_tests.bzl | 103 +++++++ .../python/tests/fake_cc_toolchain_config.bzl | 37 +++ .../python/tests/py_binary/BUILD.bazel | 17 ++ .../tests/py_binary/py_binary_tests.bzl | 28 ++ .../python/tests/py_executable_base_tests.bzl | 272 ++++++++++++++++++ .../python/tests/py_info_subject.bzl | 95 ++++++ .../python/tests/py_library/BUILD.bazel | 18 ++ .../tests/py_library/py_library_tests.bzl | 148 ++++++++++ .../python/tests/py_test/BUILD.bazel | 18 ++ .../python/tests/py_test/py_test_tests.bzl | 98 +++++++ tools/build_defs/python/tests/util.bzl | 78 +++++ 14 files changed, 1011 insertions(+) create mode 100644 tools/build_defs/python/BUILD.bazel create mode 100644 tools/build_defs/python/tests/BUILD.bazel create mode 100644 tools/build_defs/python/tests/base_tests.bzl create mode 100644 tools/build_defs/python/tests/fake_cc_toolchain_config.bzl create mode 100644 tools/build_defs/python/tests/py_binary/BUILD.bazel create mode 100644 tools/build_defs/python/tests/py_binary/py_binary_tests.bzl create mode 100644 tools/build_defs/python/tests/py_executable_base_tests.bzl create mode 100644 tools/build_defs/python/tests/py_info_subject.bzl create mode 100644 tools/build_defs/python/tests/py_library/BUILD.bazel create mode 100644 tools/build_defs/python/tests/py_library/py_library_tests.bzl create mode 100644 tools/build_defs/python/tests/py_test/BUILD.bazel create mode 100644 tools/build_defs/python/tests/py_test/py_test_tests.bzl create mode 100644 tools/build_defs/python/tests/util.bzl diff --git a/internal_deps.bzl b/internal_deps.bzl index 11c652a50d..8f52b0e7d7 100644 --- a/internal_deps.bzl +++ b/internal_deps.bzl @@ -39,6 +39,13 @@ def rules_python_internal_deps(): ], sha256 = "8a298e832762eda1830597d64fe7db58178aa84cd5926d76d5b744d6558941c2", ) + maybe( + http_archive, + name = "rules_testing", + url = "https://github.com/bazelbuild/rules_testing/releases/download/v0.0.1/rules_testing-v0.0.1.tar.gz", + sha256 = "47db8fc9c3c1837491333cdcedebf267285479bd709a1ff0a47b19a324817def", + strip_prefix = "rules_testing-0.0.1", + ) maybe( http_archive, diff --git a/tools/build_defs/python/BUILD.bazel b/tools/build_defs/python/BUILD.bazel new file mode 100644 index 0000000000..aa21042e25 --- /dev/null +++ b/tools/build_defs/python/BUILD.bazel @@ -0,0 +1,13 @@ +# Copyright 2023 The Bazel Authors. All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. diff --git a/tools/build_defs/python/tests/BUILD.bazel b/tools/build_defs/python/tests/BUILD.bazel new file mode 100644 index 0000000000..92bdc5c396 --- /dev/null +++ b/tools/build_defs/python/tests/BUILD.bazel @@ -0,0 +1,79 @@ +# Copyright 2023 The Bazel Authors. All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +load("@rules_cc//cc:defs.bzl", "cc_toolchain", "cc_toolchain_suite") +load(":fake_cc_toolchain_config.bzl", "fake_cc_toolchain_config") + +platform( + name = "mac", + constraint_values = [ + "@platforms//os:macos", + ], +) + +platform( + name = "linux", + constraint_values = [ + "@platforms//os:linux", + ], +) + +cc_toolchain_suite( + name = "cc_toolchain_suite", + tags = ["manual"], + toolchains = { + "darwin_x86_64": ":mac_toolchain", + "k8": ":linux_toolchain", + }, +) + +filegroup(name = "empty") + +cc_toolchain( + name = "mac_toolchain", + all_files = ":empty", + compiler_files = ":empty", + dwp_files = ":empty", + linker_files = ":empty", + objcopy_files = ":empty", + strip_files = ":empty", + supports_param_files = 0, + toolchain_config = ":mac_toolchain_config", + toolchain_identifier = "mac-toolchain", +) + +fake_cc_toolchain_config( + name = "mac_toolchain_config", + target_cpu = "darwin_x86_64", + toolchain_identifier = "mac-toolchain", +) + +cc_toolchain( + name = "linux_toolchain", + all_files = ":empty", + compiler_files = ":empty", + dwp_files = ":empty", + linker_files = ":empty", + objcopy_files = ":empty", + strip_files = ":empty", + supports_param_files = 0, + toolchain_config = ":linux_toolchain_config", + toolchain_identifier = "linux-toolchain", +) + +fake_cc_toolchain_config( + name = "linux_toolchain_config", + target_cpu = "k8", + toolchain_identifier = "linux-toolchain", +) diff --git a/tools/build_defs/python/tests/base_tests.bzl b/tools/build_defs/python/tests/base_tests.bzl new file mode 100644 index 0000000000..715aea7fde --- /dev/null +++ b/tools/build_defs/python/tests/base_tests.bzl @@ -0,0 +1,103 @@ +# Copyright 2023 The Bazel Authors. All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +"""Tests common to py_test, py_binary, and py_library rules.""" + +load("@rules_testing//lib:analysis_test.bzl", "analysis_test") +load("@rules_testing//lib:truth.bzl", "matching") +load("@rules_testing//lib:util.bzl", rt_util = "util") +load("//python:defs.bzl", "PyInfo") +load("//tools/build_defs/python/tests:py_info_subject.bzl", "py_info_subject") +load("//tools/build_defs/python/tests:util.bzl", pt_util = "util") + +_tests = [] + +def _produces_py_info_impl(ctx): + return [PyInfo(transitive_sources = depset(ctx.files.srcs))] + +_produces_py_info = rule( + implementation = _produces_py_info_impl, + attrs = {"srcs": attr.label_list(allow_files = True)}, +) + +def _test_consumes_provider(name, config): + rt_util.helper_target( + config.base_test_rule, + name = name + "_subject", + deps = [name + "_produces_py_info"], + ) + rt_util.helper_target( + _produces_py_info, + name = name + "_produces_py_info", + srcs = [rt_util.empty_file(name + "_produce.py")], + ) + analysis_test( + name = name, + target = name + "_subject", + impl = _test_consumes_provider_impl, + ) + +def _test_consumes_provider_impl(env, target): + env.expect.that_target(target).provider( + PyInfo, + factory = py_info_subject, + ).transitive_sources().contains("{package}/{test_name}_produce.py") + +_tests.append(_test_consumes_provider) + +def _test_requires_provider(name, config): + rt_util.helper_target( + config.base_test_rule, + name = name + "_subject", + deps = [name + "_nopyinfo"], + ) + rt_util.helper_target( + native.filegroup, + name = name + "_nopyinfo", + ) + analysis_test( + name = name, + target = name + "_subject", + impl = _test_requires_provider_impl, + expect_failure = True, + ) + +def _test_requires_provider_impl(env, target): + env.expect.that_target(target).failures().contains_predicate( + matching.str_matches("mandatory*PyInfo"), + ) + +_tests.append(_test_requires_provider) + +def _test_data_sets_uses_shared_library(name, config): + rt_util.helper_target( + config.base_test_rule, + name = name + "_subject", + data = [rt_util.empty_file(name + "_dso.so")], + ) + analysis_test( + name = name, + target = name + "_subject", + impl = _test_data_sets_uses_shared_library_impl, + ) + +def _test_data_sets_uses_shared_library_impl(env, target): + env.expect.that_target(target).provider( + PyInfo, + factory = py_info_subject, + ).uses_shared_libraries().equals(True) + +_tests.append(_test_data_sets_uses_shared_library) + +def create_base_tests(config): + return pt_util.create_tests(_tests, config = config) diff --git a/tools/build_defs/python/tests/fake_cc_toolchain_config.bzl b/tools/build_defs/python/tests/fake_cc_toolchain_config.bzl new file mode 100644 index 0000000000..b3214a61ba --- /dev/null +++ b/tools/build_defs/python/tests/fake_cc_toolchain_config.bzl @@ -0,0 +1,37 @@ +# Copyright 2023 The Bazel Authors. All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""Fake for providing CcToolchainConfigInfo.""" + +def _impl(ctx): + return cc_common.create_cc_toolchain_config_info( + ctx = ctx, + toolchain_identifier = ctx.attr.toolchain_identifier, + host_system_name = "local", + target_system_name = "local", + target_cpu = ctx.attr.target_cpu, + target_libc = "unknown", + compiler = "clang", + abi_version = "unknown", + abi_libc_version = "unknown", + ) + +fake_cc_toolchain_config = rule( + implementation = _impl, + attrs = { + "target_cpu": attr.string(), + "toolchain_identifier": attr.string(), + }, + provides = [CcToolchainConfigInfo], +) diff --git a/tools/build_defs/python/tests/py_binary/BUILD.bazel b/tools/build_defs/python/tests/py_binary/BUILD.bazel new file mode 100644 index 0000000000..17a6690b82 --- /dev/null +++ b/tools/build_defs/python/tests/py_binary/BUILD.bazel @@ -0,0 +1,17 @@ +# Copyright 2023 The Bazel Authors. All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +load(":py_binary_tests.bzl", "py_binary_test_suite") + +py_binary_test_suite(name = "py_binary_tests") diff --git a/tools/build_defs/python/tests/py_binary/py_binary_tests.bzl b/tools/build_defs/python/tests/py_binary/py_binary_tests.bzl new file mode 100644 index 0000000000..8d32632610 --- /dev/null +++ b/tools/build_defs/python/tests/py_binary/py_binary_tests.bzl @@ -0,0 +1,28 @@ +# Copyright 2023 The Bazel Authors. All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +"""Tests for py_binary.""" + +load("//python:defs.bzl", "py_binary") +load( + "//tools/build_defs/python/tests:py_executable_base_tests.bzl", + "create_executable_tests", +) + +def py_binary_test_suite(name): + config = struct(rule = py_binary) + + native.test_suite( + name = name, + tests = create_executable_tests(config), + ) diff --git a/tools/build_defs/python/tests/py_executable_base_tests.bzl b/tools/build_defs/python/tests/py_executable_base_tests.bzl new file mode 100644 index 0000000000..c66ea11e00 --- /dev/null +++ b/tools/build_defs/python/tests/py_executable_base_tests.bzl @@ -0,0 +1,272 @@ +# Copyright 2023 The Bazel Authors. All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +"""Tests common to py_binary and py_test (executable rules).""" + +load("@rules_testing//lib:analysis_test.bzl", "analysis_test") +load("@rules_testing//lib:truth.bzl", "matching") +load("@rules_testing//lib:util.bzl", rt_util = "util") +load("//tools/build_defs/python/tests:base_tests.bzl", "create_base_tests") +load("//tools/build_defs/python/tests:util.bzl", "WINDOWS_ATTR", pt_util = "util") + +_tests = [] + +def _test_executable_in_runfiles(name, config): + rt_util.helper_target( + config.rule, + name = name + "_subject", + srcs = [name + "_subject.py"], + ) + analysis_test( + name = name, + impl = _test_executable_in_runfiles_impl, + target = name + "_subject", + attrs = WINDOWS_ATTR, + ) + +_tests.append(_test_executable_in_runfiles) + +def _test_executable_in_runfiles_impl(env, target): + if pt_util.is_windows(env): + exe = ".exe" + else: + exe = "" + + env.expect.that_target(target).runfiles().contains_at_least([ + "{workspace}/{package}/{test_name}_subject" + exe, + ]) + +def _test_default_main_can_be_generated(name, config): + rt_util.helper_target( + config.rule, + name = name + "_subject", + srcs = [rt_util.empty_file(name + "_subject.py")], + ) + analysis_test( + name = name, + impl = _test_default_main_can_be_generated_impl, + target = name + "_subject", + ) + +_tests.append(_test_default_main_can_be_generated) + +def _test_default_main_can_be_generated_impl(env, target): + env.expect.that_target(target).default_outputs().contains( + "{package}/{test_name}_subject.py", + ) + +def _test_default_main_can_have_multiple_path_segments(name, config): + rt_util.helper_target( + config.rule, + name = name + "/subject", + srcs = [name + "/subject.py"], + ) + analysis_test( + name = name, + impl = _test_default_main_can_have_multiple_path_segments_impl, + target = name + "/subject", + ) + +_tests.append(_test_default_main_can_have_multiple_path_segments) + +def _test_default_main_can_have_multiple_path_segments_impl(env, target): + env.expect.that_target(target).default_outputs().contains( + "{package}/{test_name}/subject.py", + ) + +def _test_default_main_must_be_in_srcs(name, config): + # Bazel 5 will crash with a Java stacktrace when the native Python + # rules have an error. + if not pt_util.is_bazel_6_or_higher(): + rt_util.skip_test(name = name) + return + rt_util.helper_target( + config.rule, + name = name + "_subject", + srcs = ["other.py"], + ) + analysis_test( + name = name, + impl = _test_default_main_must_be_in_srcs_impl, + target = name + "_subject", + expect_failure = True, + ) + +_tests.append(_test_default_main_must_be_in_srcs) + +def _test_default_main_must_be_in_srcs_impl(env, target): + env.expect.that_target(target).failures().contains_predicate( + matching.str_matches("default*does not appear in srcs"), + ) + +def _test_default_main_cannot_be_ambiguous(name, config): + # Bazel 5 will crash with a Java stacktrace when the native Python + # rules have an error. + if not pt_util.is_bazel_6_or_higher(): + rt_util.skip_test(name = name) + return + rt_util.helper_target( + config.rule, + name = name + "_subject", + srcs = [name + "_subject.py", "other/{}_subject.py".format(name)], + ) + analysis_test( + name = name, + impl = _test_default_main_cannot_be_ambiguous_impl, + target = name + "_subject", + expect_failure = True, + ) + +_tests.append(_test_default_main_cannot_be_ambiguous) + +def _test_default_main_cannot_be_ambiguous_impl(env, target): + env.expect.that_target(target).failures().contains_predicate( + matching.str_matches("default main*matches multiple files"), + ) + +def _test_explicit_main(name, config): + rt_util.helper_target( + config.rule, + name = name + "_subject", + srcs = ["custom.py"], + main = "custom.py", + ) + analysis_test( + name = name, + impl = _test_explicit_main_impl, + target = name + "_subject", + ) + +_tests.append(_test_explicit_main) + +def _test_explicit_main_impl(env, target): + # There isn't a direct way to ask what main file was selected, so we + # rely on it being in the default outputs. + env.expect.that_target(target).default_outputs().contains( + "{package}/custom.py", + ) + +def _test_explicit_main_cannot_be_ambiguous(name, config): + # Bazel 5 will crash with a Java stacktrace when the native Python + # rules have an error. + if not pt_util.is_bazel_6_or_higher(): + rt_util.skip_test(name = name) + return + rt_util.helper_target( + config.rule, + name = name + "_subject", + srcs = ["x/foo.py", "y/foo.py"], + main = "foo.py", + ) + analysis_test( + name = name, + impl = _test_explicit_main_cannot_be_ambiguous_impl, + target = name + "_subject", + expect_failure = True, + ) + +_tests.append(_test_explicit_main_cannot_be_ambiguous) + +def _test_explicit_main_cannot_be_ambiguous_impl(env, target): + env.expect.that_target(target).failures().contains_predicate( + matching.str_matches("foo.py*matches multiple"), + ) + +def _test_files_to_build(name, config): + rt_util.helper_target( + config.rule, + name = name + "_subject", + srcs = [name + "_subject.py"], + ) + analysis_test( + name = name, + impl = _test_files_to_build_impl, + target = name + "_subject", + attrs = WINDOWS_ATTR, + ) + +_tests.append(_test_files_to_build) + +def _test_files_to_build_impl(env, target): + default_outputs = env.expect.that_target(target).default_outputs() + if pt_util.is_windows(env): + default_outputs.contains("{package}/{test_name}_subject.exe") + else: + default_outputs.contains_exactly([ + "{package}/{test_name}_subject", + "{package}/{test_name}_subject.py", + ]) + +def _test_name_cannot_end_in_py(name, config): + # Bazel 5 will crash with a Java stacktrace when the native Python + # rules have an error. + if not pt_util.is_bazel_6_or_higher(): + rt_util.skip_test(name = name) + return + rt_util.helper_target( + config.rule, + name = name + "_subject.py", + srcs = ["main.py"], + ) + analysis_test( + name = name, + impl = _test_name_cannot_end_in_py_impl, + target = name + "_subject.py", + expect_failure = True, + ) + +_tests.append(_test_name_cannot_end_in_py) + +def _test_name_cannot_end_in_py_impl(env, target): + env.expect.that_target(target).failures().contains_predicate( + matching.str_matches("name must not end in*.py"), + ) + +# Can't test this -- mandatory validation happens before analysis test +# can intercept it +# TODO(#1069): Once re-implemented in Starlark, modify rule logic to make this +# testable. +# def _test_srcs_is_mandatory(name, config): +# rt_util.helper_target( +# config.rule, +# name = name + "_subject", +# ) +# analysis_test( +# name = name, +# impl = _test_srcs_is_mandatory, +# target = name + "_subject", +# expect_failure = True, +# ) +# +# _tests.append(_test_srcs_is_mandatory) +# +# def _test_srcs_is_mandatory_impl(env, target): +# env.expect.that_target(target).failures().contains_predicate( +# matching.str_matches("mandatory*srcs"), +# ) + +# ===== +# You were gonna add a test at the end, weren't you? +# Nope. Please keep them sorted; put it in its alphabetical location. +# Here's the alphabet so you don't have to sing that song in your head: +# A B C D E F G H I J K L M N O P Q R S T U V W X Y Z +# ===== + +def create_executable_tests(config): + def _executable_with_srcs_wrapper(name, **kwargs): + if not kwargs.get("srcs"): + kwargs["srcs"] = [name + ".py"] + config.rule(name = name, **kwargs) + + config = pt_util.struct_with(config, base_test_rule = _executable_with_srcs_wrapper) + return pt_util.create_tests(_tests, config = config) + create_base_tests(config = config) diff --git a/tools/build_defs/python/tests/py_info_subject.bzl b/tools/build_defs/python/tests/py_info_subject.bzl new file mode 100644 index 0000000000..20185e55e4 --- /dev/null +++ b/tools/build_defs/python/tests/py_info_subject.bzl @@ -0,0 +1,95 @@ +# Copyright 2023 The Bazel Authors. All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +"""PyInfo testing subject.""" + +load("@rules_testing//lib:truth.bzl", "subjects") + +def py_info_subject(info, *, meta): + """Creates a new `PyInfoSubject` for a PyInfo provider instance. + + Method: PyInfoSubject.new + + Args: + info: The PyInfo object + meta: ExpectMeta object. + + Returns: + A `PyInfoSubject` struct + """ + + # buildifier: disable=uninitialized + public = struct( + # go/keep-sorted start + has_py2_only_sources = lambda *a, **k: _py_info_subject_has_py2_only_sources(self, *a, **k), + has_py3_only_sources = lambda *a, **k: _py_info_subject_has_py3_only_sources(self, *a, **k), + imports = lambda *a, **k: _py_info_subject_imports(self, *a, **k), + transitive_sources = lambda *a, **k: _py_info_subject_transitive_sources(self, *a, **k), + uses_shared_libraries = lambda *a, **k: _py_info_subject_uses_shared_libraries(self, *a, **k), + # go/keep-sorted end + ) + self = struct( + actual = info, + meta = meta, + ) + return public + +def _py_info_subject_has_py2_only_sources(self): + """Returns a `BoolSubject` for the `has_py2_only_sources` attribute. + + Method: PyInfoSubject.has_py2_only_sources + """ + return subjects.bool( + self.actual.has_py2_only_sources, + meta = self.meta.derive("has_py2_only_sources()"), + ) + +def _py_info_subject_has_py3_only_sources(self): + """Returns a `BoolSubject` for the `has_py3_only_sources` attribute. + + Method: PyInfoSubject.has_py3_only_sources + """ + return subjects.bool( + self.actual.has_py3_only_sources, + meta = self.meta.derive("has_py3_only_sources()"), + ) + +def _py_info_subject_imports(self): + """Returns a `CollectionSubject` for the `imports` attribute. + + Method: PyInfoSubject.imports + """ + return subjects.collection( + self.actual.imports, + meta = self.meta.derive("imports()"), + ) + +def _py_info_subject_transitive_sources(self): + """Returns a `DepsetFileSubject` for the `transitive_sources` attribute. + + Method: PyInfoSubject.transitive_sources + """ + return subjects.depset_file( + self.actual.transitive_sources, + meta = self.meta.derive("transitive_sources()"), + ) + +def _py_info_subject_uses_shared_libraries(self): + """Returns a `BoolSubject` for the `uses_shared_libraries` attribute. + + Method: PyInfoSubject.uses_shared_libraries + """ + return subjects.bool( + self.actual.uses_shared_libraries, + meta = self.meta.derive("uses_shared_libraries()"), + ) diff --git a/tools/build_defs/python/tests/py_library/BUILD.bazel b/tools/build_defs/python/tests/py_library/BUILD.bazel new file mode 100644 index 0000000000..9de414b31b --- /dev/null +++ b/tools/build_defs/python/tests/py_library/BUILD.bazel @@ -0,0 +1,18 @@ +# Copyright 2023 The Bazel Authors. All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +"""Tests for py_library.""" + +load(":py_library_tests.bzl", "py_library_test_suite") + +py_library_test_suite(name = "py_library_tests") diff --git a/tools/build_defs/python/tests/py_library/py_library_tests.bzl b/tools/build_defs/python/tests/py_library/py_library_tests.bzl new file mode 100644 index 0000000000..1fcb0c19b9 --- /dev/null +++ b/tools/build_defs/python/tests/py_library/py_library_tests.bzl @@ -0,0 +1,148 @@ +"""Test for py_library.""" + +load("@rules_testing//lib:analysis_test.bzl", "analysis_test") +load("@rules_testing//lib:truth.bzl", "matching") +load("@rules_testing//lib:util.bzl", rt_util = "util") +load("//python:defs.bzl", "PyRuntimeInfo", "py_library") +load("//tools/build_defs/python/tests:base_tests.bzl", "create_base_tests") +load("//tools/build_defs/python/tests:util.bzl", pt_util = "util") + +_tests = [] + +def _test_py_runtime_info_not_present(name, config): + rt_util.helper_target( + config.rule, + name = name + "_subject", + srcs = ["lib.py"], + ) + analysis_test( + name = name, + target = name + "_subject", + impl = _test_py_runtime_info_not_present_impl, + ) + +def _test_py_runtime_info_not_present_impl(env, target): + env.expect.that_bool(PyRuntimeInfo in target).equals(False) + +_tests.append(_test_py_runtime_info_not_present) + +def _test_files_to_build(name, config): + rt_util.helper_target( + config.rule, + name = name + "_subject", + srcs = ["lib.py"], + ) + analysis_test( + name = name, + target = name + "_subject", + impl = _test_files_to_build_impl, + ) + +def _test_files_to_build_impl(env, target): + env.expect.that_target(target).default_outputs().contains_exactly([ + "{package}/lib.py", + ]) + +_tests.append(_test_files_to_build) + +def _test_srcs_can_contain_rule_generating_py_and_nonpy_files(name, config): + rt_util.helper_target( + config.rule, + name = name + "_subject", + srcs = ["lib.py", name + "_gensrcs"], + ) + rt_util.helper_target( + native.genrule, + name = name + "_gensrcs", + cmd = "touch $(OUTS)", + outs = [name + "_gen.py", name + "_gen.cc"], + ) + analysis_test( + name = name, + target = name + "_subject", + impl = _test_srcs_can_contain_rule_generating_py_and_nonpy_files_impl, + ) + +def _test_srcs_can_contain_rule_generating_py_and_nonpy_files_impl(env, target): + env.expect.that_target(target).default_outputs().contains_exactly([ + "{package}/{test_name}_gen.py", + "{package}/lib.py", + ]) + +_tests.append(_test_srcs_can_contain_rule_generating_py_and_nonpy_files) + +def _test_srcs_generating_no_py_files_is_error(name, config): + rt_util.helper_target( + config.rule, + name = name + "_subject", + srcs = [name + "_gen"], + ) + rt_util.helper_target( + native.genrule, + name = name + "_gen", + cmd = "touch $(OUTS)", + outs = [name + "_gen.cc"], + ) + analysis_test( + name = name, + target = name + "_subject", + impl = _test_srcs_generating_no_py_files_is_error_impl, + expect_failure = True, + ) + +def _test_srcs_generating_no_py_files_is_error_impl(env, target): + env.expect.that_target(target).failures().contains_predicate( + matching.str_matches("does not produce*srcs files"), + ) + +_tests.append(_test_srcs_generating_no_py_files_is_error) + +def _test_files_to_compile(name, config): + rt_util.helper_target( + config.rule, + name = name + "_subject", + srcs = ["lib1.py"], + deps = [name + "_lib2"], + ) + rt_util.helper_target( + config.rule, + name = name + "_lib2", + srcs = ["lib2.py"], + deps = [name + "_lib3"], + ) + rt_util.helper_target( + config.rule, + name = name + "_lib3", + srcs = ["lib3.py"], + ) + analysis_test( + name = name, + target = name + "_subject", + impl = _test_files_to_compile_impl, + ) + +def _test_files_to_compile_impl(env, target): + target = env.expect.that_target(target) + target.output_group( + "compilation_prerequisites_INTERNAL_", + ).contains_exactly([ + "{package}/lib1.py", + "{package}/lib2.py", + "{package}/lib3.py", + ]) + target.output_group( + "compilation_outputs", + ).contains_exactly([ + "{package}/lib1.py", + "{package}/lib2.py", + "{package}/lib3.py", + ]) + +_tests.append(_test_files_to_compile) + +def py_library_test_suite(name): + config = struct(rule = py_library, base_test_rule = py_library) + native.test_suite( + name = name, + tests = pt_util.create_tests(_tests, config = config) + create_base_tests(config), + ) diff --git a/tools/build_defs/python/tests/py_test/BUILD.bazel b/tools/build_defs/python/tests/py_test/BUILD.bazel new file mode 100644 index 0000000000..2dc0e5b51d --- /dev/null +++ b/tools/build_defs/python/tests/py_test/BUILD.bazel @@ -0,0 +1,18 @@ +# Copyright 2023 The Bazel Authors. All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +"""Tests for py_test.""" + +load(":py_test_tests.bzl", "py_test_test_suite") + +py_test_test_suite(name = "py_test_tests") diff --git a/tools/build_defs/python/tests/py_test/py_test_tests.bzl b/tools/build_defs/python/tests/py_test/py_test_tests.bzl new file mode 100644 index 0000000000..f2b4875b15 --- /dev/null +++ b/tools/build_defs/python/tests/py_test/py_test_tests.bzl @@ -0,0 +1,98 @@ +# Copyright 2023 The Bazel Authors. All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +"""Tests for py_test.""" + +load("@rules_testing//lib:analysis_test.bzl", "analysis_test") +load("@rules_testing//lib:util.bzl", rt_util = "util") +load("//python:defs.bzl", "py_test") +load( + "//tools/build_defs/python/tests:py_executable_base_tests.bzl", + "create_executable_tests", +) +load("//tools/build_defs/python/tests:util.bzl", pt_util = "util") + +_tests = [] + +def _test_mac_requires_darwin_for_execution(name, config): + # Bazel 5.4 has a bug where every access of testing.ExecutionInfo is + # a different object that isn't equal to any other, which prevents + # rules_testing from detecting it properly and fails with an error. + # This is fixed in Bazel 6+. + if not pt_util.is_bazel_6_or_higher(): + rt_util.skip_test(name = name) + return + + rt_util.helper_target( + config.rule, + name = name + "_subject", + srcs = [name + "_subject.py"], + ) + analysis_test( + name = name, + impl = _test_mac_requires_darwin_for_execution_impl, + target = name + "_subject", + config_settings = { + "//command_line_option:cpu": "darwin_x86_64", + "//command_line_option:crosstool_top": "@rules_python//tools/build_defs/python/tests:cc_toolchain_suite", + #"//command_line_option:platforms": "@rules_python//tools/build_defs/python/tests:mac", + }, + ) + +def _test_mac_requires_darwin_for_execution_impl(env, target): + env.expect.that_target(target).provider( + testing.ExecutionInfo, + ).requirements().keys().contains("requires-darwin") + +_tests.append(_test_mac_requires_darwin_for_execution) + +def _test_non_mac_doesnt_require_darwin_for_execution(name, config): + # Bazel 5.4 has a bug where every access of testing.ExecutionInfo is + # a different object that isn't equal to any other, which prevents + # rules_testing from detecting it properly and fails with an error. + # This is fixed in Bazel 6+. + if not pt_util.is_bazel_6_or_higher(): + rt_util.skip_test(name = name) + return + rt_util.helper_target( + config.rule, + name = name + "_subject", + srcs = [name + "_subject.py"], + ) + analysis_test( + name = name, + impl = _test_non_mac_doesnt_require_darwin_for_execution_impl, + target = name + "_subject", + config_settings = { + "//command_line_option:cpu": "k8", + "//command_line_option:crosstool_top": "@rules_python//tools/build_defs/python/tests:cc_toolchain_suite", + #"//command_line_option:platforms": "@rules_python//tools/build_defs/python/tests:linux", + }, + ) + +def _test_non_mac_doesnt_require_darwin_for_execution_impl(env, target): + # Non-mac builds don't have the provider at all. + if testing.ExecutionInfo not in target: + return + env.expect.that_target(target).provider( + testing.ExecutionInfo, + ).requirements().keys().not_contains("requires-darwin") + +_tests.append(_test_non_mac_doesnt_require_darwin_for_execution) + +def py_test_test_suite(name): + config = struct(rule = py_test) + native.test_suite( + name = name, + tests = pt_util.create_tests(_tests, config = config) + create_executable_tests(config), + ) diff --git a/tools/build_defs/python/tests/util.bzl b/tools/build_defs/python/tests/util.bzl new file mode 100644 index 0000000000..9b386ca3bd --- /dev/null +++ b/tools/build_defs/python/tests/util.bzl @@ -0,0 +1,78 @@ +# Copyright 2023 The Bazel Authors. All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +"""Helpers and utilities multiple tests re-use.""" + +load("@bazel_skylib//lib:structs.bzl", "structs") + +# Use this with is_windows() +WINDOWS_ATTR = {"windows": attr.label(default = "@platforms//os:windows")} + +def _create_tests(tests, **kwargs): + test_names = [] + for func in tests: + test_name = _test_name_from_function(func) + func(name = test_name, **kwargs) + test_names.append(test_name) + return test_names + +def _test_name_from_function(func): + """Derives the name of the given rule implementation function. + + Args: + func: the function whose name to extract + + Returns: + The name of the given function. Note it will have leading and trailing + "_" stripped -- this allows passing a private function and having the + name of the test not start with "_". + """ + + # Starlark currently stringifies a function as "", so we use + # that knowledge to parse the "NAME" portion out. + # NOTE: This is relying on an implementation detail of Bazel + func_name = str(func) + func_name = func_name.partition("")[0] + func_name = func_name.partition(" ")[0] + return func_name.strip("_") + +def _struct_with(s, **kwargs): + struct_dict = structs.to_dict(s) + struct_dict.update(kwargs) + return struct(**struct_dict) + +def _is_bazel_6_or_higher(): + # Bazel 5.4 has a bug where every access of testing.ExecutionInfo is a + # different object that isn't equal to any other. This is fixed in bazel 6+. + return testing.ExecutionInfo == testing.ExecutionInfo + +def _is_windows(env): + """Tell if the target platform is windows. + + This assumes the `WINDOWS_ATTR` attribute was added. + + Args: + env: The test env struct + Returns: + True if the target is Windows, False if not. + """ + constraint = env.ctx.attr.windows[platform_common.ConstraintValueInfo] + return env.ctx.target_platform_has_constraint(constraint) + +util = struct( + create_tests = _create_tests, + struct_with = _struct_with, + is_bazel_6_or_higher = _is_bazel_6_or_higher, + is_windows = _is_windows, +)