Skip to content

Conversation

@compnerd
Copy link
Member

Since the SQLite3 import is marked as @_implementationOnly this is
perfectly reasonable to assume that the symbols from SQLite3 should not
be required by consumers of this library and thus the consumers of this
library do not need to link against SQLite3 themselves. Reflect this in
the build. This is needed to ensure that the dependency handling works
properly for building swift-driver.

@compnerd
Copy link
Member Author

CC: @tomerd @neonichu @abertelrud

@compnerd
Copy link
Member Author

@swift-ci please test

Copy link
Contributor

@tomerd tomerd left a comment

Choose a reason for hiding this comment

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

not sure where the implications of these are

@compnerd
Copy link
Member Author

It basically means that users of TSCBasic will no longer implicitly link against the SQLite library if they are using the imported target via a CMake dependency tree. In practice what this is amounting to is support for building swift-driver on Windows without having to introduce a SQLite dependency in that package through this dependency being leaked into the users of the library.

@tomerd
Copy link
Contributor

tomerd commented Aug 10, 2020

I'll defer to @abertelrud and @neonichu on this one

Since the SQLite3 import is marked as `@_implementationOnly` this is
perfectly reasonable to assume that the symbols from SQLite3 should not
be required by consumers of this library and thus the consumers of this
library do not need to link against SQLite3 themselves.  Reflect this in
the build.  This is needed to ensure that the dependency handling works
properly for building swift-driver.
@compnerd
Copy link
Member Author

@swift-ci please test

@neonichu neonichu merged commit 0af23ed into swiftlang:master Aug 18, 2020
@neonichu
Copy link
Contributor

I actually need this for something I'm working on, too. Thanks @compnerd!

@compnerd compnerd deleted the databases branch August 18, 2020 21:00
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.

3 participants