diff --git a/.bazelci/postsubmit.yml b/.bazelci/postsubmit.yml index e4d5e07eadbfef..f0a1eef5a12a4a 100644 --- a/.bazelci/postsubmit.yml +++ b/.bazelci/postsubmit.yml @@ -55,6 +55,13 @@ tasks: - "-//src/test/shell/bazel:bazel_coverage_cc_released_test_gcc" - "-//src/test/shell/bazel:bazel_coverage_cc_head_test_gcc" - "-//src/test/shell/bazel:bazel_coverage_sh_test" + # https://github.com/bazelbuild/bazel/issues/18776 + - "-//src/test/shell/bazel/android:android_instrumentation_test_integration_test" + - "-//src/test/shell/bazel/android:android_instrumentation_test_integration_test_with_head_android_tools" + - "-//src/test/shell/bazel/android:android_instrumentation_test_integration_test_with_platforms" + - "-//src/test/shell/bazel/android:aapt_integration_test" + - "-//src/test/shell/bazel/android:aapt_integration_test_with_head_android_tools" + - "-//src/test/shell/bazel/android:aapt_integration_test_with_platforms" include_json_profile: - build - test @@ -95,6 +102,13 @@ tasks: - "//tools/python/..." # Re-enable once fixed: https://github.com/bazelbuild/bazel/issues/8162 - "-//src/java_tools/import_deps_checker/..." + # https://github.com/bazelbuild/bazel/issues/18776 + - "-//src/test/shell/bazel/android:android_instrumentation_test_integration_test" + - "-//src/test/shell/bazel/android:android_instrumentation_test_integration_test_with_head_android_tools" + - "-//src/test/shell/bazel/android:android_instrumentation_test_integration_test_with_platforms" + - "-//src/test/shell/bazel/android:aapt_integration_test" + - "-//src/test/shell/bazel/android:aapt_integration_test_with_head_android_tools" + - "-//src/test/shell/bazel/android:aapt_integration_test_with_platforms" include_json_profile: - build - test @@ -166,6 +180,13 @@ tasks: - "//tools/python/..." # Re-enable once fixed: https://github.com/bazelbuild/bazel/issues/8162 - "-//src/java_tools/import_deps_checker/..." + # https://github.com/bazelbuild/bazel/issues/18776 + - "-//src/test/shell/bazel/android:android_instrumentation_test_integration_test" + - "-//src/test/shell/bazel/android:android_instrumentation_test_integration_test_with_head_android_tools" + - "-//src/test/shell/bazel/android:android_instrumentation_test_integration_test_with_platforms" + - "-//src/test/shell/bazel/android:aapt_integration_test" + - "-//src/test/shell/bazel/android:aapt_integration_test_with_head_android_tools" + - "-//src/test/shell/bazel/android:aapt_integration_test_with_platforms" include_json_profile: - build - test @@ -217,10 +238,13 @@ tasks: - "-//src/test/shell/bazel/apple:bazel_apple_test" # https://github.com/bazelbuild/bazel/issues/17408 - "-//src/test/shell/bazel/apple:bazel_objc_test" - # https://github.com/bazelbuild/bazel/issues/16526#issuecomment-1415858550 + # https://github.com/bazelbuild/bazel/issues/18776 - "-//src/test/shell/bazel/android:android_instrumentation_test_integration_test" - "-//src/test/shell/bazel/android:android_instrumentation_test_integration_test_with_head_android_tools" - "-//src/test/shell/bazel/android:android_instrumentation_test_integration_test_with_platforms" + - "-//src/test/shell/bazel/android:aapt_integration_test" + - "-//src/test/shell/bazel/android:aapt_integration_test_with_head_android_tools" + - "-//src/test/shell/bazel/android:aapt_integration_test_with_platforms" # https://github.com/bazelbuild/bazel/issues/17410 - "-//src/test/java/com/google/devtools/build/lib/platform:SystemMemoryPressureEventTest" # https://github.com/bazelbuild/bazel/issues/17411 @@ -274,7 +298,7 @@ tasks: - "-//src/test/shell/bazel:bazel_cc_code_coverage_test" # MacOS does not have cgroups so it can't support hardened sandbox - "-//src/test/shell/integration:bazel_hardened_sandboxed_worker_test" - # https://github.com/bazelbuild/bazel/issues/16521 + # https://github.com/bazelbuild/bazel/issues/16521 & https://github.com/bazelbuild/bazel/issues/18776 - "-//src/test/shell/bazel/android/..." - "-//src/tools/android/java/com/google/devtools/build/android/..." - "-//src/test/java/com/google/devtools/build/android/dexer:AllTests" @@ -406,6 +430,13 @@ tasks: - "-//src/test/py/bazel:bazel_repo_mapping_test" - "-//src/test/py/bazel:bazel_yanked_versions_test" - "-//src/test/shell/bazel:verify_workspace" + # https://github.com/bazelbuild/bazel/issues/18776 + - "-//src/test/shell/bazel/android:android_instrumentation_test_integration_test" + - "-//src/test/shell/bazel/android:android_instrumentation_test_integration_test_with_head_android_tools" + - "-//src/test/shell/bazel/android:android_instrumentation_test_integration_test_with_platforms" + - "-//src/test/shell/bazel/android:aapt_integration_test" + - "-//src/test/shell/bazel/android:aapt_integration_test_with_head_android_tools" + - "-//src/test/shell/bazel/android:aapt_integration_test_with_platforms" include_json_profile: - build - test diff --git a/.bazelci/presubmit.yml b/.bazelci/presubmit.yml index be4c3559e1aafa..0e8010fb16316d 100644 --- a/.bazelci/presubmit.yml +++ b/.bazelci/presubmit.yml @@ -59,6 +59,13 @@ tasks: - "-//src/test/shell/bazel:bazel_coverage_cc_released_test_gcc" - "-//src/test/shell/bazel:bazel_coverage_cc_head_test_gcc" - "-//src/test/shell/bazel:bazel_coverage_sh_test" + # https://github.com/bazelbuild/bazel/issues/18776 + - "-//src/test/shell/bazel/android:android_instrumentation_test_integration_test" + - "-//src/test/shell/bazel/android:android_instrumentation_test_integration_test_with_head_android_tools" + - "-//src/test/shell/bazel/android:android_instrumentation_test_integration_test_with_platforms" + - "-//src/test/shell/bazel/android:aapt_integration_test" + - "-//src/test/shell/bazel/android:aapt_integration_test_with_head_android_tools" + - "-//src/test/shell/bazel/android:aapt_integration_test_with_platforms" include_json_profile: - build - test @@ -100,6 +107,13 @@ tasks: - "//tools/python/..." # Re-enable once fixed: https://github.com/bazelbuild/bazel/issues/8162 - "-//src/java_tools/import_deps_checker/..." + # https://github.com/bazelbuild/bazel/issues/18776 + - "-//src/test/shell/bazel/android:android_instrumentation_test_integration_test" + - "-//src/test/shell/bazel/android:android_instrumentation_test_integration_test_with_head_android_tools" + - "-//src/test/shell/bazel/android:android_instrumentation_test_integration_test_with_platforms" + - "-//src/test/shell/bazel/android:aapt_integration_test" + - "-//src/test/shell/bazel/android:aapt_integration_test_with_head_android_tools" + - "-//src/test/shell/bazel/android:aapt_integration_test_with_platforms" include_json_profile: - build - test @@ -171,6 +185,13 @@ tasks: - "//tools/python/..." # Re-enable once fixed: https://github.com/bazelbuild/bazel/issues/8162 - "-//src/java_tools/import_deps_checker/..." + # https://github.com/bazelbuild/bazel/issues/18776 + - "-//src/test/shell/bazel/android:android_instrumentation_test_integration_test" + - "-//src/test/shell/bazel/android:android_instrumentation_test_integration_test_with_head_android_tools" + - "-//src/test/shell/bazel/android:android_instrumentation_test_integration_test_with_platforms" + - "-//src/test/shell/bazel/android:aapt_integration_test" + - "-//src/test/shell/bazel/android:aapt_integration_test_with_head_android_tools" + - "-//src/test/shell/bazel/android:aapt_integration_test_with_platforms" include_json_profile: - build - test @@ -225,10 +246,13 @@ tasks: - "-//src/test/shell/bazel/apple:bazel_apple_test" # https://github.com/bazelbuild/bazel/issues/17408 - "-//src/test/shell/bazel/apple:bazel_objc_test" - # https://github.com/bazelbuild/bazel/issues/16526#issuecomment-1415858550 + # https://github.com/bazelbuild/bazel/issues/18776 - "-//src/test/shell/bazel/android:android_instrumentation_test_integration_test" - "-//src/test/shell/bazel/android:android_instrumentation_test_integration_test_with_head_android_tools" - "-//src/test/shell/bazel/android:android_instrumentation_test_integration_test_with_platforms" + - "-//src/test/shell/bazel/android:aapt_integration_test" + - "-//src/test/shell/bazel/android:aapt_integration_test_with_head_android_tools" + - "-//src/test/shell/bazel/android:aapt_integration_test_with_platforms" # https://github.com/bazelbuild/bazel/issues/17410 - "-//src/test/java/com/google/devtools/build/lib/platform:SystemMemoryPressureEventTest" # https://github.com/bazelbuild/bazel/issues/17411 @@ -285,7 +309,7 @@ tasks: - "-//src/test/shell/bazel:bazel_cc_code_coverage_test" # MacOS does not have cgroups so it can't support hardened sandbox - "-//src/test/shell/integration:bazel_hardened_sandboxed_worker_test" - # https://github.com/bazelbuild/bazel/issues/16521 + # https://github.com/bazelbuild/bazel/issues/16521 & https://github.com/bazelbuild/bazel/issues/18776 - "-//src/test/shell/bazel/android/..." - "-//src/tools/android/java/com/google/devtools/build/android/..." - "-//src/test/java/com/google/devtools/build/android/dexer:AllTests" @@ -478,6 +502,13 @@ tasks: # Flaky on rbe_ubuntu1804 # https://github.com/bazelbuild/continuous-integration/issues/1631 - "-//src/test/shell/bazel:bazel_sandboxing_networking_test" + # https://github.com/bazelbuild/bazel/issues/18776 + - "-//src/test/shell/bazel/android:android_instrumentation_test_integration_test" + - "-//src/test/shell/bazel/android:android_instrumentation_test_integration_test_with_head_android_tools" + - "-//src/test/shell/bazel/android:android_instrumentation_test_integration_test_with_platforms" + - "-//src/test/shell/bazel/android:aapt_integration_test" + - "-//src/test/shell/bazel/android:aapt_integration_test_with_head_android_tools" + - "-//src/test/shell/bazel/android:aapt_integration_test_with_platforms" include_json_profile: - build - test diff --git a/.github/workflows/labeler.yml b/.github/workflows/labeler.yml index f228b5a6e15a95..2a6ab1c0048c94 100644 --- a/.github/workflows/labeler.yml +++ b/.github/workflows/labeler.yml @@ -2,7 +2,7 @@ name: "PR Labeler" on: pull_request_target: - types: ["opened", "ready_for_review"] + types: ["opened", "reopened", "ready_for_review"] permissions: contents: read diff --git a/BUILD b/BUILD index b03da89c2ade3b..d00a05d228d3a8 100644 --- a/BUILD +++ b/BUILD @@ -100,6 +100,7 @@ pkg_tar( "@com_google_protobuf//:protobuf_java_util", "@com_google_protobuf//:protobuf_javalite", "@zstd-jni//:zstd-jni", + "@blake3//:blake3", ], package_dir = "derived/jars", strip_prefix = "external", diff --git a/MODULE.bazel b/MODULE.bazel index 98717d2ffd1319..6ca96909a23f89 100644 --- a/MODULE.bazel +++ b/MODULE.bazel @@ -14,6 +14,7 @@ bazel_dep(name = "platforms", version = "0.0.6") bazel_dep(name = "rules_pkg", version = "0.7.0") bazel_dep(name = "stardoc", version = "0.5.3", repo_name = "io_bazel_skydoc") bazel_dep(name = "zstd-jni", version = "1.5.2-3") +bazel_dep(name = "blake3", version = "1.3.3") bazel_dep(name = "zlib", version = "1.2.13") bazel_dep(name = "rules_cc", version = "0.0.6") bazel_dep(name = "rules_java", version = "6.1.1") diff --git a/WORKSPACE b/WORKSPACE index 41d6ebf5dc115f..c7ae1fa85dae7b 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -1,7 +1,7 @@ workspace(name = "io_bazel") load("//tools/build_defs/repo:http.bzl", "http_archive") -load("//:distdir.bzl", "dist_http_archive", "distdir_tar", "dist_http_jar") +load("//:distdir.bzl", "dist_http_archive", "dist_http_jar", "distdir_tar") load("//:distdir_deps.bzl", "DIST_DEPS") load("//:repositories.bzl", "embedded_jdk_repositories") @@ -300,7 +300,9 @@ dist_http_archive( ) load("@rules_java//java:repositories.bzl", "rules_java_dependencies", "rules_java_toolchains") + rules_java_dependencies() + rules_java_toolchains() load("@io_bazel_skydoc//:setup.bzl", "stardoc_repositories") @@ -543,7 +545,7 @@ maven_install( maven.artifact( "org.mockito", "mockito-core", - "3.12.4", + "5.4.0", testonly = True, ), ], diff --git a/distdir_deps.bzl b/distdir_deps.bzl index 1a0f413f906fec..0c2d04c692836b 100644 --- a/distdir_deps.bzl +++ b/distdir_deps.bzl @@ -274,7 +274,7 @@ DIST_DEPS = { "package_version": "1.5.2-3", }, "blake3": { - "archive": "v1.3.3.zip", + "archive": "1.3.3.zip", "sha256": "bb529ba133c0256df49139bd403c17835edbf60d2ecd6463549c6a5fe279364d", "urls": [ "https://github.com/BLAKE3-team/BLAKE3/archive/refs/tags/1.3.3.zip", @@ -409,88 +409,90 @@ DIST_DEPS = { "package_version": "2.6", }, "openjdk_linux_vanilla": { - "archive": "zulu17.38.21-ca-jdk17.0.5-linux_x64.tar.gz", - "sha256": "20c91a922eec795f3181eaa70def8b99d8eac56047c9a14bfb257c85b991df1b", - "strip_prefix": "zulu17.38.21-ca-jdk17.0.5-linux_x64", + "archive": "zulu20.30.11-ca-jdk20.0.1-linux_x64.tar.gz", + "sha256": "ec5c0426a0eb2b0460968a044665ed4603b224acd5e20c379e9d7890511da683", + "strip_prefix": "zulu20.30.11-ca-jdk20.0.1-linux_x64", "urls": [ - "https://mirror.bazel.build/cdn.azul.com/zulu/bin/zulu17.38.21-ca-jdk17.0.5-linux_x64.tar.gz", - "https://cdn.azul.com/zulu/bin/zulu17.38.21-ca-jdk17.0.5-linux_x64.tar.gz", + "https://mirror.bazel.build/cdn.azul.com/zulu/bin/zulu20.30.11-ca-jdk20.0.1-linux_x64.tar.gz", + "https://cdn.azul.com/zulu/bin/zulu20.30.11-ca-jdk20.0.1-linux_x64.tar.gz", ], "used_in": [ ], }, "openjdk_linux_aarch64_vanilla": { - "archive": "zulu17.38.21-ca-jdk17.0.5-linux_aarch64.tar.gz", - "sha256": "dbc6ae9163e7ff469a9ab1f342cd1bc1f4c1fb78afc3c4f2228ee3b32c4f3e43", - "strip_prefix": "zulu17.38.21-ca-jdk17.0.5-linux_aarch64", + "archive": "zulu20.30.11-ca-jdk20.0.1-linux_aarch64.tar.gz", + "sha256": "2487cf315d1f56291c1f41fb56a34a7f863ce5bf85cadd284c79ea3f848d707c", + "strip_prefix": "zulu20.30.11-ca-jdk20.0.1-linux_aarch64", "urls": [ - "https://mirror.bazel.build/cdn.azul.com/zulu/bin/zulu17.38.21-ca-jdk17.0.5-linux_aarch64.tar.gz", - "https://cdn.azul.com/zulu/bin/zulu17.38.21-ca-jdk17.0.5-linux_aarch64.tar.gz", + "https://mirror.bazel.build/cdn.azul.com/zulu/bin/zulu20.30.11-ca-jdk20.0.1-linux_aarch64.tar.gz", + "https://cdn.azul.com/zulu/bin/zulu20.30.11-ca-jdk20.0.1-linux_aarch64.tar.gz", ], "used_in": [ ], }, + # JDK20 unavailable so use JDK19 instead for linux s390x. "openjdk_linux_s390x_vanilla": { - "archive": "OpenJDK17U-jdk_s390x_linux_hotspot_17.0.4.1_1.tar.gz", - "sha256": "fdc82f4b06c880762503b0cb40e25f46cf8190d06011b3b768f4091d3334ef7f", - "strip_prefix": "jdk-17.0.4.1+1", + "archive": "OpenJDK19U-jdk_s390x_linux_hotspot_19.0.2_7.tar.gz", + "sha256": "f2512f9a8e9847dd5d3557c39b485a8e7a1ef37b601dcbcb748d22e49f44815c", + "strip_prefix": "jdk-19.0.2+7", "urls": [ - "https://mirror.bazel.build/github.com/adoptium/temurin17-binaries/releases/download/jdk-17.0.4.1%2B1/OpenJDK17U-jdk_s390x_linux_hotspot_17.0.4.1_1.tar.gz", - "https://github.com/adoptium/temurin17-binaries/releases/download/jdk-17.0.4.1%2B1/OpenJDK17U-jdk_s390x_linux_hotspot_17.0.4.1_1.tar.gz", + "https://mirror.bazel.build/github.com/adoptium/temurin19-binaries/releases/download/jdk-19.0.2%2B7/OpenJDK19U-jdk_s390x_linux_hotspot_19.0.2_7.tar.gz", + "https://github.com/adoptium/temurin19-binaries/releases/download/jdk-19.0.2%2B7/OpenJDK19U-jdk_s390x_linux_hotspot_19.0.2_7.tar.gz", ], "used_in": [ ], }, "openjdk_linux_ppc64le_vanilla": { - "archive": "OpenJDK17U-jdk_ppc64le_linux_hotspot_17.0.4.1_1.tar.gz", - "sha256": "cbedd0a1428b3058d156e99e8e9bc8769e0d633736d6776a4c4d9136648f2fd1", - "strip_prefix": "jdk-17.0.4.1+1", + "archive": "OpenJDK20U-jdk_ppc64le_linux_hotspot_20_36.tar.gz", + "sha256": "45dde71faf8cbb78fab3c976894259655c8d3de827347f23e0ebe5710921dded", + "strip_prefix": "jdk-20+36", "urls": [ - "https://mirror.bazel.build/github.com/adoptium/temurin17-binaries/releases/download/jdk-17.0.4.1%2B1/OpenJDK17U-jdk_ppc64le_linux_hotspot_17.0.4.1_1.tar.gz", - "https://github.com/adoptium/temurin17-binaries/releases/download/jdk-17.0.4.1%2B1/OpenJDK17U-jdk_ppc64le_linux_hotspot_17.0.4.1_1.tar.gz", + "https://mirror.bazel.build/github.com/adoptium/temurin20-binaries/releases/download/jdk-20%2B36/OpenJDK20U-jdk_ppc64le_linux_hotspot_20_36.tar.gz", + "https://github.com/adoptium/temurin20-binaries/releases/download/jdk-20%2B36/OpenJDK20U-jdk_ppc64le_linux_hotspot_20_36.tar.gz", ], "used_in": [], }, "openjdk_macos_x86_64_vanilla": { - "archive": "zulu17.38.21-ca-jdk17.0.5-macosx_x64.tar.gz", - "sha256": "e6317cee4d40995f0da5b702af3f04a6af2bbd55febf67927696987d11113b53", - "strip_prefix": "zulu17.38.21-ca-jdk17.0.5-macosx_x64", + "archive": "zulu20.30.11-ca-jdk20.0.1-macosx_x64.tar.gz", + "sha256": "befee9db92345d5146945061b721d3a6c6e182471c1536f87dbadfd5aab0e241", + "strip_prefix": "zulu20.30.11-ca-jdk20.0.1-macosx_x64", "urls": [ - "https://mirror.bazel.build/cdn.azul.com/zulu/bin/zulu17.38.21-ca-jdk17.0.5-macosx_x64.tar.gz", - "https://cdn.azul.com/zulu/bin/zulu17.38.21-ca-jdk17.0.5-macosx_x64.tar.gz", + "https://mirror.bazel.build/cdn.azul.com/zulu/bin/zulu20.30.11-ca-jdk20.0.1-macosx_x64.tar.gz", + "https://cdn.azul.com/zulu/bin/zulu20.30.11-ca-jdk20.0.1-macosx_x64.tar.gz", ], "used_in": [ ], }, "openjdk_macos_aarch64_vanilla": { - "archive": "zulu17.38.21-ca-jdk17.0.5-macosx_aarch64", - "sha256": "515dd56ec99bb5ae8966621a2088aadfbe72631818ffbba6e4387b7ee292ab09", - "strip_prefix": "zulu17.38.21-ca-jdk17.0.5-macosx_aarch64", + "archive": "zulu20.30.11-ca-jdk20.0.1-macosx_aarch64.tar.gz", + "sha256": "01e59f0160d051524bb16d865652d25d00a85390581737a8f35f89057c80892d", + "strip_prefix": "zulu20.30.11-ca-jdk20.0.1-macosx_aarch64", "urls": [ - "https://mirror.bazel.build/cdn.azul.com/zulu/bin/zulu17.38.21-ca-jdk17.0.5-macosx_aarch64.tar.gz", - "https://cdn.azul.com/zulu/bin/zulu17.38.21-ca-jdk17.0.5-macosx_aarch64.tar.gz", + "https://mirror.bazel.build/cdn.azul.com/zulu/bin/zulu20.30.11-ca-jdk20.0.1-macosx_aarch64.tar.gz", + "https://cdn.azul.com/zulu/bin/zulu20.30.11-ca-jdk20.0.1-macosx_aarch64.tar.gz", ], "used_in": [ ], }, "openjdk_win_vanilla": { - "archive": "zulu17.38.21-ca-jdk17.0.5-win_x64.zip", - "sha256": "9972c5b62a61b45785d3d956c559e079d9e91f144ec46225f5deeda214d48f27", - "strip_prefix": "zulu17.38.21-ca-jdk17.0.5-win_x64", + "archive": "zulu20.30.11-ca-jdk20.0.1-win_x64.zip", + "sha256": "8a97ee11da578292f7c9e772f3edd3f083fa4f34f47a98e3abefb625ab2225ba", + "strip_prefix": "zulu20.30.11-ca-jdk20.0.1-win_x64", "urls": [ - "https://mirror.bazel.build/cdn.azul.com/zulu/bin/zulu17.38.21-ca-jdk17.0.5-win_x64.zip", - "https://cdn.azul.com/zulu/bin/zulu17.38.21-ca-jdk17.0.5-win_x64.zip", + "https://mirror.bazel.build/cdn.azul.com/zulu/bin/zulu20.30.11-ca-jdk20.0.1-win_x64.zip", + "https://cdn.azul.com/zulu/bin/zulu20.30.11-ca-jdk20.0.1-win_x64.zip", ], "used_in": [ ], }, + # JDK20 unavailable so use JDK19 instead for win aarch64. "openjdk_win_arm64_vanilla": { - "archive": "zulu17.38.21-ca-jdk17.0.5-win_aarch64.zip", - "sha256": "bc3476f2161bf99bc9a243ff535b8fc033b34ce9a2fa4b62fb8d79b6bfdc427f", - "strip_prefix": "zulu17.38.21-ca-jdk17.0.5-win_aarch64", + "archive": "zulu19.28.81-ca-jdk19.0.0-win_aarch64.zip", + "sha256": "e73e851638066c48421a60e01ce7d956c1de0935620e1b66d8bbbd6cdd4f815e", + "strip_prefix": "zulu19.28.81-ca-jdk19.0.0-win_aarch64", "urls": [ - "https://mirror.bazel.build/cdn.azul.com/zulu/bin/zulu17.38.21-ca-jdk17.0.5-win_aarch64.zip", - "https://cdn.azul.com/zulu/bin/zulu17.38.21-ca-jdk17.0.5-win_aarch64.zip", + "https://mirror.bazel.build/cdn.azul.com/zulu/bin/zulu19.28.81-ca-jdk19.0.0-win_aarch64.zip", + "https://cdn.azul.com/zulu/bin/zulu19.28.81-ca-jdk19.0.0-win_aarch64.zip", ], "used_in": [ ], diff --git a/src/java_tools/singlejar/javatests/com/google/devtools/build/singlejar/FakeZipFile.java b/src/java_tools/singlejar/javatests/com/google/devtools/build/singlejar/FakeZipFile.java index 05eaa07970dcbe..2f9de760749578 100644 --- a/src/java_tools/singlejar/javatests/com/google/devtools/build/singlejar/FakeZipFile.java +++ b/src/java_tools/singlejar/javatests/com/google/devtools/build/singlejar/FakeZipFile.java @@ -15,7 +15,6 @@ package com.google.devtools.build.singlejar; import static com.google.common.truth.Truth.assertThat; -import static com.google.common.truth.Truth.assertWithMessage; import static java.nio.charset.StandardCharsets.UTF_8; import com.google.errorprone.annotations.CanIgnoreReturnValue; @@ -25,9 +24,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Date; -import java.util.HashMap; import java.util.List; -import java.util.Map; import java.util.zip.ZipEntry; import java.util.zip.ZipInputStream; @@ -136,12 +133,6 @@ public void assertNext(ZipInputStream zipInput) throws IOException { private final List entries = new ArrayList<>(); - @CanIgnoreReturnValue - public FakeZipFile addEntry(String name, String content) { - entries.add(new FakeZipEntry(name, null, content, null, EntryMode.DONT_CARE)); - return this; - } - @CanIgnoreReturnValue public FakeZipFile addEntry(String name, String content, boolean compressed) { entries.add(new FakeZipEntry(name, null, content, null, @@ -149,66 +140,8 @@ public FakeZipFile addEntry(String name, String content, boolean compressed) { return this; } - @CanIgnoreReturnValue - public FakeZipFile addEntry(String name, Date date, String content) { - entries.add(new FakeZipEntry(name, date, content, null, EntryMode.DONT_CARE)); - return this; - } - - @CanIgnoreReturnValue - public FakeZipFile addEntry(String name, Date date, String content, boolean compressed) { - entries.add(new FakeZipEntry(name, date, content, null, - compressed ? EntryMode.EXPECT_DEFLATE : EntryMode.EXPECT_STORED)); - return this; - } - - @CanIgnoreReturnValue - public FakeZipFile addEntry(String name, ByteValidator content) { - entries.add(new FakeZipEntry(name, null, content, null, EntryMode.DONT_CARE)); - return this; - } - - @CanIgnoreReturnValue - public FakeZipFile addEntry(String name, ByteValidator content, boolean compressed) { - entries.add(new FakeZipEntry(name, null, content, null, - compressed ? EntryMode.EXPECT_DEFLATE : EntryMode.EXPECT_STORED)); - return this; - } - - @CanIgnoreReturnValue - public FakeZipFile addEntry(String name, Date date, ByteValidator content) { - entries.add(new FakeZipEntry(name, date, content, null, EntryMode.DONT_CARE)); - return this; - } - - @CanIgnoreReturnValue - public FakeZipFile addEntry(String name, Date date, ByteValidator content, boolean compressed) { - entries.add(new FakeZipEntry(name, date, content, null, - compressed ? EntryMode.EXPECT_DEFLATE : EntryMode.EXPECT_STORED)); - return this; - } - - @CanIgnoreReturnValue - public FakeZipFile addEntry(String name, byte[] extra) { - entries.add(new FakeZipEntry(name, null, (String) null, extra, EntryMode.DONT_CARE)); - return this; - } - - @CanIgnoreReturnValue - public FakeZipFile addEntry(String name, byte[] extra, boolean compressed) { - entries.add(new FakeZipEntry(name, null, (String) null, extra, - compressed ? EntryMode.EXPECT_DEFLATE : EntryMode.EXPECT_STORED)); - return this; - } - private byte[] preamble = null; - @CanIgnoreReturnValue - public FakeZipFile addPreamble(byte[] contents) { - preamble = Arrays.copyOf(contents, contents.length); - return this; - } - private int getUnsignedShort(byte[] source, int offset) { int a = source[offset + 0] & 0xff; int b = source[offset + 1] & 0xff; @@ -237,55 +170,4 @@ public void assertSame(byte[] data) throws IOException { count = getUnsignedShort(data, data.length-12); assertThat(count).isEqualTo(entries.size()); } - - /** - * Assert that {@code expected} is the same zip file as {@code actual}. It is similar to - * {@link org.junit.Assert#assertArrayEquals(byte[], byte[])} but should use a more - * helpful error message. - */ - public static void assertSame(byte[] expected, byte[] actual) throws IOException { - // First parse the zip files, then compare to have explicit comparison messages. - ZipInputStream expectedZip = new ZipInputStream(new ByteArrayInputStream(expected)); - ZipInputStream actualZip = new ZipInputStream(new ByteArrayInputStream(actual)); - StringBuffer actualFileList = new StringBuffer(); - StringBuffer expectedFileList = new StringBuffer(); - Map actualEntries = new HashMap(); - Map expectedEntries = new HashMap(); - Map actualEntryContents = new HashMap(); - Map expectedEntryContents = new HashMap(); - parseZipEntry(expectedZip, expectedFileList, expectedEntries, expectedEntryContents); - parseZipEntry(actualZip, actualFileList, actualEntries, actualEntryContents); - // Compare the ordered file list first. - assertThat(actualFileList.toString()).isEqualTo(expectedFileList.toString()); - - // Then compare each entry. - for (String name : expectedEntries.keySet()) { - ZipEntry expectedEntry = expectedEntries.get(name); - ZipEntry actualEntry = actualEntries.get(name); - assertWithMessage("Time differs for " + name) - .that(actualEntry.getTime()) - .isEqualTo(expectedEntry.getTime()); - assertWithMessage("Extraneous content differs for " + name) - .that(actualEntry.getExtra()) - .isEqualTo(expectedEntry.getExtra()); - assertWithMessage("Content differs for " + name) - .that(actualEntryContents.get(name)) - .isEqualTo(expectedEntryContents.get(name)); - } - - // Finally do a binary array comparison to be sure that test fails if files are different in - // some way we don't test. - assertThat(actual).isEqualTo(expected); - } - - private static void parseZipEntry(ZipInputStream expectedZip, StringBuffer expectedFileList, - Map expectedEntries, Map expectedEntryContents) - throws IOException { - ZipEntry expectedEntry; - while ((expectedEntry = expectedZip.getNextEntry()) != null) { - expectedFileList.append(expectedEntry.getName()).append("\n"); - expectedEntries.put(expectedEntry.getName(), expectedEntry); - expectedEntryContents.put(expectedEntry.getName(), readZipEntryContent(expectedZip)); - } - } } diff --git a/src/java_tools/singlejar/javatests/com/google/devtools/build/singlejar/ZipFactory.java b/src/java_tools/singlejar/javatests/com/google/devtools/build/singlejar/ZipFactory.java index da87d08b7d39cc..d2f45c32280872 100644 --- a/src/java_tools/singlejar/javatests/com/google/devtools/build/singlejar/ZipFactory.java +++ b/src/java_tools/singlejar/javatests/com/google/devtools/build/singlejar/ZipFactory.java @@ -68,12 +68,6 @@ public ZipFactory addFile(String name, String content, boolean compressed) { return this; } - @CanIgnoreReturnValue - public ZipFactory addFile(String name, byte[] content, boolean compressed) { - addEntry(name, content.clone(), compressed); - return this; - } - public byte[] toByteArray() { try { ByteArrayOutputStream out = new ByteArrayOutputStream(); diff --git a/src/main/java/com/google/devtools/build/lib/BUILD b/src/main/java/com/google/devtools/build/lib/BUILD index 3d61a7d7cca833..27ac03a050c926 100644 --- a/src/main/java/com/google/devtools/build/lib/BUILD +++ b/src/main/java/com/google/devtools/build/lib/BUILD @@ -314,6 +314,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/actions:middleman_type", "//src/main/java/com/google/devtools/build/lib/actions:package_roots", "//src/main/java/com/google/devtools/build/lib/actions:resource_manager", + "//src/main/java/com/google/devtools/build/lib/actions:runfiles_supplier", "//src/main/java/com/google/devtools/build/lib/actions:shared_action_event", "//src/main/java/com/google/devtools/build/lib/analysis:actions/abstract_file_write_action", "//src/main/java/com/google/devtools/build/lib/analysis:analysis_cluster", diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionContext.java b/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionContext.java index e40a6a12143140..b14cd4361fe8ba 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionContext.java +++ b/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionContext.java @@ -329,7 +329,7 @@ public void maybeReportSubcommand(Spawn spawn) { reason.append(owner.getExecutionPlatform().label()); } reason.append(", mnemonic: "); - reason.append(owner.getMnemonic()); + reason.append(spawn.getMnemonic()); reason.append("]"); } diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionLookupKeyOrProxy.java b/src/main/java/com/google/devtools/build/lib/actions/ActionLookupKeyOrProxy.java index 92b189cf5e8c3b..48f76839214673 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/ActionLookupKeyOrProxy.java +++ b/src/main/java/com/google/devtools/build/lib/actions/ActionLookupKeyOrProxy.java @@ -29,6 +29,7 @@ * are subclasses of {@link ActionLookupKeyOrProxy}. This allows callers to easily find the value * key, while remaining agnostic to what action lookup values actually exist. */ +// TODO(b/261521010): this layer of indirection is no longer needed and may be cleaned up. public interface ActionLookupKeyOrProxy extends ArtifactOwner { /** * Returns the {@link BuildConfigurationKey} for the configuration associated with this key, or diff --git a/src/main/java/com/google/devtools/build/lib/actions/BUILD b/src/main/java/com/google/devtools/build/lib/actions/BUILD index ff6058adb986f6..97518fa2306396 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/BUILD +++ b/src/main/java/com/google/devtools/build/lib/actions/BUILD @@ -29,50 +29,121 @@ java_library( # Action graph and cache. java_library( name = "actions", - srcs = glob( - [ - "*.java", - "cache/*.java", - ], - exclude = [ - "ActionInput.java", - "ActionInputHelper.java", - "ActionLookupData.java", - "ActionLookupKey.java", - "AnalysisGraphStatsEvent.java", - "Artifact.java", - "ArtifactFactory.java", - "ArtifactOwner.java", - "ArtifactPathResolver.java", - "ArtifactPrefixConflictException.java", - "ArtifactResolver.java", - "ArtifactRoot.java", - "Artifacts.java", - "CommandLineExpansionException.java", - "CommandLineItem.java", - "ExecutionRequirements.java", - "FileArtifactValue.java", - "FileContentsProxy.java", - "FileStateType.java", - "FileStateValue.java", - "FileValue.java", - "FilesetOutputSymlink.java", - "LocalHostCapacity.java", - "LocalHostResourceManagerLinux.java", - "LocalHostResourceManagerDarwin.java", - "LocalHostResourceFallback.java", - "MiddlemanType.java", - "ResourceManager.java", - "ResourceSet.java", - "ResourceSetOrBuilder.java", - "FileStatusWithMetadata.java", - "SharedActionEvent.java", - "PackageRootResolver.java", - "PackageRoots.java", - "ThreadStateReceiver.java", - "cache/MetadataDigestUtils.java", - ], - ), + srcs = [ + "AbstractAction.java", + "Action.java", + "ActionAnalysisMetadata.java", + "ActionCacheAwareAction.java", + "ActionCacheChecker.java", + "ActionCacheUtils.java", + "ActionCompletionEvent.java", + "ActionContext.java", + "ActionContextMarker.java", + "ActionEnvironment.java", + "ActionExecutedEvent.java", + "ActionExecutionContext.java", + "ActionExecutionException.java", + "ActionExecutionMetadata.java", + "ActionExecutionStatusReporter.java", + "ActionGraph.java", + "ActionGraphVisitor.java", + "ActionInputDepOwnerMap.java", + "ActionInputDepOwners.java", + "ActionInputMap.java", + "ActionInputMapSink.java", + "ActionInputPrefetcher.java", + "ActionKeyCacher.java", + "ActionKeyContext.java", + "ActionLogBufferPathGenerator.java", + "ActionLookupKeyOrProxy.java", + "ActionLookupValue.java", + "ActionMiddlemanEvent.java", + "ActionOwner.java", + "ActionProgressEvent.java", + "ActionRegistry.java", + "ActionResult.java", + "ActionResultReceivedEvent.java", + "ActionScanningCompletedEvent.java", + "ActionStartedEvent.java", + "ActionTemplate.java", + "ActionUploadFinishedEvent.java", + "ActionUploadStartedEvent.java", + "Actions.java", + "AggregatedSpawnMetrics.java", + "AlreadyReportedActionExecutionException.java", + "BaseSpawn.java", + "BasicActionLookupValue.java", + "BipartiteVisitor.java", + "BuildConfigurationEvent.java", + "BuildFailedException.java", + "CachedActionEvent.java", + "CachingActionEvent.java", + "ChangedFilesMessage.java", + "CommandAction.java", + "CommandLine.java", + "CommandLineLimits.java", + "CommandLines.java", + "CompletionContext.java", + "CompositeRunfilesSupplier.java", + "DelegateSpawn.java", + "DelegatingPairInputMetadataProvider.java", + "DigestOfDirectoryException.java", + "DiscoveredInputsEvent.java", + "DiscoveredModulesPruner.java", + "DynamicStrategyRegistry.java", + "EmptyRunfilesSupplier.java", + "EnvironmentalExecException.java", + "EventReportingArtifacts.java", + "ExecException.java", + "Executor.java", + "FailAction.java", + "FilesetManifest.java", + "FilesetTraversalParams.java", + "FilesetTraversalParamsFactory.java", + "ForbiddenActionInputException.java", + "HasDigest.java", + "InputFileErrorException.java", + "InputMetadataProvider.java", + "LostInputsActionExecutionException.java", + "LostInputsExecException.java", + "MapBasedActionGraph.java", + "MiddlemanAction.java", + "MiddlemanFactory.java", + "MissingDepExecException.java", + "MissingInputFileException.java", + "MutableActionGraph.java", + "NotifyOnActionCacheHit.java", + "ParamFileInfo.java", + "ParameterFile.java", + "PathStrippable.java", + "PathStripper.java", + "RemoteArtifactChecker.java", + "ResourceEstimator.java", + "RunningActionEvent.java", + "SandboxedSpawnStrategy.java", + "ScanningActionEvent.java", + "SchedulingActionEvent.java", + "SimpleSpawn.java", + "SingleStringArgFormatter.java", + "Spawn.java", + "SpawnExecutedEvent.java", + "SpawnMetrics.java", + "SpawnResult.java", + "SpawnStrategy.java", + "Spawns.java", + "StaticInputMetadataProvider.java", + "StoppedScanningActionEvent.java", + "TestExecException.java", + "TestMiddlemanObserver.java", + "TotalAndConfiguredTargetOnlyMetric.java", + "UserExecException.java", + "cache/ActionCache.java", + "cache/CompactPersistentActionCache.java", + "cache/MetadataInjector.java", + "cache/OutputMetadataStore.java", + "cache/PersistentStringIndexer.java", + "cache/VirtualActionInput.java", + ], deps = [ ":action_lookup_data", ":action_lookup_key", @@ -85,6 +156,7 @@ java_library( ":localhost_capacity", ":middleman_type", ":package_roots", + ":runfiles_supplier", ":thread_state_receiver", "//src/main/java/com/google/devtools/build/lib/analysis:config/build_configuration", "//src/main/java/com/google/devtools/build/lib/analysis/platform", @@ -121,7 +193,6 @@ java_library( "//src/main/java/com/google/devtools/build/lib/util:filetype", "//src/main/java/com/google/devtools/build/lib/util:os", "//src/main/java/com/google/devtools/build/lib/util:shell_escaper", - "//src/main/java/com/google/devtools/build/lib/util:string", "//src/main/java/com/google/devtools/build/lib/util:var_int", "//src/main/java/com/google/devtools/build/lib/util/io", "//src/main/java/com/google/devtools/build/lib/vfs", @@ -413,6 +484,18 @@ java_library( ], ) +java_library( + name = "runfiles_supplier", + srcs = ["RunfilesSupplier.java"], + deps = [ + ":artifacts", + "//src/main/java/com/google/devtools/build/lib/collect/nestedset", + "//src/main/java/com/google/devtools/build/lib/vfs:pathfragment", + "//src/main/java/net/starlark/java/eval", + "//third_party:guava", + ], +) + java_library( name = "shared_action_event", srcs = ["SharedActionEvent.java"], diff --git a/src/main/java/com/google/devtools/build/lib/actions/DynamicStrategyRegistry.java b/src/main/java/com/google/devtools/build/lib/actions/DynamicStrategyRegistry.java index 3c9c633a1c3c58..d481024227658c 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/DynamicStrategyRegistry.java +++ b/src/main/java/com/google/devtools/build/lib/actions/DynamicStrategyRegistry.java @@ -14,7 +14,7 @@ package com.google.devtools.build.lib.actions; -import java.util.List; +import com.google.common.collect.ImmutableCollection; /** Registry providing access to dynamic spawn strategies for both remote and local modes. */ public interface DynamicStrategyRegistry extends ActionContext { @@ -44,7 +44,8 @@ public DynamicMode other() { * Returns the spawn strategy implementations that {@linkplain SpawnStrategy#canExec can execute} * the given spawn in the order that they were registered for the provided dynamic mode. */ - List getDynamicSpawnActionContexts(Spawn spawn, DynamicMode dynamicMode); + ImmutableCollection getDynamicSpawnActionContexts( + Spawn spawn, DynamicMode dynamicMode); /** * Notifies all strategies applying to at least one mnemonic (including the empty all-catch one) diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BUILD b/src/main/java/com/google/devtools/build/lib/analysis/BUILD index 6d9af59c831e2a..32d208771ecb0c 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/BUILD +++ b/src/main/java/com/google/devtools/build/lib/analysis/BUILD @@ -230,7 +230,6 @@ java_library( "config/SymlinkDefinition.java", "config/TransitionResolver.java", "configuredtargets/AbstractConfiguredTarget.java", - "configuredtargets/ConfiguredTargetsUtil.java", "configuredtargets/EnvironmentGroupConfiguredTarget.java", "configuredtargets/FileConfiguredTarget.java", "configuredtargets/InputFileConfiguredTarget.java", @@ -392,6 +391,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/actions:fileset_output_symlink", "//src/main/java/com/google/devtools/build/lib/actions:localhost_capacity", "//src/main/java/com/google/devtools/build/lib/actions:package_roots", + "//src/main/java/com/google/devtools/build/lib/actions:runfiles_supplier", "//src/main/java/com/google/devtools/build/lib/analysis/platform", "//src/main/java/com/google/devtools/build/lib/analysis/stringtemplate", "//src/main/java/com/google/devtools/build/lib/bugreport", @@ -1562,6 +1562,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/actions:commandline_item", "//src/main/java/com/google/devtools/build/lib/actions:localhost_capacity", "//src/main/java/com/google/devtools/build/lib/actions:middleman_type", + "//src/main/java/com/google/devtools/build/lib/actions:runfiles_supplier", "//src/main/java/com/google/devtools/build/lib/collect/nestedset", "//src/main/java/com/google/devtools/build/lib/skyframe:action_template_expansion_value", "//src/main/java/com/google/devtools/build/lib/util", diff --git a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTarget.java b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTarget.java index 6795182de40624..83459523064b9f 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTarget.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTarget.java @@ -126,9 +126,10 @@ default ConfiguredTarget unwrapIfMerged() { /** * This is only intended to be called from the query dialects of Starlark. * - * @return a map of provider names to their values + * @return a map of provider names to their values, or null if there are no providers */ - default Dict getProvidersDict() { + @Nullable + default Dict getProvidersDictForQuery() { return null; } } diff --git a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java index 4ad31488eac268..44f8ce4644c061 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java @@ -239,7 +239,7 @@ public ConfiguredTarget createConfiguredTarget( return rule; } Artifact artifact = rule.findArtifactByOutputLabel(outputFile.getLabel()); - return new OutputFileConfiguredTarget(targetContext, outputFile, rule, artifact); + return new OutputFileConfiguredTarget(targetContext, artifact, rule); } else if (target instanceof InputFile) { InputFile inputFile = (InputFile) target; TargetContext targetContext = @@ -260,7 +260,7 @@ public ConfiguredTarget createConfiguredTarget( .setLabel(target.getLabel()) .setConfiguration(config) .build()); - return new InputFileConfiguredTarget(targetContext, inputFile, artifact); + return new InputFileConfiguredTarget(targetContext, artifact); } else if (target instanceof PackageGroup) { PackageGroup packageGroup = (PackageGroup) target; TargetContext targetContext = diff --git a/src/main/java/com/google/devtools/build/lib/analysis/DependencyResolver.java b/src/main/java/com/google/devtools/build/lib/analysis/DependencyResolver.java index 4d123d2831d9db..8080517b9d725b 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/DependencyResolver.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/DependencyResolver.java @@ -37,6 +37,7 @@ import com.google.devtools.build.lib.analysis.config.transitions.TransitionCollector; import com.google.devtools.build.lib.analysis.config.transitions.TransitionFactory; import com.google.devtools.build.lib.analysis.platform.PlatformInfo; +import com.google.devtools.build.lib.analysis.starlark.StarlarkAttributeTransitionProvider; import com.google.devtools.build.lib.causes.Cause; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; @@ -101,6 +102,9 @@ public abstract class DependencyResolver { * @param toolchainContexts the toolchain contexts for this target * @param trimmingTransitionFactory the transition factory used to trim rules (note: this is a * temporary feature; see the corresponding methods in ConfiguredRuleClassProvider) + * @param transitionCollector a callback that observes attribute transitions for Cquery + * @param starlarkExecTransitionFactory if not null, the Starlark transition that implements + * {@code cfg = "exec"}. Otherwise Bazel uses native exec transition logic. * @return a mapping of each attribute in this rule or aspects to its dependent nodes */ public final OrderedSetMultimap dependentNodeMap( @@ -109,7 +113,8 @@ public final OrderedSetMultimap dependentNodeMap( ImmutableMap configConditions, @Nullable ToolchainCollection toolchainContexts, @Nullable TransitionFactory trimmingTransitionFactory, - TransitionCollector transitionCollector) + TransitionCollector transitionCollector, + @Nullable StarlarkAttributeTransitionProvider starlarkExecTransitionFactory) throws Failure, InterruptedException, InconsistentAspectOrderException { NestedSetBuilder rootCauses = NestedSetBuilder.stableOrder(); OrderedSetMultimap outgoingEdges = @@ -120,7 +125,8 @@ public final OrderedSetMultimap dependentNodeMap( toolchainContexts, rootCauses, trimmingTransitionFactory, - transitionCollector); + transitionCollector, + starlarkExecTransitionFactory); if (!rootCauses.isEmpty()) { throw new IllegalStateException(rootCauses.build().toList().iterator().next().toString()); } @@ -155,6 +161,8 @@ public final OrderedSetMultimap dependentNodeMap( * temporary feature; see the corresponding methods in ConfiguredRuleClassProvider) * @param rootCauses collector for dep labels that can't be (loading phase) loaded * @param transitionCollector a callback that observes attribute transitions for Cquery + * @param starlarkExecTransitionFactory if not null, the Starlark transition that implements + * {@code cfg = "exec"}. Else Bazel uses native exec transition logic. * @return a mapping of each attribute in this rule or aspects to its dependent nodes */ public final OrderedSetMultimap dependentNodeMap( @@ -164,7 +172,8 @@ public final OrderedSetMultimap dependentNodeMap( @Nullable ToolchainCollection toolchainContexts, NestedSetBuilder rootCauses, @Nullable TransitionFactory trimmingTransitionFactory, - TransitionCollector transitionCollector) + TransitionCollector transitionCollector, + StarlarkAttributeTransitionProvider starlarkExecTransitionFactory) throws Failure, InterruptedException, InconsistentAspectOrderException { var dependencyLabels = computeDependencyLabels(node, aspects, configConditions, toolchainContexts); @@ -191,7 +200,8 @@ public final OrderedSetMultimap dependentNodeMap( dependencyLabels.attributeMap(), toolchainContexts, aspects, - transitionCollector); + transitionCollector, + starlarkExecTransitionFactory); return fullyResolveDependencies( partiallyResolvedDeps, targetMap, node.getConfiguration(), trimmingTransitionFactory); @@ -273,7 +283,8 @@ public static DependencyLabels computeDependencyLabels( @Nullable ConfiguredAttributeMapper attributeMap, @Nullable ToolchainCollection toolchainContexts, Iterable aspects, - TransitionCollector transitionCollector) + TransitionCollector transitionCollector, + StarlarkAttributeTransitionProvider starlarkExecTransitionFactory) throws Failure { OrderedSetMultimap partiallyResolvedDeps = OrderedSetMultimap.create(); @@ -348,6 +359,7 @@ public static DependencyLabels computeDependencyLabels( AttributeTransitionData.builder() .attributes(attributeMap) .executionPlatform(executionPlatformLabel) + .analysisData(starlarkExecTransitionFactory) .build(); ConfigurationTransition attributeTransition = kind.getAttribute().getTransitionFactory().create(attributeTransitionData); diff --git a/src/main/java/com/google/devtools/build/lib/analysis/TransitiveDependencyState.java b/src/main/java/com/google/devtools/build/lib/analysis/TransitiveDependencyState.java index ac9c872ea089cf..111b352f5ab504 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/TransitiveDependencyState.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/TransitiveDependencyState.java @@ -62,7 +62,7 @@ public final class TransitiveDependencyState { * *

