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

Update project to Swift 5.10 and start on Sendable updates #833

Merged
merged 12 commits into from
Aug 7, 2024
Merged
Show file tree
Hide file tree
Changes from 11 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
2 changes: 1 addition & 1 deletion Package.swift
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// swift-tools-version:5.8
// swift-tools-version:5.10

import PackageDescription

Expand Down
11 changes: 11 additions & 0 deletions Sources/XcodeProj/Extensions/NSLocking+withLock.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import Foundation

// reimplemention of `withLock` from `NSLocking` extension that is exclusive to the macOS version of `Foundation`
extension NSLocking {
func withLock<T>(_ body: () throws -> T) rethrows -> T {
lock()
defer { unlock() }
return try body()
}
}

12 changes: 0 additions & 12 deletions Sources/XcodeProj/Extensions/NSRecursiveLock+Sync.swift

This file was deleted.

51 changes: 50 additions & 1 deletion Sources/XcodeProj/Objects/Configuration/BuildSettings.swift
Original file line number Diff line number Diff line change
@@ -1,4 +1,53 @@
import Foundation

/// Build settings.
public typealias BuildSettings = [String: Any]
public typealias BuildSettings = [String: BuildSetting]

public enum BuildSetting: Sendable, Equatable {
case string(String)
case array([String])

var valueForWriting: String {
switch self {
case .string(let string):
return string
case .array(let array):
return array.joined(separator: " ")
}
}
}

extension BuildSetting: Codable {
public init(from decoder: Decoder) throws {
let container = try decoder.singleValueContainer()
do {
let string = try container.decode(String.self)
self = .string(string)
} catch {
let array = try container.decode([String].self)
self = .array(array)
}
}

public func encode(to encoder: Encoder) throws {
var container = encoder.singleValueContainer()
switch self {
case .string(let string):
try container.encode(string)
case .array(let array):
try container.encode(array)
}
}
}

extension BuildSetting: ExpressibleByArrayLiteral {
public init(arrayLiteral elements: String...) {
self = .array(elements)
}
}

extension BuildSetting: ExpressibleByStringLiteral {
public init(stringLiteral value: StringLiteralType) {
self = .string(value)
}
}
48 changes: 24 additions & 24 deletions Sources/XcodeProj/Objects/Configuration/XCBuildConfiguration.swift
Original file line number Diff line number Diff line change
Expand Up @@ -43,26 +43,26 @@ public final class XCBuildConfiguration: PBXObject {
}

// MARK: - Decodable

fileprivate enum CodingKeys: String, CodingKey {
case baseConfigurationReference
case buildSettings
case name
}

