Skip to content

Commit

Permalink
Address review comments.
Browse files Browse the repository at this point in the history
  • Loading branch information
bzbarsky-apple committed Sep 22, 2023
1 parent 6479917 commit d809587
Show file tree
Hide file tree
Showing 9 changed files with 28 additions and 45 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/darwin-tests.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ jobs:
#
# Disable availability annotations, since we are not building against a system
# Matter.framework.
run: xcodebuild -target "darwin-framework-tool" -sdk macosx -configuration Debug OTHER_CFLAGS='${inherited} -Wconversion' GCC_PREPROCESSOR_DEFINITIONS='${inherited} MTR_SKIP_AVAILABILITY_ANNOTATIONS=1'
run: xcodebuild -target "darwin-framework-tool" -sdk macosx -configuration Debug OTHER_CFLAGS='${inherited} -Wconversion' GCC_PREPROCESSOR_DEFINITIONS='${inherited} MTR_NO_AVAILABILITY=1'
- name: Delete Defaults
run: defaults delete com.apple.dt.xctest.tool
continue-on-error: true
Expand Down
20 changes: 10 additions & 10 deletions .github/workflows/darwin.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -51,14 +51,14 @@ jobs:
run: scripts/run_in_build_env.sh '(zap-cli --version && exit 1) || exit 0'
- name: Run iOS Build Debug
working-directory: src/darwin/Framework
# Disable availability annotations, since we are not building against a system
# Disable availability annotations, since we are not building a system
# Matter.framework.
run: xcodebuild -target "Matter" -sdk iphoneos -configuration Debug GCC_PREPROCESSOR_DEFINITIONS='${inherited} MTR_SKIP_AVAILABILITY_ANNOTATIONS=1'
run: xcodebuild -target "Matter" -sdk iphoneos -configuration Debug GCC_PREPROCESSOR_DEFINITIONS='${inherited} MTR_NO_AVAILABILITY=1'
- name: Run iOS Build Release
working-directory: src/darwin/Framework
# Disable availability annotations, since we are not building against a system
# Disable availability annotations, since we are not building a system
# Matter.framework.
run: xcodebuild -target "Matter" -sdk iphoneos -configuration Release GCC_PREPROCESSOR_DEFINITIONS='${inherited} MTR_SKIP_AVAILABILITY_ANNOTATIONS=1'
run: xcodebuild -target "Matter" -sdk iphoneos -configuration Release GCC_PREPROCESSOR_DEFINITIONS='${inherited} MTR_NO_AVAILABILITY=1'
- name: Clean Build
run: xcodebuild clean
working-directory: src/darwin/Framework
Expand All @@ -73,9 +73,9 @@ jobs:
# Enable -Wconversion by hand as well, because it seems to not be
# enabled by default in the Xcode config.
#
# Disable availability annotations, since we are not building against a system
# Disable availability annotations, since we are not building a system
# Matter.framework.
run: xcodebuild -target "Matter" -sdk macosx OTHER_CFLAGS='${inherited} -Werror -Wconversion' GCC_PREPROCESSOR_DEFINITIONS='${inherited} MTR_SKIP_AVAILABILITY_ANNOTATIONS=1'
run: xcodebuild -target "Matter" -sdk macosx OTHER_CFLAGS='${inherited} -Werror -Wconversion' GCC_PREPROCESSOR_DEFINITIONS='${inherited} MTR_NO_AVAILABILITY=1'
working-directory: src/darwin/Framework
- name: Clean Build
run: xcodebuild clean
Expand Down Expand Up @@ -110,14 +110,14 @@ jobs:
#
# -enableUndefinedBehaviorSanitizer instruments the code in Matter.framework,
# but to instrument the code in the underlying libCHIP we need to pass CHIP_IS_UBSAN=YES
TEST_RUNNER_ASAN_OPTIONS=__CURRENT_VALUE__:detect_stack_use_after_return=1 xcodebuild test -target "Matter" -scheme "Matter Framework Tests" -sdk macosx -enableAddressSanitizer YES -enableUndefinedBehaviorSanitizer YES OTHER_CFLAGS='${inherited} -Werror -Wconversion' CHIP_IS_UBSAN=YES CHIP_IS_BLE=NO GCC_PREPROCESSOR_DEFINITIONS='${inherited} MTR_SKIP_AVAILABILITY_ANNOTATIONS=1'> >(tee /tmp/darwin/framework-tests/darwin-tests-asan.log) 2> >(tee /tmp/darwin/framework-tests/darwin-tests-asan-err.log >&2)
TEST_RUNNER_ASAN_OPTIONS=__CURRENT_VALUE__:detect_stack_use_after_return=1 xcodebuild test -target "Matter" -scheme "Matter Framework Tests" -sdk macosx -enableAddressSanitizer YES -enableUndefinedBehaviorSanitizer YES OTHER_CFLAGS='${inherited} -Werror -Wconversion' CHIP_IS_UBSAN=YES CHIP_IS_BLE=NO GCC_PREPROCESSOR_DEFINITIONS='${inherited} MTR_NO_AVAILABILITY=1'> >(tee /tmp/darwin/framework-tests/darwin-tests-asan.log) 2> >(tee /tmp/darwin/framework-tests/darwin-tests-asan-err.log >&2)
# And the same thing, but with MTR_PER_CONTROLLER_STORAGE_ENABLED turned on.
TEST_RUNNER_ASAN_OPTIONS=__CURRENT_VALUE__:detect_stack_use_after_return=1 xcodebuild test -target "Matter" -scheme "Matter Framework Tests" -sdk macosx -enableAddressSanitizer YES -enableUndefinedBehaviorSanitizer YES OTHER_CFLAGS='${inherited} -Werror -Wconversion' CHIP_IS_UBSAN=YES CHIP_IS_BLE=NO GCC_PREPROCESSOR_DEFINITIONS='${inherited} MTR_SKIP_AVAILABILITY_ANNOTATIONS=1 MTR_PER_CONTROLLER_STORAGE_ENABLED=1' > >(tee /tmp/darwin/framework-tests/darwin-tests-asan-provisional.log) 2> >(tee /tmp/darwin/framework-tests/darwin-tests-asan-provisional-err.log >&2)
# And the same thing, but with MTR_SKIP_AVAILABILITY_ANNOTATIONS not turned on. This requires -Wno-unguarded-availability-new to avoid availability errors.
TEST_RUNNER_ASAN_OPTIONS=__CURRENT_VALUE__:detect_stack_use_after_return=1 xcodebuild test -target "Matter" -scheme "Matter Framework Tests" -sdk macosx -enableAddressSanitizer YES -enableUndefinedBehaviorSanitizer YES OTHER_CFLAGS='${inherited} -Werror -Wconversion' CHIP_IS_UBSAN=YES CHIP_IS_BLE=NO GCC_PREPROCESSOR_DEFINITIONS='${inherited} MTR_NO_AVAILABILITY=1 MTR_PER_CONTROLLER_STORAGE_ENABLED=1' > >(tee /tmp/darwin/framework-tests/darwin-tests-asan-provisional.log) 2> >(tee /tmp/darwin/framework-tests/darwin-tests-asan-provisional-err.log >&2)
# And the same thing, but with MTR_NO_AVAILABILITY not turned on. This requires -Wno-unguarded-availability-new to avoid availability errors.
TEST_RUNNER_ASAN_OPTIONS=__CURRENT_VALUE__:detect_stack_use_after_return=1 xcodebuild test -target "Matter" -scheme "Matter Framework Tests" -sdk macosx -enableAddressSanitizer YES -enableUndefinedBehaviorSanitizer YES OTHER_CFLAGS='${inherited} -Werror -Wconversion -Wno-unguarded-availability-new' CHIP_IS_UBSAN=YES CHIP_IS_BLE=NO GCC_PREPROCESSOR_DEFINITIONS='${inherited}' > >(tee /tmp/darwin/framework-tests/darwin-tests-asan-with-availability-annotations.log) 2> >(tee /tmp/darwin/framework-tests/darwin-tests-asan-with-availability-annotations-err.log >&2)
# -enableThreadSanitizer instruments the code in Matter.framework,
# but to instrument the code in the underlying libCHIP we need to pass CHIP_IS_TSAN=YES
xcodebuild test -target "Matter" -scheme "Matter Framework Tests" -sdk macosx -enableThreadSanitizer YES OTHER_CFLAGS='${inherited} -Werror -Wconversion' CHIP_IS_TSAN=YES CHIP_IS_BLE=NO GCC_PREPROCESSOR_DEFINITIONS='${inherited} MTR_SKIP_AVAILABILITY_ANNOTATIONS=1' > >(tee /tmp/darwin/framework-tests/darwin-tests-tsan.log) 2> >(tee /tmp/darwin/framework-tests/darwin-tests-tsan-err.log >&2)
xcodebuild test -target "Matter" -scheme "Matter Framework Tests" -sdk macosx -enableThreadSanitizer YES OTHER_CFLAGS='${inherited} -Werror -Wconversion' CHIP_IS_TSAN=YES CHIP_IS_BLE=NO GCC_PREPROCESSOR_DEFINITIONS='${inherited} MTR_NO_AVAILABILITY=1' > >(tee /tmp/darwin/framework-tests/darwin-tests-tsan.log) 2> >(tee /tmp/darwin/framework-tests/darwin-tests-tsan-err.log >&2)
working-directory: src/darwin/Framework
- name: Build Matter TV Casting Bridge
run: |
Expand Down
2 changes: 1 addition & 1 deletion examples/darwin-framework-tool/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ config("config") {

# Disable availability annotations in Matter.framework headers because we
# are not building against a system Matter.framework here anyway.
"MTR_SKIP_AVAILABILITY_ANNOTATIONS=1",
"MTR_NO_AVAILABILITY=1",
]