More ideally, those properties would be conveyed via providers of those dependencies, but * doing so would adversely affect resting heap usage whereas {@link ConfiguredTargetAndData} is - * ephemeral. Distributed implementations will include these properties in an extra providers. It + * ephemeral. Distributed implementations will include these properties in an extra provider. It * won't affect memory because the underlying package won't exist on the node loading it remotely. * *

It's valid to obtain {@link Package}s of dependencies from this map instead of creating an diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/CoreOptions.java b/src/main/java/com/google/devtools/build/lib/analysis/config/CoreOptions.java index 1e7e15c51a6e1f..0c996c3ee2554f 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/config/CoreOptions.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/config/CoreOptions.java @@ -71,6 +71,31 @@ public class CoreOptions extends FragmentOptions implements Cloneable { help = "If true, the genfiles directory is folded into the bin directory.") public boolean mergeGenfilesDirectory; + @Option( + name = "experimental_exec_config", + defaultValue = "null", + documentationCategory = OptionDocumentationCategory.UNDOCUMENTED, + effectTags = {OptionEffectTag.BAZEL_INTERNAL_CONFIGURATION}, + metadataTags = {OptionMetadataTag.EXPERIMENTAL}, + help = + "If set to '//some:label:my.bzl%my_transition', uses my_transition for 'cfg = \"exec\"' " + + "semantics instead of Bazel's internal exec transition logic. Else uses Bazel's " + + "internal logic.") + public String starlarkExecConfig; + + @Option( + name = "experimental_exec_config_diff", + defaultValue = "false", + documentationCategory = OptionDocumentationCategory.UNDOCUMENTED, + effectTags = {OptionEffectTag.BAZEL_INTERNAL_CONFIGURATION}, + metadataTags = {OptionMetadataTag.EXPERIMENTAL}, + help = + "For debugging --experimental_exec_config only: if set and --experimental_exec_config is" + + " set, Bazel also runs internal logic on `cfg = \"exec\"` transitions and prints " + + "the diff between that and the Starlark transition to the screen. " + + "`cfg = \"exec\"` semantics still use the Starlark transition.") + public boolean execConfigDiff; + @Option( name = "experimental_platform_in_output_dir", defaultValue = "false", @@ -1064,6 +1089,9 @@ public FragmentOptions getExec() { exec.archivedArtifactsMnemonicsFilter = archivedArtifactsMnemonicsFilter; exec.allowUnresolvedSymlinks = allowUnresolvedSymlinks; + + exec.starlarkExecConfig = starlarkExecConfig; + exec.execConfigDiff = execConfigDiff; return exec; } diff --git a/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/AbstractConfiguredTarget.java b/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/AbstractConfiguredTarget.java index b5cb82d373fca0..917d5f48bca36b 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/AbstractConfiguredTarget.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/AbstractConfiguredTarget.java @@ -23,6 +23,7 @@ import com.google.devtools.build.lib.analysis.FilesToRunProvider; import com.google.devtools.build.lib.analysis.OutputGroupInfo; import com.google.devtools.build.lib.analysis.TransitiveInfoProvider; +import com.google.devtools.build.lib.analysis.TransitiveInfoProviderMap; import com.google.devtools.build.lib.analysis.VisibilityProvider; import com.google.devtools.build.lib.collect.nestedset.NestedSet; import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; @@ -30,9 +31,11 @@ import com.google.devtools.build.lib.packages.Info; import com.google.devtools.build.lib.packages.PackageSpecification.PackageGroupContents; import com.google.devtools.build.lib.packages.Provider; +import com.google.devtools.build.lib.packages.StarlarkProvider; import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions; import java.util.function.Consumer; import javax.annotation.Nullable; +import net.starlark.java.eval.Dict; import net.starlark.java.eval.EvalException; import net.starlark.java.eval.Printer; import net.starlark.java.eval.Starlark; @@ -63,7 +66,7 @@ public abstract class AbstractConfiguredTarget implements ConfiguredTarget, Visi *