public required init(from decoder: Decoder) throws {
let objects = decoder.context.objects
let objectReferenceRepository = decoder.context.objectReferenceRepository
let container = try decoder.container(keyedBy: CodingKeys.self)
if let baseConfigurationReference: String = try container.decodeIfPresent(.baseConfigurationReference) {
self.baseConfigurationReference = objectReferenceRepository.getOrCreate(reference: baseConfigurationReference, objects: objects)
} else {
baseConfigurationReference = nil
}
buildSettings = try container.decode([String: Any].self, forKey: .buildSettings)
name = try container.decode(.name)
try super.init(from: decoder)

fileprivate enum CodingKeys: String, CodingKey {
case baseConfigurationReference
case buildSettings
case name
}

public required init(from decoder: Decoder) throws {
let objects = decoder.context.objects
let objectReferenceRepository = decoder.context.objectReferenceRepository
let container = try decoder.container(keyedBy: CodingKeys.self)
if let baseConfigurationReference: String = try container.decodeIfPresent(.baseConfigurationReference) {
self.baseConfigurationReference = objectReferenceRepository.getOrCreate(reference: baseConfigurationReference, objects: objects)
} else {
baseConfigurationReference = nil
}
buildSettings = try container.decode(BuildSettings.self, forKey: .buildSettings)
name = try container.decode(.name)
try super.init(from: decoder)
}

// MARK: - Public

Expand All @@ -75,16 +75,16 @@ public final class XCBuildConfiguration: PBXObject {
public func append(setting name: String, value: String) {
guard !value.isEmpty else { return }

let existing: Any = buildSettings[name] ?? "$(inherited)"
let existing: BuildSetting = buildSettings[name] ?? "$(inherited)"

switch existing {
case let string as String where string != value:
case let .string(string) where string != value:
let newValue = [string, value].joined(separator: " ")
buildSettings[name] = newValue
case let array as [String]:
buildSettings[name] = .string(newValue)
case let .array(array):
var newValue = array
newValue.append(value)
buildSettings[name] = newValue.uniqued()
buildSettings[name] = .array(newValue.uniqued())
default:
break
}
Expand Down
101 changes: 68 additions & 33 deletions Sources/XcodeProj/Objects/Project/PBXObjectReference.swift
Original file line number Diff line number Diff line change
@@ -1,72 +1,102 @@
import Foundation

/// Object used as a reference to PBXObjects from PBXObjects.
class PBXObjectReference: NSObject, Comparable, NSCopying {
class PBXObjectReference: NSObject, Comparable, NSCopying, @unchecked Sendable {
private let lock = NSRecursiveLock()

/// Boolean that indicates whether the id is temporary and needs
/// to be regenerated when saving it to disk.
private(set) var temporary: Bool
var temporary: Bool {
lock.withLock { _temporary }
}
private var _temporary: Bool

/// String reference.
private(set) var value: String
var value: String {
lock.withLock { _value }
}
private var _value: String


/// Weak reference to the objects instance that contains the project objects.
weak var objects: PBXObjects?
var objects: PBXObjects? {
get {
lock.withLock { _objects }
}
set {
lock.withLock {
_objects = newValue
}
}
}
private weak var _objects: PBXObjects?

// QUESTION: this is exposed to the project but so is `getThrowingObject` and `getObject`. What access patterns do we want to support?
Copy link
Contributor

Choose a reason for hiding this comment

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

Good point. I think having three APIs for the same is unnecessary. I believe (it's been a long time) that the motivation for having getThrowingObject was to instruct developers at runtime about bad usages of the library. For example adding a reference to an object that's not strongly referenced by the project. However, looking at the getThrowingObject implementation, and don't know if the error that we are throwing is more helpful than trying to do a force unwrap of the referenced object.
I'm between making getThrowingObject errors more useful, which would require adding more context to PBXObjectReference to ensure it's in sync, which is a bit of a tricky challenge, or removing getObject and getThrowingObject and having a single accessor. I'd say that we do the latter.

Copy link
Member Author

Choose a reason for hiding this comment

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

I ended up going with object() -> T? since that preserved the most current callsites. Also added object(as: T.Type) to avoid a double cast when used in compactMap

/// A weak reference to the object
private weak var object: PBXObject?
var object: PBXObject? {
get {
lock.withLock { _object }
}
}
private weak var _object: PBXObject?

/// Initializes a non-temporary reference.
///
/// - Parameter reference: reference.
init(_ reference: String, objects: PBXObjects) {
value = reference
temporary = false
self.objects = objects
_value = reference
_temporary = false
_objects = objects
}

/// Initializes a temporary reference
init(objects: PBXObjects? = nil) {
value = "TEMP_\(UUID().uuidString)"
temporary = true
self.objects = objects
_value = "TEMP_\(UUID().uuidString)"
_temporary = true
_objects = objects
}

/// Initializes the reference without objects.
///
/// - Parameter reference: reference.
init(_ reference: String) {
value = reference
temporary = false
_value = reference
_temporary = false
}

/// Initializes the object reference with another object reference, copying its values.
///
/// - Parameter objectReference: object reference to be initialized from.
required init(_ objectReference: PBXObjectReference) {
value = objectReference.value
temporary = objectReference.temporary
_value = objectReference.value
_temporary = objectReference.temporary
}

/// Fixes its value making it permanent.
///
/// - Parameter value: value.
func fix(_ value: String) {
let object = objects?.delete(reference: self)
self.value = value
temporary = false
if let object = object {
objects?.add(object: object)
lock.withLock {
let object = objects?.delete(reference: self)
_value = value
_temporary = false
if let object = object {
objects?.add(object: object)
}
}
}

/// Invalidates the reference making it temporary.
func invalidate() {
let object = objects?.delete(reference: self)
value = "TEMP_\(UUID().uuidString)"
temporary = true
if let object = object {
objects?.add(object: object)
lock.withLock {
let object = _objects?.delete(reference: self)
_value = "TEMP_\(UUID().uuidString)"
_temporary = true
if let object = object {
_objects?.add(object: object)
}
}

}

/// Hash value.
Expand Down Expand Up @@ -112,7 +142,9 @@ class PBXObjectReference: NSObject, Comparable, NSCopying {
///
/// - Parameter object: The object
func setObject(_ object: PBXObject) {
self.object = object
lock.withLock {
_object = object
}
}

/// Returns the object the reference is referfing to.
Expand All @@ -130,14 +162,17 @@ class PBXObjectReference: NSObject, Comparable, NSCopying {
if let object = object as? T {
return object
}
guard let objects = objects else {
throw PBXObjectError.objectsReleased
}
guard let object = objects.get(reference: self) as? T else {
throw PBXObjectError.objectNotFound(value)
return try lock.withLock {
guard let objects = objects else {
throw PBXObjectError.objectsReleased
}
guard let object = objects.get(reference: self) as? T else {
throw PBXObjectError.objectNotFound(value)
}
_object = object
return object
}
self.object = object
return object

}
}

Expand Down
Loading
Loading