diagnose-api-breaking-changes is incorrectly reporting "No breaking changes detected"

[epau]

Description

Running swift package diagnose-api-breaking-changes fails to detect API breaking changes.

Working within this package, no matter what changes I make, diagnose-api-breaking-changes never reports any breaking changes.

When I test the the same types of changes (such as removing an enum case) in a new Package, diagnose-api-breaking-changes works as expected.

Expected behavior

diagnose-api-breaking-changes will report breaking changes if breaking changes exist, otherwise it will report "No breaking changes detected"

Actual behavior

diagnose-api-breaking-changes is always reporting "No breaking changes detected" even though breaking changes do exist

Steps to reproduce

Given

• This baseline branch
• This branch with breaking change

1. Checkout the branch with breaking change
2. Run swift package diagnose-api-breaking-changes epau/sample/api-breakage-test-main
3. Observe the output No breaking changes detected in ClientRuntime

Swift Package Manager version/commit hash

Swift Package Manager - Swift 5.7.0

Swift & OS version (output of swift --version && uname -a)

swift-driver version: 1.62.8 Apple Swift version 5.7 (swiftlang-5.7.0.127.4 clang-1400.0.29.50) Target: arm64-apple-macosx12.0 Darwin c889f3be185c 21.6.0 Darwin Kernel Version 21.6.0: Thu Sep 29 20:13:56 PDT 2022; root:xnu-8020.240.7~1/RELEASE_ARM64_T6000 arm64

[neonichu]

Looks like the generated JSON is devoid of any symbols here (tool_arguments intentionally removed):

{
  "ABIRoot": {
    "kind": "Root",
    "name": "TopLevel",
    "printedName": "TopLevel",
    "json_format_version": 8,
    "tool_arguments": [ ...
    ]
  }
}

[neonichu]

I see this when manually executing the command SwiftPM runs:

<module-includes>:1:9: note: in file included from <module-includes>:1:
#import "/Users/neonacho/Desktop/smithy-swift/.build/checkouts/aws-crt-swift/aws-common-runtime/aws-c-common/include/aws/common/allocator.h"
        ^
/Users/neonacho/Desktop/smithy-swift/.build/checkouts/aws-crt-swift/aws-common-runtime/aws-c-common/include/aws/common/allocator.h:8:10: error: 'aws/common/macros.h' file not found
#include <aws/common/macros.h>
         ^
<unknown>:0: error: could not build Objective-C module 'AwsCCommon'
<module-includes>:1:9: note: in file included from <module-includes>:1:
#import "/Users/neonacho/Desktop/smithy-swift/.build/checkouts/aws-crt-swift/aws-common-runtime/aws-c-auth/include/aws/auth/auth.h"
        ^
Failed to load module: SmithyTestUtil

[neonichu]

So it seems like there are at least two issues here:

• we are missing search paths or other flags from the invocation which leads to the errors
• we are not displaying the errors when running the tool, leading to the false negative results

[neonichu]

Doesn't seem to me as if BuildPlan.createAPIToolCommonArgs() is correct, e.g. it is using targetDescription.moduleMap?.parentDirectory as the way to compute the include directory. It seems to me as if that method should be taking into account many more parameters of the underlying target description, e.g. also any extra flags configured in the package manifest.

[neonichu]

In terms of error reporting, it looks like swift-api-digester still returns zero in this case, might be right since it is still technically producing the file, but it makes the integration tricky. We're also not properly capturing stderr here, but that doesn't matter much since we anyway wouldn't display it.

Barring a change from swift-api-digester itself, I believe we would need to check whether the produced JSON contains any symbols and if not assume a failure and print the captured stderr.

[neonichu]

https://github.com/apple/swift-package-manager/pull/5891 should improve the situation here somewhat and fail the command in the given scenario.