Skip to content

Commit

Permalink
Fix Darwin handling of provisional fields in non-provisional commands…
Browse files Browse the repository at this point in the history
…/structs. (#30279)

We want to make sure that our framework-internal code knows about provisional
things whether the API client enables them or not.

The xcodebuild bit in the darwin workflow that was testing that
MTR_ENABLE_PROVISIONAL compiled is now redundant.

I verified that trying to use provisional things in the XCTest unit tests fails
to compile unless MTR_ENABLE_PROVISIONAL=1 is explicitly used for the
"xcodebuild test" command.
  • Loading branch information
bzbarsky-apple authored and pull[bot] committed Feb 3, 2024
1 parent acac2d5 commit c66b537
Show file tree
Hide file tree
Showing 8 changed files with 2 additions and 515 deletions.
2 changes: 0 additions & 2 deletions .github/workflows/darwin.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,6 @@ jobs:
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 off, so we test that it does not break for now.
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=0' > >(tee /tmp/darwin/framework-tests/darwin-tests-asan-controller-storage.log) 2> >(tee /tmp/darwin/framework-tests/darwin-tests-asan-controller-storage-err.log >&2)
# And the same thing, but with MTR_ENABLE_PROVISIONAL also 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_NO_AVAILABILITY=1 MTR_PER_CONTROLLER_STORAGE_ENABLED=1 MTR_ENABLE_PROVISIONAL=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,
Expand Down
6 changes: 0 additions & 6 deletions examples/darwin-framework-tool/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -113,12 +113,6 @@ action("build-darwin-framework") {
args += [ "--no-enable-encoding-sentinel-enum-values" ]
}

if (enable_provisional_features) {
args += [ "--enable-provisional-framework-features" ]
} else {
args += [ "--no-enable-provisional-framework-features" ]
}

output_name = "Matter.framework"
outputs = [
"${root_out_dir}/macos_framework_output/Build/Products/${output_sdk_type}/${output_name}",
Expand Down
3 changes: 0 additions & 3 deletions scripts/build/build_darwin_framework.py
Original file line number Diff line number Diff line change
Expand Up @@ -97,8 +97,6 @@ def build_darwin_framework(args):
command += ["{}={}".format(option, "YES" if options[option] else "NO")]

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

command += [defines]

Expand Down Expand Up @@ -174,7 +172,6 @@ def build_darwin_framework(args):
parser.add_argument('--ble', action=argparse.BooleanOptionalAction)
parser.add_argument('--clang', action=argparse.BooleanOptionalAction)
parser.add_argument('--enable-encoding-sentinel-enum-values', action=argparse.BooleanOptionalAction)
parser.add_argument('--enable-provisional-framework-features', action=argparse.BooleanOptionalAction)

args = parser.parse_args()
build_darwin_framework(args)
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,6 @@ static id _Nullable DecodeGlobalAttributeValue(AttributeId aAttributeId, TLV::TL
{{#zcl_attributes_server removeKeys='isOptional'}}
{{#unless clusterRef}}
{{#if (isSupported "" globalAttribute=(asUpperCamelCase name preserveAcronyms=true))}}
{{#if (isProvisional "" globalAttribute=(asUpperCamelCase name preserveAcronyms=true))}}
#if MTR_ENABLE_PROVISIONAL
{{/if}}
case Attributes::{{asUpperCamelCase name}}::Id: {
using TypeInfo = Attributes::{{asUpperCamelCase name}}::TypeInfo;
TypeInfo::DecodableType cppValue;
Expand All @@ -39,9 +36,6 @@ static id _Nullable DecodeGlobalAttributeValue(AttributeId aAttributeId, TLV::TL
{{>decode_value target="value" source="cppValue" cluster="" errorCode="*aError = err; return nil;" depth=0}}
return value;
}
{{#if (isProvisional "" globalAttribute=(asUpperCamelCase name preserveAcronyms=true))}}
#endif // MTR_ENABLE_PROVISIONAL
{{/if}}
{{/if}}
{{/unless}}
{{/zcl_attributes_server}}
Expand All @@ -63,9 +57,6 @@ static id _Nullable DecodeAttributeValueFor{{asUpperCamelCase name preserveAcron
{{#zcl_attributes_server removeKeys='isOptional'}}
{{#if clusterRef}}
{{#if (isSupported (asUpperCamelCase ../name preserveAcronyms=true) attribute=(asUpperCamelCase name preserveAcronyms=true))}}
{{#if (isProvisional (asUpperCamelCase ../name preserveAcronyms=true) attribute=(asUpperCamelCase name preserveAcronyms=true))}}
#if MTR_ENABLE_PROVISIONAL
{{/if}}
case Attributes::{{asUpperCamelCase name}}::Id: {
using TypeInfo = Attributes::{{asUpperCamelCase name}}::TypeInfo;
TypeInfo::DecodableType cppValue;
Expand All @@ -78,9 +69,6 @@ static id _Nullable DecodeAttributeValueFor{{asUpperCamelCase name preserveAcron
{{>decode_value target="value" source="cppValue" cluster=parent.name errorCode="*aError = err; return nil;" depth=0}}
return value;
}
{{#if (isProvisional (asUpperCamelCase ../name preserveAcronyms=true) attribute=(asUpperCamelCase name preserveAcronyms=true))}}
#endif // MTR_ENABLE_PROVISIONAL
{{/if}}
{{/if}}
{{/if}}
{{/zcl_attributes_server}}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,6 @@ static id _Nullable DecodeEventPayloadFor{{asUpperCamelCase name preserveAcronym
switch (aEventId) {
{{#zcl_events}}
{{#if (isSupported (asUpperCamelCase ../name preserveAcronyms=true) event=(asUpperCamelCase name preserveAcronyms=true))}}
{{#if (isProvisional (asUpperCamelCase ../name preserveAcronyms=true) event=(asUpperCamelCase name preserveAcronyms=true))}}
#if MTR_ENABLE_PROVISIONAL
{{/if}}
case Events::{{asUpperCamelCase name}}::Id: {
Events::{{asUpperCamelCase name}}::DecodableType cppValue;
*aError = DataModel::Decode(aReader, cppValue);
Expand All @@ -52,9 +49,6 @@ static id _Nullable DecodeEventPayloadFor{{asUpperCamelCase name preserveAcronym

return value;
}
{{#if (isProvisional (asUpperCamelCase ../name preserveAcronyms=true) event=(asUpperCamelCase name preserveAcronyms=true))}}
#endif // MTR_ENABLE_PROVISIONAL
{{/if}}
{{/if}}
{{/zcl_events}}
default: {
Expand Down
Loading

0 comments on commit c66b537

Please sign in to comment.