Skip to content

adopt swift-collections #3590

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

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ string(COMPARE EQUAL ${CMAKE_SYSTEM_NAME} Windows CMAKE_INSTALL_DEFAULT)
option(USE_CMAKE_INSTALL
"Install build products using cmake's install() instead of the bootstrap script's install()"
${CMAKE_INSTALL_DEFAULT})

if(BUILD_SHARED_LIBS)
set(CMAKE_POSITION_INDEPENDENT_CODE YES)
endif()
Expand All @@ -53,6 +53,7 @@ if(FIND_PM_DEPS)
find_package(ArgumentParser CONFIG REQUIRED)
find_package(SwiftCrypto CONFIG REQUIRED)
find_package(SwiftDriver CONFIG REQUIRED)
find_package(SwiftCollections CONFIG REQUIRED)
endif()

find_package(dispatch QUIET)
Expand Down
8 changes: 7 additions & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,13 @@ For example, if the latest tag is 1.1.3:
$> git clone https://github.com/apple/swift-crypto --branch 1.1.3
```

5. Clone [swift-collections](https://github.com/apple/swift-collections) beside the SwiftPM directory and check out tag with the [latest version](https://github.com/apple/swift-collections/tags).
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be list number 6, instead of 5


For example, if the latest tag is 0.0.3:
```bash
$> git clone https://github.com/apple/swift-collections --branch 0.0.3
```

Comment on lines +168 to +173
Copy link
Contributor

Choose a reason for hiding this comment

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

Could the command examples be indented by 3 spaces so it's structurally more clear that they're part of the list items?

Also, could argument parser and swift crypto's versions in the commands be updated to 0.4.3 and 1.1.4, to align with what's in Package.swift?

#### Building

```bash
Expand Down Expand Up @@ -374,4 +381,3 @@ $> swift package update
```
Alternatively, if you are using Xcode, you can update to the latest version of all packages:
**Xcode App** > *File* > *Swift Packages* > *Update to Latest Package Versions*

11 changes: 8 additions & 3 deletions Package.swift
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import class Foundation.ProcessInfo


/** SwiftPMDataModel is the subset of SwiftPM product that includes just its data model.
This allowis some clients (such as IDEs) that use SwiftPM's data model but not its build system
This allows some clients (such as IDEs) that use SwiftPM's data model but not its build system
to not have to depend on SwiftDriver, SwiftLLBuild, etc. We should probably have better names here,
though that could break some clients.
*/
Expand All @@ -33,7 +33,7 @@ let swiftPMDataModelProduct = (
]
)

/** The `libSwiftPM` set of interfaces to programatically work with Swift
/** The `libSwiftPM` set of interfaces to programmatically work with Swift
packages. `libSwiftPM` includes all of the SwiftPM code except the
command line tools, while `libSwiftPMDataModel` includes only the data model.

Expand Down Expand Up @@ -126,7 +126,10 @@ let package = Package(

.target(
name: "Basics",
dependencies: ["SwiftToolsSupport-auto"]),
dependencies: [
"SwiftToolsSupport-auto",
.product(name: "OrderedCollections", package: "swift-collections")
]),

.target(
/** The llbuild manifest model */
Expand Down Expand Up @@ -356,12 +359,14 @@ if ProcessInfo.processInfo.environment["SWIFTCI_USE_LOCAL_DEPS"] == nil {
.package(url: "https://github.com/apple/swift-argument-parser.git", .upToNextMinor(from: "0.4.3")),
.package(url: "https://github.com/apple/swift-driver.git", .branch(relatedDependenciesBranch)),
.package(url: "https://github.com/apple/swift-crypto.git", .upToNextMinor(from: "1.1.4")),
.package(url: "https://github.com/apple/swift-collections.git", .upToNextMinor(from: "0.0.3")),
]
} else {
package.dependencies += [
.package(path: "../swift-tools-support-core"),
.package(path: "../swift-argument-parser"),
.package(path: "../swift-driver"),
.package(path: "../swift-crypto"),
.package(path: "../swift-collections"),
]
}
2 changes: 0 additions & 2 deletions Sources/Basics/ByteString+Extensions.swift
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@
See http://swift.org/CONTRIBUTORS.txt for Swift project authors
*/

import TSCBasic

extension ByteString {
/// A lowercase, hexadecimal representation of the SHA256 hash
/// generated for the byte string's contents.
Expand Down
1 change: 1 addition & 0 deletions Sources/Basics/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ add_library(Basics
Dictionary+Extensions.swift
DispatchTimeInterval+Extensions.swift
Errors.swift
Exports.swift
FileSystem+Extensions.swift
HTPClient+URLSession.swift
HTTPClient.swift
Expand Down
2 changes: 1 addition & 1 deletion Sources/Basics/ConcurrencyHelpers.swift
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@
See http://swift.org/CONTRIBUTORS.txt for Swift project authors
*/


import Dispatch
import class Foundation.ProcessInfo
import TSCBasic

/// Thread-safe dictionary like structure
public final class ThreadSafeKeyValueStore<Key, Value> where Key: Hashable {
Expand Down
2 changes: 0 additions & 2 deletions Sources/Basics/Dictionary+Extensions.swift
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@
See http://swift.org/CONTRIBUTORS.txt for Swift project authors
*/

import TSCBasic

extension Dictionary {
@inlinable
@discardableResult
Expand Down
16 changes: 16 additions & 0 deletions Sources/Basics/Exports.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
/*
This source file is part of the Swift.org open source project

Copyright (c) 2021 Apple Inc. and the Swift project authors
Licensed under Apache License v2.0 with Runtime Library Exception

See http://swift.org/LICENSE.txt for license information
See http://swift.org/CONTRIBUTORS.txt for Swift project authors
*/

@_exported import OrderedCollections
@_exported import TSCBasic
// override TSC versions until deprecated
// TODO: remove once TSC removes these
public typealias OrderedSet = OrderedCollections.OrderedSet
public typealias OrderedDictionary = OrderedCollections.OrderedDictionary
Comment on lines +11 to +16
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a much cleaner solution than what I did in #3533.

Just one question, though: Since there are only a few files that use OrderedCollections, is there any performance disadvantage for files that don't use OrderedCollections to import Basics?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh, I totally missed #3533. sorry about that!

is there any performance disadvantage for files that don't use OrderedCollections to import Basics?

I dont believe so

1 change: 0 additions & 1 deletion Sources/Basics/FileSystem+Extensions.swift
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
See http://swift.org/CONTRIBUTORS.txt for Swift project authors
*/

import TSCBasic
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we should be removing the TSCBasic imports from all files since it currently has plenty of other types besides collections. We may want to remove the exports file prior not having TSCBasic anymore and then we have to put all those imports back again. If we have any files that only import TSCBasic for collections, we can remove the import from those of course.

Copy link
Contributor Author

@tomerd tomerd Jul 2, 2021

Choose a reason for hiding this comment

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

what I did here was to replace the direct import on TSCBasic with an import of Basics which currently re-exports by TSCBasics. The idea is that we can evolve Basics over time until we can eventually eliminate the need in TSCBasic

Copy link
Contributor

Choose a reason for hiding this comment

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

I kinda feel like that could make it harder to remove TSCBasic? Since now every client of Basics will see the API of TSCBasic for an extended period of time. IMO, it would be good to get rid of the re-export quickly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

is #3595 better?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yah, I think I prefer that approach.

import class Foundation.FileManager

// MARK: - user level
Expand Down
1 change: 0 additions & 1 deletion Sources/Basics/HTPClient+URLSession.swift
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
*/

import Foundation
import TSCBasic
import struct TSCUtility.Versioning
#if canImport(FoundationNetworking)
// FIXME: this brings OpenSSL dependency on Linux
Expand Down
1 change: 0 additions & 1 deletion Sources/Basics/HTTPClient.swift
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import struct Foundation.Date
import class Foundation.JSONDecoder
import class Foundation.NSError
import struct Foundation.URL
import TSCBasic
import TSCUtility

#if canImport(Glibc)
Expand Down
2 changes: 0 additions & 2 deletions Sources/Basics/SQLiteBackedCache.swift
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@
*/

import Foundation

import TSCBasic
import TSCUtility

/// SQLite backed persistent cache.
Expand Down
1 change: 0 additions & 1 deletion Sources/Basics/Sandbox.swift
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
See http://swift.org/CONTRIBUTORS.txt for Swift project authors
*/

import TSCBasic
import TSCUtility

public enum Sandbox {
Expand Down
1 change: 0 additions & 1 deletion Sources/Build/BuildOperation.swift
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import PackageGraph
import PackageModel
import SPMBuildCore
import SPMLLBuild
import TSCBasic
import TSCUtility

public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildSystem, BuildErrorAdviceProvider {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,13 @@
*/

import Basics
import TSCBasic
import TSCUtility
import SPMLLBuild
import PackageModel
import Dispatch
import Foundation
import LLBuildManifest
import PackageModel
import SPMBuildCore
import SPMLLBuild
import TSCUtility

#if canImport(llbuildSwift)
typealias LLBuildBuildSystemDelegate = llbuildSwift.BuildSystemDelegate
Expand Down
7 changes: 3 additions & 4 deletions Sources/Build/BuildPlan.swift
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,13 @@
*/

import Basics
import TSCBasic
import TSCUtility
import PackageModel
import Foundation
import PackageGraph
import PackageLoading
import Foundation
import PackageModel
import SPMBuildCore
@_implementationOnly import SwiftDriver
import TSCUtility

extension String {
fileprivate var asSwiftStringLiteralConstant: String {
Expand Down
8 changes: 2 additions & 6 deletions Sources/Build/ManifestBuilder.swift
Original file line number Diff line number Diff line change
Expand Up @@ -8,17 +8,13 @@
See http://swift.org/CONTRIBUTORS.txt for Swift project authors
*/

import LLBuildManifest

import Basics
import TSCBasic
import TSCUtility

import LLBuildManifest
import PackageModel
import PackageGraph
import SPMBuildCore

@_implementationOnly import SwiftDriver
import TSCUtility

public class LLBuildManifestBuilder {
public enum TargetKind {
Expand Down
1 change: 0 additions & 1 deletion Sources/Build/SPMSwiftDriverExecutor.swift
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
*/

import Basics
import TSCBasic
import Foundation
@_implementationOnly import SwiftDriver

Expand Down
2 changes: 1 addition & 1 deletion Sources/Build/SwiftCompilerOutputParser.swift
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@
See http://swift.org/CONTRIBUTORS.txt for Swift project authors
*/

import Basics
import Foundation
import TSCBasic
import TSCUtility

/// Represents a message output by the Swift compiler in JSON output mode.
Expand Down
8 changes: 2 additions & 6 deletions Sources/Commands/APIDigester.swift
Original file line number Diff line number Diff line change
Expand Up @@ -8,17 +8,13 @@
See http://swift.org/CONTRIBUTORS.txt for Swift project authors
*/

import Dispatch

import TSCBasic
import TSCUtility

import SPMBuildCore
import Basics
import Build
import Dispatch
import PackageGraph
import PackageModel
import SourceControl
import SPMBuildCore
import Workspace

/// Helper for emitting a JSON API baseline for a module.
Expand Down
5 changes: 2 additions & 3 deletions Sources/Commands/Describe.swift
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,9 @@
See http://swift.org/CONTRIBUTORS.txt for Swift project authors
*/

import TSCBasic
import PackageModel
import Basics
import Foundation

import PackageModel

enum DescribeMode: String {
/// JSON format (guaranteed to be parsable and stable across time).
Expand Down
4 changes: 2 additions & 2 deletions Sources/Commands/Error.swift
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,12 @@
See http://swift.org/CONTRIBUTORS.txt for Swift project authors
*/

import TSCBasic
import Basics
import PackageLoading
import PackageModel
import SourceControl
import TSCUtility
import func TSCLibc.exit
import TSCUtility
import Workspace

enum Error: Swift.Error {
Expand Down
1 change: 0 additions & 1 deletion Sources/Commands/GenerateLinuxMain.swift
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ See http://swift.org/CONTRIBUTORS.txt for Swift project authors
import Basics
import PackageGraph
import PackageModel
import TSCBasic

/// A utility for generating test entries on linux.
///
Expand Down
7 changes: 3 additions & 4 deletions Sources/Commands/MultiRootSupport.swift
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,13 @@
See http://swift.org/CONTRIBUTORS.txt for Swift project authors
*/

import TSCBasic
import TSCUtility
import class PackageModel.Manifest
import Basics
import Foundation

#if canImport(FoundationXML)
import FoundationXML
#endif
import class PackageModel.Manifest
import TSCUtility

/// A bare minimum loader for Xcode workspaces.
///
Expand Down
6 changes: 3 additions & 3 deletions Sources/Commands/Options.swift
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,11 @@
*/

import ArgumentParser
import TSCBasic
import TSCUtility
import Basics
import Build
import PackageModel
import SPMBuildCore
import Build
import TSCUtility

struct BuildFlagsGroup: ParsableArguments {
@Option(name: .customLong("Xcc", withSingleDash: true),
Expand Down
1 change: 0 additions & 1 deletion Sources/Commands/SwiftBuildTool.swift
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import Basics
import Build
import PackageGraph
import SPMBuildCore
import TSCBasic
Copy link
Contributor

Choose a reason for hiding this comment

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

By a rough count, there are about 10% of changes to imports not because of the changes to Basics. Would it be better to separate them into their own PR, to reduce the amount of files changed in this PR? Although, the PR probably still would contain changes to about 200 files.

Copy link
Contributor Author

@tomerd tomerd Jul 2, 2021

Choose a reason for hiding this comment

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

exactly, this is why I ended up doing together since it would anyways be a huge (# of files) PR

import TSCUtility

extension BuildSubset {
Expand Down
1 change: 0 additions & 1 deletion Sources/Commands/SwiftPackageCollectionsTool.swift
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import Basics
import Foundation
import PackageCollections
import PackageModel
import TSCBasic
import TSCUtility

private enum CollectionsError: Swift.Error {
Expand Down
11 changes: 5 additions & 6 deletions Sources/Commands/SwiftPackageTool.swift
Original file line number Diff line number Diff line change
Expand Up @@ -10,18 +10,17 @@

import ArgumentParser
import Basics
import TSCBasic
import SPMBuildCore
import Build
import PackageModel
import PackageLoading
import Foundation
import PackageGraph
import PackageLoading
import PackageModel
import SourceControl
import SPMBuildCore
import TSCUtility
import Workspace
import Xcodeproj
import XCBuildSupport
import Workspace
import Foundation

/// swift-package tool namespace
public struct SwiftPackageTool: ParsableCommand {
Expand Down
1 change: 0 additions & 1 deletion Sources/Commands/SwiftRunTool.swift
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import Basics
import Build
import PackageGraph
import PackageModel
import TSCBasic
import TSCUtility

/// An enumeration of the errors that can be generated by the run tool.
Expand Down
Loading