From 8e26f64f891f719e52f64f294cbd206638a99488 Mon Sep 17 00:00:00 2001 From: Paolo Tranquilli Date: Fri, 31 May 2024 11:45:30 +0200 Subject: [PATCH 1/5] Bazel: fix non-swift macOS builds This is meant to be cleaned up in a later PR with respect to the TODOs. --- .bazelrc | 5 -- .github/workflows/go-tests-other-os.yml | 3 + .github/workflows/go-tests.yml | 3 + misc/bazel/os.bzl | 38 ++++++++ misc/bazel/pkg.bzl | 32 +------ swift/rules.bzl | 111 ++++++++++++++++++++++-- 6 files changed, 154 insertions(+), 38 deletions(-) create mode 100644 misc/bazel/os.bzl diff --git a/.bazelrc b/.bazelrc index 3035b0beb395..87b611fe954f 100644 --- a/.bazelrc +++ b/.bazelrc @@ -10,11 +10,6 @@ common --override_module=semmle_code=%workspace%/misc/bazel/semmle_code_stub build --repo_env=CC=clang --repo_env=CXX=clang++ -build:linux --cxxopt=-std=c++20 --host_cxxopt=-std=c++20 -# we currently cannot built the swift extractor for ARM -build:macos --cxxopt=-std=c++20 --host_cxxopt=-std=c++20 --copt=-arch --copt=x86_64 --linkopt=-arch --linkopt=x86_64 -build:windows --cxxopt=/std:c++20 --cxxopt=/Zc:preprocessor --host_cxxopt=/std:c++20 --host_cxxopt=/Zc:preprocessor - # this requires developer mode, but is required to have pack installer functioning startup --windows_enable_symlinks common --enable_runfiles diff --git a/.github/workflows/go-tests-other-os.yml b/.github/workflows/go-tests-other-os.yml index 9915b0869db7..2760a8a4b51a 100644 --- a/.github/workflows/go-tests-other-os.yml +++ b/.github/workflows/go-tests-other-os.yml @@ -7,6 +7,9 @@ on: - .github/workflows/go-tests-other-os.yml - .github/actions/** - codeql-workspace.yml + - MODULE.bazel + - .bazelrc + - misc/bazel/** permissions: contents: read diff --git a/.github/workflows/go-tests.yml b/.github/workflows/go-tests.yml index 63e2b7c49740..f5e7d33fb1f6 100644 --- a/.github/workflows/go-tests.yml +++ b/.github/workflows/go-tests.yml @@ -15,6 +15,9 @@ on: - .github/workflows/go-tests.yml - .github/actions/** - codeql-workspace.yml + - MODULE.bazel + - .bazelrc + - misc/bazel/** permissions: contents: read diff --git a/misc/bazel/os.bzl b/misc/bazel/os.bzl new file mode 100644 index 000000000000..34093e76331d --- /dev/null +++ b/misc/bazel/os.bzl @@ -0,0 +1,38 @@ +""" Os detection facilities. """ + +def os_select( + ctx = None, + *, + linux = None, + windows = None, + macos = None, + default = None): + """ + This can work both in a macro and a rule context to choose something based on the current OS. + If used in a rule implementation, you need to pass `ctx` and add `OS_DETECTION_ATTRS` to the + rule attributes. + """ + choices = { + "linux": linux or default, + "windows": windows or default, + "macos": macos or default, + } + if not ctx: + return select({ + "@platforms//os:%s" % os: v + for os, v in choices.items() + if v != None + }) + + for os, v in choices.items(): + if ctx.target_platform_has_constraint(getattr(ctx.attr, "_%s_constraint" % os)[platform_common.ConstraintValueInfo]): + if v == None: + fail("%s not supported by %s" % (os, ctx.label)) + return v + fail("Unknown OS detected") + +OS_DETECTION_ATTRS = { + "_windows_constraint": attr.label(default = "@platforms//os:windows"), + "_macos_constraint": attr.label(default = "@platforms//os:macos"), + "_linux_constraint": attr.label(default = "@platforms//os:linux"), +} diff --git a/misc/bazel/pkg.bzl b/misc/bazel/pkg.bzl index fdfdb6be746b..8ba69677a1f2 100644 --- a/misc/bazel/pkg.bzl +++ b/misc/bazel/pkg.bzl @@ -7,6 +7,7 @@ load("@rules_pkg//pkg:mappings.bzl", "pkg_attributes", "pkg_filegroup", "pkg_fil load("@rules_pkg//pkg:pkg.bzl", "pkg_zip") load("@rules_pkg//pkg:providers.bzl", "PackageFilegroupInfo", "PackageFilesInfo") load("@rules_python//python:defs.bzl", "py_binary") +load("//misc/bazel:os.bzl", "OS_DETECTION_ATTRS", "os_select") def _make_internal(name): def internal(suffix = "internal", *args): @@ -15,11 +16,6 @@ def _make_internal(name): return internal -_PLAT_DETECTION_ATTRS = { - "_windows": attr.label(default = "@platforms//os:windows"), - "_macos": attr.label(default = "@platforms//os:macos"), -} - _PLAT_PLACEHOLDER = "{CODEQL_PLATFORM}" def _expand_path(path, platform): @@ -28,28 +24,8 @@ def _expand_path(path, platform): return ("arch", path) return ("generic", path) -def _platform_select( - ctx = None, - *, - linux, - windows, - macos): - if ctx: - if ctx.target_platform_has_constraint(ctx.attr._windows[platform_common.ConstraintValueInfo]): - return windows - elif ctx.target_platform_has_constraint(ctx.attr._macos[platform_common.ConstraintValueInfo]): - return macos - else: - return linux - else: - return select({ - "@platforms//os:linux": linux, - "@platforms//os:macos": macos, - "@platforms//os:windows": windows, - }) - def _detect_platform(ctx = None): - return _platform_select(ctx, linux = "linux64", macos = "osx64", windows = "win64") + return os_select(ctx, linux = "linux64", macos = "osx64", windows = "win64") def codeql_pkg_files( *, @@ -131,7 +107,7 @@ _extract_pkg_filegroup = rule( attrs = { "src": attr.label(providers = [PackageFilegroupInfo, DefaultInfo]), "kind": attr.string(doc = "What part to extract", values = ["generic", "arch"]), - } | _PLAT_DETECTION_ATTRS, + } | OS_DETECTION_ATTRS, ) _ZipInfo = provider(fields = {"zips_to_prefixes": "mapping of zip files to prefixes"}) @@ -181,7 +157,7 @@ _zip_info_filter = rule( attrs = { "srcs": attr.label_list(doc = "_ZipInfos to transform", providers = [_ZipInfo]), "kind": attr.string(doc = "Which zip kind to consider", values = ["generic", "arch"]), - } | _PLAT_DETECTION_ATTRS, + } | OS_DETECTION_ATTRS, ) def _imported_zips_manifest_impl(ctx): diff --git a/swift/rules.bzl b/swift/rules.bzl index 6ed085de63c3..3a70dbbb556d 100644 --- a/swift/rules.bzl +++ b/swift/rules.bzl @@ -1,11 +1,106 @@ +load("//misc/bazel:os.bzl", "os_select") + +# TODO: make a shared library with the internal repos for transitions +# unfortunately github.com/fmeum/rules_meta doesn't work any more with latest bazel + +def _transition_impl(settings, attr): + return { + "macos": { + "//command_line_option:copt": [ + "-fno-rtti", + # we currently cannot built the swift extractor for ARM + "-arch", + "x86_64", + ], + "//command_line_option:cxxopt": [ + "-std=c++20", + # we currently cannot built the swift extractor for ARM + "-arch", + "x86_64", + ], + "//command_line_option:linkopt": [ + # we currently cannot built the swift extractor for ARM + "-arch", + "x86_64", + ], + }, + "linux": { + "//command_line_option:copt": [ + "-fno-rtti", + ], + "//command_line_option:cxxopt": [ + "-std=c++20", + ], + }, + "windows": { + "//command_line_option:cxxopt": [ + "/std:c++20", + "--cxxopt=/Zc:preprocessor", + ], + }, + }[attr.os] + +_transition = transition( + implementation = _transition_impl, + inputs = [], + outputs = ["//command_line_option:copt", "//command_line_option:cxxopt", "//command_line_option:linkopt"], +) + +def _cc_transition_impl(ctx): + src = ctx.attr.src[0] + default_info = src[DefaultInfo] + executable = default_info.files_to_run.executable + runfiles = default_info.default_runfiles + direct = [] + if executable: + original_executable = executable + executable = ctx.actions.declare_file(original_executable.basename) + command = "cp %s %s" % (original_executable.path, executable.path) + + ctx.actions.run_shell( + inputs = [original_executable], + outputs = [executable], + command = command, + ) + + # costly, but no other way to remove the internal exe from the runfiles + files = runfiles.files.to_list() + files.remove(original_executable) + files.append(executable) + runfiles = ctx.runfiles(files) + + direct = [executable] + + providers = [ + DefaultInfo( + files = depset(direct), + runfiles = runfiles, + executable = executable, + ), + ] + for p in (OutputGroupInfo, CcInfo): + if p in src: + providers.append(src[p]) + + return providers + +_cc_transition = rule( + implementation = _cc_transition_impl, + attrs = { + "_allowlist_function_transition": attr.label( + default = "@bazel_tools//tools/allowlists/function_transition_allowlist", + ), + "src": attr.label(mandatory = True, cfg = _transition), + "os": attr.string(), + }, +) + def _add_args(kwargs, kwarg, value): kwargs[kwarg] = kwargs.get(kwarg, []) + value def _wrap_cc(rule, kwargs): - _add_args(kwargs, "copts", [ - # Required by LLVM/Swift - "-fno-rtti", - ]) + name = kwargs.pop("name") + visibility = kwargs.pop("visibility", None) _add_args(kwargs, "features", [ # temporary, before we do universal merging "-universal_binaries", @@ -17,7 +112,13 @@ def _wrap_cc(rule, kwargs): "@platforms//os:macos": [], "//conditions:default": ["@platforms//:incompatible"], })) - rule(**kwargs) + rule(name = "internal/" + name, visibility = ["//visibility:private"], **kwargs) + _cc_transition( + name = name, + visibility = visibility, + src = ":internal/" + name, + os = os_select(linux = "linux", macos = "macos", windows = "windows"), + ) def swift_cc_binary(**kwargs): _wrap_cc(native.cc_binary, kwargs) From 07f4288e1f8746b15031028f70685d6a8162405d Mon Sep 17 00:00:00 2001 From: Paolo Tranquilli Date: Fri, 31 May 2024 12:12:58 +0200 Subject: [PATCH 2/5] Fix zipmerge build and test --- misc/bazel/internal/zipmerge/BUILD.bazel | 3 +++ 1 file changed, 3 insertions(+) diff --git a/misc/bazel/internal/zipmerge/BUILD.bazel b/misc/bazel/internal/zipmerge/BUILD.bazel index cae83d529211..07cbb34ce978 100644 --- a/misc/bazel/internal/zipmerge/BUILD.bazel +++ b/misc/bazel/internal/zipmerge/BUILD.bazel @@ -4,6 +4,7 @@ cc_library( "zipmerge.cpp", ], hdrs = ["zipmerge.h"], + copts = ["-std=c++20"], ) cc_binary( @@ -11,6 +12,7 @@ cc_binary( srcs = [ "zipmerge_main.cpp", ], + copts = ["-std=c++20"], visibility = ["//visibility:public"], deps = [ ":lib", @@ -21,6 +23,7 @@ cc_test( name = "test", size = "small", srcs = ["zipmerge_test.cpp"], + copts = ["-std=c++20"], data = glob(["test-files/*"]), linkstatic = True, # required to build the test in the internal repo deps = [ From b3e29bd8b576f104034e970eec28b52d2ba85141 Mon Sep 17 00:00:00 2001 From: Paolo Tranquilli Date: Fri, 31 May 2024 12:13:26 +0200 Subject: [PATCH 3/5] Bazel: add `--build_tests_only` in swift CI --- swift/actions/build-and-test/action.yml | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/swift/actions/build-and-test/action.yml b/swift/actions/build-and-test/action.yml index 4553952f2c28..25034ab77318 100644 --- a/swift/actions/build-and-test/action.yml +++ b/swift/actions/build-and-test/action.yml @@ -44,10 +44,6 @@ runs: mkdir -p bazel-cache/{repository,disk} echo build --repository_cache=bazel-cache/repository --disk_cache=bazel-cache/disk > local.bazelrc echo test --test_output=errors >> local.bazelrc - # - name: Print unextracted entities - # shell: bash - # run: | - # bazel run //swift/extractor/print_unextracted - uses: ./swift/actions/share-extractor-pack - name: Build Swift extractor shell: bash @@ -62,7 +58,7 @@ runs: if: ${{ github.event_name == 'pull_request' }} shell: bash run: | - bazel test //swift/... + bazel test --build_tests_only //swift/... - name: Evict bazel cache if: ${{ github.event_name != 'pull_request' }} shell: bash From 3f19974bb6fde5491a26662892f630a1b0b341b6 Mon Sep 17 00:00:00 2001 From: Paolo Tranquilli Date: Fri, 31 May 2024 12:14:13 +0200 Subject: [PATCH 4/5] Bazel: fix transition on non-macOS --- swift/rules.bzl | 3 +++ 1 file changed, 3 insertions(+) diff --git a/swift/rules.bzl b/swift/rules.bzl index 3a70dbbb556d..cb16ca4382ad 100644 --- a/swift/rules.bzl +++ b/swift/rules.bzl @@ -31,12 +31,15 @@ def _transition_impl(settings, attr): "//command_line_option:cxxopt": [ "-std=c++20", ], + "//command_line_option:linkopt": [], }, "windows": { + "//command_line_option:copt": [], "//command_line_option:cxxopt": [ "/std:c++20", "--cxxopt=/Zc:preprocessor", ], + "//command_line_option:linkopt": [], }, }[attr.os] From bfc37fddffd19ea3eefb1dc2984095b1531a4abc Mon Sep 17 00:00:00 2001 From: Paolo Tranquilli Date: Fri, 31 May 2024 12:35:52 +0200 Subject: [PATCH 5/5] Bazel: move `--build_tests_only` from swift action to `.bazelrc` --- .bazelrc | 3 +++ swift/actions/build-and-test/action.yml | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/.bazelrc b/.bazelrc index 87b611fe954f..e2ccb69be902 100644 --- a/.bazelrc +++ b/.bazelrc @@ -10,6 +10,9 @@ common --override_module=semmle_code=%workspace%/misc/bazel/semmle_code_stub build --repo_env=CC=clang --repo_env=CXX=clang++ +# we use transitions that break builds of `...`, so for `test` to work with that we need the following +test --build_tests_only + # this requires developer mode, but is required to have pack installer functioning startup --windows_enable_symlinks common --enable_runfiles diff --git a/swift/actions/build-and-test/action.yml b/swift/actions/build-and-test/action.yml index 25034ab77318..2522f545c05f 100644 --- a/swift/actions/build-and-test/action.yml +++ b/swift/actions/build-and-test/action.yml @@ -58,7 +58,7 @@ runs: if: ${{ github.event_name == 'pull_request' }} shell: bash run: | - bazel test --build_tests_only //swift/... + bazel test //swift/... - name: Evict bazel cache if: ${{ github.event_name != 'pull_request' }} shell: bash