From 51aab9be78afb22b1d87dc1e3cb0fb3e9f1e639b Mon Sep 17 00:00:00 2001 From: Michael Carroll Date: Thu, 2 Nov 2023 19:32:53 +0000 Subject: [PATCH 1/9] Move python dep to upstream rules_python Signed-off-by: Michael Carroll --- lint/BUILD.bazel | 11 +++++------ lint/bazel_lint.bzl | 6 +++--- skylark/BUILD.bazel | 8 +------- skylark/build_defs.bzl | 1 + 4 files changed, 10 insertions(+), 16 deletions(-) diff --git a/lint/BUILD.bazel b/lint/BUILD.bazel index 4f00122..9f9af07 100644 --- a/lint/BUILD.bazel +++ b/lint/BUILD.bazel @@ -1,13 +1,12 @@ -package(default_visibility = ["//visibility:public"]) - -load("@gz//bazel/lint:lint.bzl", "add_lint_tests") load( - "@gz//bazel/skylark:gz_py.bzl", + "@gz//bazel/skylark:build_defs.bzl", + "add_lint_tests", "gz_py_binary", - "gz_py_library", - "gz_py_unittest", + "gz_py_library" ) +package(default_visibility = ["//visibility:public"]) + gz_py_library( name = "find_data", srcs = ["find_data.py"], diff --git a/lint/bazel_lint.bzl b/lint/bazel_lint.bzl index 49b0fd9..c3b1625 100644 --- a/lint/bazel_lint.bzl +++ b/lint/bazel_lint.bzl @@ -1,7 +1,7 @@ # -*- mode: python -*- # vi: set ft=python : -load("@gz//bazel/skylark:gz_py.bzl", "py_test_isolated") +load("@rules_python//python:defs.bzl", "py_test") #------------------------------------------------------------------------------ # Internal helper; set up test given name and list of files. Will do nothing @@ -17,7 +17,7 @@ def _bazel_lint(name, files, ignore): ignores_as_arg = ["--ignore=" + ",".join(ignores)] locations = ["$(locations %s)" % f for f in files] - py_test_isolated( + py_test( name = name + "_codestyle", size = "small", srcs = ["@gz//bazel/lint:bzlcodestyle"], @@ -27,7 +27,7 @@ def _bazel_lint(name, files, ignore): tags = ["bzlcodestyle", "lint"], ) - py_test_isolated( + py_test( name = name + "_buildifier", size = "small", srcs = ["@gz//bazel/lint:buildifier"], diff --git a/skylark/BUILD.bazel b/skylark/BUILD.bazel index ae7767b..3558e69 100644 --- a/skylark/BUILD.bazel +++ b/skylark/BUILD.bazel @@ -1,12 +1,6 @@ # -*- python -*- -load("@gz//bazel/skylark:gz_py.bzl", "gz_py_binary") - -# Used by :python_env.bzl. -config_setting( - name = "linux", - values = {"cpu": "k8"}, -) +load(":build_defs.bzl", "gz_py_binary") gz_py_binary( name = "gz_configure_file", diff --git a/skylark/build_defs.bzl b/skylark/build_defs.bzl index 64086d8..ad1b200 100644 --- a/skylark/build_defs.bzl +++ b/skylark/build_defs.bzl @@ -40,6 +40,7 @@ GZ_FEATURES = [ cmake_configure_file = _cmake_configure_file gz_configure_header = _gz_configure_header +gz_configure_file = _gz_configure_header gz_export_header = _gz_export_header gz_include_header = _gz_include_header add_lint_tests = _add_lint_tests From 3eb59056a54daf291c5997d466cf6b12c2d21dec Mon Sep 17 00:00:00 2001 From: Michael Carroll Date: Thu, 2 Nov 2023 19:42:03 +0000 Subject: [PATCH 2/9] Lint Signed-off-by: Michael Carroll --- lint/BUILD.bazel | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lint/BUILD.bazel b/lint/BUILD.bazel index 9f9af07..244a914 100644 --- a/lint/BUILD.bazel +++ b/lint/BUILD.bazel @@ -2,7 +2,7 @@ load( "@gz//bazel/skylark:build_defs.bzl", "add_lint_tests", "gz_py_binary", - "gz_py_library" + "gz_py_library", ) package(default_visibility = ["//visibility:public"]) From 18316e1b4d3d5b36d9a91b70c2db08069f9ea3d2 Mon Sep 17 00:00:00 2001 From: Michael Carroll Date: Wed, 9 Aug 2023 23:09:26 +0000 Subject: [PATCH 3/9] Switch host to exec Signed-off-by: Michael Carroll --- BUILD.bazel | 2 ++ skylark/cmake_configure_file.bzl | 2 +- skylark/gz_configure_file.bzl | 2 +- 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/BUILD.bazel b/BUILD.bazel index 4819fe4..9bc967b 100644 --- a/BUILD.bazel +++ b/BUILD.bazel @@ -16,3 +16,5 @@ licenses(["notice"]) exports_files(["LICENSE"]) add_lint_tests() + +exports_files(["LICENSE"]) diff --git a/skylark/cmake_configure_file.bzl b/skylark/cmake_configure_file.bzl index e911168..61ad8e6 100644 --- a/skylark/cmake_configure_file.bzl +++ b/skylark/cmake_configure_file.bzl @@ -38,7 +38,7 @@ _cmake_configure_file_gen = rule( "undefines": attr.string_list(), "cmakelists": attr.label_list(allow_files = True), "cmake_configure_file_py": attr.label( - cfg = "host", + cfg = "exec", executable = True, default = Label("@gz//bazel/skylark:cmake_configure_file"), ), diff --git a/skylark/gz_configure_file.bzl b/skylark/gz_configure_file.bzl index 06d2b38..5bc3f93 100644 --- a/skylark/gz_configure_file.bzl +++ b/skylark/gz_configure_file.bzl @@ -42,7 +42,7 @@ _gz_configure_file_gen = rule( "undefines": attr.string_list(), "cmakelists": attr.label_list(allow_files = True), "gz_configure_file_py": attr.label( - cfg = "host", + cfg = "exec", executable = True, default = Label("@gz//bazel/skylark:gz_configure_file"), ), From 5d42ff99da7b1f7f1defdcf3fd3dbf9455b9523a Mon Sep 17 00:00:00 2001 From: Michael Carroll Date: Thu, 10 Aug 2023 13:27:08 +0000 Subject: [PATCH 4/9] Add support for Debian-based systems Signed-off-by: Michael Carroll --- BUILD.bazel | 2 -- bazel.rc | 3 --- 2 files changed, 5 deletions(-) diff --git a/BUILD.bazel b/BUILD.bazel index 9bc967b..4819fe4 100644 --- a/BUILD.bazel +++ b/BUILD.bazel @@ -16,5 +16,3 @@ licenses(["notice"]) exports_files(["LICENSE"]) add_lint_tests() - -exports_files(["LICENSE"]) diff --git a/bazel.rc b/bazel.rc index 809d79d..673cb0b 100644 --- a/bazel.rc +++ b/bazel.rc @@ -11,13 +11,10 @@ build --force_pic build --strip=never build --strict_system_includes build --workspace_status_command=./bazel/workspace/workspace-status.bash -build --define enable_ifaddrs=true build --fission=dbg build --features=per_object_debug_info -test --define enable_ifaddrs=true - # -- Options for explicitly using GCC. common:gcc --repo_env=CC=gcc common:gcc --repo_env=CXX=g++ From 4b0ae45696725cbf8b699864d11cd70907d3a7dc Mon Sep 17 00:00:00 2001 From: Michael Carroll Date: Thu, 10 Aug 2023 13:39:14 +0000 Subject: [PATCH 5/9] Remove gz-specific python in favor of upstream rules_python Signed-off-by: Michael Carroll --- lint/BUILD.bazel | 2 +- skylark/gz_py.bzl | 260 ----------------------------------------- skylark/py.bzl | 14 --- skylark/python_env.bzl | 22 ---- 4 files changed, 1 insertion(+), 297 deletions(-) delete mode 100644 skylark/gz_py.bzl delete mode 100644 skylark/py.bzl delete mode 100644 skylark/python_env.bzl diff --git a/lint/BUILD.bazel b/lint/BUILD.bazel index 244a914..c2c8b84 100644 --- a/lint/BUILD.bazel +++ b/lint/BUILD.bazel @@ -3,7 +3,7 @@ load( "add_lint_tests", "gz_py_binary", "gz_py_library", -) +t) package(default_visibility = ["//visibility:public"]) diff --git a/skylark/gz_py.bzl b/skylark/gz_py.bzl deleted file mode 100644 index a40c7b5..0000000 --- a/skylark/gz_py.bzl +++ /dev/null @@ -1,260 +0,0 @@ -# -*- python -*- - -load("@gz//bazel/skylark:py.bzl", "py_binary", "py_library", "py_test") - -def gz_py_library( - name, - deps = None, - **kwargs): - """A wrapper to insert Drake-specific customizations.""" - - # Work around https://github.com/bazelbuild/bazel/issues/1567. - deps = (deps or []) - py_library( - name = name, - deps = deps, - srcs_version = "PY3", - **kwargs - ) - -def _disable_test_impl(ctx): - info = dict( - bad_target = ctx.attr.bad_target, - good_target = ctx.attr.good_target, - ) - content = """#!/bin/bash -echo "ERROR: Please use '{good_target}'; the label '{bad_target}'" \ - "has been removed." >&2 -exit 1 -""".format(**info) - ctx.actions.write( - output = ctx.outputs.executable, - content = content, - ) - return [DefaultInfo()] - -# Defines a test which will fail when run via `bazel run` or `bazel test`, -# pointing the user to the correct binary to use. This should typically have -# a "manual" tag. -_disable_test = rule( - attrs = { - "bad_target": attr.string(mandatory = True), - "good_target": attr.string(mandatory = True), - }, - test = True, - implementation = _disable_test_impl, -) - -def _py_target_isolated( - name, - py_target = None, - srcs = None, - main = None, - isolate = True, - visibility = None, - **kwargs): - # See #8041 for more details. - # TODO(eric.cousineau): See if we can remove these shims once we stop - # supporting Python 2 (#10606). - if py_target == None: - fail("Must supply macro function for defining `py_target`.") - - # Do not isolate targets that are already isolated. This generally happens - # when linting tests (which are isolated) are invoked for isolated Python - # targets. Without this check, the actual test turns into - # `py/py/{name}`. - prefix = "py/" - if isolate and not name.startswith(prefix): - actual = prefix + name - - # Preserve original functionality. - if not main: - main = name + ".py" - if not srcs: - srcs = [name + ".py"] - py_target( - name = actual, - srcs = srcs, - main = main, - visibility = visibility, - **kwargs - ) - - # Disable and redirect original name. - package_prefix = "//" + native.package_name() + ":" - - # N.B. We make the disabled rule a test, even if the original was not. - # This ensures that developers will see the redirect using both - # `bazel run` or `bazel test`. - _disable_test( - name = name, - good_target = package_prefix + actual, - bad_target = package_prefix + name, - tags = ["manual"], - visibility = visibility, - ) - else: - py_target( - name = name, - srcs = srcs, - main = main, - visibility = visibility, - **kwargs - ) - -def gz_py_binary( - name, - srcs = None, - main = None, - data = [], - deps = None, - isolate = False, - tags = [], - add_test_rule = 0, - test_rule_args = [], - test_rule_data = [], - test_rule_tags = None, - test_rule_size = None, - test_rule_timeout = None, - test_rule_flaky = 0, - **kwargs): - """A wrapper to insert Drake-specific customizations. - - @param isolate (optional, default is False) - If True, the binary will be placed in a folder isolated from the - library code. This prevents submodules from leaking in as top-level - submodules. For more detail, see #8041. - """ - - # Work around https://github.com/bazelbuild/bazel/issues/1567. - deps = deps or [] - if main == None and len(srcs) == 1: - main = srcs[0] - _py_target_isolated( - name = name, - py_target = py_binary, - isolate = isolate, - srcs = srcs, - main = main, - data = data, - deps = deps, - tags = tags, - python_version = "PY3", - srcs_version = "PY3", - **kwargs - ) - if add_test_rule: - gz_py_test( - name = name + "_test", - srcs = srcs, - main = main, - deps = deps, - isolate = isolate, - args = test_rule_args, - data = data + test_rule_data, - size = test_rule_size, - timeout = test_rule_timeout, - flaky = test_rule_flaky, - tags = (test_rule_tags or []) + ["nolint"], - # N.B. Same as the warning in `drake_pybind_cc_googletest`: numpy - # imports unittest unconditionally. - allow_import_unittest = True, - **kwargs - ) - -def gz_py_unittest( - name, - **kwargs): - """Declares a `unittest`-based python test. - - This macro should be preferred instead of the basic drake_py_test for tests - that use the `unittest` framework. Tests that use this macro should *not* - contain a __main__ handler nor a shebang line. By default, sets test size - to "small" to indicate a unit test. - """ - helper = "//common/test_utilities:gz_py_unittest_main.py" - if kwargs.pop("srcs", None): - fail("Changing srcs= is not allowed by gz_py_unittest." + - " Use gz_py_test instead, if you need something weird.") - srcs = ["test/%s.py" % name, helper] - gz_py_test( - name = name, - srcs = srcs, - main = helper, - allow_import_unittest = True, - _gz_py_unittest_shard_count = kwargs.pop("shard_count", None), - deps = kwargs.pop("deps", []) + [ - "@xmlrunner_py", - ], - **kwargs - ) - -def gz_py_test( - name, - size = None, - srcs = None, - deps = None, - isolate = True, - allow_import_unittest = False, - tags = [], - **kwargs): - """A wrapper to insert Drake-specific customizations. - - @param isolate (optional, default is True) - If True, the test binary will be placed in a folder isolated from the - library code. This prevents submodules from leaking in as top-level - submodules. For more detail, see #8041. - - @param allow_import_unittest (optional, default is False) - If False, this test (and anything it imports) is prevented from doing - `import unittest`. This is a guard against writing `unittest`-based - cases that accidentally never get run. In general, `unittest`-based - tests should use the `drake_py_unittest` macro instead of this one - (thus disabling this interlock), but can override this parameter in - case something unique is happening and the other macro can't be used. - - By default, sets test size to "small" to indicate a unit test. Adds the tag - "py" if not already present. - - This macro does not allow a shard_count; use drake_py_unittest for that. - """ - if size == None: - size = "small" - if srcs == None: - srcs = ["test/%s.py" % name] - if kwargs.get("shard_count") != None: - fail("Only gz_py_unittest can use sharding") - shard_count = kwargs.pop("_gz_py_unittest_shard_count", None) - - # Work around https://github.com/bazelbuild/bazel/issues/1567. - deps = deps or [] - if not allow_import_unittest: - deps = deps + ["//common/test_utilities:disable_python_unittest"] - if "py" not in tags: - tags = tags + ["py"] - _py_target_isolated( - name = name, - py_target = py_test, - isolate = isolate, - size = size, - shard_count = shard_count, - srcs = srcs, - deps = deps, - tags = tags, - python_version = "PY3", - srcs_version = "PY3", - **kwargs - ) - -def py_test_isolated( - name, - **kwargs): - """Provides a directory-isolated Python test, robust against shadowing - (#8041). - """ - _py_target_isolated( - name = name, - py_target = py_test, - isolate = True, - **kwargs - ) diff --git a/skylark/py.bzl b/skylark/py.bzl deleted file mode 100644 index 0df5415..0000000 --- a/skylark/py.bzl +++ /dev/null @@ -1,14 +0,0 @@ -# -*- python -*- - -load( - "@rules_python//python:defs.bzl", - _py_binary = "py_binary", - _py_library = "py_library", - _py_test = "py_test", -) - -py_binary = _py_binary - -py_library = _py_library - -py_test = _py_test diff --git a/skylark/python_env.bzl b/skylark/python_env.bzl deleted file mode 100644 index e39b9e4..0000000 --- a/skylark/python_env.bzl +++ /dev/null @@ -1,22 +0,0 @@ -# -*- python -*- - -def hermetic_python_env(): - # In general, we do not want to use Python's "user site-packages" - # (e.g., $HOME/.local) directory because it's not hermetic. Thus, - # we set PYTHONNOUSERSITE to disable the user site-packages. - # - # However, our macOS setup instructions provide for some dependencies - # (e.g., PyYAML) to come from pip, and in some reasonable configurations it - # could be done via `pip install --user` and so be part of $HOME. Thus, in - # order to support that configuration, we only set PYTHONNOUSERSITE under - # linux. We can revisit this decision if we changes how python packages on - # macOS are brought into the workspace. - # - # If https://github.com/bazelbuild/bazel/issues/4939 gets fixed, we can - # revisit whether manually specifying a hermetic env is still necessary. - return select({ - "@gz//bazel/skylark:linux": { - "PYTHONNOUSERSITE": "1", - }, - "//conditions:default": {}, - }) From 36595a16842b57cac972edbdcb0f5556722cd6dc Mon Sep 17 00:00:00 2001 From: Michael Carroll Date: Thu, 10 Aug 2023 16:53:15 +0000 Subject: [PATCH 6/9] Fix cmake substitution Signed-off-by: Michael Carroll --- skylark/gz_configure_file.py | 13 +++++++------ workspace/workspace-status.bash | 5 ++++- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/skylark/gz_configure_file.py b/skylark/gz_configure_file.py index 4abda12..9cb5d22 100644 --- a/skylark/gz_configure_file.py +++ b/skylark/gz_configure_file.py @@ -39,11 +39,11 @@ def _transform(line, definitions): blank, maybe01, var, rest, newline = match.groups() defined = definitions.get(var) is not None if maybe01: - return blank + '#define ' + var + [' 0', ' 1'][defined] + newline + return blank + '#define ' + str(var) + [' 0', ' 1'][defined] + newline elif defined: - line = blank + '#define ' + var + rest + newline + line = blank + '#define ' + str(var) + rest + newline else: - return blank + '/* #undef ' + var + ' */' + newline + return blank + '/* #undef ' + str(var) + ' */' + newline # Replace variable substitutions. while True: @@ -51,6 +51,7 @@ def _transform(line, definitions): if not match: break before, xvarx, after, newline = match.groups() + var = '' if xvarx[0] == '$': assert len(xvarx) >= 4 assert xvarx[1] == '{' @@ -60,14 +61,15 @@ def _transform(line, definitions): assert len(xvarx) >= 3 assert xvarx[-1] == '@' var = xvarx[1:-1] - assert len(var) > 0 + assert var if var not in definitions: raise KeyError('Missing definition for ' + var) value = definitions.get(var) + if value is None: value = '' - line = before + value + after + newline + line = before + str(value) + after + newline return line @@ -189,4 +191,3 @@ def main(): if __name__ == "__main__": main() - diff --git a/workspace/workspace-status.bash b/workspace/workspace-status.bash index 89da63b..ff1cbb3 100755 --- a/workspace/workspace-status.bash +++ b/workspace/workspace-status.bash @@ -12,9 +12,12 @@ LIBS=( plugin rendering sensors + sim + tools transport utils - sdformat) + sdformat +) for LIB in "${LIBS[@]}" do From b5e85b23bd61a9d3028ffa349327f61585b0ca33 Mon Sep 17 00:00:00 2001 From: Michael Carroll Date: Tue, 31 Oct 2023 13:39:01 +0000 Subject: [PATCH 7/9] Use msgs branch for now Signed-off-by: Michael Carroll --- example/bazel.repos | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/example/bazel.repos b/example/bazel.repos index 48df445..da98c78 100644 --- a/example/bazel.repos +++ b/example/bazel.repos @@ -22,7 +22,7 @@ repositories: msgs: type: git url: https://github.com/gazebosim/gz-msgs - version: gz-msgs9 + version: mjcarroll/garden_bazel physics: type: git url: https://github.com/gazebosim/gz-physics From 587f42840fb1ec54bcf5cf1a3029e14f40fc6da6 Mon Sep 17 00:00:00 2001 From: Michael Carroll Date: Thu, 21 Sep 2023 14:14:50 +0000 Subject: [PATCH 8/9] Add rules_license for checking package licenses Signed-off-by: Michael Carroll --- example/bazel.repos | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/example/bazel.repos b/example/bazel.repos index da98c78..a2407cc 100644 --- a/example/bazel.repos +++ b/example/bazel.repos @@ -14,7 +14,7 @@ repositories: fuel_tools: type: git url: https://github.com/gazebosim/gz-fuel-tools - version: gz-fuel-tools8 + version: gz-fuel-tools9 math: type: git url: https://github.com/gazebosim/gz-math From 77dbf5cbb0023307bd7239dad35892e83dbda0ec Mon Sep 17 00:00:00 2001 From: Michael Carroll Date: Tue, 31 Oct 2023 14:34:19 +0000 Subject: [PATCH 9/9] Incorrect fuel version Signed-off-by: Michael Carroll --- example/bazel.repos | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/example/bazel.repos b/example/bazel.repos index a2407cc..da98c78 100644 --- a/example/bazel.repos +++ b/example/bazel.repos @@ -14,7 +14,7 @@ repositories: fuel_tools: type: git url: https://github.com/gazebosim/gz-fuel-tools - version: gz-fuel-tools9 + version: gz-fuel-tools8 math: type: git url: https://github.com/gazebosim/gz-math