cflags = [
Expand Down
2 changes: 1 addition & 1 deletion scripts/build/build_darwin_framework.py
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ def build_darwin_framework(args):
for option in options:
command += ["{}={}".format(option, "YES" if options[option] else "NO")]

defines = 'GCC_PREPROCESSOR_DEFINITIONS=${inherited} MTR_SKIP_AVAILABILITY_ANNOTATIONS=1'
defines = 'GCC_PREPROCESSOR_DEFINITIONS=${inherited} MTR_NO_AVAILABILITY=1'
if args.enable_provisional_framework_features:
defines += ' MTR_ENABLE_PROVISIONAL=1'

Expand Down
30 changes: 7 additions & 23 deletions src/darwin/Framework/CHIP/MTRDefines.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,43 +38,27 @@
#pragma mark - Deprecation macros (can be overriden via build system)

/**
* MTR_SKIP_AVAILABILITY_ANNOTATIONS can be used to turn off availability
* MTR_NO_AVAILABILITY can be used to turn off availability
* annotations, to allow compiling a version of Matter.framework for "local
* use", not as a system framework.
*/
#if !defined(MTR_SKIP_AVAILABILITY_ANNOTATIONS)
#define MTR_SKIP_AVAILABILITY_ANNOTATIONS 0
#if !defined(MTR_NO_AVAILABILITY)
#define MTR_NO_AVAILABILITY 0
#endif

#ifndef MTR_DEPRECATED
#if MTR_SKIP_AVAILABILITY_ANNOTATIONS
#if MTR_NO_AVAILABILITY
#define MTR_DEPRECATED(...) MTR_SWIFT_DISFAVORED_OVERLOAD
#else
#define MTR_DEPRECATED(...) API_DEPRECATED(__VA_ARGS__) MTR_SWIFT_DISFAVORED_OVERLOAD
#endif // MTR_SKIP_AVAILABILITY_ANNOTATIONS
#endif // MTR_DEPRECATED

#ifndef MTR_DEPRECATED_WITH_REPLACEMENT
#if MTR_SKIP_AVAILABILITY_ANNOTATIONS
#define MTR_DEPRECATED_WITH_REPLACEMENT(...) MTR_SWIFT_DISFAVORED_OVERLOAD
#else
#define MTR_DEPRECATED_WITH_REPLACEMENT(...) API_DEPRECATED_WITH_REPLACEMENT(__VA_ARGS__) MTR_SWIFT_DISFAVORED_OVERLOAD
#endif // MTR_SKIP_AVAILABILITY_ANNOTATIONS
#endif // MTR_DEPRECATED_WITH_REPLACEMENT

#if MTR_SKIP_AVAILABILITY_ANNOTATIONS
#define MTR_AVAILABLE(...)
#else
#define MTR_DEPRECATED(...) API_DEPRECATED(__VA_ARGS__) MTR_SWIFT_DISFAVORED_OVERLOAD
#define MTR_DEPRECATED_WITH_REPLACEMENT(...) API_DEPRECATED_WITH_REPLACEMENT(__VA_ARGS__) MTR_SWIFT_DISFAVORED_OVERLOAD
#define MTR_AVAILABLE(...) API_AVAILABLE(__VA_ARGS__)
#endif // MTR_SKIP_AVAILABILITY_ANNOTATIONS
#endif // MTR_NO_AVAILABILITY

#ifndef MTR_NEWLY_DEPRECATED
#define MTR_NEWLY_DEPRECATED(message)
#endif

#ifndef MTR_NEWLY_AVAILABLE
#define MTR_NEWLY_AVAILABLE
#endif

#if !defined(MTR_ENABLE_PROVISIONAL)
#define MTR_ENABLE_PROVISIONAL 0
Expand Down
2 changes: 1 addition & 1 deletion src/darwin/Framework/CHIP/MTRDeviceControllerFactory.mm
Original file line number Diff line number Diff line change
Expand Up @@ -671,7 +671,7 @@ - (MTRDeviceController * _Nullable)_startDeviceController:(MTRDeviceController *
// matches a running controller.
auto * controllersCopy = [self getRunningControllers];
for (MTRDeviceController * existing in controllersCopy) {
if (existing != controller && [existing.uniqueIdentifier compare:params.uniqueIdentifier] == NSOrderedSame) {
if (existing != controller && [existing.uniqueIdentifier isEqual:params.uniqueIdentifier]) {
MTR_LOG_ERROR("Already have running controller with uniqueIdentifier %@", existing.uniqueIdentifier);
fabricError = CHIP_ERROR_INVALID_ARGUMENT;
params = nil;
Expand Down
1 change: 1 addition & 0 deletions src/darwin/Framework/CHIP/MTRDeviceController_Internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
#import "MTRDeviceController.h"
#import "MTRDeviceControllerDataStore.h"

#import <Matter/MTRDefines.h>
#import <Matter/MTRDeviceControllerStartupParams.h>
#if MTR_PER_CONTROLLER_STORAGE_ENABLED
#import <Matter/MTRDeviceControllerStorageDelegate.h>
Expand Down
6 changes: 6 additions & 0 deletions src/darwin/Framework/CHIPTests/MTRSwiftPairingTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ struct Constants {
static let timeoutInSeconds : UInt16 = 3
}

// Because we are using things from Matter.framework that are flagged
// as only being available starting with macOS 13.3, we need to flag our
// code with the same availabiluty annotation.
@available(macOS, introduced: 13.3)
class MTRSwiftPairingTestControllerDelegate : NSObject, MTRDeviceControllerDelegate {
let expectation: XCTestExpectation
Expand Down Expand Up @@ -43,6 +46,9 @@ class MTRSwiftPairingTestControllerDelegate : NSObject, MTRDeviceControllerDeleg
}

class MTRSwiftPairingTests : XCTestCase {
// Because we are using things from Matter.framework that are flagged
// as only being available starting with macOS 13.3, we need to flag our
// code with the same availabiluty annotation.
@available(macOS, introduced: 13.3)
func test001_BasicPairing() {
let factory = MTRDeviceControllerFactory.sharedInstance()
Expand Down
8 changes: 0 additions & 8 deletions src/darwin/Framework/Matter.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -2032,8 +2032,6 @@
CHIP_HAVE_CONFIG_H,
"CHIP_ADDRESS_RESOLVE_IMPL_INCLUDE_HEADER=<lib/address_resolve/AddressResolve_DefaultImpl.h>",
"$(inherited)",
"MTR_NEWLY_AVAILABLE=",
"MTR_NEWLY_DEPRECATED(message)=",
"CHIP_CONFIG_SKIP_APP_SPECIFIC_GENERATED_HEADER_INCLUDES=1",
);
HEADER_SEARCH_PATHS = (
Expand All @@ -2054,7 +2052,6 @@
INSTALL_PATH = "$(LOCAL_LIBRARY_DIR)/Frameworks";
IPHONEOS_DEPLOYMENT_TARGET = 13.4;
LIBRARY_SEARCH_PATHS = "$(TEMP_DIR)/out/lib";
MACOSX_DEPLOYMENT_TARGET = 12.0;
OTHER_CFLAGS = "-fmacro-prefix-map=$(SRCROOT)/CHIP/=";
OTHER_LDFLAGS = "";
"OTHER_LDFLAGS[sdk=*]" = (
Expand Down Expand Up @@ -2110,7 +2107,6 @@
"@executable_path/Frameworks",
"@loader_path/Frameworks",
);
MACOSX_DEPLOYMENT_TARGET = 12.0;
PRODUCT_BUNDLE_IDENTIFIER = com.chip.CHIPTests;
PRODUCT_NAME = "$(TARGET_NAME)";
SWIFT_OBJC_BRIDGING_HEADER = "CHIPTests/MatterTests-Bridging-Header.h";
Expand Down Expand Up @@ -2200,8 +2196,6 @@
CHIP_HAVE_CONFIG_H,
"CHIP_ADDRESS_RESOLVE_IMPL_INCLUDE_HEADER=<lib/address_resolve/AddressResolve_DefaultImpl.h>",
"$(inherited)",
"MTR_NEWLY_AVAILABLE=",
"MTR_NEWLY_DEPRECATED(message)=",
"CHIP_CONFIG_SKIP_APP_SPECIFIC_GENERATED_HEADER_INCLUDES=1",
);
HEADER_SEARCH_PATHS = (
Expand All @@ -2222,7 +2216,6 @@
INSTALL_PATH = "$(LOCAL_LIBRARY_DIR)/Frameworks";
IPHONEOS_DEPLOYMENT_TARGET = 13.4;
LIBRARY_SEARCH_PATHS = "$(TEMP_DIR)/out/lib";
MACOSX_DEPLOYMENT_TARGET = 12.0;
OTHER_CFLAGS = "-fmacro-prefix-map=$(SRCROOT)/CHIP/=";
OTHER_LDFLAGS = "";
"OTHER_LDFLAGS[sdk=*]" = (
Expand Down Expand Up @@ -2280,7 +2273,6 @@
"@executable_path/Frameworks",
"@loader_path/Frameworks",
);
MACOSX_DEPLOYMENT_TARGET = 12.0;
PRODUCT_BUNDLE_IDENTIFIER = com.chip.CHIPTests;
PRODUCT_NAME = "$(TARGET_NAME)";
SWIFT_OBJC_BRIDGING_HEADER = "CHIPTests/MatterTests-Bridging-Header.h";
Expand Down

0 comments on commit d809587

Please sign in to comment.