Skip to content

Add swift-crypto dependency #3202

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

Merged
merged 4 commits into from
Feb 5, 2021
Merged

Add swift-crypto dependency #3202

merged 4 commits into from
Feb 5, 2021

Conversation

yim-lee
Copy link
Contributor

@yim-lee yim-lee commented Jan 16, 2021

Motivation:
Upcoming package collections feature will require swift-crypto dependency.

Modifications:

  • Add swift-crypto dependency
  • Bump minimum macOS version to 10.15, for it's required by swift-crypto

@yim-lee
Copy link
Contributor Author

yim-lee commented Jan 16, 2021

@swift-ci please smoke test

1 similar comment
@yim-lee
Copy link
Contributor Author

yim-lee commented Jan 17, 2021

@swift-ci please smoke test

@yim-lee
Copy link
Contributor Author

yim-lee commented Jan 17, 2021

Does this mean we need to add CMakeLists files to swift-crypto?

11:03:28 -- Check for working Swift compiler: /home/buildnode/jenkins/workspace/swift-package-manager-Linux-smoke-test/branch-main/build/buildbot_incremental/toolchain-linux-x86_64/usr/bin/swiftc
11:03:29 -- Check for working Swift compiler: /home/buildnode/jenkins/workspace/swift-package-manager-Linux-smoke-test/branch-main/build/buildbot_incremental/toolchain-linux-x86_64/usr/bin/swiftc -- works
11:03:29 CMake Error at CMakeLists.txt:50 (find_package):
11:03:29   Could not find a package configuration file provided by "Crypto" with any
11:03:29   of the following names:
11:03:29 
11:03:29     CryptoConfig.cmake
11:03:29     crypto-config.cmake
11:03:29 
11:03:29   Add the installation prefix of "Crypto" to CMAKE_PREFIX_PATH or set
11:03:29   "Crypto_DIR" to a directory containing one of the above files.  If "Crypto"
11:03:29   provides a separate development package or SDK, be sure it has been
11:03:29   installed.

@abertelrud
Copy link
Contributor

Does this mean we need to add CMakeLists files to swift-crypto?

11:03:28 -- Check for working Swift compiler: /home/buildnode/jenkins/workspace/swift-package-manager-Linux-smoke-test/branch-main/build/buildbot_incremental/toolchain-linux-x86_64/usr/bin/swiftc
11:03:29 -- Check for working Swift compiler: /home/buildnode/jenkins/workspace/swift-package-manager-Linux-smoke-test/branch-main/build/buildbot_incremental/toolchain-linux-x86_64/usr/bin/swiftc -- works
11:03:29 CMake Error at CMakeLists.txt:50 (find_package):
11:03:29   Could not find a package configuration file provided by "Crypto" with any
11:03:29   of the following names:
11:03:29 
11:03:29     CryptoConfig.cmake
11:03:29     crypto-config.cmake
11:03:29 
11:03:29   Add the installation prefix of "Crypto" to CMAKE_PREFIX_PATH or set
11:03:29   "Crypto_DIR" to a directory containing one of the above files.  If "Crypto"
11:03:29   provides a separate development package or SDK, be sure it has been
11:03:29   installed.

Right, it looks as if swift-crypto isn't able to build with CMake. But is it needed for bootstrapping? CMake is only used in order to get a swift-build that can build itself. Would it be too convoluted to factor the dependency on swift-crypto so it isn't needed by swift-build itself?

@tomerd
Copy link
Contributor

tomerd commented Jan 19, 2021

Right, it looks as if swift-crypto isn't able to build with CMake. But is it needed for bootstrapping? CMake is only used in order to get a swift-build that can build itself

I dont know that that is true for Windows (tho would be nice to have minimal pre step) @compnerd ?

@neonichu neonichu self-assigned this Jan 19, 2021
yim-lee added a commit to yim-lee/swift-crypto that referenced this pull request Jan 20, 2021
Motivation:
This is to support swiftlang/swift-package-manager#3202, which adds swift-crypto as a dependency to SwiftPM.

Modifications:
Add CMake files.