If you respond to this key you are expected to return a list of actions belonging to this * configured target. */ - public static final String ACTIONS_FIELD_NAME = "actions"; + static final String ACTIONS_FIELD_NAME = "actions"; // A set containing all field names which may be specially handled (and thus may not be // attributed to normal user-specified providers). @@ -77,7 +80,7 @@ public abstract class AbstractConfiguredTarget implements ConfiguredTarget, Visi OutputGroupInfo.STARLARK_NAME, ACTIONS_FIELD_NAME); - public AbstractConfiguredTarget(ActionLookupKeyOrProxy actionLookupKey) { + AbstractConfiguredTarget(ActionLookupKeyOrProxy actionLookupKey) { this(actionLookupKey, NestedSetBuilder.emptySet(Order.STABLE_ORDER)); } @@ -251,4 +254,54 @@ public String getRuleClassString() { public void repr(Printer printer) { printer.append(""); } + + /** + * Returns a {@link Dict} of provider names to their values for a configured target, intended to + * be called from {@link #getProvidersDictForQuery}. + * + *

{@link #getProvidersDictForQuery} is intended to be used from Starlark query output methods, + * so all values must be accessible in Starlark. If the value of a provider is not convertible to + * a Starlark value, that name/value pair is left out of the {@link Dict}. + */ + static Dict toProvidersDictForQuery(TransitiveInfoProviderMap providers) { + Dict.Builder dict = Dict.builder(); + for (int i = 0; i < providers.getProviderCount(); i++) { + tryAddProviderForQuery( + dict, providers.getProviderKeyAt(i), providers.getProviderInstanceAt(i)); + } + return dict.buildImmutable(); + } + + /** + * Attempts to add a provider instance to {@code dict} under an unspecified stringification of the + * given key. Takes no action if the provider instance is not a valid Starlark value or if the key + * is of an unknown type. + * + *

Intended to be called from {@link #getProvidersDictForQuery}. + */ + static void tryAddProviderForQuery( + Dict.Builder dict, Object key, Object providerInstance) { + // The key may be of many types, but we need a string for the intended use. + String keyAsString; + if (key instanceof String) { + keyAsString = (String) key; + } else if (key instanceof Provider.Key) { + if (key instanceof StarlarkProvider.Key) { + StarlarkProvider.Key k = (StarlarkProvider.Key) key; + keyAsString = k.getExtensionLabel() + "%" + k.getExportedName(); + } else { + keyAsString = key.toString(); + } + } else if (key instanceof Class) { + keyAsString = ((Class) key).getSimpleName(); + } else { + // ??? + return; + } + try { + dict.put(keyAsString, Starlark.fromJava(providerInstance, /* mutability= */ null)); + } catch (Starlark.InvalidStarlarkValueException e) { + // This is OK. If this is not a valid StarlarkValue, we just leave it out of the map. + } + } } diff --git a/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/ConfiguredTargetsUtil.java b/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/ConfiguredTargetsUtil.java deleted file mode 100644 index c5525656625e22..00000000000000 --- a/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/ConfiguredTargetsUtil.java +++ /dev/null @@ -1,65 +0,0 @@ -// Copyright 2020 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. -package com.google.devtools.build.lib.analysis.configuredtargets; - -import com.google.devtools.build.lib.analysis.TransitiveInfoProviderMap; -import com.google.devtools.build.lib.packages.Provider; -import com.google.devtools.build.lib.packages.StarlarkProvider; -import net.starlark.java.eval.Dict; -import net.starlark.java.eval.Starlark; - -/** Utility methods for configured gargets. */ -public final class ConfiguredTargetsUtil { - - private ConfiguredTargetsUtil() {} - - /** - * Returns a Dict of provider names to their values for a configured target. - * - *

This map is intended to be used from Starlark query output methods, so all values must be - * accessible in Starlark. If the value of a provider is not convertible to a Starlark value, that - * name/value pair is left out of the map. - */ - public static Dict getProvidersDict( - AbstractConfiguredTarget target, TransitiveInfoProviderMap providers) { - Dict.Builder res = Dict.builder(); - for (int i = 0; i < providers.getProviderCount(); i++) { - // The key may be of many types, but we need a string for the intended use. - Object key = providers.getProviderKeyAt(i); - Object v = providers.getProviderInstanceAt(i); - String keyAsString; - if (key instanceof String) { - keyAsString = key.toString(); - } else if (key instanceof Provider.Key) { - if (key instanceof StarlarkProvider.Key) { - StarlarkProvider.Key k = (StarlarkProvider.Key) key; - keyAsString = k.getExtensionLabel().toString() + "%" + k.getExportedName(); - } else { - keyAsString = key.toString(); - } - } else if (key instanceof Class) { - keyAsString = ((Class) key).getSimpleName(); - } else { - // ??? - continue; - } - try { - res.put(keyAsString, Starlark.fromJava(v, null)); - } catch (IllegalArgumentException ex) { - // This is OK. If this is not a valid StarlarkValue, we just leave it out of the map. - } - } - return res.buildImmutable(); - } -} diff --git a/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/FileConfiguredTarget.java b/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/FileConfiguredTarget.java index f91fdb74dfc192..033d17914175b4 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/FileConfiguredTarget.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/FileConfiguredTarget.java @@ -14,26 +14,18 @@ package com.google.devtools.build.lib.analysis.configuredtargets; -import com.google.devtools.build.lib.actions.ActionLookupKeyOrProxy; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.analysis.AnalysisUtils; import com.google.devtools.build.lib.analysis.FileProvider; import com.google.devtools.build.lib.analysis.FilesToRunProvider; import com.google.devtools.build.lib.analysis.LicensesProvider; -import com.google.devtools.build.lib.analysis.OutputGroupInfo; -import com.google.devtools.build.lib.analysis.RequiredConfigFragmentsProvider; +import com.google.devtools.build.lib.analysis.TargetContext; import com.google.devtools.build.lib.analysis.TransitiveInfoProvider; -import com.google.devtools.build.lib.analysis.TransitiveInfoProviderMap; -import com.google.devtools.build.lib.analysis.TransitiveInfoProviderMapBuilder; import com.google.devtools.build.lib.analysis.VisibilityProvider; -import com.google.devtools.build.lib.analysis.test.InstrumentedFilesInfo; import com.google.devtools.build.lib.collect.nestedset.NestedSet; import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; import com.google.devtools.build.lib.collect.nestedset.Order; import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; -import com.google.devtools.build.lib.packages.Info; -import com.google.devtools.build.lib.packages.PackageSpecification.PackageGroupContents; -import com.google.devtools.build.lib.packages.Provider; import com.google.devtools.build.lib.util.FileType; import javax.annotation.Nullable; import net.starlark.java.eval.Dict; @@ -46,60 +38,24 @@ public abstract class FileConfiguredTarget extends AbstractConfiguredTarget implements FileType.HasFileType, LicensesProvider { - private final TransitiveInfoProviderMap providers; + private final NestedSet singleFile; - FileConfiguredTarget( - ActionLookupKeyOrProxy actionLookupKey, - NestedSet visibility, - Artifact artifact, - @Nullable InstrumentedFilesInfo instrumentedFilesInfo, - @Nullable RequiredConfigFragmentsProvider configFragmentsProvider, - @Nullable OutputGroupInfo generatingRuleOutputGroupInfo) { - - super(actionLookupKey, visibility); - - NestedSet filesToBuild = NestedSetBuilder.create(Order.STABLE_ORDER, artifact); - FilesToRunProvider filesToRunProvider = - FilesToRunProvider.create( - filesToBuild, /* runfilesSupport= */ null, /* executable= */ artifact); - TransitiveInfoProviderMapBuilder providerBuilder = - new TransitiveInfoProviderMapBuilder() - .put(VisibilityProvider.class, this) - .put(LicensesProvider.class, this) - .add(FileProvider.of(filesToBuild)) - .add(filesToRunProvider); - - if (instrumentedFilesInfo != null) { - providerBuilder.put(instrumentedFilesInfo); - } - - if (generatingRuleOutputGroupInfo != null) { - NestedSet validationOutputs = - generatingRuleOutputGroupInfo.getOutputGroup(OutputGroupInfo.VALIDATION); - if (!validationOutputs.isEmpty()) { - providerBuilder.put( - OutputGroupInfo.singleGroup(OutputGroupInfo.VALIDATION, validationOutputs)); - } - } - - if (configFragmentsProvider != null) { - providerBuilder.add(configFragmentsProvider); - } - - this.providers = providerBuilder.build(); + FileConfiguredTarget(TargetContext targetContext, Artifact artifact) { + super(targetContext.getAnalysisEnvironment().getOwner(), targetContext.getVisibility()); + this.singleFile = NestedSetBuilder.create(Order.STABLE_ORDER, artifact); } - public abstract Artifact getArtifact(); + public Artifact getArtifact() { + return singleFile.getSingleton(); + } - /** - * Returns the file name of this file target. - */ - public String getFilename() { + /** Returns the file name of this file target. */ + public final String getFilename() { return getLabel().getName(); } @Override - public String filePathForFileTypeMatcher() { + public final String filePathForFileTypeMatcher() { return getFilename(); } @@ -107,23 +63,50 @@ public String filePathForFileTypeMatcher() { @Nullable public

P getProvider(Class

providerClass) { AnalysisUtils.checkProvider(providerClass); - return providers.getProvider(providerClass); + return providerClass.cast(getProviderInternal(providerClass)); } - @Override @Nullable - protected Info rawGetStarlarkProvider(Provider.Key providerKey) { - return providers.get(providerKey); + private TransitiveInfoProvider getProviderInternal( + Class providerClass) { + // The set of possible providers is small and predictable, so to save memory, this method does + // simple identity checks so that we don't need to store a TransitiveInfoProviderMap. + // Additionally, file providers are created on-demand when requested. These optimizations + // combine to save over 1% of analysis heap. + if (providerClass == VisibilityProvider.class || providerClass == LicensesProvider.class) { + return this; + } + if (providerClass == FileProvider.class) { + return createFileProvider(); + } + if (providerClass == FilesToRunProvider.class) { + return createFilesToRunProvider(); + } + return null; + } + + private FileProvider createFileProvider() { + return FileProvider.of(singleFile); + } + + private FilesToRunProvider createFilesToRunProvider() { + return FilesToRunProvider.create( + singleFile, /* runfilesSupport= */ null, /* executable= */ getArtifact()); } @Override @Nullable - protected Object rawGetStarlarkProvider(String providerKey) { + protected final Object rawGetStarlarkProvider(String providerKey) { return null; } @Override - public Dict getProvidersDict() { - return ConfiguredTargetsUtil.getProvidersDict(this, providers); + public Dict getProvidersDictForQuery() { + Dict.Builder dict = Dict.builder(); + tryAddProviderForQuery(dict, VisibilityProvider.class, this); + tryAddProviderForQuery(dict, LicensesProvider.class, this); + tryAddProviderForQuery(dict, FileProvider.class, createFileProvider()); + tryAddProviderForQuery(dict, FilesToRunProvider.class, createFilesToRunProvider()); + return dict.buildImmutable(); } } diff --git a/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/InputFileConfiguredTarget.java b/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/InputFileConfiguredTarget.java index 3a0705ce7bef42..fa5270ed8e4971 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/InputFileConfiguredTarget.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/InputFileConfiguredTarget.java @@ -14,16 +14,19 @@ package com.google.devtools.build.lib.analysis.configuredtargets; -import com.google.common.base.Preconditions; -import com.google.devtools.build.lib.actions.Artifact; +import static com.google.common.base.Preconditions.checkArgument; + import com.google.devtools.build.lib.actions.Artifact.SourceArtifact; import com.google.devtools.build.lib.analysis.TargetContext; import com.google.devtools.build.lib.collect.nestedset.NestedSet; import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; import com.google.devtools.build.lib.collect.nestedset.Order; import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; +import com.google.devtools.build.lib.packages.Info; import com.google.devtools.build.lib.packages.InputFile; import com.google.devtools.build.lib.packages.License; +import com.google.devtools.build.lib.packages.Provider; +import com.google.devtools.build.lib.packages.Target; import java.util.Objects; import javax.annotation.Nullable; import net.starlark.java.eval.Printer; @@ -36,25 +39,17 @@ */ @Immutable public final class InputFileConfiguredTarget extends FileConfiguredTarget { - private final SourceArtifact artifact; + private final NestedSet licenses; - public InputFileConfiguredTarget( - TargetContext targetContext, InputFile inputFile, SourceArtifact artifact) { - super( - targetContext.getAnalysisEnvironment().getOwner(), - targetContext.getVisibility(), - artifact, - /* instrumentedFilesInfo= */ null, - /* configFragmentsProvider= */ null, - /* generatingRuleOutputGroupInfo= */ null); - Preconditions.checkArgument(getConfigurationKey() == null, getLabel()); - Preconditions.checkArgument(targetContext.getTarget() == inputFile, getLabel()); - this.artifact = artifact; - this.licenses = makeLicenses(inputFile); + public InputFileConfiguredTarget(TargetContext targetContext, SourceArtifact artifact) { + super(targetContext, artifact); + this.licenses = makeLicenses(targetContext.getTarget()); + checkArgument(targetContext.getTarget() instanceof InputFile, targetContext.getTarget()); + checkArgument(getConfigurationKey() == null, getLabel()); } - private static NestedSet makeLicenses(InputFile inputFile) { + private static NestedSet makeLicenses(Target inputFile) { License license = inputFile.getLicense(); return Objects.equals(license, License.NO_LICENSE) ? NestedSetBuilder.emptySet(Order.LINK_ORDER) @@ -63,17 +58,19 @@ private static NestedSet makeLicenses(InputFile inputFile) { } @Override - public final Artifact getArtifact() { - return artifact; + public SourceArtifact getArtifact() { + return (SourceArtifact) super.getArtifact(); } @Override - public String toString() { - return "InputFileConfiguredTarget(" + getLabel() + ")"; + @Nullable + protected Info rawGetStarlarkProvider(Provider.Key providerKey) { + // FileConfiguredTarget only defines native providers, which are not available to Starlark. + return null; } @Override - public final NestedSet getTransitiveLicenses() { + public NestedSet getTransitiveLicenses() { return licenses; } @@ -93,7 +90,8 @@ public void repr(Printer printer) { printer.append(""); } - public SourceArtifact getSourceArtifact() { - return artifact; + @Override + public String toString() { + return "InputFileConfiguredTarget(" + getLabel() + ")"; } } diff --git a/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/MergedConfiguredTarget.java b/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/MergedConfiguredTarget.java index 6433005a3b0a26..40d6882bc6a453 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/MergedConfiguredTarget.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/MergedConfiguredTarget.java @@ -298,8 +298,8 @@ public void repr(Printer printer) { } @Override - public Dict getProvidersDict() { - return ConfiguredTargetsUtil.getProvidersDict(this, nonBaseProviders); + public Dict getProvidersDictForQuery() { + return toProvidersDictForQuery(nonBaseProviders); } @Override diff --git a/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/OutputFileConfiguredTarget.java b/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/OutputFileConfiguredTarget.java index af6a19b563a7a0..444b931e904e19 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/OutputFileConfiguredTarget.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/OutputFileConfiguredTarget.java @@ -14,92 +14,126 @@ package com.google.devtools.build.lib.analysis.configuredtargets; -import com.google.common.base.Preconditions; +import static com.google.common.base.MoreObjects.firstNonNull; +import static com.google.common.base.Preconditions.checkArgument; +import static com.google.common.base.Preconditions.checkNotNull; + import com.google.devtools.build.lib.actions.Artifact; -import com.google.devtools.build.lib.analysis.ConfiguredTarget; import com.google.devtools.build.lib.analysis.LicensesProvider; import com.google.devtools.build.lib.analysis.LicensesProviderImpl; import com.google.devtools.build.lib.analysis.OutputGroupInfo; import com.google.devtools.build.lib.analysis.RequiredConfigFragmentsProvider; import com.google.devtools.build.lib.analysis.TargetContext; -import com.google.devtools.build.lib.analysis.TransitiveInfoCollection; import com.google.devtools.build.lib.analysis.TransitiveInfoProvider; import com.google.devtools.build.lib.analysis.test.InstrumentedFilesInfo; import com.google.devtools.build.lib.collect.nestedset.NestedSet; import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; +import com.google.devtools.build.lib.packages.Info; import com.google.devtools.build.lib.packages.OutputFile; +import com.google.devtools.build.lib.packages.Provider; +import javax.annotation.Nullable; +import net.starlark.java.eval.Dict; import net.starlark.java.eval.Printer; /** A ConfiguredTarget for an OutputFile. */ @Immutable -public class OutputFileConfiguredTarget extends FileConfiguredTarget { +public final class OutputFileConfiguredTarget extends FileConfiguredTarget { - private final Artifact artifact; - private final ConfiguredTarget generatingRule; + private final RuleConfiguredTarget generatingRule; public OutputFileConfiguredTarget( - TargetContext targetContext, - OutputFile outputFile, - ConfiguredTarget generatingRule, - Artifact outputArtifact) { - super( - targetContext.getAnalysisEnvironment().getOwner(), - targetContext.getVisibility(), - outputArtifact, - instrumentedFilesInfo(generatingRule), - generatingRule.getProvider(RequiredConfigFragmentsProvider.class), - Preconditions.checkNotNull(generatingRule).get(OutputGroupInfo.STARLARK_CONSTRUCTOR)); - Preconditions.checkArgument(targetContext.getTarget() == outputFile); - this.artifact = outputArtifact; - this.generatingRule = Preconditions.checkNotNull(generatingRule); + TargetContext targetContext, Artifact outputArtifact, RuleConfiguredTarget generatingRule) { + super(targetContext, outputArtifact); + this.generatingRule = checkNotNull(generatingRule); + checkArgument(targetContext.getTarget() instanceof OutputFile, targetContext.getTarget()); } - private static InstrumentedFilesInfo instrumentedFilesInfo( - TransitiveInfoCollection generatingRule) { - Preconditions.checkNotNull(generatingRule); - InstrumentedFilesInfo provider = generatingRule.get(InstrumentedFilesInfo.STARLARK_CONSTRUCTOR); - return provider == null ? InstrumentedFilesInfo.EMPTY : provider; + public RuleConfiguredTarget getGeneratingRule() { + return generatingRule; } - public ConfiguredTarget getGeneratingRule() { - return generatingRule; + @Override + @Nullable + public

P getProvider(Class

providerClass) { + P provider = super.getProvider(providerClass); + if (provider != null) { + return provider; + } + if (providerClass == RequiredConfigFragmentsProvider.class) { + return generatingRule.getProvider(providerClass); + } + return null; } + @Nullable @Override - public final Artifact getArtifact() { - return artifact; + protected Info rawGetStarlarkProvider(Provider.Key providerKey) { + // The following Starlark providers do not implement TransitiveInfoProvider and thus may only be + // requested via this method using a Provider.Key, not via getProvider(Class) above. + + if (providerKey.equals(InstrumentedFilesInfo.STARLARK_CONSTRUCTOR.getKey())) { + return firstNonNull( + generatingRule.get(InstrumentedFilesInfo.STARLARK_CONSTRUCTOR), + InstrumentedFilesInfo.EMPTY); + } + + if (providerKey.equals(OutputGroupInfo.STARLARK_CONSTRUCTOR.getKey())) { + // We have an OutputFileConfiguredTarget, so the generating rule must have OutputGroupInfo. + NestedSet validationOutputs = + generatingRule + .get(OutputGroupInfo.STARLARK_CONSTRUCTOR) + .getOutputGroup(OutputGroupInfo.VALIDATION); + if (!validationOutputs.isEmpty()) { + return OutputGroupInfo.singleGroup(OutputGroupInfo.VALIDATION, validationOutputs); + } + } + + return null; + } + + @Override + public Dict getProvidersDictForQuery() { + Dict.Builder dict = Dict.builder(); + dict.putAll(super.getProvidersDictForQuery()); + addStarlarkProviderIfPresent(dict, InstrumentedFilesInfo.STARLARK_CONSTRUCTOR); + addStarlarkProviderIfPresent(dict, OutputGroupInfo.STARLARK_CONSTRUCTOR); + addNativeProviderFromRuleIfPresent(dict, RequiredConfigFragmentsProvider.class); + return dict.buildImmutable(); + } + + private void addStarlarkProviderIfPresent(Dict.Builder dict, Provider provider) { + Info info = rawGetStarlarkProvider(provider.getKey()); + if (info != null) { + tryAddProviderForQuery(dict, provider.getKey(), info); + } + } + + private void addNativeProviderFromRuleIfPresent( + Dict.Builder dict, Class providerClass) { + TransitiveInfoProvider provider = generatingRule.getProvider(providerClass); + if (provider != null) { + tryAddProviderForQuery(dict, providerClass, provider); + } } @Override public NestedSet getTransitiveLicenses() { - return getProvider(LicensesProvider.class, LicensesProviderImpl.EMPTY) - .getTransitiveLicenses(); + return getLicencesProviderFromGeneratingRule().getTransitiveLicenses(); } @Override public TargetLicense getOutputLicenses() { - return getProvider(LicensesProvider.class, LicensesProviderImpl.EMPTY) - .getOutputLicenses(); + return getLicencesProviderFromGeneratingRule().getOutputLicenses(); } @Override public boolean hasOutputLicenses() { - return getProvider(LicensesProvider.class, LicensesProviderImpl.EMPTY) - .hasOutputLicenses(); + return getLicencesProviderFromGeneratingRule().hasOutputLicenses(); } - /** - * Returns the corresponding provider from the generating rule, if it is non-null, or {@code - * defaultValue} otherwise. - */ - private T getProvider(Class clazz, T defaultValue) { - if (generatingRule != null) { - T result = generatingRule.getProvider(clazz); - if (result != null) { - return result; - } - } - return defaultValue; + private LicensesProvider getLicencesProviderFromGeneratingRule() { + return firstNonNull( + generatingRule.getProvider(LicensesProvider.class), LicensesProviderImpl.EMPTY); } @Override diff --git a/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/RuleConfiguredTarget.java b/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/RuleConfiguredTarget.java index dbd362e65f3219..a99b8a51b2fc30 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/RuleConfiguredTarget.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/RuleConfiguredTarget.java @@ -290,7 +290,7 @@ public Artifact findArtifactByOutputLabel(Label outputLabel) { } @Override - public Dict getProvidersDict() { - return ConfiguredTargetsUtil.getProvidersDict(this, providers); + public Dict getProvidersDictForQuery() { + return toProvidersDictForQuery(providers); } } diff --git a/src/main/java/com/google/devtools/build/lib/analysis/producers/AttributeConfiguration.java b/src/main/java/com/google/devtools/build/lib/analysis/producers/AttributeConfiguration.java index c927e3c38467a0..106f2ef3836c48 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/producers/AttributeConfiguration.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/producers/AttributeConfiguration.java @@ -28,6 +28,13 @@ enum Kind { * target is known, it should be verified to be a {@link PackageGroup}. */ VISIBILITY, + /** + * The configuration is null. + * + *

This is only applied when the dependency is in the same package as the parent and it is + * not configurable. + */ + NULL_CONFIGURATION, /** * There is a single configuration. * @@ -46,6 +53,8 @@ enum Kind { abstract void visibility(); + abstract void nullConfiguration(); + abstract BuildConfigurationKey unary(); abstract ImmutableMap split(); @@ -53,6 +62,7 @@ enum Kind { public int count() { switch (kind()) { case VISIBILITY: + case NULL_CONFIGURATION: case UNARY: return 1; case SPLIT: @@ -65,6 +75,10 @@ static AttributeConfiguration ofVisibility() { return AutoOneOf_AttributeConfiguration.visibility(); } + static AttributeConfiguration ofNullConfiguration() { + return AutoOneOf_AttributeConfiguration.nullConfiguration(); + } + static AttributeConfiguration ofUnary(BuildConfigurationKey key) { return AutoOneOf_AttributeConfiguration.unary(key); } diff --git a/src/main/java/com/google/devtools/build/lib/analysis/producers/ConfigConditionsProducer.java b/src/main/java/com/google/devtools/build/lib/analysis/producers/ConfigConditionsProducer.java index c9d91a559ce06a..7b71751f9bd179 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/producers/ConfigConditionsProducer.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/producers/ConfigConditionsProducer.java @@ -14,7 +14,7 @@ package com.google.devtools.build.lib.analysis.producers; import com.google.common.collect.ImmutableMap; -import com.google.devtools.build.lib.analysis.InvalidVisibilityDependencyException; +import com.google.devtools.build.lib.analysis.InconsistentNullConfigException; import com.google.devtools.build.lib.analysis.TargetAndConfiguration; import com.google.devtools.build.lib.analysis.TransitiveDependencyState; import com.google.devtools.build.lib.analysis.config.ConfigConditions; @@ -130,17 +130,12 @@ public void acceptConfiguredTargetAndDataError(ConfiguredValueCreationException } @Override - public void acceptConfiguredTargetAndDataError(InvalidVisibilityDependencyException error) { - // After removing the rule transition from dependency resolution, a ConfiguredTargetKey in - // Skyframe with a null BuildConfigurationKey will only be used to request visibility - // dependencies. This will never be the case for ConfigConditions, which are always requested - // with the parent configuration. At the moment, nothing throws - // InvalidVisibilityDependencyException. - // - // TODO(b/261521010): update this comment once rule transitions are removed from dependency - // resolution. + public void acceptConfiguredTargetAndDataError(InconsistentNullConfigException error) { + // A config label was evaluated with a null configuration. This should never happen as + // ConfigConditions are only present if the parent is a Rule, then always evaluated with the + // parent configuration. throw new IllegalArgumentException( - "ConfigCondition dependency should never be marked visibility.", error); + "ConfigCondition dependency should never be evaluated with a null configuration.", error); } private StateMachine constructConfigConditions(Tasks tasks, ExtendedEventHandler listener) { diff --git a/src/main/java/com/google/devtools/build/lib/analysis/producers/ConfiguredTargetAndDataProducer.java b/src/main/java/com/google/devtools/build/lib/analysis/producers/ConfiguredTargetAndDataProducer.java index 2ca4a755ff50b2..50c475a1824509 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/producers/ConfiguredTargetAndDataProducer.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/producers/ConfiguredTargetAndDataProducer.java @@ -16,7 +16,7 @@ import com.google.common.collect.ImmutableList; import com.google.devtools.build.lib.analysis.ConfiguredTarget; import com.google.devtools.build.lib.analysis.ConfiguredTargetValue; -import com.google.devtools.build.lib.analysis.InvalidVisibilityDependencyException; +import com.google.devtools.build.lib.analysis.InconsistentNullConfigException; import com.google.devtools.build.lib.analysis.TransitiveDependencyState; import com.google.devtools.build.lib.analysis.config.BuildConfigurationValue; import com.google.devtools.build.lib.events.ExtendedEventHandler; @@ -42,14 +42,14 @@ public final class ConfiguredTargetAndDataProducer implements StateMachine, Consumer, StateMachine.ValueOrException2Sink< - ConfiguredValueCreationException, InvalidVisibilityDependencyException> { + ConfiguredValueCreationException, InconsistentNullConfigException> { /** Interface for accepting values produced by this class. */ public interface ResultSink { void acceptConfiguredTargetAndData(ConfiguredTargetAndData value, int index); void acceptConfiguredTargetAndDataError(ConfiguredValueCreationException error); - void acceptConfiguredTargetAndDataError(InvalidVisibilityDependencyException error); + void acceptConfiguredTargetAndDataError(InconsistentNullConfigException error); } // -------------------- Input -------------------- @@ -86,9 +86,8 @@ public StateMachine step(Tasks tasks, ExtendedEventHandler listener) { tasks.lookUp( key.toKey(), ConfiguredValueCreationException.class, - InvalidVisibilityDependencyException.class, - (ValueOrException2Sink< - ConfiguredValueCreationException, InvalidVisibilityDependencyException>) + InconsistentNullConfigException.class, + (ValueOrException2Sink) this); return this::fetchConfigurationAndPackage; } @@ -97,7 +96,7 @@ public StateMachine step(Tasks tasks, ExtendedEventHandler listener) { public void acceptValueOrException2( @Nullable SkyValue value, @Nullable ConfiguredValueCreationException error, - @Nullable InvalidVisibilityDependencyException visibilityError) { + @Nullable InconsistentNullConfigException visibilityError) { if (value != null) { var configuredTargetValue = (ConfiguredTargetValue) value; this.configuredTarget = configuredTargetValue.getConfiguredTarget(); diff --git a/src/main/java/com/google/devtools/build/lib/analysis/producers/DependencyProducer.java b/src/main/java/com/google/devtools/build/lib/analysis/producers/DependencyProducer.java index 2940409889ba9d..e6bddce74310ef 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/producers/DependencyProducer.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/producers/DependencyProducer.java @@ -30,11 +30,16 @@ import com.google.devtools.build.lib.analysis.config.BuildConfigurationValue; import com.google.devtools.build.lib.analysis.config.DependencyEvaluationException; import com.google.devtools.build.lib.analysis.starlark.StarlarkTransition.TransitionException; +import com.google.devtools.build.lib.causes.LoadingFailedCause; import com.google.devtools.build.lib.cmdline.Label; +import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.events.ExtendedEventHandler; import com.google.devtools.build.lib.packages.Aspect; import com.google.devtools.build.lib.packages.Attribute; import com.google.devtools.build.lib.packages.AttributeTransitionData; +import com.google.devtools.build.lib.packages.NoSuchTargetException; +import com.google.devtools.build.lib.packages.Target; +import com.google.devtools.build.lib.packages.TargetUtils; import com.google.devtools.build.lib.skyframe.BuildConfigurationKey; import com.google.devtools.build.lib.skyframe.ConfiguredTargetAndData; import com.google.devtools.build.lib.skyframe.ConfiguredValueCreationException; @@ -112,6 +117,25 @@ public StateMachine step(Tasks tasks, ExtendedEventHandler listener) { AttributeConfiguration.ofVisibility(), /* executionPlatformLabel= */ null); } + Target parentTarget = parameters.target(); + if (parentTarget.getLabel().getPackageIdentifier().equals(toLabel.getPackageIdentifier())) { + try { + Target toTarget = parentTarget.getPackage().getTarget(toLabel.getName()); + if (!toTarget.isConfigurable()) { + return computePrerequisites( + AttributeConfiguration.ofNullConfiguration(), /* executionPlatformLabel= */ null); + } + } catch (NoSuchTargetException e) { + parameters + .transitiveState() + .addTransitiveCause(new LoadingFailedCause(toLabel, e.getDetailedExitCode())); + listener.handle( + Event.error( + TargetUtils.getLocationMaybe(parentTarget), + TargetUtils.formatMissingEdge(parentTarget, toLabel, e, kind.getAttribute()))); + } + } + // The logic of `DependencyResolver.computeDependencyLabels` implies that // `parameters.configurationKey()` is non-null for everything that follows. BuildConfigurationKey configurationKey = checkNotNull(parameters.configurationKey()); diff --git a/src/main/java/com/google/devtools/build/lib/analysis/producers/PrerequisiteParameters.java b/src/main/java/com/google/devtools/build/lib/analysis/producers/PrerequisiteParameters.java index 4cdd28db3de305..7ab5fd1300888c 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/producers/PrerequisiteParameters.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/producers/PrerequisiteParameters.java @@ -25,6 +25,7 @@ import com.google.devtools.build.lib.packages.Aspect; import com.google.devtools.build.lib.packages.ConfiguredAttributeMapper; import com.google.devtools.build.lib.packages.Rule; +import com.google.devtools.build.lib.packages.Target; import com.google.devtools.build.lib.skyframe.BuildConfigurationKey; import com.google.devtools.build.lib.skyframe.ConfiguredTargetKey; import javax.annotation.Nullable; @@ -33,7 +34,7 @@ /** Common parameters for computing prerequisites. */ public final class PrerequisiteParameters { private final ConfiguredTargetKey configuredTargetKey; - @Nullable private final Rule associatedRule; + private final Target target; private final ImmutableList aspects; private final StarlarkTransitionCache transitionCache; @@ -44,14 +45,14 @@ public final class PrerequisiteParameters { public PrerequisiteParameters( ConfiguredTargetKey configuredTargetKey, - @Nullable Rule associatedRule, + Target target, Iterable aspects, StarlarkTransitionCache transitionCache, @Nullable ToolchainCollection toolchainContexts, @Nullable ConfiguredAttributeMapper attributeMap, TransitiveDependencyState transitiveState) { this.configuredTargetKey = configuredTargetKey; - this.associatedRule = associatedRule; + this.target = target; this.aspects = ImmutableList.copyOf(aspects); this.transitionCache = transitionCache; this.toolchainContexts = toolchainContexts; @@ -63,9 +64,13 @@ public Label label() { return configuredTargetKey.getLabel(); } + public Target target() { + return target; + } + @Nullable public Rule associatedRule() { - return associatedRule; + return target.getAssociatedRule(); } @Nullable @@ -91,12 +96,8 @@ public ConfiguredAttributeMapper attributeMap() { return attributeMap; } - @Nullable public Location location() { - if (associatedRule == null) { - return null; - } - return associatedRule.getLocation(); + return target.getLocation(); } public BuildEventId eventId() { diff --git a/src/main/java/com/google/devtools/build/lib/analysis/producers/PrerequisitesProducer.java b/src/main/java/com/google/devtools/build/lib/analysis/producers/PrerequisitesProducer.java index 295eedb29706f3..43a286440cb2d9 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/producers/PrerequisitesProducer.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/producers/PrerequisitesProducer.java @@ -15,6 +15,7 @@ import static com.google.common.base.Preconditions.checkState; import static com.google.devtools.build.lib.analysis.AspectResolutionHelpers.computeAspectCollection; +import static com.google.devtools.build.lib.analysis.producers.AttributeConfiguration.Kind.VISIBILITY; import static com.google.devtools.build.lib.skyframe.ConfiguredTargetAndData.SPLIT_DEP_ORDERING; import static java.util.Arrays.sort; @@ -22,9 +23,11 @@ import com.google.devtools.build.lib.analysis.AspectCollection; import com.google.devtools.build.lib.analysis.DuplicateException; import com.google.devtools.build.lib.analysis.InconsistentAspectOrderException; +import com.google.devtools.build.lib.analysis.InconsistentNullConfigException; import com.google.devtools.build.lib.analysis.InvalidVisibilityDependencyException; import com.google.devtools.build.lib.analysis.config.BuildConfigurationValue; import com.google.devtools.build.lib.analysis.config.DependencyEvaluationException; +import com.google.devtools.build.lib.analysis.configuredtargets.PackageGroupConfiguredTarget; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.events.ExtendedEventHandler; import com.google.devtools.build.lib.packages.Aspect; @@ -101,6 +104,7 @@ interface ResultSink { public StateMachine step(Tasks tasks, ExtendedEventHandler listener) { switch (configuration.kind()) { case VISIBILITY: + case NULL_CONFIGURATION: tasks.enqueue( new ConfiguredTargetAndDataProducer( getPrerequisiteKey(/* configurationKey= */ null), @@ -141,9 +145,18 @@ public void acceptConfiguredTargetAndData(ConfiguredTargetAndData value, int ind } @Override - public void acceptConfiguredTargetAndDataError(InvalidVisibilityDependencyException error) { + public void acceptConfiguredTargetAndDataError(InconsistentNullConfigException error) { hasError = true; - sink.acceptPrerequisitesError(error); + if (configuration.kind() == VISIBILITY) { + // The target was configurable, but used as a visibility dependency. This is invalid because + // only `PackageGroup`s are accepted as visibility dependencies and those are not + // configurable. Propagates the exception with more precise information. + sink.acceptPrerequisitesError(new InvalidVisibilityDependencyException(label)); + return; + } + // `configuration.kind()` was `NULL_CONFIGURATION`. This is only used when the target is in the + // same package as the parent and not configurable so this should never happen. + throw new IllegalStateException(error); } @Override @@ -157,6 +170,15 @@ private StateMachine computeConfiguredAspects(Tasks tasks, ExtendedEventHandler return DONE; } + if (configuration.kind() == VISIBILITY) { + // Verifies that the dependency is a `package_group`. The value is always at index 0 because + // the `VISIBILITY` configuration is always unary. + if (!(configuredTargets[0].getConfiguredTarget() instanceof PackageGroupConfiguredTarget)) { + sink.acceptPrerequisitesError(new InvalidVisibilityDependencyException(label)); + return DONE; + } + } + cleanupValues(); AspectCollection aspects; diff --git a/src/main/java/com/google/devtools/build/lib/analysis/producers/TargetAndConfigurationProducer.java b/src/main/java/com/google/devtools/build/lib/analysis/producers/TargetAndConfigurationProducer.java index c8fdc99f0de7c1..b64d5fa7c0a507 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/producers/TargetAndConfigurationProducer.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/producers/TargetAndConfigurationProducer.java @@ -22,7 +22,6 @@ import com.google.devtools.build.lib.actions.ActionLookupKey; import com.google.devtools.build.lib.analysis.ConfiguredTargetValue; import com.google.devtools.build.lib.analysis.InconsistentNullConfigException; -import com.google.devtools.build.lib.analysis.InvalidVisibilityDependencyException; import com.google.devtools.build.lib.analysis.TargetAndConfiguration; import com.google.devtools.build.lib.analysis.TransitiveDependencyState; import com.google.devtools.build.lib.analysis.config.BuildConfigurationValue; @@ -35,7 +34,6 @@ import com.google.devtools.build.lib.events.ExtendedEventHandler; import com.google.devtools.build.lib.packages.NoSuchPackageException; import com.google.devtools.build.lib.packages.NoSuchTargetException; -import com.google.devtools.build.lib.packages.PackageGroup; import com.google.devtools.build.lib.packages.Rule; import com.google.devtools.build.lib.packages.RuleTransitionData; import com.google.devtools.build.lib.packages.Target; @@ -54,13 +52,8 @@ * Computes the target and configuration for a configured target key. * *

If the key has a configuration and the target is configurable, attempts to apply a rule side - * transition. If the target is not configurable, directly transitions to the null configuration. If - * the resulting configuration already has an owner, delegates to the owner instead of recomputing - * the configured target. - * - *

If the key does not have a configuration, it was requested as a visibility dependency. - * Verifies that the {@link Target} is a {@link PackageGroup}, throwing {@link - * InvalidVisibilityDependencyException} if that's not the case. + * transition. If the configuration changes, delegates to a target with the new configuration. If + * the target is not configurable, directly delegates to the null configuration. */ public final class TargetAndConfigurationProducer implements StateMachine, Consumer, TargetProducer.ResultSink { @@ -79,7 +72,6 @@ public abstract static class TargetAndConfigurationError { /** Tags the error type. */ public enum Kind { CONFIGURED_VALUE_CREATION, - INVALID_VISIBILITY_DEPENDENCY, INCONSISTENT_NULL_CONFIG } @@ -87,8 +79,6 @@ public enum Kind { public abstract ConfiguredValueCreationException configuredValueCreation(); - public abstract InvalidVisibilityDependencyException invalidVisibilityDependency(); - public abstract InconsistentNullConfigException inconsistentNullConfig(); private static TargetAndConfigurationError of(ConfiguredValueCreationException e) { @@ -96,15 +86,6 @@ private static TargetAndConfigurationError of(ConfiguredValueCreationException e .configuredValueCreation(e); } - // TODO(b/261521010): enable this error once Rule transitions are removed from dependency - // resolution. - // private static TargetAndConfigurationError of(InvalidVisibilityDependencyException e) { - // return AutoOneOf_TargetAndConfigurationProducer_TargetAndConfigurationError - // .invalidVisibilityDependency(e); - // } - - // TODO(b/261521010): delete this error once Rule transitions are removed from dependency - // resolution. private static TargetAndConfigurationError of(InconsistentNullConfigException e) { return AutoOneOf_TargetAndConfigurationProducer_TargetAndConfigurationError .inconsistentNullConfig(e); @@ -173,41 +154,29 @@ private StateMachine determineConfiguration(Tasks tasks, ExtendedEventHandler li if (configurationKey == null) { if (target.isConfigurable()) { // We somehow ended up in a target that requires a non-null configuration but with a key - // that doesn't have a configuration. This is always an error, but we need to analyze the - // dependencies of the latter target to realize that. Short-circuit the evaluation to avoid - // doing useless work and running code with a null configuration that's not prepared for it. + // that doesn't have a configuration. This is always an error, but we need to bubble this + // up to the parent to provide more context. sink.acceptTargetAndConfigurationError( TargetAndConfigurationError.of(new InconsistentNullConfigException())); return DONE; } - // TODO(b/261521010): after removing the rule transition from dependency resolution, the logic - // here changes. - // - // A null configuration key will only be used for visibility dependencies so when that's - // true, a check that the target is a PackageGroup will be performed, throwing - // InvalidVisibilityDependencyException on failure. - // - // The ConfiguredTargetKey cannot fan-in in this case. sink.acceptTargetAndConfiguration( new TargetAndConfiguration(target, /* configuration= */ null), preRuleTransitionKey); return DONE; } - // This may happen for top-level ConfiguredTargets. - // - // TODO(b/261521010): this may also happen for targets that are not top-level after removing - // rule transitions from dependency resolution. Update this comment. if (!target.isConfigurable()) { - var nullConfiguredTargetKey = - ConfiguredTargetKey.builder().setDelegate(preRuleTransitionKey).build(); - ActionLookupKey delegate = nullConfiguredTargetKey.toKey(); - if (!delegate.equals(preRuleTransitionKey)) { - // Delegates to the key that already owns the null configuration. - delegateTo(tasks, delegate); - return DONE; - } - sink.acceptTargetAndConfiguration( - new TargetAndConfiguration(target, /* configuration= */ null), nullConfiguredTargetKey); + // If target is not configurable, but requested with a configuration. Delegates to a key with + // the null configuration. This is expected to be uncommon. The common case of a + // non-configurable target is an input file, but those are usually package local and requested + // correctly with the null configuration. + delegateTo( + tasks, + ConfiguredTargetKey.builder() + .setLabel(preRuleTransitionKey.getLabel()) + .setExecutionPlatformLabel(preRuleTransitionKey.getExecutionPlatformLabel()) + .build() + .toKey()); return DONE; } @@ -267,17 +236,15 @@ private StateMachine processTransitionedKey(Tasks tasks, ExtendedEventHandler li } if (!configurationKey.equals(preRuleTransitionKey.getConfigurationKey())) { - fullKey = + delegateTo( + tasks, ConfiguredTargetKey.builder() - .setDelegate(preRuleTransitionKey) + .setLabel(preRuleTransitionKey.getLabel()) + .setExecutionPlatformLabel(preRuleTransitionKey.getExecutionPlatformLabel()) .setConfigurationKey(configurationKey) - .build(); - ActionLookupKey delegate = fullKey.toKey(); - if (!delegate.equals(preRuleTransitionKey)) { - // Delegates to the key that already owns this configuration. - delegateTo(tasks, delegate); - return DONE; - } + .build() + .toKey()); + return DONE; } else { fullKey = preRuleTransitionKey; } diff --git a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkAttributeTransitionProvider.java b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkAttributeTransitionProvider.java index 035380e5224c8d..6ba639fd9105e4 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkAttributeTransitionProvider.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkAttributeTransitionProvider.java @@ -53,7 +53,7 @@ public class StarlarkAttributeTransitionProvider implements TransitionFactory, SplitTransitionProviderApi { private final StarlarkDefinedConfigTransition starlarkDefinedConfigTransition; - StarlarkAttributeTransitionProvider( + public StarlarkAttributeTransitionProvider( StarlarkDefinedConfigTransition starlarkDefinedConfigTransition) { this.starlarkDefinedConfigTransition = starlarkDefinedConfigTransition; } diff --git a/src/main/java/com/google/devtools/build/lib/authandtls/GoogleAuthUtils.java b/src/main/java/com/google/devtools/build/lib/authandtls/GoogleAuthUtils.java index 409263929c7cdc..70e235b394fbcf 100644 --- a/src/main/java/com/google/devtools/build/lib/authandtls/GoogleAuthUtils.java +++ b/src/main/java/com/google/devtools/build/lib/authandtls/GoogleAuthUtils.java @@ -330,7 +330,7 @@ public static Credentials newGoogleCredentialsFromFile( creds = creds.createScoped(authScopes); } return creds; - } catch (IOException e) { + } catch (Exception e) { String message = "Failed to init auth credentials: " + e.getMessage(); throw new IOException(message, e); } diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BUILD b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BUILD index 49c52e86135997..b04cd1d270c42e 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BUILD +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BUILD @@ -7,7 +7,9 @@ package( filegroup( name = "srcs", - srcs = glob(["*"]), + srcs = glob(["*"]) + [ + "//src/main/java/com/google/devtools/build/lib/bazel/bzlmod/modquery:srcs", + ], visibility = ["//src:__subpackages__"], ) @@ -92,10 +94,13 @@ java_library( ":resolution", ":resolution_impl", "//src/main/java/com/google/devtools/build/lib:runtime", + "//src/main/java/com/google/devtools/build/lib/bazel/bzlmod:module_extension", "//src/main/java/com/google/devtools/build/lib/bazel/repository:repository_options", "//src/main/java/com/google/devtools/build/lib/cmdline", + "//src/main/java/com/google/devtools/build/lib/util:abrupt_exit_exception", "//src/main/java/com/google/devtools/build/lib/vfs", "//third_party:flogger", + "//third_party:gson", "//third_party:guava", "//third_party:jsr305", ], @@ -108,14 +113,17 @@ java_library( "ArchiveOverride.java", "BazelDepGraphValue.java", "BazelLockFileValue.java", + "BazelModuleResolutionEvent.java", "BazelModuleResolutionValue.java", "BzlmodFlagsAndEnvVars.java", "GitOverride.java", "InterimModule.java", "LocalPathOverride.java", + "LockFileModuleExtension.java", "Module.java", "ModuleBase.java", "ModuleExtensionEvalStarlarkThreadContext.java", + "ModuleExtensionResolutionEvent.java", "ModuleFileValue.java", "ModuleOverride.java", "MultipleVersionOverride.java", diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelDepGraphFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelDepGraphFunction.java index 1add0745d61404..94ea47b7fdb4d8 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelDepGraphFunction.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelDepGraphFunction.java @@ -41,7 +41,6 @@ import com.google.devtools.build.skyframe.SkyFunctionException.Transience; import com.google.devtools.build.skyframe.SkyKey; import com.google.devtools.build.skyframe.SkyValue; -import java.util.List; import java.util.Map; import java.util.Map.Entry; import javax.annotation.Nullable; @@ -69,13 +68,13 @@ public SkyValue compute(SkyKey skyKey, Environment env) ImmutableMap localOverrideHashes = null; ImmutableMap depGraph = null; BzlmodFlagsAndEnvVars flags = null; - BazelLockFileValue lockFile = null; + BazelLockFileValue lockfile = null; // If the module has not changed (has the same contents and flags as the lockfile), // read the dependency graph from the lock file, else run resolution and update lockfile if (!lockfileMode.equals(LockfileMode.OFF)) { - lockFile = (BazelLockFileValue) env.getValue(BazelLockFileValue.KEY); - if (lockFile == null) { + lockfile = (BazelLockFileValue) env.getValue(BazelLockFileValue.KEY); + if (lockfile == null) { return null; } flags = getFlagsAndEnvVars(env); @@ -83,17 +82,17 @@ public SkyValue compute(SkyKey skyKey, Environment env) return null; } localOverrideHashes = getLocalOverridesHashes(root.getOverrides(), env); - if (localOverrideHashes == null) { // trying to read override module + if (localOverrideHashes == null) { // still reading an override "module" return null; } - if (root.getModuleFileHash().equals(lockFile.getModuleFileHash()) - && flags.equals(lockFile.getFlags()) - && localOverrideHashes.equals(lockFile.getLocalOverrideHashes())) { - depGraph = lockFile.getModuleDepGraph(); + if (root.getModuleFileHash().equals(lockfile.getModuleFileHash()) + && flags.equals(lockfile.getFlags()) + && localOverrideHashes.equals(lockfile.getLocalOverrideHashes())) { + depGraph = lockfile.getModuleDepGraph(); } else if (lockfileMode.equals(LockfileMode.ERROR)) { - List diffLockfile = - lockFile.getDiffLockfile(root.getModuleFileHash(), localOverrideHashes, flags); + ImmutableList diffLockfile = + lockfile.getModuleAndFlagsDiff(root.getModuleFileHash(), localOverrideHashes, flags); throw new BazelDepGraphFunctionException( ExternalDepsException.withMessage( Code.BAD_MODULE, @@ -110,28 +109,34 @@ public SkyValue compute(SkyKey skyKey, Environment env) return null; } depGraph = selectionResult.getResolvedDepGraph(); - if (lockfileMode.equals(LockfileMode.UPDATE)) { - BazelLockFileValue updatedLockFile = - lockFile.toBuilder() - .setModuleFileHash(root.getModuleFileHash()) - .setFlags(flags) - .setLocalOverrideHashes(localOverrideHashes) - .setModuleDepGraph(depGraph) - .build(); - env.getListener().post(updatedLockFile); - } } ImmutableMap canonicalRepoNameLookup = depGraph.keySet().stream() .collect(toImmutableMap(ModuleKey::getCanonicalRepoName, key -> key)); - ImmutableTable extensionUsagesById = - getExtensionUsagesById(depGraph); + ImmutableTable extensionUsagesById; + try { + extensionUsagesById = getExtensionUsagesById(depGraph); + } catch (ExternalDepsException e) { + throw new BazelDepGraphFunctionException(e, Transience.PERSISTENT); + } ImmutableBiMap extensionUniqueNames = calculateUniqueNameForUsedExtensionId(extensionUsagesById); + if (!lockfileMode.equals(LockfileMode.OFF)) { + BazelLockFileValue updateLockfile = + lockfile.toBuilder() + .setModuleFileHash(root.getModuleFileHash()) + .setFlags(flags) + .setLocalOverrideHashes(localOverrideHashes) + .setModuleDepGraph(depGraph) + .build(); + env.getListener() + .post(BazelModuleResolutionEvent.create(updateLockfile, extensionUsagesById)); + } + return BazelDepGraphValue.create( depGraph, canonicalRepoNameLookup, @@ -202,8 +207,9 @@ static BzlmodFlagsAndEnvVars getFlagsAndEnvVars(Environment env) throws Interrup * For each extension usage, we resolve (i.e. canonicalize) its bzl file label. Then we can group * all usages by the label + name (the ModuleExtensionId). */ - private ImmutableTable getExtensionUsagesById( - ImmutableMap depGraph) throws BazelDepGraphFunctionException { + public static ImmutableTable + getExtensionUsagesById(ImmutableMap depGraph) + throws ExternalDepsException { ImmutableTable.Builder extensionUsagesTableBuilder = ImmutableTable.builder(); for (Module module : depGraph.values()) { @@ -220,22 +226,18 @@ private ImmutableTable getEx usage.getExtensionName(), usage.getIsolationKey()); } catch (LabelSyntaxException e) { - throw new BazelDepGraphFunctionException( - ExternalDepsException.withCauseAndMessage( - Code.BAD_MODULE, - e, - "invalid label for module extension found at %s", - usage.getLocation()), - Transience.PERSISTENT); + throw ExternalDepsException.withCauseAndMessage( + Code.BAD_MODULE, + e, + "invalid label for module extension found at %s", + usage.getLocation()); } if (!moduleExtensionId.getBzlFileLabel().getRepository().isVisible()) { - throw new BazelDepGraphFunctionException( - ExternalDepsException.withMessage( - Code.BAD_MODULE, - "invalid label for module extension found at %s: no repo visible as '@%s' here", - usage.getLocation(), - moduleExtensionId.getBzlFileLabel().getRepository().getName()), - Transience.PERSISTENT); + throw ExternalDepsException.withMessage( + Code.BAD_MODULE, + "invalid label for module extension found at %s: no repo visible as '@%s' here", + usage.getLocation(), + moduleExtensionId.getBzlFileLabel().getRepository().getName()); } extensionUsagesTableBuilder.put(moduleExtensionId, module.getKey(), usage); } diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileFunction.java index 5200381621fc7e..41cff393000ac1 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileFunction.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileFunction.java @@ -57,6 +57,7 @@ public class BazelLockFileFunction implements SkyFunction { .setFlags(EMPTY_FLAGS) .setLocalOverrideHashes(ImmutableMap.of()) .setModuleDepGraph(ImmutableMap.of()) + .setModuleExtensions(ImmutableMap.of()) .build(); public BazelLockFileFunction(Path rootDirectory) { @@ -75,12 +76,8 @@ public SkyValue compute(SkyKey skyKey, Environment env) return null; } - BazelLockFileValue bazelLockFileValue; try { - String json = FileSystemUtils.readContent(lockfilePath.asPath(), UTF_8); - bazelLockFileValue = LOCKFILE_GSON.fromJson(json, BazelLockFileValue.class); - } catch (FileNotFoundException e) { - bazelLockFileValue = EMPTY_LOCKFILE; + return getLockfileValue(lockfilePath); } catch (IOException | JsonSyntaxException | NullPointerException e) { throw new BazelLockfileFunctionException( ExternalDepsException.withMessage( @@ -90,9 +87,20 @@ public SkyValue compute(SkyKey skyKey, Environment env) e.getMessage()), Transience.PERSISTENT); } + } + + public static BazelLockFileValue getLockfileValue(RootedPath lockfilePath) throws IOException { + BazelLockFileValue bazelLockFileValue; + try { + String json = FileSystemUtils.readContent(lockfilePath.asPath(), UTF_8); + bazelLockFileValue = LOCKFILE_GSON.fromJson(json, BazelLockFileValue.class); + } catch (FileNotFoundException e) { + bazelLockFileValue = EMPTY_LOCKFILE; + } return bazelLockFileValue; } + static final class BazelLockfileFunctionException extends SkyFunctionException { BazelLockfileFunctionException(ExternalDepsException cause, Transience transience) { diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileModule.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileModule.java index 962c70a8f98d9c..521dd09a1a6dde 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileModule.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileModule.java @@ -17,6 +17,7 @@ import static com.google.devtools.build.lib.bazel.bzlmod.GsonTypeAdapterUtil.LOCKFILE_GSON; import static java.nio.charset.StandardCharsets.UTF_8; +import com.google.common.collect.ImmutableMap; import com.google.common.eventbus.Subscribe; import com.google.common.flogger.GoogleLogger; import com.google.devtools.build.lib.bazel.repository.RepositoryOptions; @@ -24,11 +25,16 @@ import com.google.devtools.build.lib.cmdline.LabelConstants; import com.google.devtools.build.lib.runtime.BlazeModule; import com.google.devtools.build.lib.runtime.CommandEnvironment; +import com.google.devtools.build.lib.util.AbruptExitException; import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.Root; import com.google.devtools.build.lib.vfs.RootedPath; +import com.google.gson.JsonSyntaxException; import java.io.IOException; +import java.util.ArrayList; +import java.util.List; +import java.util.Map; import javax.annotation.Nullable; /** @@ -38,8 +44,8 @@ public class BazelLockFileModule extends BlazeModule { private Path workspaceRoot; - - @Nullable private BazelLockFileValue lockfileValue; + @Nullable private BazelModuleResolutionEvent moduleResolutionEvent; + private final List extensionResolutionEvents = new ArrayList<>(); private static final GoogleLogger logger = GoogleLogger.forEnclosingClass(); @@ -53,23 +59,74 @@ public void beforeCommand(CommandEnvironment env) { } @Override - public void afterCommand() { - if (lockfileValue == null) { // module didn't change --> no event was posted - return; + public void afterCommand() throws AbruptExitException { + if (moduleResolutionEvent == null && extensionResolutionEvents.isEmpty()) { + return; // nothing changed, do nothing! + } + + RootedPath lockfilePath = + RootedPath.toRootedPath(Root.fromPath(workspaceRoot), LabelConstants.MODULE_LOCKFILE_NAME); + + // Create an updated version of the lockfile with the events updates + BazelLockFileValue lockfile; + if (moduleResolutionEvent != null) { + lockfile = moduleResolutionEvent.getLockfileValue(); + } else { + // Read the existing lockfile (if none exists, will get an empty lockfile value) + try { + lockfile = BazelLockFileFunction.getLockfileValue(lockfilePath); + } catch (IOException | JsonSyntaxException | NullPointerException e) { + logger.atSevere().withCause(e).log( + "Failed to read and parse the MODULE.bazel.lock file with error: %s." + + " Try deleting it and rerun the build.", + e.getMessage()); + return; + } } - updateLockfile(workspaceRoot, lockfileValue); - this.lockfileValue = null; + lockfile = + lockfile.toBuilder() + .setModuleExtensions(combineModuleExtensions(lockfile.getModuleExtensions())) + .build(); + + // Write the new value to the file + updateLockfile(lockfilePath, lockfile); + this.moduleResolutionEvent = null; + this.extensionResolutionEvents.clear(); + } + + /** + * Combines the old extensions stored in the lockfile -that are still used- with the new + * extensions from the events (if any) + * + * @param oldModuleExtensions Module extensions stored in the current lockfile + */ + private ImmutableMap combineModuleExtensions( + ImmutableMap oldModuleExtensions) { + ImmutableMap.Builder updatedExtensionMap = + ImmutableMap.builder(); + + // Add the old extensions (stored in the lockfile) only if it still has a usage somewhere + for (Map.Entry extensionEntry : + oldModuleExtensions.entrySet()) { + if (moduleResolutionEvent.getExtensionUsagesById().containsRow(extensionEntry.getKey())) { + updatedExtensionMap.put(extensionEntry); + } + } + + // Add the new resolved extensions + for (ModuleExtensionResolutionEvent extensionEvent : extensionResolutionEvents) { + updatedExtensionMap.put(extensionEvent.getExtensionId(), extensionEvent.getModuleExtension()); + } + return updatedExtensionMap.buildKeepingLast(); } /** * Updates the data stored in the lockfile (MODULE.bazel.lock) * - * @param workspaceRoot Where to update the lockfile + * @param lockfilePath Rooted path to lockfile * @param updatedLockfile The updated lockfile data to save */ - public static void updateLockfile(Path workspaceRoot, BazelLockFileValue updatedLockfile) { - RootedPath lockfilePath = - RootedPath.toRootedPath(Root.fromPath(workspaceRoot), LabelConstants.MODULE_LOCKFILE_NAME); + public static void updateLockfile(RootedPath lockfilePath, BazelLockFileValue updatedLockfile) { try { FileSystemUtils.writeContent( lockfilePath.asPath(), UTF_8, LOCKFILE_GSON.toJson(updatedLockfile)); @@ -80,7 +137,12 @@ public static void updateLockfile(Path workspaceRoot, BazelLockFileValue updated } @Subscribe - public void bazelModuleResolved(BazelLockFileValue lockfileValue) { - this.lockfileValue = lockfileValue; + public void bazelModuleResolved(BazelModuleResolutionEvent moduleResolutionEvent) { + this.moduleResolutionEvent = moduleResolutionEvent; + } + + @Subscribe + public void moduleExtensionResolved(ModuleExtensionResolutionEvent extensionResolutionEvent) { + this.extensionResolutionEvents.add(extensionResolutionEvent); } } diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileValue.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileValue.java index fa93c4f39874e4..c0d0c06aed41e4 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileValue.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileValue.java @@ -17,6 +17,7 @@ import com.google.auto.value.AutoValue; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.devtools.build.lib.events.ExtendedEventHandler.Postable; import com.google.devtools.build.lib.skyframe.SkyFunctions; @@ -24,7 +25,7 @@ import com.google.devtools.build.skyframe.SkyKey; import com.google.devtools.build.skyframe.SkyValue; import com.ryanharter.auto.value.gson.GenerateTypeAdapter; -import java.util.ArrayList; +import java.util.Arrays; import java.util.Map; /** @@ -41,7 +42,9 @@ public abstract class BazelLockFileValue implements SkyValue, Postable { @SerializationConstant public static final SkyKey KEY = () -> SkyFunctions.BAZEL_LOCK_FILE; static Builder builder() { - return new AutoValue_BazelLockFileValue.Builder().setLockFileVersion(LOCK_FILE_VERSION); + return new AutoValue_BazelLockFileValue.Builder() + .setLockFileVersion(LOCK_FILE_VERSION) + .setModuleExtensions(ImmutableMap.of()); } /** Current version of the lock file */ @@ -59,6 +62,9 @@ static Builder builder() { /** The post-selection dep graph retrieved from the lock file. */ public abstract ImmutableMap getModuleDepGraph(); + /** Mapping the extension id to the module extension data */ + public abstract ImmutableMap getModuleExtensions(); + public abstract Builder toBuilder(); /** Builder type for {@link BazelLockFileValue}. */ @@ -74,30 +80,55 @@ public abstract static class Builder { public abstract Builder setModuleDepGraph(ImmutableMap value); + public abstract Builder setModuleExtensions( + ImmutableMap value); + public abstract BazelLockFileValue build(); } /** Returns the difference between the lockfile and the current module & flags */ - public ArrayList getDiffLockfile( + public ImmutableList getModuleAndFlagsDiff( String moduleFileHash, ImmutableMap localOverrideHashes, BzlmodFlagsAndEnvVars flags) { - ArrayList diffLockfile = new ArrayList<>(); + ImmutableList.Builder moduleDiff = new ImmutableList.Builder<>(); if (!moduleFileHash.equals(getModuleFileHash())) { - diffLockfile.add("the root MODULE.bazel has been modified"); + moduleDiff.add("the root MODULE.bazel has been modified"); } - diffLockfile.addAll(getFlags().getDiffFlags(flags)); + moduleDiff.addAll(getFlags().getDiffFlags(flags)); for (Map.Entry entry : localOverrideHashes.entrySet()) { String currentValue = entry.getValue(); String lockfileValue = getLocalOverrideHashes().get(entry.getKey()); // If the lockfile value is null, the module hash would be different anyway if (lockfileValue != null && !currentValue.equals(lockfileValue)) { - diffLockfile.add( + moduleDiff.add( "The MODULE.bazel file has changed for the overriden module: " + entry.getKey()); } } + return moduleDiff.build(); + } - return diffLockfile; + public ImmutableList getModuleExtensionDiff( + LockFileModuleExtension lockedExtension, + ImmutableMap lockedExtensionUsages, + ModuleExtensionId extensionId, + byte[] transitiveDigest, + ImmutableMap extensionUsages) { + ImmutableList.Builder extDiff = new ImmutableList.Builder<>(); + if (lockedExtension == null) { + extDiff.add("The module extension '" + extensionId + "' does not exist in the lockfile"); + } else { + if (!Arrays.equals(transitiveDigest, lockedExtension.getBzlTransitiveDigest())) { + extDiff.add( + "The implementation of the extension '" + + extensionId + + "' or one of its transitive .bzl files has changed"); + } + if (!extensionUsages.equals(lockedExtensionUsages)) { + extDiff.add("The usages of the extension named '" + extensionId + "' has changed"); + } + } + return extDiff.build(); } } diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModuleInspectorValue.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModuleInspectorValue.java index 667f94e296dbdf..ca4632d32ad8bb 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModuleInspectorValue.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModuleInspectorValue.java @@ -196,17 +196,27 @@ public AugmentedModule.Builder addDepReason(String repoName, ResolutionReason re /** The reason why a final dependency of a module was resolved the way it was. */ public enum ResolutionReason { /** The dependency is the original dependency defined in the MODULE.bazel file. */ - ORIGINAL, + ORIGINAL(""), /** The dependency was replaced by the Minimal-Version Selection algorithm. */ - MINIMAL_VERSION_SELECTION, + MINIMAL_VERSION_SELECTION("MVS"), /** The dependency was replaced by a {@code single_version_override} rule. */ - SINGLE_VERSION_OVERRIDE, + SINGLE_VERSION_OVERRIDE("SVO"), /** The dependency was replaced by a {@code multiple_version_override} rule. */ - MULTIPLE_VERSION_OVERRIDE, + MULTIPLE_VERSION_OVERRIDE("MVO"), /** The dependency was replaced by one of the {@link NonRegistryOverride} rules. */ - ARCHIVE_OVERRIDE, - GIT_OVERRIDE, - LOCAL_PATH_OVERRIDE, + ARCHIVE_OVERRIDE("archive"), + GIT_OVERRIDE("git"), + LOCAL_PATH_OVERRIDE("local"); + + private final String label; + + ResolutionReason(String label) { + this.label = label; + } + + public String getLabel() { + return label; + } } } } diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModuleResolutionEvent.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModuleResolutionEvent.java new file mode 100644 index 00000000000000..81a0dbaa144b9c --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModuleResolutionEvent.java @@ -0,0 +1,39 @@ +// 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. + +package com.google.devtools.build.lib.bazel.bzlmod; + +import com.google.auto.value.AutoValue; +import com.google.common.collect.ImmutableTable; +import com.google.devtools.build.lib.events.ExtendedEventHandler.Postable; + +/** + * After resolving bazel module this event is sent from {@link BazelDepGraphFunction} holding the + * lockfile value with module updates, and the module extension usages. It will be received in + * {@link BazelLockFileModule} to be used to update the lockfile content + */ +@AutoValue +public abstract class BazelModuleResolutionEvent implements Postable { + + public static BazelModuleResolutionEvent create( + BazelLockFileValue lockFileValue, + ImmutableTable extensionUsagesById) { + return new AutoValue_BazelModuleResolutionEvent(lockFileValue, extensionUsagesById); + } + + public abstract BazelLockFileValue getLockfileValue(); + + public abstract ImmutableTable + getExtensionUsagesById(); +} diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BzlmodFlagsAndEnvVars.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BzlmodFlagsAndEnvVars.java index f5f483f716c04b..a69e53791f247f 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BzlmodFlagsAndEnvVars.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BzlmodFlagsAndEnvVars.java @@ -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. -// package com.google.devtools.build.lib.bazel.bzlmod; @@ -19,7 +18,6 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.ryanharter.auto.value.gson.GenerateTypeAdapter; -import java.util.ArrayList; /** Stores the values of flags and environment variables that affect the resolution */ @AutoValue @@ -65,8 +63,8 @@ public static BzlmodFlagsAndEnvVars create( /** Error level of bazel compatability check */ public abstract String compatibilityMode(); - public ArrayList getDiffFlags(BzlmodFlagsAndEnvVars flags) { - ArrayList diffFlags = new ArrayList<>(); + public ImmutableList getDiffFlags(BzlmodFlagsAndEnvVars flags) { + ImmutableList.Builder diffFlags = new ImmutableList.Builder<>(); if (!flags.cmdRegistries().equals(cmdRegistries())) { diffFlags.add("the value of --registry flag has been modified"); } @@ -89,6 +87,6 @@ public ArrayList getDiffFlags(BzlmodFlagsAndEnvVars flags) { if (!flags.compatibilityMode().equals(compatibilityMode())) { diffFlags.add("the value of --check_bazel_compatibility flag has been modified"); } - return diffFlags; + return diffFlags.build(); } } diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/GsonTypeAdapterUtil.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/GsonTypeAdapterUtil.java index ba008528516802..c558e806e9d6d3 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/GsonTypeAdapterUtil.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/GsonTypeAdapterUtil.java @@ -23,6 +23,8 @@ import com.google.common.base.Preconditions; import com.google.common.base.Splitter; import com.google.devtools.build.lib.bazel.bzlmod.Version.ParseException; +import com.google.devtools.build.lib.cmdline.Label; +import com.google.devtools.build.lib.cmdline.LabelSyntaxException; import com.google.gson.Gson; import com.google.gson.GsonBuilder; import com.google.gson.JsonParseException; @@ -36,6 +38,7 @@ import java.io.IOException; import java.lang.reflect.ParameterizedType; import java.lang.reflect.Type; +import java.util.Base64; import java.util.List; import java.util.Optional; import javax.annotation.Nullable; @@ -96,6 +99,45 @@ public ModuleKey read(JsonReader jsonReader) throws IOException { } }; + // TODO(salmasamy) need to handle "isolated" in module extensions when it is stable + public static final TypeAdapter MODULE_EXTENSION_ID_TYPE_ADAPTER = + new TypeAdapter<>() { + @Override + public void write(JsonWriter jsonWriter, ModuleExtensionId moduleExtId) throws IOException { + jsonWriter.value(moduleExtId.getBzlFileLabel() + "%" + moduleExtId.getExtensionName()); + } + + @Override + public ModuleExtensionId read(JsonReader jsonReader) throws IOException { + String jsonString = jsonReader.nextString(); + // [0] is labelString, [1] is extensionName + List extIdParts = Splitter.on("%").splitToList(jsonString); + try { + return ModuleExtensionId.create( + Label.parseCanonical(extIdParts.get(0)), extIdParts.get(1), Optional.empty()); + } catch (LabelSyntaxException e) { + throw new JsonParseException( + String.format( + "Unable to parse ModuleExtensionID bzl file label: '%s' from the lockfile", + extIdParts.get(0)), + e); + } + } + }; + + public static final TypeAdapter BYTE_ARRAY_TYPE_ADAPTER = + new TypeAdapter<>() { + @Override + public void write(JsonWriter jsonWriter, byte[] value) throws IOException { + jsonWriter.value(Base64.getEncoder().encodeToString(value)); + } + + @Override + public byte[] read(JsonReader jsonReader) throws IOException { + return Base64.getDecoder().decode(jsonReader.nextString()); + } + }; + public static final TypeAdapterFactory OPTIONAL = new TypeAdapterFactory() { @Nullable @@ -150,6 +192,7 @@ public Optional read(JsonReader jsonReader) throws IOException { new GsonBuilder() .setPrettyPrinting() .disableHtmlEscaping() + .enableComplexMapKeySerialization() .registerTypeAdapterFactory(GenerateTypeAdapter.FACTORY) .registerTypeAdapterFactory(DICT) .registerTypeAdapterFactory(IMMUTABLE_MAP) @@ -159,7 +202,9 @@ public Optional read(JsonReader jsonReader) throws IOException { .registerTypeAdapterFactory(OPTIONAL) .registerTypeAdapter(Version.class, VERSION_TYPE_ADAPTER) .registerTypeAdapter(ModuleKey.class, MODULE_KEY_TYPE_ADAPTER) + .registerTypeAdapter(ModuleExtensionId.class, MODULE_EXTENSION_ID_TYPE_ADAPTER) .registerTypeAdapter(AttributeValues.class, new AttributeValuesAdapter()) + .registerTypeAdapter(byte[].class, BYTE_ARRAY_TYPE_ADAPTER) .create(); private GsonTypeAdapterUtil() {} diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/LockFileModuleExtension.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/LockFileModuleExtension.java new file mode 100644 index 00000000000000..af4da1e131ee18 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/LockFileModuleExtension.java @@ -0,0 +1,40 @@ +// Copyright 2021 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. + +package com.google.devtools.build.lib.bazel.bzlmod; + +import com.google.auto.value.AutoValue; +import com.google.common.collect.ImmutableMap; +import com.google.devtools.build.lib.events.ExtendedEventHandler.Postable; +import com.ryanharter.auto.value.gson.GenerateTypeAdapter; + +/** + * This object serves as a container for the transitive digest (obtained from transitive .bzl files) + * and the generated repositories from evaluating a module extension. Its purpose is to store this + * information within the lockfile. + */ +@AutoValue +@GenerateTypeAdapter +public abstract class LockFileModuleExtension implements Postable { + + @SuppressWarnings("AutoValueImmutableFields") + public abstract byte[] getBzlTransitiveDigest(); + + public abstract ImmutableMap getGeneratedRepoSpecs(); + + public static LockFileModuleExtension create( + byte[] transitiveDigest, ImmutableMap generatedRepoSpecs) { + return new AutoValue_LockFileModuleExtension(transitiveDigest, generatedRepoSpecs); + } +} diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionResolutionEvent.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionResolutionEvent.java new file mode 100644 index 00000000000000..4013e7bb4c5965 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionResolutionEvent.java @@ -0,0 +1,36 @@ +// 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. + +package com.google.devtools.build.lib.bazel.bzlmod; + +import com.google.auto.value.AutoValue; +import com.google.devtools.build.lib.events.ExtendedEventHandler.Postable; + +/** + * After evaluating any module extension this event is sent from {@link SingleExtensionEvalFunction} + * holding the extension id and the resolution data LockFileModuleExtension. It will be received in + * {@link BazelLockFileModule} to be used to update the lockfile content + */ +@AutoValue +public abstract class ModuleExtensionResolutionEvent implements Postable { + + public static ModuleExtensionResolutionEvent create( + ModuleExtensionId extensionId, LockFileModuleExtension lockfileModuleExtension) { + return new AutoValue_ModuleExtensionResolutionEvent(extensionId, lockfileModuleExtension); + } + + public abstract ModuleExtensionId getExtensionId(); + + public abstract LockFileModuleExtension getModuleExtension(); +} diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunction.java index 9f373295ebd75f..cccc0d79b7c627 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunction.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunction.java @@ -162,6 +162,18 @@ private SkyValue computeForRootModule(StarlarkSemantics starlarkSemantics, Envir starlarkSemantics, env); InterimModule module = moduleFileGlobals.buildModule(); + for (ModuleExtensionUsage usage : module.getExtensionUsages()) { + if (usage.getIsolationKey().isPresent() && usage.getImports().isEmpty()) { + throw errorf( + Code.BAD_MODULE, + "the isolated usage at %s of extension %s defined in %s has no effect as no " + + "repositories are imported from it. Either import one or more repositories " + + "generated by the extension with use_repo or remove the usage.", + usage.getLocation(), + usage.getExtensionName(), + usage.getExtensionBzlFile()); + } + } ImmutableMap moduleOverrides = moduleFileGlobals.buildOverrides(); Map commandOverrides = MODULE_OVERRIDES.get(env); diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SingleExtensionEvalFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SingleExtensionEvalFunction.java index 26d8557fdf4c98..29a32ef988ec17 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SingleExtensionEvalFunction.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SingleExtensionEvalFunction.java @@ -18,10 +18,15 @@ import static com.google.common.collect.ImmutableBiMap.toImmutableBiMap; import static com.google.common.collect.ImmutableSet.toImmutableSet; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; +import com.google.common.collect.ImmutableTable; import com.google.devtools.build.lib.analysis.BlazeDirectories; +import com.google.devtools.build.lib.bazel.repository.RepositoryOptions.LockfileMode; import com.google.devtools.build.lib.bazel.repository.downloader.DownloadManager; import com.google.devtools.build.lib.cmdline.BazelModuleContext; +import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.cmdline.LabelConstants; import com.google.devtools.build.lib.cmdline.LabelSyntaxException; import com.google.devtools.build.lib.cmdline.RepositoryName; @@ -30,6 +35,7 @@ import com.google.devtools.build.lib.runtime.ProcessWrapper; import com.google.devtools.build.lib.runtime.RepositoryRemoteExecutor; import com.google.devtools.build.lib.server.FailureDetails.ExternalDeps; +import com.google.devtools.build.lib.server.FailureDetails.ExternalDeps.Code; import com.google.devtools.build.lib.skyframe.BzlLoadFunction; import com.google.devtools.build.lib.skyframe.BzlLoadFunction.BzlLoadFailedException; import com.google.devtools.build.lib.skyframe.BzlLoadValue; @@ -42,6 +48,7 @@ import com.google.devtools.build.skyframe.SkyValue; import java.io.IOException; import java.util.ArrayList; +import java.util.Arrays; import java.util.Map; import java.util.Map.Entry; import java.util.function.Function; @@ -108,36 +115,8 @@ public SkyValue compute(SkyKey skyKey, Environment env) } Location sampleUsageLocation = usagesValue.getExtensionUsages().values().iterator().next().getLocation(); - - // Check that the .bzl label isn't crazy. - try { - BzlLoadFunction.checkValidLoadLabel(extensionId.getBzlFileLabel(), starlarkSemantics); - } catch (LabelSyntaxException e) { - throw new SingleExtensionEvalFunctionException( - ExternalDepsException.withCauseAndMessage( - ExternalDeps.Code.BAD_MODULE, e, "invalid module extension label"), - Transience.PERSISTENT); - } - - // Load the .bzl file pointed to by the label. - BzlLoadValue bzlLoadValue; - try { - bzlLoadValue = - (BzlLoadValue) - env.getValueOrThrow( - BzlLoadValue.keyForBzlmod(extensionId.getBzlFileLabel()), - BzlLoadFailedException.class); - } catch (BzlLoadFailedException e) { - throw new SingleExtensionEvalFunctionException( - ExternalDepsException.withCauseAndMessage( - ExternalDeps.Code.BAD_MODULE, - e, - "Error loading '%s' for module extensions, requested by %s: %s", - extensionId.getBzlFileLabel(), - sampleUsageLocation, - e.getMessage()), - Transience.PERSISTENT); - } + BzlLoadValue bzlLoadValue = + loadBzlFile(extensionId.getBzlFileLabel(), sampleUsageLocation, starlarkSemantics, env); if (bzlLoadValue == null) { return null; } @@ -165,13 +144,176 @@ public SkyValue compute(SkyKey skyKey, Environment env) Transience.PERSISTENT); } + // Check the lockfile first for that module extension + byte[] bzlTransitiveDigest = + BazelModuleContext.of(bzlLoadValue.getModule()).bzlTransitiveDigest(); + LockfileMode lockfileMode = BazelLockFileFunction.LOCKFILE_MODE.get(env); + if (!lockfileMode.equals(LockfileMode.OFF)) { + BazelLockFileValue lockfile = (BazelLockFileValue) env.getValue(BazelLockFileValue.KEY); + if (lockfile == null) { + return null; + } + SingleExtensionEvalValue singleExtensionEvalValue = + tryGettingValueFromLockFile( + extensionId, usagesValue, bzlTransitiveDigest, lockfileMode, lockfile); + if (singleExtensionEvalValue != null) { + return singleExtensionEvalValue; + } + } + // Run that extension! ModuleExtension extension = (ModuleExtension) exported; + ImmutableMap generatedRepoSpecs = + runModuleExtension( + extensionId, extension, usagesValue, bzlLoadValue.getModule(), starlarkSemantics, env); + if (generatedRepoSpecs == null) { + return null; + } + // Check that all imported repos have been actually generated + validateAllImportsAreGenerated(generatedRepoSpecs, usagesValue, extensionId); + + if (lockfileMode.equals(LockfileMode.UPDATE)) { + env.getListener() + .post( + ModuleExtensionResolutionEvent.create( + extensionId, + LockFileModuleExtension.create(bzlTransitiveDigest, generatedRepoSpecs))); + } + return createSingleExtentionValue(generatedRepoSpecs, usagesValue); + } + + @Nullable + private SingleExtensionEvalValue tryGettingValueFromLockFile( + ModuleExtensionId extensionId, + SingleExtensionUsagesValue usagesValue, + byte[] bzlTransitiveDigest, + LockfileMode lockfileMode, + BazelLockFileValue lockfile) + throws SingleExtensionEvalFunctionException { + LockFileModuleExtension lockedExtension = lockfile.getModuleExtensions().get(extensionId); + ImmutableMap lockedExtensionUsages; + try { + // TODO(salmasamy) might be nicer to precompute this table when we construct + // BazelLockFileValue, without adding it to the json file + ImmutableTable extensionUsagesById = + BazelDepGraphFunction.getExtensionUsagesById(lockfile.getModuleDepGraph()); + lockedExtensionUsages = extensionUsagesById.row(extensionId); + } catch (ExternalDepsException e) { + throw new SingleExtensionEvalFunctionException(e, Transience.PERSISTENT); + } + + // If we have the extension, check if the implementation and usage haven't changed + if (lockedExtension != null + && Arrays.equals(bzlTransitiveDigest, lockedExtension.getBzlTransitiveDigest()) + && usagesValue.getExtensionUsages().equals(lockedExtensionUsages)) { + return createSingleExtentionValue(lockedExtension.getGeneratedRepoSpecs(), usagesValue); + } else if (lockfileMode.equals(LockfileMode.ERROR)) { + ImmutableList extDiff = + lockfile.getModuleExtensionDiff( + lockedExtension, + lockedExtensionUsages, + extensionId, + bzlTransitiveDigest, + usagesValue.getExtensionUsages()); + throw new SingleExtensionEvalFunctionException( + ExternalDepsException.withMessage( + Code.BAD_MODULE, + "Lock file is no longer up-to-date because: %s", + String.join(", ", extDiff)), + Transience.PERSISTENT); + } + return null; + } + + private SingleExtensionEvalValue createSingleExtentionValue( + ImmutableMap generatedRepoSpecs, SingleExtensionUsagesValue usagesValue) { + return SingleExtensionEvalValue.create( + generatedRepoSpecs, + generatedRepoSpecs.keySet().stream() + .collect( + toImmutableBiMap( + e -> + RepositoryName.createUnvalidated( + usagesValue.getExtensionUniqueName() + "~" + e), + Function.identity()))); + } + + private void validateAllImportsAreGenerated( + ImmutableMap generatedRepoSpecs, + SingleExtensionUsagesValue usagesValue, + ModuleExtensionId extensionId) + throws SingleExtensionEvalFunctionException { + for (ModuleExtensionUsage usage : usagesValue.getExtensionUsages().values()) { + for (Entry repoImport : usage.getImports().entrySet()) { + if (!generatedRepoSpecs.containsKey(repoImport.getValue())) { + throw new SingleExtensionEvalFunctionException( + ExternalDepsException.withMessage( + Code.BAD_MODULE, + "module extension \"%s\" from \"%s\" does not generate repository \"%s\", yet it" + + " is imported as \"%s\" in the usage at %s%s", + extensionId.getExtensionName(), + extensionId.getBzlFileLabel(), + repoImport.getValue(), + repoImport.getKey(), + usage.getLocation(), + SpellChecker.didYouMean(repoImport.getValue(), generatedRepoSpecs.keySet())), + Transience.PERSISTENT); + } + } + } + } + + private BzlLoadValue loadBzlFile( + Label bzlFileLabel, + Location sampleUsageLocation, + StarlarkSemantics starlarkSemantics, + Environment env) + throws SingleExtensionEvalFunctionException, InterruptedException { + // Check that the .bzl label isn't crazy. + try { + BzlLoadFunction.checkValidLoadLabel(bzlFileLabel, starlarkSemantics); + } catch (LabelSyntaxException e) { + throw new SingleExtensionEvalFunctionException( + ExternalDepsException.withCauseAndMessage( + Code.BAD_MODULE, e, "invalid module extension label"), + Transience.PERSISTENT); + } + + // Load the .bzl file pointed to by the label. + BzlLoadValue bzlLoadValue; + try { + bzlLoadValue = + (BzlLoadValue) + env.getValueOrThrow( + BzlLoadValue.keyForBzlmod(bzlFileLabel), BzlLoadFailedException.class); + } catch (BzlLoadFailedException e) { + throw new SingleExtensionEvalFunctionException( + ExternalDepsException.withCauseAndMessage( + Code.BAD_MODULE, + e, + "Error loading '%s' for module extensions, requested by %s: %s", + bzlFileLabel, + sampleUsageLocation, + e.getMessage()), + Transience.PERSISTENT); + } + return bzlLoadValue; + } + + @Nullable + private ImmutableMap runModuleExtension( + ModuleExtensionId extensionId, + ModuleExtension extension, + SingleExtensionUsagesValue usagesValue, + net.starlark.java.eval.Module module, + StarlarkSemantics starlarkSemantics, + Environment env) + throws SingleExtensionEvalFunctionException, InterruptedException { ModuleExtensionEvalStarlarkThreadContext threadContext = new ModuleExtensionEvalStarlarkThreadContext( usagesValue.getExtensionUniqueName() + "~", extensionId.getBzlFileLabel().getPackageIdentifier(), - BazelModuleContext.of(bzlLoadValue.getModule()).repoMapping(), + BazelModuleContext.of(module).repoMapping(), directories, env.getListener()); try (Mutability mu = @@ -229,37 +371,7 @@ public SkyValue compute(SkyKey skyKey, Environment env) Transience.TRANSIENT); } } - - // Check that all imported repos have been actually generated - for (ModuleExtensionUsage usage : usagesValue.getExtensionUsages().values()) { - for (Entry repoImport : usage.getImports().entrySet()) { - if (!threadContext.getGeneratedRepoSpecs().containsKey(repoImport.getValue())) { - throw new SingleExtensionEvalFunctionException( - ExternalDepsException.withMessage( - ExternalDeps.Code.BAD_MODULE, - "module extension \"%s\" from \"%s\" does not generate repository \"%s\", yet it" - + " is imported as \"%s\" in the usage at %s%s", - extensionId.getExtensionName(), - extensionId.getBzlFileLabel(), - repoImport.getValue(), - repoImport.getKey(), - usage.getLocation(), - SpellChecker.didYouMean( - repoImport.getValue(), threadContext.getGeneratedRepoSpecs().keySet())), - Transience.PERSISTENT); - } - } - } - - return SingleExtensionEvalValue.create( - threadContext.getGeneratedRepoSpecs(), - threadContext.getGeneratedRepoSpecs().keySet().stream() - .collect( - toImmutableBiMap( - e -> - RepositoryName.createUnvalidated( - usagesValue.getExtensionUniqueName() + "~" + e), - Function.identity()))); + return threadContext.getGeneratedRepoSpecs(); } private ModuleExtensionContext createContext( diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/modquery/BUILD b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/modquery/BUILD new file mode 100644 index 00000000000000..6c0e7657b54973 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/modquery/BUILD @@ -0,0 +1,29 @@ +load("@rules_java//java:defs.bzl", "java_library") + +package( + default_applicable_licenses = ["//:license"], + default_visibility = ["//src:__subpackages__"], +) + +filegroup( + name = "srcs", + srcs = glob(["*"]), + visibility = ["//src:__subpackages__"], +) + +java_library( + name = "modquery", + srcs = glob(["*.java"]), + deps = [ + "//src/main/java/com/google/devtools/build/lib/bazel/bzlmod:common", + "//src/main/java/com/google/devtools/build/lib/bazel/bzlmod:inspection", + "//src/main/java/com/google/devtools/build/lib/bazel/bzlmod:repo_rule_value", + "//src/main/java/com/google/devtools/build/lib/packages", + "//src/main/java/com/google/devtools/build/lib/query2/query/output", + "//src/main/java/com/google/devtools/common/options", + "//third_party:auto_value", + "//third_party:gson", + "//third_party:guava", + "//third_party:jsr305", + ], +) diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/modquery/GraphvizOutputFormatter.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/modquery/GraphvizOutputFormatter.java new file mode 100644 index 00000000000000..91395832d013aa --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/modquery/GraphvizOutputFormatter.java @@ -0,0 +1,111 @@ +// Copyright 2022 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. + +package com.google.devtools.build.lib.bazel.bzlmod.modquery; + +import com.google.common.base.Preconditions; +import com.google.devtools.build.lib.bazel.bzlmod.BazelModuleInspectorValue.AugmentedModule; +import com.google.devtools.build.lib.bazel.bzlmod.ModuleKey; +import com.google.devtools.build.lib.bazel.bzlmod.Version; +import com.google.devtools.build.lib.bazel.bzlmod.modquery.ModqueryExecutor.ResultNode; +import com.google.devtools.build.lib.bazel.bzlmod.modquery.ModqueryExecutor.ResultNode.IsIndirect; +import com.google.devtools.build.lib.bazel.bzlmod.modquery.ModqueryExecutor.ResultNode.NodeMetadata; +import com.google.devtools.build.lib.bazel.bzlmod.modquery.OutputFormatters.OutputFormatter; +import java.util.ArrayDeque; +import java.util.Deque; +import java.util.HashSet; +import java.util.Map.Entry; +import java.util.Set; + +/** + * Outputs graph-based results of {@link ModqueryExecutor} in the Graphviz dot format which + * can be further pipelined to create an image graph visualization. + */ +public class GraphvizOutputFormatter extends OutputFormatter { + + @Override + public void output() { + StringBuilder str = new StringBuilder(); + str.append("digraph mygraph {\n") + .append(" ") + .append("node [ shape=box ]\n") + .append(" ") + .append("edge [ fontsize=8 ]\n"); + Set seen = new HashSet<>(); + Deque toVisit = new ArrayDeque<>(); + seen.add(ModuleKey.ROOT); + toVisit.add(ModuleKey.ROOT); + + while (!toVisit.isEmpty()) { + ModuleKey key = toVisit.pop(); + AugmentedModule module = depGraph.get(key); + ResultNode node = result.get(key); + Preconditions.checkNotNull(module); + Preconditions.checkNotNull(node); + String sourceId = toId(key); + + if (key.equals(ModuleKey.ROOT)) { + String rootLabel = String.format("root (%s@%s)", module.getName(), module.getVersion()); + str.append(String.format(" root [ label=\"%s\" ]\n", rootLabel)); + } else if (node.isTarget() || !module.isUsed()) { + String shapeString = node.isTarget() ? "diamond" : "box"; + String styleString = module.isUsed() ? "solid" : "dotted"; + str.append( + String.format(" %s [ shape=%s style=%s ]\n", toId(key), shapeString, styleString)); + } + + for (Entry e : node.getChildrenSortedByKey()) { + ModuleKey childKey = e.getKey(); + IsIndirect childIndirect = e.getValue().isIndirect(); + String childId = toId(childKey); + if (childIndirect == IsIndirect.FALSE) { + String reasonLabel = getReasonLabel(childKey, key); + str.append(String.format(" %s -> %s [ %s ]\n", sourceId, childId, reasonLabel)); + } else { + str.append(String.format(" %s -> %s [ style=dashed ]\n", sourceId, childId)); + } + if (seen.add(childKey)) { + toVisit.add(childKey); + } + } + } + str.append("}"); + printer.println(str); + printer.flush(); + } + + private String toId(ModuleKey key) { + if (key.equals(ModuleKey.ROOT)) { + return "root"; + } + return String.format( + "\"%s@%s\"", + key.getName(), key.getVersion().equals(Version.EMPTY) ? "_" : key.getVersion()); + } + + private String getReasonLabel(ModuleKey key, ModuleKey parent) { + if (!options.extra) { + return ""; + } + Explanation explanation = getExtraResolutionExplanation(key, parent); + if (explanation == null) { + return ""; + } + String label = explanation.getResolutionReason().getLabel(); + if (!label.isEmpty()) { + return String.format("label=%s", label); + } + return ""; + } +} diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/modquery/JsonOutputFormatter.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/modquery/JsonOutputFormatter.java new file mode 100644 index 00000000000000..3a7091b394a1a3 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/modquery/JsonOutputFormatter.java @@ -0,0 +1,102 @@ +// Copyright 2022 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. + +package com.google.devtools.build.lib.bazel.bzlmod.modquery; + +import com.google.devtools.build.lib.bazel.bzlmod.BazelModuleInspectorValue.AugmentedModule; +import com.google.devtools.build.lib.bazel.bzlmod.ModuleKey; +import com.google.devtools.build.lib.bazel.bzlmod.modquery.ModqueryExecutor.ResultNode; +import com.google.devtools.build.lib.bazel.bzlmod.modquery.ModqueryExecutor.ResultNode.IsCycle; +import com.google.devtools.build.lib.bazel.bzlmod.modquery.ModqueryExecutor.ResultNode.IsExpanded; +import com.google.devtools.build.lib.bazel.bzlmod.modquery.ModqueryExecutor.ResultNode.IsIndirect; +import com.google.devtools.build.lib.bazel.bzlmod.modquery.ModqueryExecutor.ResultNode.NodeMetadata; +import com.google.devtools.build.lib.bazel.bzlmod.modquery.OutputFormatters.OutputFormatter; +import com.google.gson.GsonBuilder; +import com.google.gson.JsonArray; +import com.google.gson.JsonObject; +import java.util.Map.Entry; + +/** Outputs graph-based results of {@link ModqueryExecutor} in JSON format. */ +public class JsonOutputFormatter extends OutputFormatter { + @Override + public void output() { + JsonObject root = printTree(ModuleKey.ROOT, null, IsExpanded.TRUE, IsIndirect.FALSE); + root.addProperty("root", true); + printer.println(new GsonBuilder().setPrettyPrinting().create().toJson(root)); + } + + public String printKey(ModuleKey key) { + if (key.equals(ModuleKey.ROOT)) { + return "root"; + } + return key.toString(); + } + + JsonObject printTree(ModuleKey key, ModuleKey parent, IsExpanded expanded, IsIndirect indirect) { + ResultNode node = result.get(key); + AugmentedModule module = depGraph.get(key); + JsonObject json = new JsonObject(); + json.addProperty("key", printKey(key)); + if (!key.getName().equals(module.getName())) { + json.addProperty("name", module.getName()); + } + if (!key.getVersion().equals(module.getVersion())) { + json.addProperty("version", module.getVersion().toString()); + } + + if (indirect == IsIndirect.FALSE && options.extra && parent != null) { + Explanation explanation = getExtraResolutionExplanation(key, parent); + if (explanation != null) { + if (!module.isUsed()) { + json.addProperty("unused", true); + json.addProperty("resolvedVersion", explanation.getChangedVersion().toString()); + } else { + json.addProperty("originalVersion", explanation.getChangedVersion().toString()); + } + json.addProperty("resolutionReason", explanation.getChangedVersion().toString()); + if (explanation.getRequestedByModules() != null) { + JsonArray requestedBy = new JsonArray(); + explanation.getRequestedByModules().forEach(k -> requestedBy.add(printKey(k))); + json.add("resolvedRequestedBy", requestedBy); + } + } + } + + if (expanded == IsExpanded.FALSE) { + json.addProperty("unexpanded", true); + return json; + } + + JsonArray deps = new JsonArray(); + JsonArray indirectDeps = new JsonArray(); + JsonArray cycles = new JsonArray(); + for (Entry e : node.getChildrenSortedByEdgeType()) { + ModuleKey childKey = e.getKey(); + IsExpanded childExpanded = e.getValue().isExpanded(); + IsIndirect childIndirect = e.getValue().isIndirect(); + IsCycle childCycles = e.getValue().isCycle(); + if (childCycles == IsCycle.TRUE) { + cycles.add(printTree(childKey, key, IsExpanded.FALSE, IsIndirect.FALSE)); + } else if (childIndirect == IsIndirect.TRUE) { + indirectDeps.add(printTree(childKey, key, childExpanded, IsIndirect.TRUE)); + } else { + deps.add(printTree(childKey, key, childExpanded, IsIndirect.FALSE)); + } + } + json.add("dependencies", deps); + json.add("indirectDependencies", indirectDeps); + json.add("cycles", cycles); + return json; + } +} diff --git a/src/main/java/com/google/devtools/build/lib/bazel/commands/ModqueryExecutor.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/modquery/ModqueryExecutor.java similarity index 66% rename from src/main/java/com/google/devtools/build/lib/bazel/commands/ModqueryExecutor.java rename to src/main/java/com/google/devtools/build/lib/bazel/bzlmod/modquery/ModqueryExecutor.java index 2fcb9fa4ffc239..c68c6b86594844 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/commands/ModqueryExecutor.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/modquery/ModqueryExecutor.java @@ -12,9 +12,10 @@ // See the License for the specific language governing permissions and // limitations under the License. -package com.google.devtools.build.lib.bazel.commands; +package com.google.devtools.build.lib.bazel.bzlmod.modquery; -import static com.google.common.collect.ImmutableMap.toImmutableMap; +import static com.google.common.collect.ImmutableSortedMap.toImmutableSortedMap; +import static java.util.Comparator.reverseOrder; import static java.util.stream.Collectors.toCollection; import com.google.auto.value.AutoValue; @@ -22,12 +23,21 @@ import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; -import com.google.common.collect.ImmutableSortedMap; +import com.google.common.collect.ImmutableSetMultimap; +import com.google.common.collect.ImmutableSortedSet; import com.google.devtools.build.lib.bazel.bzlmod.BazelModuleInspectorValue.AugmentedModule; import com.google.devtools.build.lib.bazel.bzlmod.BzlmodRepoRuleValue; import com.google.devtools.build.lib.bazel.bzlmod.ModuleKey; -import com.google.devtools.build.lib.bazel.commands.ModqueryExecutor.ResultNode.IsExpanded; +import com.google.devtools.build.lib.bazel.bzlmod.modquery.ModqueryExecutor.ResultNode.IsExpanded; +import com.google.devtools.build.lib.bazel.bzlmod.modquery.ModqueryExecutor.ResultNode.IsIndirect; +import com.google.devtools.build.lib.bazel.bzlmod.modquery.ModqueryExecutor.ResultNode.NodeMetadata; +import com.google.devtools.build.lib.packages.RawAttributeMapper; +import com.google.devtools.build.lib.packages.Rule; +import com.google.devtools.build.lib.query2.query.output.BuildOutputFormatter.AttributeReader; +import com.google.devtools.build.lib.query2.query.output.BuildOutputFormatter.TargetOutputter; +import com.google.devtools.build.lib.query2.query.output.PossibleAttributeValues; import com.google.errorprone.annotations.CanIgnoreReturnValue; +import java.io.IOException; import java.io.PrintWriter; import java.io.Writer; import java.util.ArrayDeque; @@ -37,12 +47,14 @@ import java.util.HashSet; import java.util.Map; import java.util.Map.Entry; +import java.util.Objects; import java.util.Set; import javax.annotation.Nullable; /** * Executes inspection queries for {@link - * com.google.devtools.build.lib.bazel.commands.ModqueryCommand}. + * com.google.devtools.build.lib.bazel.commands.ModqueryCommand} and prints the resulted output to + * the reporter's output stream using the different defined {@link OutputFormatters}. */ public class ModqueryExecutor { @@ -59,24 +71,26 @@ public ModqueryExecutor( public void tree(ImmutableSet targets) { ImmutableMap result = expandAndPrune(targets, ImmutableSet.of(), false); - printer.println(result); - printer.println("OUTPUT NOT IMPLEMENTED YET"); + OutputFormatters.getFormatter(options.outputFormat).output(result, depGraph, printer, options); } public void path(ImmutableSet from, ImmutableSet to) { ImmutableMap result = expandAndPrune(from, to, true); - printer.println(result); - printer.println("OUTPUT NOT IMPLEMENTED YET"); + OutputFormatters.getFormatter(options.outputFormat).output(result, depGraph, printer, options); } public void allPaths(ImmutableSet from, ImmutableSet to) { ImmutableMap result = expandAndPrune(from, to, false); - printer.println(result); - printer.println("OUTPUT NOT IMPLEMENTED YET"); + OutputFormatters.getFormatter(options.outputFormat).output(result, depGraph, printer, options); } public void show(ImmutableMap repoRuleValues) { - printer.println("OUTPUT NOT IMPLEMENTED YET"); + RuleDisplayOutputter outputter = new RuleDisplayOutputter(printer); + for (Entry e : repoRuleValues.entrySet()) { + printer.printf("## %s:", e.getKey()); + outputter.outputRule(e.getValue().getRule()); + } + printer.flush(); } /** @@ -104,13 +118,11 @@ ImmutableMap expandAndPrune( rootPinnedChildren.stream() .filter(coloredPaths::contains) .forEach( - moduleKey -> { - if (rootDirectChildren.contains(moduleKey)) { - rootBuilder.addChild(moduleKey, IsExpanded.TRUE); - } else { - rootBuilder.addIndirectChild(moduleKey, IsExpanded.TRUE); - } - }); + moduleKey -> + rootBuilder.addChild( + moduleKey, + IsExpanded.TRUE, + rootDirectChildren.contains(moduleKey) ? IsIndirect.FALSE : IsIndirect.TRUE)); resultBuilder.put(ModuleKey.ROOT, rootBuilder.build()); Set seen = new HashSet<>(rootPinnedChildren); @@ -137,11 +149,11 @@ ImmutableMap expandAndPrune( // wrong answer in cycle edge-case A -> B -> C -> B with target D will not find ABD // \__ D if (!singlePath) { - nodeBuilder.addChild(childKey, IsExpanded.FALSE); + nodeBuilder.addChild(childKey, IsExpanded.FALSE, IsIndirect.FALSE); } continue; } - nodeBuilder.addChild(childKey, IsExpanded.TRUE); + nodeBuilder.addChild(childKey, IsExpanded.TRUE, IsIndirect.FALSE); seen.add(childKey); toVisit.add(childKey); if (singlePath) { @@ -184,19 +196,18 @@ private ImmutableMap pruneByDepth() { parentStack.add(ModuleKey.ROOT); - for (ModuleKey childKey : oldResult.get(ModuleKey.ROOT).getChildren().keySet()) { - rootBuilder.addChild(childKey, IsExpanded.TRUE); - visitVisible(childKey, 1, ModuleKey.ROOT, IsExpanded.TRUE); - } - for (ModuleKey childKey : oldResult.get(ModuleKey.ROOT).getIndirectChildren().keySet()) { - rootBuilder.addIndirectChild(childKey, IsExpanded.TRUE); - visitVisible(childKey, 1, ModuleKey.ROOT, IsExpanded.TRUE); + for (Entry e : + oldResult.get(ModuleKey.ROOT).getChildrenSortedByKey()) { + rootBuilder.addChild(e.getKey(), IsExpanded.TRUE, e.getValue().isIndirect()); + visitVisible(e.getKey(), 1, ModuleKey.ROOT, IsExpanded.TRUE); } // Build everything at the end to allow children to add themselves to their parent's // adjacency list. return resultBuilder.entrySet().stream() - .collect(toImmutableMap(Entry::getKey, e -> e.getValue().build())); + .collect( + toImmutableSortedMap( + ModuleKey.LEXICOGRAPHIC_COMPARATOR, Entry::getKey, e -> e.getValue().build())); } // Handles graph traversal within the specified depth. @@ -209,16 +220,16 @@ private void visitVisible( resultBuilder.put(moduleKey, nodeBuilder); nodeBuilder.setTarget(oldNode.isTarget()); if (depth > 1) { - resultBuilder.get(parentKey).addChild(moduleKey, expanded); + resultBuilder.get(parentKey).addChild(moduleKey, expanded, IsIndirect.FALSE); } if (expanded == IsExpanded.FALSE) { parentStack.remove(moduleKey); return; } - for (Entry e : oldNode.getChildren().entrySet()) { + for (Entry e : oldNode.getChildrenSortedByKey()) { ModuleKey childKey = e.getKey(); - IsExpanded childExpanded = e.getValue(); + IsExpanded childExpanded = e.getValue().isExpanded(); if (notCycle(childKey)) { if (depth < options.depth) { visitVisible(childKey, depth + 1, moduleKey, childExpanded); @@ -226,7 +237,7 @@ private void visitVisible( visitDetached(childKey, moduleKey, moduleKey, childExpanded); } } else if (options.cycles) { - nodeBuilder.addChild(childKey, IsExpanded.FALSE); + nodeBuilder.addCycle(childKey); } } parentStack.remove(moduleKey); @@ -246,11 +257,9 @@ private void visitDetached( if (oldNode.isTarget() || oldNode.isTargetParent()) { ResultNode.Builder parentBuilder = resultBuilder.get(lastVisibleParentKey); - if (lastVisibleParentKey.equals(parentKey)) { - parentBuilder.addChild(moduleKey, expanded); - } else { - parentBuilder.addIndirectChild(moduleKey, expanded); - } + IsIndirect childIndirect = + lastVisibleParentKey.equals(parentKey) ? IsIndirect.FALSE : IsIndirect.TRUE; + parentBuilder.addChild(moduleKey, expanded, childIndirect); resultBuilder.put(moduleKey, nodeBuilder); lastVisibleParentKey = moduleKey; } @@ -259,13 +268,13 @@ private void visitDetached( parentStack.remove(moduleKey); return; } - for (Entry e : oldNode.getChildren().entrySet()) { + for (Entry e : oldNode.getChildrenSortedByKey()) { ModuleKey childKey = e.getKey(); - IsExpanded childExpanded = e.getValue(); + IsExpanded childExpanded = e.getValue().isExpanded(); if (notCycle(childKey)) { visitDetached(childKey, moduleKey, lastVisibleParentKey, childExpanded); } else if (options.cycles) { - nodeBuilder.addChild(childKey, IsExpanded.FALSE); + nodeBuilder.addCycle(childKey); } } parentStack.remove(moduleKey); @@ -323,14 +332,16 @@ private MaybeCompleteSet colorReversePathsToRoot(ImmutableSet dependenciesGraph) { - AugmentedModule module = dependenciesGraph.get(key); + AugmentedModule module = Objects.requireNonNull(dependenciesGraph.get(key)); if (key.equals(ModuleKey.ROOT)) { return false; } @@ -343,8 +354,9 @@ static boolean filterUnused( return true; } + /** A node representing a module that forms the result graph. */ @AutoValue - abstract static class ResultNode { + public abstract static class ResultNode { /** Whether the module is one of the targets in a paths query. */ abstract boolean isTarget(); @@ -355,15 +367,59 @@ abstract static class ResultNode { abstract boolean isTargetParent(); enum IsExpanded { - TRUE, - FALSE + FALSE, + TRUE } - /** List of direct children. True if the children will be expanded in this branch. */ - abstract ImmutableSortedMap getChildren(); + enum IsIndirect { + FALSE, + TRUE + } - /** List of indirect children. True if the children will be expanded in this branch. */ - abstract ImmutableSortedMap getIndirectChildren(); + enum IsCycle { + FALSE, + TRUE + } + + /** Detailed edge type for the {@link ResultNode} graph. */ + @AutoValue + public abstract static class NodeMetadata { + /** + * Whether the node should be expanded from this edge (the same node can appear in multiple + * places in a flattened graph). + */ + public abstract IsExpanded isExpanded(); + + /** Whether the edge is a direct edge or an indirect (transitive) one. */ + public abstract IsIndirect isIndirect(); + + /** Whether the edge is cycling back inside the flattened graph. */ + public abstract IsCycle isCycle(); + + private static NodeMetadata create( + IsExpanded isExpanded, IsIndirect isIndirect, IsCycle isCycle) { + return new AutoValue_ModqueryExecutor_ResultNode_NodeMetadata( + isExpanded, isIndirect, isCycle); + } + } + + /** List of children mapped to detailed edge types. */ + protected abstract ImmutableSetMultimap getChildren(); + + public ImmutableSortedSet> getChildrenSortedByKey() { + return ImmutableSortedSet.copyOf( + Entry.comparingByKey(ModuleKey.LEXICOGRAPHIC_COMPARATOR), getChildren().entries()); + } + + public ImmutableSortedSet> getChildrenSortedByEdgeType() { + return ImmutableSortedSet.copyOf( + Comparator., IsCycle>comparing( + e -> e.getValue().isCycle(), reverseOrder()) + .thenComparing(e -> e.getValue().isExpanded()) + .thenComparing(e -> e.getValue().isIndirect()) + .thenComparing(Entry::getKey, ModuleKey.LEXICOGRAPHIC_COMPARATOR), + getChildren().entries()); + } static ResultNode.Builder builder() { return new AutoValue_ModqueryExecutor_ResultNode.Builder() @@ -376,28 +432,20 @@ abstract static class Builder { abstract ResultNode.Builder setTargetParent(boolean value); - private final ImmutableSortedMap.Builder childrenBuilder = - childrenBuilder(ModuleKey.LEXICOGRAPHIC_COMPARATOR); - private final ImmutableSortedMap.Builder indirectChildrenBuilder = - indirectChildrenBuilder(ModuleKey.LEXICOGRAPHIC_COMPARATOR); - abstract ResultNode.Builder setTarget(boolean value); - abstract ImmutableSortedMap.Builder childrenBuilder( - Comparator comparator); - - abstract ImmutableSortedMap.Builder indirectChildrenBuilder( - Comparator comparator); + abstract ImmutableSetMultimap.Builder childrenBuilder(); @CanIgnoreReturnValue - final Builder addChild(ModuleKey value, IsExpanded expanded) { - childrenBuilder.put(value, expanded); + final Builder addChild(ModuleKey value, IsExpanded expanded, IsIndirect indirect) { + childrenBuilder().put(value, NodeMetadata.create(expanded, indirect, IsCycle.FALSE)); return this; } @CanIgnoreReturnValue - final Builder addIndirectChild(ModuleKey value, IsExpanded expanded) { - indirectChildrenBuilder.put(value, expanded); + final Builder addCycle(ModuleKey value) { + childrenBuilder() + .put(value, NodeMetadata.create(IsExpanded.FALSE, IsIndirect.FALSE, IsCycle.TRUE)); return this; } @@ -432,4 +480,34 @@ static MaybeCompleteSet completeSet() { return new AutoValue_ModqueryExecutor_MaybeCompleteSet<>(null); } } + + /** + * Uses Query's {@link TargetOutputter} to display the generating repo rule and other information. + */ + static class RuleDisplayOutputter { + private static final AttributeReader attrReader = + (rule, attr) -> + // Query's implementation copied + PossibleAttributeValues.forRuleAndAttribute( + rule, attr, /* mayTreatMultipleAsNone= */ true); + private final TargetOutputter targetOutputter; + private final PrintWriter printer; + + RuleDisplayOutputter(PrintWriter printer) { + this.printer = printer; + this.targetOutputter = + new TargetOutputter( + this.printer, + (rule, attr) -> RawAttributeMapper.of(rule).isConfigurable(attr.getName()), + "\n"); + } + + private void outputRule(Rule rule) { + try { + targetOutputter.outputRule(rule, attrReader, this.printer); + } catch (IOException e) { + throw new IllegalStateException(e); + } + } + } } diff --git a/src/main/java/com/google/devtools/build/lib/bazel/commands/ModqueryOptions.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/modquery/ModqueryOptions.java similarity index 93% rename from src/main/java/com/google/devtools/build/lib/bazel/commands/ModqueryOptions.java rename to src/main/java/com/google/devtools/build/lib/bazel/bzlmod/modquery/ModqueryOptions.java index c12675bfd03c4f..08e02f9ab12576 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/commands/ModqueryOptions.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/modquery/ModqueryOptions.java @@ -11,7 +11,7 @@ // 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. -package com.google.devtools.build.lib.bazel.commands; +package com.google.devtools.build.lib.bazel.bzlmod.modquery; import static java.util.Arrays.stream; import static java.util.stream.Collectors.joining; @@ -115,7 +115,11 @@ public class ModqueryOptions extends OptionsBase { + "text, json, graph") public OutputFormat outputFormat; - enum QueryType { + /** + * Possible subcommands that can be specified for {@link + * com.google.devtools.build.lib.bazel.commands.ModqueryCommand} + */ + public enum QueryType { DEPS(1), TREE(0), ALL_PATHS(1), @@ -151,7 +155,11 @@ public QueryTypeConverter() { } } - enum Charset { + /** + * Charset to be used in outputting the {@link + * com.google.devtools.build.lib.bazel.commands.ModqueryCommand} result. + */ + public enum Charset { UTF8, ASCII } @@ -163,7 +171,11 @@ public CharsetConverter() { } } - enum OutputFormat { + /** + * Possible formats of the {@link com.google.devtools.build.lib.bazel.commands.ModqueryCommand} + * result. + */ + public enum OutputFormat { TEXT, JSON, GRAPH @@ -178,19 +190,19 @@ public OutputFormatConverter() { /** Argument of a modquery converted from the form name@version or name. */ @AutoValue - abstract static class TargetModule { + public abstract static class TargetModule { static TargetModule create(String name, Version version) { return new AutoValue_ModqueryOptions_TargetModule(name, version); } - abstract String getName(); + public abstract String getName(); /** * If it is null, it represents any (one or multiple) present versions of the module in the dep * graph, which is different from the empty version */ @Nullable - abstract Version getVersion(); + public abstract Version getVersion(); } /** Converts a module target argument string to a properly typed {@link TargetModule} */ diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/modquery/OutputFormatters.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/modquery/OutputFormatters.java new file mode 100644 index 00000000000000..78d40b0596f38d --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/modquery/OutputFormatters.java @@ -0,0 +1,164 @@ +// Copyright 2022 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. + +package com.google.devtools.build.lib.bazel.bzlmod.modquery; + +import static java.util.stream.Collectors.joining; + +import com.google.auto.value.AutoValue; +import com.google.common.base.Ascii; +import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; +import com.google.devtools.build.lib.bazel.bzlmod.BazelModuleInspectorValue.AugmentedModule; +import com.google.devtools.build.lib.bazel.bzlmod.BazelModuleInspectorValue.AugmentedModule.ResolutionReason; +import com.google.devtools.build.lib.bazel.bzlmod.ModuleKey; +import com.google.devtools.build.lib.bazel.bzlmod.Version; +import com.google.devtools.build.lib.bazel.bzlmod.modquery.ModqueryExecutor.ResultNode; +import com.google.devtools.build.lib.bazel.bzlmod.modquery.ModqueryOptions.OutputFormat; +import java.io.PrintWriter; +import javax.annotation.Nullable; + +/** + * Contains the output formatters for the graph-based results of {@link ModqueryExecutor} that can + * be specified using {@link ModqueryOptions#outputFormat}. + */ +public final class OutputFormatters { + + private static final OutputFormatter textFormatter = new TextOutputFormatter(); + private static final OutputFormatter jsonFormatter = new JsonOutputFormatter(); + private static final OutputFormatter graphvizFormatter = new GraphvizOutputFormatter(); + + private OutputFormatters() {} + + static OutputFormatter getFormatter(OutputFormat format) { + switch (format) { + case TEXT: + return textFormatter; + case JSON: + return jsonFormatter; + case GRAPH: + return graphvizFormatter; + } + throw new IllegalArgumentException("Output format cannot be null."); + } + + abstract static class OutputFormatter { + + protected ImmutableMap result; + protected ImmutableMap depGraph; + protected PrintWriter printer; + protected ModqueryOptions options; + + /** Compact representation of the data provided by the {@link ModqueryOptions#extra} flag. */ + @AutoValue + abstract static class Explanation { + + /** The version from/to which the module was changed after resolution. */ + abstract Version getChangedVersion(); + + abstract ResolutionReason getResolutionReason(); + + /** + * The list of modules who originally requested the selected version in the case of + * Minimal-Version-Selection. + */ + @Nullable + abstract ImmutableSet getRequestedByModules(); + + static Explanation create( + Version version, ResolutionReason reason, ImmutableSet requestedByModules) { + return new AutoValue_OutputFormatters_OutputFormatter_Explanation( + version, reason, requestedByModules); + } + + /** + * Gets the exact label that is printed next to the module if the {@link + * ModqueryOptions#extra} flag is enabled. + */ + String toExplanationString(boolean unused) { + String changedVersionLabel = + getChangedVersion().equals(Version.EMPTY) ? "_" : getChangedVersion().toString(); + String toOrWasString = unused ? "to" : "was"; + String reasonString = + getRequestedByModules() != null + ? getRequestedByModules().stream().map(ModuleKey::toString).collect(joining(", ")) + : Ascii.toLowerCase(getResolutionReason().toString()); + return String.format("(%s %s, cause %s)", toOrWasString, changedVersionLabel, reasonString); + } + } + + /** Exposed API of the formatter during which the necessary objects are injected. */ + void output( + ImmutableMap result, + ImmutableMap depGraph, + PrintWriter printer, + ModqueryOptions options) { + this.result = result; + this.depGraph = depGraph; + this.printer = printer; + this.options = options; + output(); + printer.flush(); + } + + /** Internal implementation of the formatter output function. */ + protected abstract void output(); + + /** + * Exists only for testing, because normally the depGraph and options are injected inside the + * public API call. + */ + protected Explanation getExtraResolutionExplanation( + ModuleKey key, + ModuleKey parent, + ImmutableMap depGraph, + ModqueryOptions options) { + this.depGraph = depGraph; + this.options = options; + return getExtraResolutionExplanation(key, parent); + } + + /** + * Returns {@code null} if the module version has not changed during resolution or if the module + * is <root>. + */ + @Nullable + protected Explanation getExtraResolutionExplanation(ModuleKey key, ModuleKey parent) { + if (key.equals(ModuleKey.ROOT)) { + return null; + } + AugmentedModule module = depGraph.get(key); + AugmentedModule parentModule = depGraph.get(parent); + String repoName = parentModule.getAllDeps(options.includeUnused).get(key); + Version changedVersion; + ImmutableSet changedByModules = null; + ResolutionReason reason = parentModule.getDepReasons().get(repoName); + AugmentedModule replacement = + module.isUsed() ? module : depGraph.get(parentModule.getDeps().get(repoName)); + if (reason != ResolutionReason.ORIGINAL) { + if (!module.isUsed()) { + changedVersion = replacement.getVersion(); + } else { + AugmentedModule old = depGraph.get(parentModule.getUnusedDeps().get(repoName)); + changedVersion = old.getVersion(); + } + if (reason == ResolutionReason.MINIMAL_VERSION_SELECTION) { + changedByModules = replacement.getOriginalDependants(); + } + return Explanation.create(changedVersion, reason, changedByModules); + } + return null; + } + } +} diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/modquery/TextOutputFormatter.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/modquery/TextOutputFormatter.java new file mode 100644 index 00000000000000..52b995d78e1dca --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/modquery/TextOutputFormatter.java @@ -0,0 +1,174 @@ +// Copyright 2022 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. + +package com.google.devtools.build.lib.bazel.bzlmod.modquery; + +import com.google.common.base.Preconditions; +import com.google.devtools.build.lib.bazel.bzlmod.BazelModuleInspectorValue.AugmentedModule; +import com.google.devtools.build.lib.bazel.bzlmod.ModuleKey; +import com.google.devtools.build.lib.bazel.bzlmod.Version; +import com.google.devtools.build.lib.bazel.bzlmod.modquery.ModqueryExecutor.ResultNode; +import com.google.devtools.build.lib.bazel.bzlmod.modquery.ModqueryExecutor.ResultNode.IsCycle; +import com.google.devtools.build.lib.bazel.bzlmod.modquery.ModqueryExecutor.ResultNode.IsExpanded; +import com.google.devtools.build.lib.bazel.bzlmod.modquery.ModqueryExecutor.ResultNode.IsIndirect; +import com.google.devtools.build.lib.bazel.bzlmod.modquery.ModqueryExecutor.ResultNode.NodeMetadata; +import com.google.devtools.build.lib.bazel.bzlmod.modquery.ModqueryOptions.Charset; +import com.google.devtools.build.lib.bazel.bzlmod.modquery.OutputFormatters.OutputFormatter; +import java.util.ArrayDeque; +import java.util.Deque; +import java.util.Iterator; +import java.util.Map.Entry; +import java.util.Objects; + +/** + * Outputs graph-based results of {@link + * com.google.devtools.build.lib.bazel.bzlmod.modquery.ModqueryExecutor} in a human-readable text + * format. + */ +public class TextOutputFormatter extends OutputFormatter { + + private Deque isLastChildStack; + private DrawCharset drawCharset; + + @Override + public void output() { + if (options.charset == Charset.ASCII) { + drawCharset = DrawCharset.ASCII; + } else { + drawCharset = DrawCharset.UTF8; + } + isLastChildStack = new ArrayDeque<>(); + printTree(ModuleKey.ROOT, null, IsExpanded.TRUE, IsIndirect.FALSE, IsCycle.FALSE, 0); + } + + // Depth-first traversal to print the actual output + void printTree( + ModuleKey key, + ModuleKey parent, + IsExpanded expanded, + IsIndirect indirect, + IsCycle cycle, + int depth) { + ResultNode node = Objects.requireNonNull(result.get(key)); + StringBuilder str = new StringBuilder(); + + if (depth > 0) { + int indents = isLastChildStack.size() - 1; + Iterator value = isLastChildStack.descendingIterator(); + for (int i = 0; i < indents; i++) { + boolean isLastChild = value.next(); + if (isLastChild) { + str.append(drawCharset.emptyIndent); + } else { + str.append(drawCharset.prevChildIndent); + } + } + if (indirect == IsIndirect.TRUE) { + if (isLastChildStack.getFirst()) { + str.append(drawCharset.lastIndirectChildIndent); + } else { + str.append(drawCharset.indirectChildIndent); + } + } else { + if (isLastChildStack.getFirst()) { + str.append(drawCharset.lastChildIndent); + + } else { + str.append(drawCharset.childIndent); + } + } + } + + int totalChildrenNum = node.getChildren().size(); + + if (key.equals(ModuleKey.ROOT)) { + AugmentedModule rootModule = depGraph.get(ModuleKey.ROOT); + Preconditions.checkNotNull(rootModule); + str.append( + String.format( + "root (%s@%s)", + rootModule.getName(), + rootModule.getVersion().equals(Version.EMPTY) ? "_" : rootModule.getVersion())); + } else { + str.append(key).append(" "); + } + + if (cycle == IsCycle.TRUE) { + str.append("(cycle) "); + } else if (expanded == IsExpanded.FALSE) { + str.append("(*) "); + } else { + if (totalChildrenNum != 0 && node.isTarget()) { + str.append("# "); + } + } + AugmentedModule module = Objects.requireNonNull(depGraph.get(key)); + + if (!options.extra && !module.isUsed()) { + str.append("(unused) "); + } + // If the edge is indirect, the parent is not only unknown, but the node could have come + // from + // multiple paths merged in the process, so we skip the resolution explanation. + if (indirect == IsIndirect.FALSE && options.extra && parent != null) { + Explanation explanation = getExtraResolutionExplanation(key, parent); + if (explanation != null) { + str.append(explanation.toExplanationString(!module.isUsed())); + } + } + + this.printer.println(str); + + if (expanded == IsExpanded.FALSE) { + return; + } + + int currChild = 1; + for (Entry e : node.getChildrenSortedByEdgeType()) { + ModuleKey childKey = e.getKey(); + IsExpanded childExpanded = e.getValue().isExpanded(); + IsIndirect childIndirect = e.getValue().isIndirect(); + IsCycle childCycles = e.getValue().isCycle(); + isLastChildStack.push(currChild++ == totalChildrenNum); + printTree(childKey, key, childExpanded, childIndirect, childCycles, depth + 1); + isLastChildStack.pop(); + } + } + + enum DrawCharset { + ASCII(" ", "| ", "|___", "|...", "|___", "|..."), + UTF8(" ", "│ ", "├───", "├╌╌╌", "└───", "└╌╌╌"); + final String emptyIndent; + final String prevChildIndent; + final String childIndent; + final String indirectChildIndent; + final String lastChildIndent; + final String lastIndirectChildIndent; + + DrawCharset( + String emptyIndent, + String prevChildIndent, + String childIndent, + String indirectChildIndent, + String lastChildIndent, + String lastIndirectChildIndent) { + this.emptyIndent = emptyIndent; + this.prevChildIndent = prevChildIndent; + this.childIndent = childIndent; + this.indirectChildIndent = indirectChildIndent; + this.lastChildIndent = lastChildIndent; + this.lastIndirectChildIndent = lastIndirectChildIndent; + } + } +} diff --git a/src/main/java/com/google/devtools/build/lib/bazel/commands/BUILD b/src/main/java/com/google/devtools/build/lib/bazel/commands/BUILD index 54918ab3dc0ef3..f8a290a66e334e 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/commands/BUILD +++ b/src/main/java/com/google/devtools/build/lib/bazel/commands/BUILD @@ -32,6 +32,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/bazel/bzlmod:common", "//src/main/java/com/google/devtools/build/lib/bazel/bzlmod:inspection", "//src/main/java/com/google/devtools/build/lib/bazel/bzlmod:repo_rule_value", + "//src/main/java/com/google/devtools/build/lib/bazel/bzlmod/modquery", "//src/main/java/com/google/devtools/build/lib/bazel/repository", "//src/main/java/com/google/devtools/build/lib/bazel/repository/starlark", "//src/main/java/com/google/devtools/build/lib/cmdline", @@ -58,8 +59,6 @@ java_library( "//src/main/java/com/google/devtools/common/options", "//src/main/java/net/starlark/java/eval", "//src/main/protobuf:failure_details_java_proto", - "//third_party:auto_value", "//third_party:guava", - "//third_party:jsr305", ], ) diff --git a/src/main/java/com/google/devtools/build/lib/bazel/commands/ModqueryCommand.java b/src/main/java/com/google/devtools/build/lib/bazel/commands/ModqueryCommand.java index f249e178ee8c43..b7b773f3c7e1fd 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/commands/ModqueryCommand.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/commands/ModqueryCommand.java @@ -15,6 +15,7 @@ import static com.google.common.collect.ImmutableMap.toImmutableMap; import static com.google.common.collect.ImmutableSet.toImmutableSet; +import static com.google.devtools.build.lib.bazel.bzlmod.modquery.ModqueryOptions.Charset.UTF8; import static java.nio.charset.StandardCharsets.US_ASCII; import static java.nio.charset.StandardCharsets.UTF_8; @@ -29,11 +30,12 @@ import com.google.devtools.build.lib.bazel.bzlmod.BzlmodRepoRuleValue; import com.google.devtools.build.lib.bazel.bzlmod.ModuleKey; import com.google.devtools.build.lib.bazel.bzlmod.Version; -import com.google.devtools.build.lib.bazel.commands.ModqueryOptions.Charset; -import com.google.devtools.build.lib.bazel.commands.ModqueryOptions.QueryType; -import com.google.devtools.build.lib.bazel.commands.ModqueryOptions.QueryTypeConverter; -import com.google.devtools.build.lib.bazel.commands.ModqueryOptions.TargetModule; -import com.google.devtools.build.lib.bazel.commands.ModqueryOptions.TargetModuleListConverter; +import com.google.devtools.build.lib.bazel.bzlmod.modquery.ModqueryExecutor; +import com.google.devtools.build.lib.bazel.bzlmod.modquery.ModqueryOptions; +import com.google.devtools.build.lib.bazel.bzlmod.modquery.ModqueryOptions.QueryType; +import com.google.devtools.build.lib.bazel.bzlmod.modquery.ModqueryOptions.QueryTypeConverter; +import com.google.devtools.build.lib.bazel.bzlmod.modquery.ModqueryOptions.TargetModule; +import com.google.devtools.build.lib.bazel.bzlmod.modquery.ModqueryOptions.TargetModuleListConverter; import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.pkgcache.PackageOptions; import com.google.devtools.build.lib.runtime.BlazeCommand; @@ -226,7 +228,7 @@ public BlazeCommandResult exec(CommandEnvironment env, OptionsParsingResult opti modqueryOptions, new OutputStreamWriter( env.getReporter().getOutErr().getOutputStream(), - modqueryOptions.charset == Charset.UTF8 ? UTF_8 : US_ASCII)); + modqueryOptions.charset == UTF8 ? UTF_8 : US_ASCII)); switch (query) { case TREE: diff --git a/src/main/java/com/google/devtools/build/lib/bazel/coverage/BUILD b/src/main/java/com/google/devtools/build/lib/bazel/coverage/BUILD index 5e18b61b0b5395..67e095e132b533 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/coverage/BUILD +++ b/src/main/java/com/google/devtools/build/lib/bazel/coverage/BUILD @@ -22,6 +22,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/actions:artifacts", "//src/main/java/com/google/devtools/build/lib/actions:execution_requirements", "//src/main/java/com/google/devtools/build/lib/actions:localhost_capacity", + "//src/main/java/com/google/devtools/build/lib/actions:runfiles_supplier", "//src/main/java/com/google/devtools/build/lib/analysis:actions/compression", "//src/main/java/com/google/devtools/build/lib/analysis:analysis_cluster", "//src/main/java/com/google/devtools/build/lib/analysis:blaze_directories", diff --git a/src/main/java/com/google/devtools/build/lib/bazel/execlog/BUILD b/src/main/java/com/google/devtools/build/lib/bazel/execlog/BUILD index 5987af0694885d..2620c1ee0e60e8 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/execlog/BUILD +++ b/src/main/java/com/google/devtools/build/lib/bazel/execlog/BUILD @@ -15,6 +15,7 @@ java_library( name = "stable_sort", srcs = glob(["*.java"]), deps = [ + "//src/main/java/com/google/devtools/build/lib/profiler", "//src/main/java/com/google/devtools/build/lib/util/io", "//src/main/protobuf:spawn_java_proto", "//third_party:guava", diff --git a/src/main/java/com/google/devtools/build/lib/bazel/execlog/StableSort.java b/src/main/java/com/google/devtools/build/lib/bazel/execlog/StableSort.java index fb5a283a82cfca..7343be0b10bae8 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/execlog/StableSort.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/execlog/StableSort.java @@ -19,6 +19,8 @@ import com.google.common.collect.MultimapBuilder; import com.google.devtools.build.lib.exec.Protos.File; import com.google.devtools.build.lib.exec.Protos.SpawnExec; +import com.google.devtools.build.lib.profiler.Profiler; +import com.google.devtools.build.lib.profiler.SilentCloseable; import com.google.devtools.build.lib.util.io.MessageOutputStream; import java.io.IOException; import java.io.InputStream; @@ -54,7 +56,13 @@ private static ImmutableList read(InputStream in) throws IOException * output. We assume that there are no cyclic dependencies. */ public static void stableSort(InputStream in, MessageOutputStream out) throws IOException { - stableSort(read(in), out); + try (SilentCloseable c = Profiler.instance().profile("stableSort")) { + ImmutableList inputs; + try (SilentCloseable c2 = Profiler.instance().profile("stableSort/read")) { + inputs = read(in); + } + stableSort(inputs, out); + } } private static void stableSort(List inputs, MessageOutputStream out) diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRulesModule.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRulesModule.java index b6535de791f7b9..5a4112308ba75f 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRulesModule.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRulesModule.java @@ -20,19 +20,10 @@ import com.google.devtools.build.lib.bazel.rules.sh.BazelShRuleClasses; import com.google.devtools.build.lib.buildtool.BuildRequest; import com.google.devtools.build.lib.exec.ModuleActionContextRegistry; -import com.google.devtools.build.lib.remote.options.RemoteOptions; -import com.google.devtools.build.lib.remote.options.RemoteOutputsMode; -import com.google.devtools.build.lib.rules.cpp.CppOptions; import com.google.devtools.build.lib.rules.java.JavaCompileActionContext; -import com.google.devtools.build.lib.rules.java.JavaOptions; import com.google.devtools.build.lib.runtime.BlazeModule; import com.google.devtools.build.lib.runtime.Command; import com.google.devtools.build.lib.runtime.CommandEnvironment; -import com.google.devtools.build.lib.server.FailureDetails.FailureDetail; -import com.google.devtools.build.lib.server.FailureDetails.RemoteExecution; -import com.google.devtools.build.lib.server.FailureDetails.RemoteExecution.Code; -import com.google.devtools.build.lib.util.AbruptExitException; -import com.google.devtools.build.lib.util.DetailedExitCode; import com.google.devtools.build.lib.util.ResourceFileLoader; import com.google.devtools.common.options.Converters; import com.google.devtools.common.options.Option; @@ -386,6 +377,15 @@ public static class BuildGraveyardOptions extends OptionsBase { effectTags = {OptionEffectTag.EXECUTION}, help = "Deprecated no-op.") public boolean collectLocalSandboxExecutionStatistics; + + @Option( + name = "experimental_enable_starlark_doc_extract", + defaultValue = "true", + documentationCategory = OptionDocumentationCategory.UNDOCUMENTED, + effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS}, + metadataTags = {OptionMetadataTag.EXPERIMENTAL}, + help = "Deprecated no-op.") + public boolean enableBzlDocDump; } /** This is where deprecated Bazel-specific options only used by the build command go to die. */ @@ -638,11 +638,6 @@ public void initializeRuleClasses(ConfiguredRuleClassProvider.Builder builder) { } } - @Override - public void beforeCommand(CommandEnvironment env) throws AbruptExitException { - validateRemoteOutputsMode(env); - } - @Override public Iterable> getCommandOptions(Command command) { return "build".equals(command.name()) @@ -657,37 +652,4 @@ public void registerActionContexts( BuildRequest buildRequest) { registryBuilder.register(JavaCompileActionContext.class, new JavaCompileActionContext()); } - - private static void validateRemoteOutputsMode(CommandEnvironment env) throws AbruptExitException { - RemoteOptions remoteOptions = env.getOptions().getOptions(RemoteOptions.class); - if (remoteOptions == null) { - return; - } - if (remoteOptions.remoteOutputsMode != RemoteOutputsMode.ALL) { - JavaOptions javaOptions = env.getOptions().getOptions(JavaOptions.class); - if (javaOptions != null && !javaOptions.inmemoryJdepsFiles) { - throw createRemoteExecutionExitException( - "--experimental_remote_download_outputs={toplevel,minimal} requires" - + " --experimental_inmemory_jdeps_files to be enabled", - Code.REMOTE_DOWNLOAD_OUTPUTS_MINIMAL_WITHOUT_INMEMORY_JDEPS); - } - CppOptions cppOptions = env.getOptions().getOptions(CppOptions.class); - if (cppOptions != null && !cppOptions.inmemoryDotdFiles) { - throw createRemoteExecutionExitException( - "--experimental_remote_download_outputs={toplevel,minimal} requires" - + " --experimental_inmemory_dotd_files to be enabled", - Code.REMOTE_DOWNLOAD_OUTPUTS_MINIMAL_WITHOUT_INMEMORY_DOTD); - } - } - } - - private static AbruptExitException createRemoteExecutionExitException( - String message, Code remoteExecutionCode) { - return new AbruptExitException( - DetailedExitCode.of( - FailureDetail.newBuilder() - .setMessage(message) - .setRemoteExecution(RemoteExecution.newBuilder().setCode(remoteExecutionCode)) - .build())); - } } diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/GenericRules.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/GenericRules.java index c8602ff089da6a..08e6004b9a1848 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/GenericRules.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/GenericRules.java @@ -50,7 +50,7 @@ public void init(ConfiguredRuleClassProvider.Builder builder) { GenQueryRule.register(builder); builder.addRuleDefinition(new LabelBuildSettingRule()); builder.addRuleDefinition(new LabelBuildFlagRule()); - StarlarkDocExtractRule.register(builder); + builder.addRuleDefinition(new StarlarkDocExtractRule()); try { builder.addWorkspaceFilePrefix( diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/JavaRules.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/JavaRules.java index 814dc77b795987..8a623846060c79 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/JavaRules.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/JavaRules.java @@ -85,11 +85,13 @@ public void init(ConfiguredRuleClassProvider.Builder builder) { builder.addStarlarkBootstrap( new JavaBootstrap( - new JavaStarlarkCommon(BazelJavaSemantics.INSTANCE), JavaInfo.PROVIDER, JavaPluginInfo.PROVIDER, ProguardSpecProvider.PROVIDER)); + builder.addStarlarkBuiltinsInternal( + "java_common_internal_do_not_use", new JavaStarlarkCommon(BazelJavaSemantics.INSTANCE)); + builder.addBzlToplevel( "experimental_java_library_export_do_not_use", FlagGuardedValue.onlyWhenExperimentalFlagIsTrue( diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/BuildRequestOptions.java b/src/main/java/com/google/devtools/build/lib/buildtool/BuildRequestOptions.java index 89ca6ed7544858..540f86c151ba7a 100644 --- a/src/main/java/com/google/devtools/build/lib/buildtool/BuildRequestOptions.java +++ b/src/main/java/com/google/devtools/build/lib/buildtool/BuildRequestOptions.java @@ -428,7 +428,7 @@ public boolean useTopLevelTargetsForSymlinks() { OptionEffectTag.BAZEL_INTERNAL_CONFIGURATION, OptionEffectTag.HOST_MACHINE_RESOURCE_OPTIMIZATIONS }, - help = "Whether to store output metadata in the action cache") + help = "no-op") public boolean actionCacheStoreOutputMetadata; @Option( diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionTool.java b/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionTool.java index aeb4ffb9ae2743..506c1875343b7a 100644 --- a/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionTool.java +++ b/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionTool.java @@ -305,9 +305,16 @@ public void prepareForExecution(Stopwatch executionTimer) } SkyframeBuilder skyframeBuilder; try (SilentCloseable c = Profiler.instance().profile("createBuilder")) { + var shouldStoreRemoteMetadataInActionCache = + outputService != null && outputService.shouldStoreRemoteOutputMetadataInActionCache(); skyframeBuilder = (SkyframeBuilder) - createBuilder(request, actionCache, skyframeExecutor, modifiedOutputFiles); + createBuilder( + request, + actionCache, + skyframeExecutor, + modifiedOutputFiles, + shouldStoreRemoteMetadataInActionCache); } skyframeExecutor.drainChangedFiles(); @@ -395,7 +402,15 @@ void executeBuild( SkyframeExecutor skyframeExecutor = env.getSkyframeExecutor(); Builder builder; try (SilentCloseable c = Profiler.instance().profile("createBuilder")) { - builder = createBuilder(request, actionCache, skyframeExecutor, modifiedOutputFiles); + var shouldStoreRemoteMetadataInActionCache = + outputService != null && outputService.shouldStoreRemoteOutputMetadataInActionCache(); + builder = + createBuilder( + request, + actionCache, + skyframeExecutor, + modifiedOutputFiles, + shouldStoreRemoteMetadataInActionCache); } // @@ -925,7 +940,8 @@ private Builder createBuilder( BuildRequest request, @Nullable ActionCache actionCache, SkyframeExecutor skyframeExecutor, - ModifiedFileSet modifiedOutputFiles) { + ModifiedFileSet modifiedOutputFiles, + boolean shouldStoreRemoteOutputMetadataInActionCache) { BuildRequestOptions options = request.getBuildOptions(); skyframeExecutor.setActionOutputRoot(env.getActionTempsDirectory()); @@ -944,7 +960,7 @@ private Builder createBuilder( ActionCacheChecker.CacheConfig.builder() .setEnabled(options.useActionCache) .setVerboseExplanations(options.verboseExplanations) - .setStoreOutputMetadata(options.actionCacheStoreOutputMetadata) + .setStoreOutputMetadata(shouldStoreRemoteOutputMetadataInActionCache) .build()), modifiedOutputFiles, env.getFileCache(), diff --git a/src/main/java/com/google/devtools/build/lib/cmdline/BazelModuleContext.java b/src/main/java/com/google/devtools/build/lib/cmdline/BazelModuleContext.java index 9ea41d5aab2fa6..d330efe6103cdc 100644 --- a/src/main/java/com/google/devtools/build/lib/cmdline/BazelModuleContext.java +++ b/src/main/java/com/google/devtools/build/lib/cmdline/BazelModuleContext.java @@ -16,6 +16,7 @@ import com.google.auto.value.AutoValue; import com.google.common.collect.ImmutableList; +import com.google.errorprone.annotations.CanIgnoreReturnValue; import net.starlark.java.eval.Module; /** @@ -45,10 +46,42 @@ public abstract class BazelModuleContext { * Returns a list of modules loaded by this .bzl file, in source order. * *

By traversing these modules' loads, it is possible to reconstruct the complete load DAG (not - * including {@code @_builtins} .bzl files). + * including {@code @_builtins} .bzl files). See {@link #visitLoadGraphRecursively}. */ public abstract ImmutableList loads(); + /** + * Consumes labels of loaded Starlark files during a call to {@link #visitLoadGraphRecursively}. + * + *

The value returned by {@link #visit} determines whether the traversal should continue (true) + * or backtrack (false). Using a method reference to {@link Set#add} is a convenient way to + * aggregate Starlark files while pruning branches when a file was already seen. The same set may + * be reused across multiple calls to {@link #visitLoadGraphRecursively} in order to prune the + * graph at files already seen during a previous traversal. + */ + @FunctionalInterface + public interface LoadGraphVisitor { + /** + * Processes a single loaded Starlark file and determines whether to recurse into that file's + * loads. + * + * @return true if the visitation should recurse into the loads of the given file + */ + @CanIgnoreReturnValue + boolean visit(Label load) throws E1, E2; + } + + /** Performs an online visitation of the load graph rooted at a given list of loads. */ + public static void visitLoadGraphRecursively( + Iterable loads, LoadGraphVisitor visitor) throws E1, E2 { + for (Module module : loads) { + BazelModuleContext ctx = BazelModuleContext.of(module); + if (visitor.visit(ctx.label())) { + visitLoadGraphRecursively(ctx.loads(), visitor); + } + } + } + /** * Transitive digest of the .bzl file of the {@link net.starlark.java.eval.Module} itself and all * files it transitively loads. diff --git a/src/main/java/com/google/devtools/build/lib/cmdline/Label.java b/src/main/java/com/google/devtools/build/lib/cmdline/Label.java index 25e432e4b9346d..0b8f66c94580e3 100644 --- a/src/main/java/com/google/devtools/build/lib/cmdline/Label.java +++ b/src/main/java/com/google/devtools/build/lib/cmdline/Label.java @@ -19,7 +19,6 @@ import com.google.common.base.Preconditions; import com.google.common.collect.ComparisonChain; import com.google.common.collect.ImmutableSet; -import com.google.common.collect.Interner; import com.google.common.util.concurrent.Striped; import com.google.devtools.build.docgen.annot.DocCategory; import com.google.devtools.build.lib.actions.CommandLineItem; @@ -34,7 +33,6 @@ import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.skyframe.SkyFunctionName; import com.google.devtools.build.skyframe.SkyKey; -import com.google.devtools.build.skyframe.UsePooledLabelInterningFlag; import java.util.Arrays; import java.util.concurrent.locks.Lock; import java.util.concurrent.locks.ReadWriteLock; @@ -86,16 +84,10 @@ public final class Label implements Comparable

It is safe to call {@link #updateRunfilesDirectory} concurrently. + *

It is safe to call {@link #updateRunfiles} concurrently. */ @ThreadSafe public class RunfilesTreeUpdater { + private final Path execRoot; + private final BinTools binTools; + private final XattrProvider xattrProvider; - public static final RunfilesTreeUpdater INSTANCE = new RunfilesTreeUpdater(); + /** + * Deduplicates multiple attempts to update the same runfiles tree. + * + *

Attempts may occur concurrently, e.g. if multiple local actions have the same input. + * + *

The presence of an entry in the map signifies that an earlier attempt to update the + * corresponding runfiles tree was started, and will (have) set the future upon completion. + */ + private final ConcurrentHashMap> updatedTrees = + new ConcurrentHashMap<>(); - private final Object lock = new Object(); + public static RunfilesTreeUpdater forCommandEnvironment(CommandEnvironment env) { + return new RunfilesTreeUpdater( + env.getExecRoot(), env.getBlazeWorkspace().getBinTools(), env.getXattrProvider()); + } - private static final class LockWithRefcnt { - int refcnt = 1; + public RunfilesTreeUpdater(Path execRoot, BinTools binTools, XattrProvider xattrProvider) { + this.execRoot = execRoot; + this.binTools = binTools; + this.xattrProvider = xattrProvider; } - /** - * Poor man's reference counted object pool. - * - *

Maintains a mapping of runfiles directory to a monitor. The monitor maintains a counter that - * tracks how many threads it is acquired by at the moment. If the count drops to zero the mapping - * is removed. - */ - @GuardedBy("lock") - private final Map locksWithRefcnt = new HashMap<>(); + /** Creates or updates input runfiles trees for a spawn. */ + public void updateRunfiles( + RunfilesSupplier runfilesSupplier, ImmutableMap env, OutErr outErr) + throws ExecException, IOException, InterruptedException { + for (Map.Entry> runfiles : + runfilesSupplier.getMappings().entrySet()) { + PathFragment runfilesDir = runfiles.getKey(); + if (runfilesSupplier.isBuildRunfileLinks(runfilesDir)) { + continue; + } - private RunfilesTreeUpdater() {} + var freshFuture = new CompletableFuture(); + CompletableFuture priorFuture = updatedTrees.putIfAbsent(runfilesDir, freshFuture); - private static void updateRunfilesTree( - Path execRoot, + if (priorFuture == null) { + // We are the first attempt; update the runfiles tree and mark the future complete. + try { + updateRunfilesTree( + runfilesDir, env, outErr, runfilesSupplier.isRunfileLinksEnabled(runfilesDir)); + freshFuture.complete(null); + } catch (Exception e) { + freshFuture.completeExceptionally(e); + throw e; + } + } else { + // There was a previous attempt; wait for it to complete. + priorFuture.join(); + } + } + } + + private void updateRunfilesTree( PathFragment runfilesDir, - BinTools binTools, ImmutableMap env, OutErr outErr, - boolean enableRunfiles, - XattrProvider xattrProvider) + boolean enableRunfiles) throws IOException, ExecException, InterruptedException { Path runfilesDirPath = execRoot.getRelative(runfilesDir); Path inputManifest = RunfilesSupport.inputManifestPath(runfilesDirPath); @@ -101,69 +133,4 @@ private static void updateRunfilesTree( new SymlinkTreeHelper(inputManifest, runfilesDirPath, /* filesetTree= */ false); helper.createSymlinks(execRoot, outErr, binTools, env, enableRunfiles); } - - private LockWithRefcnt getLockAndIncrementRefcnt(PathFragment runfilesDirectory) { - synchronized (lock) { - LockWithRefcnt lock = locksWithRefcnt.get(runfilesDirectory); - if (lock != null) { - lock.refcnt++; - return lock; - } - lock = new LockWithRefcnt(); - locksWithRefcnt.put(runfilesDirectory, lock); - return lock; - } - } - - private void decrementRefcnt(PathFragment runfilesDirectory) { - synchronized (lock) { - LockWithRefcnt lock = locksWithRefcnt.get(runfilesDirectory); - lock.refcnt--; - if (lock.refcnt == 0) { - if (!locksWithRefcnt.remove(runfilesDirectory, lock)) { - throw new IllegalStateException( - String.format( - "Failed to remove lock for dir '%s'." + " This is a bug.", runfilesDirectory)); - } - } - } - } - - public void updateRunfilesDirectory( - Path execRoot, - RunfilesSupplier runfilesSupplier, - BinTools binTools, - ImmutableMap env, - OutErr outErr, - XattrProvider xattrProvider) - throws ExecException, IOException, InterruptedException { - for (Map.Entry> runfiles : - runfilesSupplier.getMappings().entrySet()) { - PathFragment runfilesDir = runfiles.getKey(); - if (runfilesSupplier.isBuildRunfileLinks(runfilesDir)) { - continue; - } - - try { - long startTime = Profiler.nanoTimeMaybe(); - // Synchronize runfiles tree generation on the runfiles directory in order to prevent - // concurrent modifications of the runfiles tree. In particular this can happen for sharded - // tests and --runs_per_test > 1 in which case multiple actions use the same runfiles tree. - synchronized (getLockAndIncrementRefcnt(runfilesDir)) { - Profiler.instance() - .logSimpleTask(startTime, ProfilerTask.WAIT, "Waiting to create runfiles tree"); - updateRunfilesTree( - execRoot, - runfilesDir, - binTools, - env, - outErr, - runfilesSupplier.isRunfileLinksEnabled(runfilesDir), - xattrProvider); - } - } finally { - decrementRefcnt(runfilesDir); - } - } - } } diff --git a/src/main/java/com/google/devtools/build/lib/exec/SpawnLogContext.java b/src/main/java/com/google/devtools/build/lib/exec/SpawnLogContext.java index 33abbcde29712f..92ffaa1c410959 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/SpawnLogContext.java +++ b/src/main/java/com/google/devtools/build/lib/exec/SpawnLogContext.java @@ -31,6 +31,8 @@ import com.google.devtools.build.lib.exec.Protos.Digest; import com.google.devtools.build.lib.exec.Protos.File; import com.google.devtools.build.lib.exec.Protos.SpawnExec; +import com.google.devtools.build.lib.profiler.Profiler; +import com.google.devtools.build.lib.profiler.SilentCloseable; import com.google.devtools.build.lib.remote.options.RemoteOptions; import com.google.devtools.build.lib.util.io.MessageOutputStream; import com.google.devtools.build.lib.vfs.DigestHashFunction; @@ -100,7 +102,7 @@ public void logSpawn( builder.addEnvironmentVariablesBuilder().setName(var).setValue(env.get(var)); } - try { + try (SilentCloseable c = Profiler.instance().profile("logSpawn/inputs")) { for (Map.Entry e : inputMap.entrySet()) { ActionInput input = e.getValue(); if (input instanceof VirtualActionInput.EmptyActionInput) { @@ -117,24 +119,26 @@ public void logSpawn( } catch (IOException e) { logger.atWarning().withCause(e).log("Error computing spawn inputs"); } - ArrayList outputPaths = new ArrayList<>(); - for (ActionInput output : spawn.getOutputFiles()) { - outputPaths.add(output.getExecPathString()); - } - Collections.sort(outputPaths); - builder.addAllListedOutputs(outputPaths); - for (Map.Entry e : existingOutputs.entrySet()) { - Path path = e.getKey(); - if (path.isDirectory()) { - listDirectoryContents(path, builder::addActualOutputs, inputMetadataProvider); - } else { - File.Builder outputBuilder = builder.addActualOutputsBuilder(); - outputBuilder.setPath(path.relativeTo(execRoot).toString()); - try { - outputBuilder.setDigest( - computeDigest(e.getValue(), path, inputMetadataProvider, xattrProvider)); - } catch (IOException ex) { - logger.atWarning().withCause(ex).log("Error computing spawn event output properties"); + try (SilentCloseable c = Profiler.instance().profile("logSpawn/outputs")) { + ArrayList outputPaths = new ArrayList<>(); + for (ActionInput output : spawn.getOutputFiles()) { + outputPaths.add(output.getExecPathString()); + } + Collections.sort(outputPaths); + builder.addAllListedOutputs(outputPaths); + for (Map.Entry e : existingOutputs.entrySet()) { + Path path = e.getKey(); + if (path.isDirectory()) { + listDirectoryContents(path, builder::addActualOutputs, inputMetadataProvider); + } else { + File.Builder outputBuilder = builder.addActualOutputsBuilder(); + outputBuilder.setPath(path.relativeTo(execRoot).toString()); + try { + outputBuilder.setDigest( + computeDigest(e.getValue(), path, inputMetadataProvider, xattrProvider)); + } catch (IOException ex) { + logger.atWarning().withCause(ex).log("Error computing spawn event output properties"); + } } } } @@ -219,7 +223,9 @@ public void logSpawn( } } - executionLog.write(builder.build()); + try (SilentCloseable c = Profiler.instance().profile("logSpawn/write")) { + executionLog.write(builder.build()); + } } private static com.google.protobuf.Duration millisToProto(int t) { diff --git a/src/main/java/com/google/devtools/build/lib/exec/SpawnStrategyRegistry.java b/src/main/java/com/google/devtools/build/lib/exec/SpawnStrategyRegistry.java index 8bef73435dde8f..55675127a5ea45 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/SpawnStrategyRegistry.java +++ b/src/main/java/com/google/devtools/build/lib/exec/SpawnStrategyRegistry.java @@ -19,6 +19,7 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Joiner; import com.google.common.base.Preconditions; +import com.google.common.collect.ImmutableCollection; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableListMultimap; import com.google.common.collect.ImmutableMultimap; @@ -140,16 +141,19 @@ public void notifyUsedDynamic(ActionContext.ActionContextRegistry actionContextR } @Override - public List getDynamicSpawnActionContexts( + public ImmutableCollection getDynamicSpawnActionContexts( Spawn spawn, DynamicMode dynamicMode) { ImmutableMultimap mnemonicToDynamicStrategies = dynamicMode == DynamicStrategyRegistry.DynamicMode.REMOTE ? mnemonicToRemoteDynamicStrategies : mnemonicToLocalDynamicStrategies; - return ImmutableList.builder() - .addAll(mnemonicToDynamicStrategies.get(spawn.getMnemonic())) - .addAll(mnemonicToDynamicStrategies.get("")) - .build(); + if (mnemonicToDynamicStrategies.containsKey(spawn.getMnemonic())) { + return mnemonicToDynamicStrategies.get(spawn.getMnemonic()); + } + if (mnemonicToDynamicStrategies.containsKey("")) { + return mnemonicToDynamicStrategies.get(""); + } + return ImmutableList.of(); } @Nullable diff --git a/src/main/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunner.java index 6cf84d466df0a2..3d9ce428385932 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunner.java +++ b/src/main/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunner.java @@ -59,7 +59,6 @@ import com.google.devtools.build.lib.util.OS; import com.google.devtools.build.lib.util.io.FileOutErr; import com.google.devtools.build.lib.vfs.Path; -import com.google.devtools.build.lib.vfs.XattrProvider; import com.google.errorprone.annotations.FormatMethod; import com.google.errorprone.annotations.FormatString; import java.io.File; @@ -94,7 +93,6 @@ public class LocalSpawnRunner implements SpawnRunner { private final String hostName; private final LocalExecutionOptions localExecutionOptions; - private final XattrProvider xattrProvider; @Nullable private final ProcessWrapper processWrapper; @@ -110,12 +108,10 @@ public LocalSpawnRunner( LocalEnvProvider localEnvProvider, BinTools binTools, ProcessWrapper processWrapper, - XattrProvider xattrProvider, RunfilesTreeUpdater runfilesTreeUpdater) { this.execRoot = execRoot; this.processWrapper = processWrapper; this.localExecutionOptions = Preconditions.checkNotNull(localExecutionOptions); - this.xattrProvider = xattrProvider; this.hostName = NetUtil.getCachedShortHostName(); this.resourceManager = resourceManager; this.localEnvProvider = localEnvProvider; @@ -135,13 +131,8 @@ public SpawnResult exec(Spawn spawn, SpawnExecutionContext context) SpawnMetrics.Builder spawnMetrics = SpawnMetrics.Builder.forLocalExec(); Stopwatch totalTimeStopwatch = Stopwatch.createStarted(); Stopwatch setupTimeStopwatch = Stopwatch.createStarted(); - runfilesTreeUpdater.updateRunfilesDirectory( - execRoot, - spawn.getRunfilesSupplier(), - binTools, - spawn.getEnvironment(), - context.getFileOutErr(), - xattrProvider); + runfilesTreeUpdater.updateRunfiles( + spawn.getRunfilesSupplier(), spawn.getEnvironment(), context.getFileOutErr()); if (Spawns.shouldPrefetchInputsForLocalExecution(spawn)) { context.prefetchInputsAndWait(); } diff --git a/src/main/java/com/google/devtools/build/lib/includescanning/BUILD b/src/main/java/com/google/devtools/build/lib/includescanning/BUILD index 974db80cae1dcd..d50623e0d5646d 100644 --- a/src/main/java/com/google/devtools/build/lib/includescanning/BUILD +++ b/src/main/java/com/google/devtools/build/lib/includescanning/BUILD @@ -23,6 +23,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/actions:file_metadata", "//src/main/java/com/google/devtools/build/lib/actions:localhost_capacity", "//src/main/java/com/google/devtools/build/lib/actions:middleman_type", + "//src/main/java/com/google/devtools/build/lib/actions:runfiles_supplier", "//src/main/java/com/google/devtools/build/lib/analysis:blaze_directories", "//src/main/java/com/google/devtools/build/lib/analysis/platform", "//src/main/java/com/google/devtools/build/lib/cmdline", diff --git a/src/main/java/com/google/devtools/build/lib/includescanning/IncludeScannerSupplier.java b/src/main/java/com/google/devtools/build/lib/includescanning/IncludeScannerSupplier.java index c5211462990f4d..d9ec0a02828857 100644 --- a/src/main/java/com/google/devtools/build/lib/includescanning/IncludeScannerSupplier.java +++ b/src/main/java/com/google/devtools/build/lib/includescanning/IncludeScannerSupplier.java @@ -94,6 +94,16 @@ public IncludeScannerSupplier( PathExistenceCache pathCache = new PathExistenceCache(execRoot, artifactFactory); scanners = Caffeine.newBuilder() + // We choose to make cache values weak referenced due to LegacyIncludeScanner can hold + // on to a memory expensive InclusionCache. However, a lot of IncludeScannerParams are + // not in use so they are eligible for garbage collection. As a matter of fact, this + // reduces peak heap on an example cpp-heavy build by ~5%. + + // + // We could also choose to use softValues() but avoid doing so. The reason is that we + // want to keep blaze memory usage deterministic and to guarantee collection before + // blaze initiated-OOMs. + .weakValues() .build( key -> new LegacyIncludeScanner( diff --git a/src/main/java/com/google/devtools/build/lib/packages/AttributeTransitionData.java b/src/main/java/com/google/devtools/build/lib/packages/AttributeTransitionData.java index 115ff241db6067..9016d3868b24ad 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/AttributeTransitionData.java +++ b/src/main/java/com/google/devtools/build/lib/packages/AttributeTransitionData.java @@ -34,6 +34,16 @@ public abstract class AttributeTransitionData implements TransitionFactory.Data @Nullable public abstract Label executionPlatform(); + /** + * Optional parameter to let callers instantiate objects that the {@code lib.packages} library + * can't resolve. This class is both defined in {@code lib.packages} and referenced by other files + * in that package. + * + *

Callers are responsible for ensuring correct casting between writes and reads. + */ + @Nullable + public abstract Object analysisData(); + /** Returns a new {@link Builder} for {@link AttributeTransitionData}. */ public static Builder builder() { return new AutoValue_AttributeTransitionData.Builder(); @@ -48,6 +58,8 @@ public abstract static class Builder { /** Sets the execution platform label. */ public abstract Builder executionPlatform(@Nullable Label executionPlatform); + public abstract Builder analysisData(Object analysisData); + /** Returns the new {@link AttributeTransitionData}. */ public abstract AttributeTransitionData build(); } diff --git a/src/main/java/com/google/devtools/build/lib/packages/BazelStarlarkEnvironment.java b/src/main/java/com/google/devtools/build/lib/packages/BazelStarlarkEnvironment.java index d74febc4a6e4b9..120d6b2c7acb69 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/BazelStarlarkEnvironment.java +++ b/src/main/java/com/google/devtools/build/lib/packages/BazelStarlarkEnvironment.java @@ -93,8 +93,6 @@ public final class BazelStarlarkEnvironment { private final ImmutableMap uninjectedWorkspaceBzlEnv; /** The top-level predeclared symbols for a bzl module in the {@code @_builtins} pseudo-repo. */ private final ImmutableMap builtinsBzlEnv; - /** The top-level predeclared symbols for a bzl module in the Bzlmod system. */ - private final ImmutableMap bzlmodBzlEnv; /** * Constructs a new {@code BazelStarlarkEnvironment} that will have complete knowledge of the @@ -135,11 +133,6 @@ public BazelStarlarkEnvironment( createUninjectedBuildBzlEnv(bzlToplevelsWithoutNative, uninjectedBuildBzlNativeBindings); this.uninjectedWorkspaceBzlEnv = createWorkspaceBzlEnv(bzlToplevelsWithoutNative, workspaceBzlNativeBindings); - // TODO(#11954): We should converge all .bzl dialects regardless of whether they're loaded by - // BUILD, WORKSPACE, or MODULE. At the moment, WORKSPACE-loaded and MODULE-loaded .bzl files are - // already converged, hence why bzlmodBzlEnv is populated with a "Workspace" helper function. - this.bzlmodBzlEnv = - createWorkspaceBzlEnv(bzlToplevelsWithoutNative, workspaceBzlNativeBindings); this.builtinsBzlEnv = createBuiltinsBzlEnv( starlarkGlobals, @@ -213,14 +206,6 @@ public ImmutableMap getBuiltinsBzlEnv() { return builtinsBzlEnv; } - /** - * Returns the environment for Bzlmod-loaded bzl files. Excludes symbols in {@link - * Starlark#UNIVERSE}. - */ - public ImmutableMap getBzlmodBzlEnv() { - return bzlmodBzlEnv; - } - private static ImmutableMap createBzlToplevelsWithoutNative( StarlarkGlobals starlarkGlobals, Map registeredBzlToplevels) { ImmutableMap.Builder env = new ImmutableMap.Builder<>(); diff --git a/src/main/java/com/google/devtools/build/lib/packages/License.java b/src/main/java/com/google/devtools/build/lib/packages/License.java index 0f3c3bf06efdcd..4fe246a4224c8c 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/License.java +++ b/src/main/java/com/google/devtools/build/lib/packages/License.java @@ -14,6 +14,9 @@ package com.google.devtools.build.lib.packages; +import static com.google.common.collect.ImmutableList.toImmutableList; + +import com.google.common.base.Ascii; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Sets; import com.google.devtools.build.lib.cmdline.Label; @@ -27,6 +30,7 @@ import java.util.List; import java.util.Locale; import java.util.Set; +import java.util.stream.Stream; import net.starlark.java.eval.Printer; /** Support for license and distribution checking. */ @@ -35,6 +39,7 @@ public final class License implements LicenseApi { private final ImmutableSet licenseTypes; private final ImmutableSet

The value returned by {@link #visit} determines whether the traversal should continue (true) - * or backtrack (false). Using a method reference to {@link Set#add} is a convenient way to - * aggregate Starlark files while pruning branches when a file was already seen. The same set may - * be reused across multiple calls to {@link #visitLoadGraph} for different packages in order to - * prune the graph at files already seen during a previous traversal. - */ - @FunctionalInterface - public interface LoadGraphVisitor { - /** - * Processes a single loaded Starlark file and determines whether to recurse into that file's - * loads. - * - * @return true if the visitation should recurse into the loads of the given file; ignored if - * transitive loads were {@linkplain PackageSettings#precomputeTransitiveLoads precomputed} - */ - @CanIgnoreReturnValue - boolean visit(Label load) throws E1, E2; - } - /** * Performs an online visitation of the load graph rooted at this package. * @@ -504,23 +483,13 @@ public void visitLoadGraph( visitor.visit(load); } } else { - visitLoadGraphRecursively(directLoads, visitor); - } - } - - private static void visitLoadGraphRecursively( - Iterable loads, LoadGraphVisitor visitor) throws E1, E2 { - for (Module module : loads) { - BazelModuleContext ctx = BazelModuleContext.of(module); - if (visitor.visit(ctx.label())) { - visitLoadGraphRecursively(ctx.loads(), visitor); - } + BazelModuleContext.visitLoadGraphRecursively(directLoads, visitor); } } private static ImmutableList