-
Notifications
You must be signed in to change notification settings - Fork 283
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
build: partially repair cross-repository testing #1826
Conversation
When doing cross-repository testing, we need to ensure that we use the proper set of dependencies. With a change to IndexStoreDB relying on swift-lmdb, we uncovered this latent issue which filtered out the dependency and resulted in undefined symbol references.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume that you’re seeing missing symbols from swift-lmdb? If so, I don’t think this is the right approach because it re-builds modules that have already been built using CMake. You should add the appropriate search paths for swift-lmdb to https://github.com/swiftlang/swift/blob/8bf940614627a58ca708ce2639220c5e59172a69/utils/build.ps1#L2536
However, that wouldn't allow you the necessary control per se. You need to manually manage every single dependency per target as the C/C++ dependencies will not flow (unlikely Swift where the autolinking would allow you do that). |
Yes, that is the downside of the current design. As I see it we have two ways to go:
I don’t think that we should do some mixed in-between way of re-using some dependencies from the CMake build and re-building other dependencies. I believe that (1) is the better approach for now because it avoids building code twice, reducing CI times and tests the object files that are going to be used in the toolchain, aligning the tests more closely with what we really ship. I agree that having to manage the list of C/C++ targets that we link against is annoying but I think it’s the best option we have right now. |
The problem is that the approach you are proposing makes it impossible to use SPM plugins, macros, and will overlink and potentially incorrectly build binaries. But, if that is the preferred solution and is what is needed to enable this for now, I'll go ahead and make that change. |
I agree that it’s not a perfect solution but I think given the state that we’re currently in, it’s the best option we have. |
How about we fix SwiftPM so these issues don't happen :). My goal is to never need CMake to build Swift projects. You may need a work around today, but we need to make sure we save investment to clean things up once we don't need them. |
When doing cross-repository testing, we need to ensure that we use the proper set of dependencies. With a change to IndexStoreDB relying on swift-lmdb, we uncovered this latent issue which filtered out the dependency and resulted in undefined symbol references.