Result:
Can build swift-crypto with cmake.
yim-lee added a commit to yim-lee/swift-crypto that referenced this pull request Jan 20, 2021
Motivation:
This is to support swiftlang/swift-package-manager#3202, which adds swift-crypto as a dependency to SwiftPM.

Modifications:
Add CMake files.

Result:
Can build swift-crypto with cmake.
@tomerd tomerd added the next waiting for next merge window label Jan 20, 2021
@finagolfin
Copy link
Member

Unless you plan on using these new crypto features when building SPM with itself, which shouldn't be necessary since build-script should have already checked out SPM's dependencies, I think it's fine to not add this swift-crypto dependency for the CMake bootstrap and avoid adding a CMake config for swift-crypto.

@compnerd
Copy link
Member

Windows definitely relies on the CMake build to get a fully functional SPM. The manifest execution is a problem - there is no concept of rpath, and static linking for Swift modules is unsupported. We currently are reliant on CMake for the complete build.

compnerd pushed a commit to compnerd/swift-crypto that referenced this pull request Jan 22, 2021
Motivation:
This is to support swiftlang/swift-package-manager#3202, which adds swift-crypto as a dependency to SwiftPM.

Modifications:
Add CMake files.

Result:
Can build swift-crypto with cmake.
compnerd pushed a commit to compnerd/swift-crypto that referenced this pull request Jan 22, 2021
Motivation:
This is to support swiftlang/swift-package-manager#3202, which adds swift-crypto as a dependency to SwiftPM.

Modifications:
Add CMake files.

Result:
Can build swift-crypto with cmake.
@finagolfin
Copy link
Member

Alright, I didn't know you weren't building SPM with SPM on Windows and are using the CMake-configured build of SPM instead on that platform.

compnerd pushed a commit to compnerd/swift-crypto that referenced this pull request Jan 22, 2021
Motivation:
This is to support swiftlang/swift-package-manager#3202, which adds swift-crypto as a dependency to SwiftPM.

Modifications:
Add CMake files.

Result:
Can build swift-crypto with cmake.
@neonichu
Copy link
Contributor

@yim-lee BTW, if we land swiftlang/swift#35507, we could use cross-repository testing together with the swift-crypto PR -- might be worth it to get a fully end-to-end test on CI before we land the changes in swift-crypto? Then maybe pinning a version in update-checkout could be a follow-up PR.

yim-lee added a commit to yim-lee/swift-crypto that referenced this pull request Jan 23, 2021
Motivation:
This is to support swiftlang/swift-package-manager#3202, which adds swift-crypto as a dependency to SwiftPM.

Modifications:
Add CMake files.

Result:
Can build swift-crypto with cmake.
yim-lee added a commit to yim-lee/swift-crypto that referenced this pull request Jan 26, 2021
Motivation:
This is to support swiftlang/swift-package-manager#3202, which adds swift-crypto as a dependency to SwiftPM.

Modifications:
Add CMake files.

Result:
Can build swift-crypto with cmake.
yim-lee added a commit to yim-lee/swift-crypto that referenced this pull request Jan 26, 2021
Motivation:
This is to support swiftlang/swift-package-manager#3202, which adds swift-crypto as a dependency to SwiftPM.

Modifications:
Add CMake files.

Result:
Can build swift-crypto with cmake.
Lukasa pushed a commit to apple/swift-crypto that referenced this pull request Jan 27, 2021
Motivation:
This is to support swiftlang/swift-package-manager#3202, which adds swift-crypto as a dependency to SwiftPM.

Modifications:
Add CMake files.

Result:
Can build swift-crypto with cmake.

Co-authored-by: Saleem Abdulrasool <[email protected]>
Co-authored-by: Saleem Abdulrasool <[email protected]>
@yim-lee
Copy link
Contributor Author

yim-lee commented Feb 3, 2021

@swift-ci please smoke test

@yim-lee
Copy link
Contributor Author

yim-lee commented Feb 3, 2021

Build error seems unrelated:

/home/buildnode/jenkins/workspace/swift-package-manager-Linux-smoke-test/branch-main/swift-driver/Sources/SwiftDriver/IncrementalCompilation/ModuleDependencyGraph.swift:938:86: error: 'SwiftDeps' is not a member type of class 'SwiftDriver.ModuleDependencyGraph'
20:05:02 extension BidirectionalMap where T1 == TypedVirtualPath, T2 == ModuleDependencyGraph.SwiftDeps {
20:05:02                                                                ~~~~~~~~~~~~~~~~~~~~~ ^
20:05:02 /home/buildnode/jenkins/workspace/swift-package-manager-Linux-smoke-test/branch-main/swift-driver/Sources/SwiftDriver/IncrementalCompilation/ModuleDependencyGraph.swift:20:39: note: 'ModuleDependencyGraph' declared here
20:05:02 /*@_spi(Testing)*/ public final class ModuleDependencyGraph {
20:05:02                                       ^
20:05:02 /home/buildnode/jenkins/workspace/swift-package-manager-Linux-smoke-test/branch-main/swift-driver/Sources/SwiftDriver/IncrementalCompilation/ModuleDependencyGraph.swift:923:7: error: cannot find 'sourceSwiftDepsMap' in scope
20:05:02       sourceSwiftDepsMap.matches(other.sourceSwiftDepsMap),
20:05:02       ^~~~~~~~~~~~~~~~~~

@yim-lee
Copy link
Contributor Author

yim-lee commented Feb 3, 2021

@swift-ci please smoke test

@yim-lee
Copy link
Contributor Author

yim-lee commented Feb 3, 2021

We've moved past the cmake build issues. The "Swift Test macOS Platform (smoke test)" failure is:

22:32:38 error: the library 'SKSwiftPMWorkspace' requires macos 10.10, but depends on the product 'SwiftPM-auto' which requires macos 10.15; consider changing the library 'SKSwiftPMWorkspace' to require macos 10.15 or later, or the product 'SwiftPM-auto' to require macos 10.10 or earlier.

@neonichu I can't find where that is specified actually. Or I guess 10.10 is the default?

yim-lee added a commit to yim-lee/sourcekit-lsp that referenced this pull request Feb 3, 2021
Motivation:
The same requirement is being added to SwiftPM as part of swiftlang/swift-package-manager#3202. Without this change SourceKit-LSP will fail to build with error:

```
the library 'SKSwiftPMWorkspace' requires macos 10.10, but depends on the product 'SwiftPM-auto' which requires macos 10.15; consider changing the library 'SKSwiftPMWorkspace' to require macos 10.15 or later, or the product 'SwiftPM-auto' to require macos 10.10 or earlier.
```

Modification:
Require macOS 10.15.

Result:
SourceKit-LSP builds successfully with changeset
swiftlang/swift-package-manager#3202.
@yim-lee
Copy link
Contributor Author

yim-lee commented Feb 4, 2021

@swift-ci please smoke test

2 similar comments
@yim-lee
Copy link
Contributor Author

yim-lee commented Feb 4, 2021

@swift-ci please smoke test

@yim-lee
Copy link
Contributor Author

yim-lee commented Feb 4, 2021

@swift-ci please smoke test

Motivation:
Package collections feature will require swift-crypto dependency.

Modifications:
- Add swift-crypto dependency
- Bump minimum macOS version to 10.15, for it's required by swift-crypto
@yim-lee
Copy link
Contributor Author

yim-lee commented Feb 4, 2021

@swift-ci please smoke test

@yim-lee
Copy link
Contributor Author

yim-lee commented Feb 5, 2021

🥳 all green!

Copy link
Contributor

@abertelrud abertelrud left a comment

Choose a reason for hiding this comment

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

LGTM

@yim-lee
Copy link
Contributor Author

yim-lee commented Feb 5, 2021

Testing apple/swift-crypto#67

@swift-ci please smoke test

@yim-lee
Copy link
Contributor Author

yim-lee commented Feb 5, 2021

@swift-ci please smoke test

@yim-lee yim-lee merged commit 60d6da6 into swiftlang:main Feb 5, 2021
@yim-lee yim-lee deleted the add-crypto-dep branch February 5, 2021 19:12
@tomerd
Copy link
Contributor

tomerd commented Feb 5, 2021

🥳

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

Successfully merging this pull request may close these issues.

6 participants