Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add include dir from a clang module into the build args in diagnose-api-breaking-changes command. #8209

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

yyvch
Copy link
Contributor

@yyvch yyvch commented Jan 10, 2025

Fixes #8073: Update include paths for clang module in diagnose-api-breaking-changes command.

Motivation:

Swift package manager's command diagnose-api-breaking-changes is a wrapper on top of swift-api-digester. Purpose of swift-api-digester is to detect API incompatible changes between two revisions of code. diagnose-api-breaking-changes makes it more convenient to use with packages and does a few things under the hood:

  • it copies git repo into temp dir and checks out its working dir to "baseline" commit
  • it calls swift-api-digester to compare current package to the baseline commit
  • it calls swift-api-digester with necessary build flags determined by toolchain, configuration and environment

@PeterAdams-A reported in #8073 that swift-api-digester failed to build a baseline commit for a specific package with vendored C library. This PR resolves the issue.

Modifications:

Turns out, diagnose-api-breaking-changes was missing some include paths which were necessary for the package to be built. The PR corrects that and adds a new include path for CLang targets. The build arguments are used only by swift-api-digester and do not impact other commands. I distilled the failing scenario to a test case and added it to APIDiff tests.

Modifications:

  • Each clang module has public include dir. When building current module then it typically works as-is, but when a module has non-standard source location, then explicit include of include is necessary.
  • A new test to make sure diagnose-api-breaking-changes can check API compatibility for a package with non-default source location and include files.

Result:

Each clang module has public `include` dir. When building current module
then it typically works as-is, but when a module has non-standard
source location, then explicit include of `include` is necessary.
@yyvch
Copy link
Contributor Author

yyvch commented Jan 15, 2025

@swift-ci please test

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

diagnose-api-breaking-changes does not understand submodules
2 participants