Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

First step towards using bzlmod #1570

Closed
wants to merge 1 commit into from

Conversation

hzeller
Copy link
Member

@hzeller hzeller commented Aug 28, 2024

  • Move projects that are available already in https://registry.bazel.build/ from from load_external to MODULE.bazel.
  • This is not complete: there are probably more steps to simplify and clean-up after the first submit.
  • Switching to bzlmod broke making the compilation-db as the action hooks don't seem to work anymore ( xls/dev_tools//make-compilation-db.sh ). The used com_grail_bazel_compdb is not maintained anymore, so this needs to be updated to something more modern (hedronvision?). Since this is not actively needed in daily development (and I am the only one really using it), postponed for later.

Issues: #931

Also, needed to make Python rules to work well with --incompatible_default_to_explicit_init_py (bazelbuild/bazel#10076)

@hzeller
Copy link
Member Author

hzeller commented Aug 28, 2024

Continuation of #1565

@hzeller hzeller force-pushed the feature-20240828-bzlmod2 branch 5 times, most recently from 8762e52 to 8804821 Compare August 28, 2024 18:06
@hzeller
Copy link
Member Author

hzeller commented Aug 28, 2024

@cdleary : PR moved to here.

Copy link
Collaborator

@ericastor ericastor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, though it sounds like we should wait for @cdleary to respond.

@hzeller hzeller force-pushed the feature-20240828-bzlmod2 branch 2 times, most recently from 6c88c96 to 5fd46c1 Compare August 28, 2024 21:20
@proppy
Copy link
Member

proppy commented Aug 30, 2024

on macosx (14.6.1) when doing bazel build //..., there are a few warnings related to python:

DEBUG: /private/var/tmp/_bazel_proppy/243779b655cdfb332dd4fc5af63d7557/external/rules_python~/python/private/bzlmod/python.bzl:46:10: WARNING: Ignoring toolchain 'python_3_8' from module 'protobuf': Toolchain 'python_3_8' from module 'grpc' already registered Python version 3.8 and has precedence
DEBUG: /private/var/tmp/_bazel_proppy/243779b655cdfb332dd4fc5af63d7557/external/rules_python~/python/private/bzlmod/python.bzl:46:10: WARNING: Ignoring toolchain 'python_3_9' from module 'protobuf': Toolchain 'python_3_9' from module 'grpc' already registered Python version 3.9 and has precedence
DEBUG: /private/var/tmp/_bazel_proppy/243779b655cdfb332dd4fc5af63d7557/external/rules_python~/python/private/bzlmod/python.bzl:46:10: WARNING: Ignoring toolchain 'python_3_10' from module 'protobuf': Toolchain 'python_3_10' from module 'grpc' already registered Python version 3.10 and has precedence
DEBUG: /private/var/tmp/_bazel_proppy/243779b655cdfb332dd4fc5af63d7557/external/rules_python~/python/private/bzlmod/python.bzl:46:10: WARNING: Ignoring toolchain 'python_3_11' from module 'protobuf': Toolchain 'python_3_11' from module 'grpc' already registered Python version 3.11 and has precedence
DEBUG: /private/var/tmp/_bazel_proppy/243779b655cdfb332dd4fc5af63d7557/external/rules_python~/python/private/bzlmod/python.bzl:46:10: WARNING: Ignoring toolchain 'python_3_12' from module 'protobuf': Toolchain 'python_3_12' from module 'grpc' already registered Python version 3.12 and has precedence
DEBUG: /private/var/tmp/_bazel_proppy/243779b655cdfb332dd4fc5af63d7557/external/rules_python~/python/private/bzlmod/python.bzl:46:10: WARNING: Ignoring toolchain 'python_3_8' from module 'riegeli': Toolchain 'python_3_8' from module 'grpc' already registered Python version 3.8 and has precedence
DEBUG: /private/var/tmp/_bazel_proppy/243779b655cdfb332dd4fc5af63d7557/external/rules_python~/python/private/bzlmod/python.bzl:46:10: WARNING: Ignoring toolchain 'python_3_9' from module 'riegeli': Toolchain 'python_3_9' from module 'grpc' already registered Python version 3.9 and has precedence
DEBUG: /private/var/tmp/_bazel_proppy/243779b655cdfb332dd4fc5af63d7557/external/rules_python~/python/private/bzlmod/python.bzl:46:10: WARNING: Ignoring toolchain 'python_3_10' from module 'riegeli': Toolchain 'python_3_10' from module 'grpc' already registered Python version 3.10 and has precedence
DEBUG: /private/var/tmp/_bazel_proppy/243779b655cdfb332dd4fc5af63d7557/external/rules_python~/python/private/bzlmod/python.bzl:46:10: WARNING: Ignoring toolchain 'python_3_11' from module 'riegeli': Toolchain 'python_3_11' from module 'grpc' already registered Python version 3.11 and has precedence
DEBUG: /private/var/tmp/_bazel_proppy/243779b655cdfb332dd4fc5af63d7557/external/rules_python~/python/private/bzlmod/python.bzl:46:10: WARNING: Ignoring toolchain 'python_3_12' from module 'riegeli': Toolchain 'python_3_12' from module 'grpc' already registered Python version 3.12 and has precedence
DEBUG: /private/var/tmp/_bazel_proppy/243779b655cdfb332dd4fc5af63d7557/external/rules_python~/python/private/bzlmod/python.bzl:46:10: WARNING: Ignoring toolchain 'python_3_11' from module 'rules_python': Toolchain 'python_3_11' from module 'grpc' already registered Python version 3.11 and has precedence
DEBUG: /private/var/tmp/_bazel_proppy/243779b655cdfb332dd4fc5af63d7557/external/rules_python~/python/private/bzlmod/python.bzl:46:10: WARNING: Ignoring toolchain 'python_3_8' from module 'rules_fuzzing': Toolchain 'python_3_8' from module 'grpc' already registered Python version 3.8 and has precedence
DEBUG: /private/var/tmp/_bazel_proppy/243779b655cdfb332dd4fc5af63d7557/external/rules_python~/python/private/bzlmod/python.bzl:46:10: WARNING: Ignoring toolchain 'python_3_9' from module 'rules_fuzzing': Toolchain 'python_3_9' from module 'grpc' already registered Python version 3.9 and has precedence
DEBUG: /private/var/tmp/_bazel_proppy/243779b655cdfb332dd4fc5af63d7557/external/rules_python~/python/private/bzlmod/python.bzl:46:10: WARNING: Ignoring toolchain 'python_3_10' from module 'rules_fuzzing': Toolchain 'python_3_10' from module 'grpc' already registered Python version 3.10 and has precedence
DEBUG: /private/var/tmp/_bazel_proppy/243779b655cdfb332dd4fc5af63d7557/external/rules_python~/python/private/bzlmod/python.bzl:46:10: WARNING: Ignoring toolchain 'python_3_11' from module 'rules_fuzzing': Toolchain 'python_3_11' from module 'grpc' already registered Python version 3.11 and has precedence
DEBUG: /private/var/tmp/_bazel_proppy/243779b655cdfb332dd4fc5af63d7557/external/rules_python~/python/private/bzlmod/python.bzl:46:10: WARNING: Ignoring toolchain 'python_3_12' from module 'rules_fuzzing': Toolchain 'python_3_12' from module 'grpc' already registered Python version 3.12 and has precedence

and a hard dependency missing on sha265sum (on macosx it's apparently spelled shasum)

ERROR: /Users/proppy/src/github.com/google/xls/xls/build_rules/tests/BUILD:81:23: Checking sha256sum on file: bazel-out/darwin_arm64-fastbuild/bin/xls/build_rules/tests/generated.x failed: (Exit 255): bash failed: error executing CheckSHA256SumFrozen command (from target //xls/build_rules/tests:generated_x_sha256sum_frozen) /bin/bash -c ... (remaining 1 argument skipped)

the later looks orthogonal to this change, as this rule was already present on main:

"#!/usr/bin/env bash",
# Note two spaces is required between the sha256sum and filename to
# comply with the sha256sum input format.
"echo \"{} {}\" | sha256sum -c -".format(
ctx.attr.sha256sum,
src.short_path,
),

then tried to switch to the local llvm toolchain (similar to what @cdleary did in 76d4ad5):

diff --git a/MODULE.bazel b/MODULE.bazel
index a06dbb3cf..7a92ff031 100644
--- a/MODULE.bazel
+++ b/MODULE.bazel
@@ -6,15 +6,6 @@ module(
 # Compiler toolchain
 bazel_dep(name = "toolchains_llvm", version = "1.1.2")
 
-# Configure and register the toolchain.
-llvm = use_extension("@toolchains_llvm//toolchain/extensions:llvm.bzl", "llvm")
-llvm.toolchain(
-    llvm_version = "17.0.6",
-)
-use_repo(llvm, "llvm_toolchain")
-
-register_toolchains("@llvm_toolchain//:all")
-
 #
 bazel_dep(name = "abseil-cpp", version = "20240116.2", repo_name = "com_google_absl")
 bazel_dep(name = "abseil-py", version = "2.1.0", repo_name = "com_google_absl_py")
diff --git a/xls/build_rules/xls_ir_rules.bzl b/xls/build_rules/xls_ir_rules.bzl
index 7346b2138..897c6427e 100644
--- a/xls/build_rules/xls_ir_rules.bzl
+++ b/xls/build_rules/xls_ir_rules.bzl
@@ -1247,7 +1247,6 @@ xls_ir_cc_library = rule(
                 executable = True,
                 allow_files = True,
                 cfg = "exec",
-                default = Label("@llvm_toolchain//:clang-format"),
             ),
         },
     ),
diff --git a/xls/delay_model/build_defs.bzl b/xls/delay_model/build_defs.bzl
index adc956c3a..d0f43fe3f 100644
--- a/xls/delay_model/build_defs.bzl
+++ b/xls/delay_model/build_defs.bzl
@@ -47,11 +47,9 @@ def delay_model(
         outs = ["{}.cc".format(name)],
         cmd = ("$(location //xls/delay_model:generate_delay_lookup) " +
                "--model_name={model_name} --precedence={precedence} $< " +
-               "| $(location @llvm_toolchain//:clang-format)" +
                " > $(OUTS)").format(model_name = model_name, precedence = precedence),
         tools = [
             "//xls/delay_model:generate_delay_lookup",
-            "@llvm_toolchain//:clang-format",
         ],
         **kwargs
     )

and was a able to build a restricted set of targets:

xls 🍡 bazel build -c opt -- //xls/dslx:interpreter_main //xls/dslx/ir_convert:ir_converter_main //xls/tools:opt_main //xls/tools:codegen_main
...
INFO: From Linking xls/dslx/interpreter_main:
ld: warning: ignoring duplicate libraries: '-lm', '-lpthread'
INFO: Found 4 targets...
bazel: Leaving directory `/private/var/tmp/_bazel_proppy/243779b655cdfb332dd4fc5af63d7557/execroot/_main/'
INFO: Elapsed time: 811.461s, Critical Path: 43.87s
INFO: 10975 processes: 3825 internal, 7150 darwin-sandbox.
INFO: Build completed successfully, 10975 total actions

@hzeller
Copy link
Member Author

hzeller commented Aug 30, 2024

  • The Python warnings also happen on Linux, it is just an artifact of the dependency resolution in bzlmod which we can ignore.
  • sha256sum: maybe this is one of the instances in which bazel attempts to cut down on the shell utilities it supplies by default. Maybe we already compile sha256 from C-source somewhere anyway and should use that (but not in this change)
  • llvm: I suspect the equivalent has to be done before and after this change, so would not be influenced by this change.

But essentially what you say is looks good, regular SNAFU (in its original sense), but not impacted by this change.

@hzeller
Copy link
Member Author

hzeller commented Aug 30, 2024

@cdleary : good to submit ?

 * Move projects that are available already in https://registry.bazel.build/
   from from load_external to MODULE.bazel.
 * This is not complete: there are probably more steps to simplify and
   clean-up after the first submit.
 * Switching to bzlmod broke making the compilation-db as the action hooks
   don't seem to work anymore ( xls/dev_tools//make-compilation-db.sh ).
   The used com_grail_bazel_compdb is not maintained anymore, so this needs
   to be updated to something more modern (hedronvision?).
   Since this is not actively needed in daily development (and I am the only
   one really using it), postponed for later.
 * Clean out things not needed anymore in dependency_support/$(various-dirs)

Issues: google#931

Also, needed to make Python rules to work well with
--incompatible_default_to_explicit_init_py (bazelbuild/bazel#10076)
@hzeller
Copy link
Member Author

hzeller commented Sep 3, 2024

(Postponing for later)

@hzeller hzeller closed this Sep 3, 2024
@hzeller hzeller mentioned this pull request Sep 3, 2024
hzeller added a commit that referenced this pull request Sep 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants