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

Helper methods required for adding build file to the target #213

Merged
merged 7 commits into from
Jan 18, 2018
Merged
Show file tree
Hide file tree
Changes from 6 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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
- **Breaking:** `XCWorkspace.Data` renamed to `XCWorkspaceData` and removed `references`.
- Improved README examples. https://github.com/xcodeswift/xcproj/pull/212 by @ilyapuchka
- Added methods to get paths to workspace, project and breakpoints and shemes files, added public methods to write them separatery. https://github.com/xcodeswift/xcproj/pull/215 by @ilyapuchka
- Added helper methods for adding source file to the project. https://github.com/xcodeswift/xcproj/pull/213 by @ilyapuchka

## 2.0.0

Expand Down
Empty file added Fixtures/newfile.swift
Empty file.
22 changes: 22 additions & 0 deletions Sources/xcproj/PBXProj+Helpers.swift
Original file line number Diff line number Diff line change
@@ -1,6 +1,28 @@
import Foundation
import PathKit

// MARK: - PBXProj Extension (Public)

public extension PBXProj {

/// Returns root project.
public var rootProject: PBXProject? {
return objects.projects[rootObject]
}

/// Returns root project's root group.
public var rootGroup: PBXGroup {
guard let rootProject = self.rootProject else {
fatalError("Missing root project")
Copy link
Contributor

Choose a reason for hiding this comment

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

Is fatalError suitable here?
Maybe we can make rootGroup optional (public var rootGroup: PBXGroup?).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it's more suitable than requiring to unwrap this property which will be nil only in specific situations when accessed before project is correctly setup. In my opinion it would be better if API would not allow to create objects in such invalid or incomplete states (what it does now as far as I understand), but not sure what it will take to change it in this direction.

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree with @ilyapuchka here. If we use an optional attribute, the developer could wrongly access that attribute and get unexpected behaviours as a result. If we fatalErrror they would know that they are doing something wrong and it'd save them some debugging time.

Copy link
Contributor

Choose a reason for hiding this comment

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

Understood!πŸ‘Œ

}
guard let rootGroup = objects.groups[rootProject.mainGroup] else {
fatalError("Root project has no root group")
}
return rootGroup
}

}

// MARK: - PBXProj Extension (Getters)

extension PBXProj {
Expand Down
220 changes: 219 additions & 1 deletion Sources/xcproj/PBXProjObjects+Helpers.swift
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import Foundation
import PathKit

// MARK: - PBXProj.Objects Extension (Public)

Expand All @@ -15,7 +16,224 @@ public extension PBXProj.Objects {
targets.append(contentsOf: Array(aggregateTargets.values) as [PBXTarget])
return targets.filter { $0.name == name }
}


/// Retruns target's sources build phase.
///
/// - Parameter target: target object.
/// - Returns: target's sources build phase, if found.
public func sourcesBuildPhase(target: PBXTarget) -> PBXSourcesBuildPhase? {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice additions @ilyapuchka πŸ‘

Copy link
Contributor

Choose a reason for hiding this comment

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

sourcesBuildPhases is deprecated. Instead you should use objects.sourcesBuildPhases.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is exactly what is being used, this is extension on PBXObjects.

Copy link
Contributor

Choose a reason for hiding this comment

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

πŸ‘ sorry, missed that.

return sourcesBuildPhases.first(where: { target.buildPhases.contains($0.key) })?.value
}

/// Returns all files in target's sources build phase.
///
/// - Parameter target: target object.
/// - Returns: all files in target's sources build phase, or empty array if sources build phase is not found.
public func sourceFiles(target: PBXTarget) -> [(reference: String, file: PBXFileElement)] {
return sourcesBuildPhase(target: target)?.files
.flatMap({ buildFiles[$0]?.fileRef })
.flatMap({ fileRef in getFileElement(reference: fileRef).map({ (fileRef, $0) })})
?? []
}

/// Returns group with the given name contained in the given parent group and its reference.
///
/// - Parameter groupName: group name.
/// - Parameter inGroup: parent group.
/// - Returns: group with the given name contained in the given parent group and its reference.
public func group(named groupName: String, inGroup: PBXGroup) -> (reference: String, group: PBXGroup)? {
let children = inGroup.children
return groups.first(where: {
children.contains($0.key) && ($0.value.name == groupName || $0.value.path == groupName)
}).map({ ($0.0, $0.1) })
}

/// Adds new group with the give name to the given parent group.
/// Group name can be a path with components separated by `/`.
/// Will create new groups for intermediate path components or use existing groups.
/// Returns all new or existing groups in the path and their references.
///
/// - Parameters:
/// - groupName: group name, can be a path with components separated by `/`
/// - toGroup: parent group
/// - options: additional options, default is empty set.
/// - Returns: all new or existing groups in the path and their references.
public func addGroup(named groupName: String, to toGroup: PBXGroup, options: GroupAddingOptions = []) -> [(reference: String, group: PBXGroup)] {
return addGroups(groupName.components(separatedBy: "/"), to: toGroup, options: options)
}

private func addGroups(_ groupNames: [String], to toGroup: PBXGroup, options: GroupAddingOptions) -> [(reference: String, group: PBXGroup)] {
guard !groupNames.isEmpty else { return [] }
let newGroup = createOrGetGroup(named: groupNames[0], in: toGroup, options: options)
return [newGroup] + addGroups(Array(groupNames.dropFirst()), to: newGroup.1, options: options)
}

private func createOrGetGroup(named groupName: String, in parentGroup: PBXGroup, options: GroupAddingOptions) -> (reference: String, group: PBXGroup) {
if let existingGroup = group(named: groupName, inGroup: parentGroup) {
return existingGroup
}

let newGroup = PBXGroup(
children: [],
sourceTree: .group,
name: groupName,
path: options.contains(.withoutFolder) ? nil : groupName
)
let reference = generateReference(newGroup, groupName)
addObject(newGroup, reference: reference)
parentGroup.children.append(reference)
return (reference, newGroup)
}

/// Adds file at the give path to the project and given group or returns existing file and its reference.
///
/// - Parameters:
Copy link
Contributor

Choose a reason for hiding this comment

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

These parameter descriptions do not match function.

/// - filePath: path to the file.
/// - toGroup: group to add file to.
/// - sourceTree: file sourceTree, default is `.group`
/// - sourceRoot: path to project's source root.
/// - Returns: new or existing file and its reference.
public func addFile(at filePath: Path, toGroup: PBXGroup, sourceTree: PBXSourceTree = .group, sourceRoot: Path) throws -> (reference: String, file: PBXFileReference) {
guard filePath.isFile else {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is it necessary to check if the file really exists?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

First to not make project buildable otherwise (it will not build if this file reference is then added to the build phase). Second to not allow paths to folders.

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually you can non existing add file to build phase. This is common case with generated files. But as this is a helper function, I believe it is ok to make these kinds of checks here.

throw XCodeProjEditingError.notAFile(path: filePath)
}

guard let groupReference = groups.first(where: { $0.value == toGroup })?.key else {
throw XCodeProjEditingError.groupNotFound(group: toGroup)
}
let groupPath = fullPath(fileElement: toGroup, reference: groupReference, sourceRoot: sourceRoot)

if let existingFileReference = fileReferences.first(where: {
filePath == fullPath(fileElement: $0.value, reference: $0.key, sourceRoot: sourceRoot)
}) {
let file = (existingFileReference.key, existingFileReference.value)
if !toGroup.children.contains(file.0) {
file.1.path = groupPath.flatMap({ filePath.relativeTo($0) })?.string
toGroup.children.append(file.0)
}
return file
}

let path: Path?
switch sourceTree {
case .group:
path = groupPath.map({ filePath.relativeTo($0) })
case .sourceRoot:
path = filePath.relativeTo(sourceRoot)
case .absolute:
path = filePath.absolute()
default:
path = nil
}

let fileReference = PBXFileReference(
sourceTree: sourceTree,
name: filePath.lastComponent,
explicitFileType: PBXFileReference.fileType(path: filePath),
lastKnownFileType: PBXFileReference.fileType(path: filePath),
path: path?.string
)
let reference = generateReference(fileReference, filePath.string)
addObject(fileReference, reference: reference)

if !toGroup.children.contains(reference) {
toGroup.children.append(reference)
}

return (reference, fileReference)
}

/// Adds file to the given target's sources build phase or returns existing build file and its reference.
/// If target's sources build phase can't be found returns nil.
///
/// - Parameter target: target object
/// - Parameter reference: file reference
/// - Returns: new or existing build file and its reference
public func addBuildFile(toTarget target: PBXTarget, reference: String) -> (reference: String, file: PBXBuildFile)? {
guard let sourcesBuildPhase = sourcesBuildPhase(target: target) else { return nil }
if let existingBuildFile = buildFiles.first(where: { $0.value.fileRef == reference }) {
return (existingBuildFile.key, existingBuildFile.value)
}

let buildFile = PBXBuildFile(fileRef: reference)
let reference = generateReference(buildFile, reference)
addObject(buildFile, reference: reference)
sourcesBuildPhase.files.append(reference)
return (reference, buildFile)
}

/// Returns full path of the file element.
///
/// - Parameters:
/// - fileElement: a file element
/// - reference: a reference to this file element
/// - sourceRoot: path to the project's sourceRoot
/// - Returns: fully qualified file element path
public func fullPath(fileElement: PBXFileElement, reference: String, sourceRoot: Path) -> Path? {
Copy link
Contributor

Choose a reason for hiding this comment

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

could you add few unit tests to this function. I am not sure what cases it has to cover and how. Especially with groups.

switch fileElement.sourceTree {
case .absolute?:
return fileElement.path.flatMap({ Path($0) })
case .sourceRoot?:
return fileElement.path.flatMap({ Path($0, relativeTo: sourceRoot) })
case .group?:
guard let group = groups.first(where: { $0.value.children.contains(reference) }) else { return sourceRoot }
guard let groupPath = fullPath(fileElement: group.value, reference: group.key, sourceRoot: sourceRoot) else { return nil }
return fileElement.path.flatMap({ Path($0, relativeTo: groupPath) })
default:
return nil
}
}

}

public struct GroupAddingOptions: OptionSet {
public let rawValue: Int
public init(rawValue: Int) {
self.rawValue = rawValue
}
/// Create group without reference to folder
static let withoutFolder = GroupAddingOptions(rawValue: 1 << 0)
}

public enum XCodeProjEditingError: Error, CustomStringConvertible {
case notAFile(path: Path)
case groupNotFound(group: PBXGroup)

public var description: String {
switch self {
case .notAFile(let path):
return "\(path) is not a file path"
case .groupNotFound(let group):
return "Group not found in project: \(group)"
}
}
}

extension Path {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it should be made fileprivate or moved to separate file.

fileprivate init(_ string: String, relativeTo relativePath: Path) {
var path = Path(string)
if !path.isAbsolute {
path = (relativePath + path).absolute()
}
self.init(path.string)
}

public func relativeTo(_ relativePath: Path) -> Path {
let components = self.absolute().components
let relativePathComponents = relativePath.absolute().components

var commonPathComponents = [String]()
for component in components {
guard relativePathComponents.count > commonPathComponents.count else { break }
guard relativePathComponents[commonPathComponents.count] == component else { break }
commonPathComponents.append(component)
}

let relative = Array(repeating: "..", count: (relativePathComponents.count - commonPathComponents.count))
let suffix = components.suffix(components.count - commonPathComponents.count)
let path = Path(components: relative + suffix)
return path
}
}

// MARK: - PBXProj.Objects Extension (Internal)
Expand Down
Loading