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

[cmake] Build swift-testing with CMake. #387

Merged
merged 14 commits into from
May 20, 2024

Conversation

jeffdav
Copy link
Contributor

@jeffdav jeffdav commented Apr 30, 2024

Add files needed to build swift-testing with CMake.

Motivation:

Being able to build with CMake is useful in many scenarios. The immediate need is to bring swift-testing to Windows. To do so we need to support a non-SPM path to allow building targets that would otherwise hit the export symbol limit on Windows.

Modifications:

  • Added CMake files to build Testing, TestingInternal, and TestingMacros.
  • Build TestingMacros as an ExternalProject targeting the build machine.
  • Add SwiftTesting_GenerateDerivedSources which generates a runner-swift-testing.swift and returns a variable that allows consumers to compile it with their sources.

Result:

  • Can build swift-testing with CMake. Example command lines:
cmake -S . -B b -G Ninja -D CMAKE_CXX_COMPILER=clang
cmake --build .\b
  • Example diff that converts swift-webdriver to use swift-testing via CMake.

Checklist:

  • Code and documentation should follow the style of the Style Guide.
  • If public symbols are renamed or modified, DocC references should be updated.

@grynspan grynspan added enhancement New feature or request tools integration Integration of swift-testing into tools/IDEs labels Apr 30, 2024
cmake/Runner.cmake Outdated Show resolved Hide resolved
CMakeLists.txt Outdated Show resolved Hide resolved
cmake/Runner.cmake Outdated Show resolved Hide resolved
cmake/Settings.cmake Outdated Show resolved Hide resolved
cmake/Settings.cmake Outdated Show resolved Hide resolved
Copy link
Contributor

@grynspan grynspan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for opening this PR! There's still some work to be done, but this is a great start.

Sources/TestingMacros/CMakeLists.txt Show resolved Hide resolved
CMakeLists.txt Outdated Show resolved Hide resolved
cmake/Runner.cmake Outdated Show resolved Hide resolved
Sources/TestingMacros/CMakeLists.txt Show resolved Hide resolved
Sources/TestingMacros/CMakeLists.txt Show resolved Hide resolved
cmake/Settings.cmake Outdated Show resolved Hide resolved
@stmontgomery
Copy link
Contributor

Thank you @jeffdav! Appreciate the helpful contribution. I have several comments so far and will take a second pass tomorrow

@grynspan
Copy link
Contributor

grynspan commented May 1, 2024

Open question: how do we test this change?

@jeffdav jeffdav force-pushed the jeff/cmake branch 5 times, most recently from e51a1a8 to 9e6e6d9 Compare May 1, 2024 21:18
cmake/Settings.cmake Outdated Show resolved Hide resolved
Copy link
Member

@etcwilde etcwilde left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is looking good. I think disabling C++ exceptions on _TestingInternal will help make the experience a little smoother, but beyond that, it seems to be working as I would expect. Install story can come later.

Sources/_TestingInternals/CMakeLists.txt Show resolved Hide resolved
Copy link
Member

@etcwilde etcwilde left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jeffdav
Copy link
Contributor Author

jeffdav commented May 16, 2024

@grynspan any further concerns on your side?

@grynspan
Copy link
Contributor

Nope, I defer to @etcwilde here.

(FYI we don't use Swift's C++ interop feature, so its lack of support for exceptions doesn't make a difference to us. However, we also don't throw any exceptions or intend to catch any, so it's moot.)

@grynspan
Copy link
Contributor

Oh—please update the file lists before merging. There are a number of new files added since you opened this PR.

@jeffdav
Copy link
Contributor Author

jeffdav commented May 16, 2024

Oh—please update the file lists before merging. There are a number of new files added since you opened this PR.

Done!

Copy link
Contributor

@grynspan grynspan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@grynspan
Copy link
Contributor

@swift-ci please test

@grynspan
Copy link
Contributor

@jeffdav I know you folks obviously work with Windows. The Windows failure we're seeing is a compiler bug unrelated to your PR. If you're comfortable with merging the PR despite the failure, we can proceed. (I'm unsure if you can see a merge button—if not, ping me and I'll do it.)

@jeffdav
Copy link
Contributor Author

jeffdav commented May 19, 2024

@jeffdav I know you folks obviously work with Windows. The Windows failure we're seeing is a compiler bug unrelated to your PR. If you're comfortable with merging the PR despite the failure, we can proceed. (I'm unsure if you can see a merge button—if not, ping me and I'll do it.)

Yeah, I'm familiar with that compiler bug for other reasons. :)

@jeffdav
Copy link
Contributor Author

jeffdav commented May 20, 2024

@grynspan Thanks for your help! I don't have a merge button.

@grynspan
Copy link
Contributor

@swift-ci please test

@grynspan grynspan merged commit a12f93e into apple:main May 20, 2024
2 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request tools integration Integration of swift-testing into tools/IDEs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants