From 7e4b05e8e73714a79c0260fdfe3af13f3e0bdba0 Mon Sep 17 00:00:00 2001 From: Chuck Grindel Date: Tue, 2 Jan 2024 14:27:05 -0700 Subject: [PATCH] fix: filter out `maccatalyst` SPM platform (#829) Bazel currently does not support `maccatalyst` as a platform. We were mapping it to `ios` as they are Mac apps that use iOS frameworks. However, this can lead to warnings if define values exist with the same name mapped for `ios` and `maccatalyst` (e.g. Firebase). This PR filters out `maccatalyst` conditions. Closes #801. --- config_settings/spm/platform/platforms.bzl | 13 +++-- .../spm/tests/platform_tests/BUILD.bazel | 6 ++ .../tests/platform_tests/platforms_tests.bzl | 57 +++++++++++++++++++ examples/firebase_example/MODULE.bazel.lock | 8 +-- swiftpkg/internal/bzl_selects.bzl | 24 ++++---- swiftpkg/internal/pkginfos.bzl | 8 +-- 6 files changed, 88 insertions(+), 28 deletions(-) create mode 100644 config_settings/spm/tests/platform_tests/BUILD.bazel create mode 100644 config_settings/spm/tests/platform_tests/platforms_tests.bzl diff --git a/config_settings/spm/platform/platforms.bzl b/config_settings/spm/platform/platforms.bzl index f7cce63f0..376653655 100644 --- a/config_settings/spm/platform/platforms.bzl +++ b/config_settings/spm/platform/platforms.bzl @@ -46,9 +46,6 @@ _PLATFORM_INFOS = [ _platform_info(spm = p, bzl = None, os = p) for p in _NON_APPLE_PLATFORMS ] + [ - # Treat `maccatalyst` as an alias of sorts for macos. This will be handled - # in the `platforms.label` function. - _platform_info(spm = "maccatalyst", bzl = None, os = None), # Map `driverkit` as `macos`. This will be handled in the # `platforms.label()` function. _platform_info(spm = "driverkit", bzl = None, os = None), @@ -67,11 +64,17 @@ def _label(name): # There is currently no support Mac Catalyst in Bazel. These are Mac apps # that use iOS frameworks. Treat it like iOS for now. if name == "maccatalyst": - name = "ios" + fail("Unsupported swift package manager platform: maccatalyst.") if name == "driverkit": name = "macos" return "@rules_swift_package_manager//config_settings/spm/platform:{}".format(name) +def _is_supported(name): + return name != "maccatalyst" + +def _supported(names): + return [n for n in names if _is_supported(n)] + platforms = struct( macos = "macos", maccatalyst = "maccatalyst", @@ -86,4 +89,6 @@ platforms = struct( all_values = [pi.spm for pi in _PLATFORM_INFOS], all_platform_infos = _PLATFORM_INFOS, label = _label, + is_supported = _is_supported, + supported = _supported, ) diff --git a/config_settings/spm/tests/platform_tests/BUILD.bazel b/config_settings/spm/tests/platform_tests/BUILD.bazel new file mode 100644 index 000000000..6726e076a --- /dev/null +++ b/config_settings/spm/tests/platform_tests/BUILD.bazel @@ -0,0 +1,6 @@ +load("@cgrindel_bazel_starlib//bzlformat:defs.bzl", "bzlformat_pkg") +load(":platforms_tests.bzl", "platforms_test_suite") + +bzlformat_pkg(name = "bzlformat") + +platforms_test_suite() diff --git a/config_settings/spm/tests/platform_tests/platforms_tests.bzl b/config_settings/spm/tests/platform_tests/platforms_tests.bzl new file mode 100644 index 000000000..3f7607296 --- /dev/null +++ b/config_settings/spm/tests/platform_tests/platforms_tests.bzl @@ -0,0 +1,57 @@ +"""Tests for `platforms` module.""" + +load("@bazel_skylib//lib:unittest.bzl", "asserts", "unittest") +load("//config_settings/spm/platform:platforms.bzl", "platforms") + +def _is_supported_test(ctx): + env = unittest.begin(ctx) + + tests = [ + struct(name = "maccatalyst", exp = False), + ] + [ + struct(name = name, exp = True) + for name in platforms.all_values + ] + for t in tests: + actual = platforms.is_supported(t.name) + msg = getattr(t, "msg", t.name) + asserts.equals(env, t.exp, actual, msg) + + return unittest.end(env) + +is_supported_test = unittest.make(_is_supported_test) + +def _supported_test(ctx): + env = unittest.begin(ctx) + + tests = [ + struct( + msg = "all valid names", + names = platforms.all_values, + exp = platforms.all_values, + ), + struct( + msg = "some invalid names", + names = [platforms.ios, platforms.maccatalyst, platforms.macos], + exp = [platforms.ios, platforms.macos], + ), + struct( + msg = "only invalid names", + names = [platforms.maccatalyst], + exp = [], + ), + ] + for t in tests: + actual = platforms.supported(t.names) + asserts.equals(env, t.exp, actual, t.msg) + + return unittest.end(env) + +supported_test = unittest.make(_supported_test) + +def platforms_test_suite(name = "platforms_tests"): + return unittest.suite( + name, + is_supported_test, + supported_test, + ) diff --git a/examples/firebase_example/MODULE.bazel.lock b/examples/firebase_example/MODULE.bazel.lock index 69b2b4117..327b54c5b 100644 --- a/examples/firebase_example/MODULE.bazel.lock +++ b/examples/firebase_example/MODULE.bazel.lock @@ -3870,9 +3870,9 @@ }, "@@rules_swift_package_manager~override//:extensions.bzl%swift_deps": { "general": { - "bzlTransitiveDigest": "a13Oc0hXT+L5cvojiIZIfzIFXt+8RemLnU6Rf/kkLXI=", + "bzlTransitiveDigest": "t/yVS4ZpkoDsgehxpfo1Yf+75r2Ca2oamGJRN5b3wMs=", "accumulatedFileDigests": { - "@@//:swift_deps_index.json": "f1aecd2c87eee351e25949d68bbcd3dfd375e06f44c4e81c2b2a193ecb17e9d2" + "@@//:swift_deps_index.json": "33407860e1c821371bc07952746aee8e1db5cbdfc53a5804fd2abf0da6877cc9" }, "envVariables": {}, "generatedRepoSpecs": { @@ -3982,7 +3982,7 @@ "attributes": { "name": "rules_swift_package_manager~override~swift_deps~swiftpkg_googledatatransport", "bazel_package_name": "swiftpkg_googledatatransport", - "commit": "5056b15c5acbb90cd214fe4d6138bdf5a740e5a8", + "commit": "a732a4b47f59e4f725a2ea10f0c77e93a7131117", "remote": "https://github.com/google/GoogleDataTransport.git", "dependencies_index": "@@//:swift_deps_index.json", "init_submodules": false, @@ -4002,7 +4002,7 @@ "attributes": { "name": "rules_swift_package_manager~override~swift_deps~swiftpkg_firebase_ios_sdk", "bazel_package_name": "swiftpkg_firebase_ios_sdk", - "commit": "5de0369ee79ad096c164eb3afeb7921d92a43b58", + "commit": "c60c958e707c50a9cf8bcb7cfd7d51c566d726c5", "remote": "https://github.com/firebase/firebase-ios-sdk", "dependencies_index": "@@//:swift_deps_index.json", "init_submodules": false, diff --git a/swiftpkg/internal/bzl_selects.bzl b/swiftpkg/internal/bzl_selects.bzl index 4732ece0a..dad167f33 100644 --- a/swiftpkg/internal/bzl_selects.bzl +++ b/swiftpkg/internal/bzl_selects.bzl @@ -62,20 +62,19 @@ def _new_from_build_setting(build_setting, values_map_fn = None): for v in values ] - platforms_len = len(bsc.platforms) + supported_platforms = spm_platforms.supported(bsc.platforms) + platforms_len = len(supported_platforms) if platforms_len > 0 and bsc.configuration != None: conditions = [ spm_platform_configurations.label(p, bsc.configuration) - for p in bsc.platforms + for p in supported_platforms ] elif platforms_len > 0: - conditions = [spm_platforms.label(p) for p in bsc.platforms] + conditions = [spm_platforms.label(p) for p in supported_platforms] elif bsc.configuration != None: conditions = [spm_configurations.label(bsc.configuration)] else: - fail("""\ -Found a build setting condition that had no platforms or a configuration. {}\ -""".format(build_setting)) + return [] return [ _new(kind = build_setting.kind, value = v, condition = c) @@ -97,13 +96,12 @@ def _new_from_target_dependency_condition(kind, labels, condition = None): """ if condition == None: return [_new(kind = kind, value = labels)] - platforms_len = len(condition.platforms) - if platforms_len > 0: - conditions = [spm_platforms.label(p) for p in condition.platforms] - else: - fail("""\ -Found a target dependency condition that had no platforms. {}\ -""".format(condition)) + + conditions = [ + spm_platforms.label(p) + for p in spm_platforms.supported(condition.platforms) + ] + return [ _new(kind = kind, value = labels, condition = c) for c in conditions diff --git a/swiftpkg/internal/pkginfos.bzl b/swiftpkg/internal/pkginfos.bzl index 4f8232aaf..1e97c1203 100644 --- a/swiftpkg/internal/pkginfos.bzl +++ b/swiftpkg/internal/pkginfos.bzl @@ -1157,13 +1157,7 @@ def _new_build_setting_condition(platforms = [], configuration = None): if platforms == [] and configuration == None: return None - for platform in platforms: - validations.in_list( - spm_platforms.all_values, - platform, - "Unrecognized platform. platform:", - ) - + platforms = spm_platforms.supported(platforms) if configuration != None: validations.in_list( spm_configurations.all_values,