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

Resolving relative paths with custom project destination #681

Merged
merged 13 commits into from Oct 22, 2019
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -11,6 +11,7 @@
- Add `rcproject` files to sources build phase instead of resources [#669](https://github.com/yonaskolb/XcodeGen/pull/669) @Qusic
- Prefer default configuration names for generated schemes [#673](https://github.com/yonaskolb/XcodeGen/pull/673) @giginet
- Fixed some resource files being placed to "Recovered References" group [#679](https://github.com/yonaskolb/XcodeGen/pull/679) @nivanchikov
- Fixed resolving relative paths with custom project destination [#681](https://github.com/yonaskolb/XcodeGen/pull/681) @giginet

#### Internal

Expand Down
2 changes: 1 addition & 1 deletion Sources/XcodeGenCLI/GenerateCommand.swift
Expand Up @@ -125,7 +125,7 @@ class GenerateCommand: Command {
let xcodeProject: XcodeProj
do {
let projectGenerator = ProjectGenerator(project: project)
xcodeProject = try projectGenerator.generateXcodeProject()
xcodeProject = try projectGenerator.generateXcodeProject(to: projectDirectory)
} catch {
throw GenerationError.generationError(error)
}
Expand Down
6 changes: 4 additions & 2 deletions Sources/XcodeGenKit/PBXProjGenerator.swift
Expand Up @@ -24,11 +24,13 @@ public class PBXProjGenerator {

var generated = false

public init(project: Project) {
public init(project: Project, projectDirectory: Path? = nil) {
self.project = project
carthageResolver = CarthageDependencyResolver(project: project)
pbxProj = PBXProj(rootObject: nil, objectVersion: project.objectVersion)
sourceGenerator = SourceGenerator(project: project, pbxProj: pbxProj)
sourceGenerator = SourceGenerator(project: project,
pbxProj: pbxProj,
projectDirectory: projectDirectory)
}

@discardableResult
Expand Down
5 changes: 3 additions & 2 deletions Sources/XcodeGenKit/ProjectGenerator.swift
Expand Up @@ -13,10 +13,11 @@ public class ProjectGenerator {
self.project = project
}

public func generateXcodeProject() throws -> XcodeProj {
public func generateXcodeProject(to projectDirectory: Path? = nil) throws -> XcodeProj {
kateinoigakukun marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Owner

Choose a reason for hiding this comment

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

minor nitpick if using naming like that:

Suggested change
public func generateXcodeProject(to projectDirectory: Path? = nil) throws -> XcodeProj {
public func generateXcodeProject(in projectDirectory: Path? = nil) throws -> XcodeProj {


// generate PBXProj
let pbxProjGenerator = PBXProjGenerator(project: project)
let pbxProjGenerator = PBXProjGenerator(project: project,
projectDirectory: projectDirectory)
let pbxProj = try pbxProjGenerator.generate()

// generate Schemes
Expand Down
17 changes: 13 additions & 4 deletions Sources/XcodeGenKit/SourceGenerator.swift
Expand Up @@ -13,6 +13,7 @@ struct SourceFile {
class SourceGenerator {

var rootGroups: Set<PBXFileElement> = []
private let projectDirectory: Path?
private var fileReferencesByPath: [String: PBXFileElement] = [:]
private var groupsByPath: [Path: PBXGroup] = [:]
private var variantGroupsByPath: [Path: PBXVariantGroup] = [:]
Expand All @@ -30,9 +31,18 @@ class SourceGenerator {

private(set) var knownRegions: Set<String> = []

init(project: Project, pbxProj: PBXProj) {
init(project: Project, pbxProj: PBXProj, projectDirectory: Path?) {
self.project = project
self.pbxProj = pbxProj
self.projectDirectory = projectDirectory
}

private func resolveGroupPath(_ path: Path, isTopLevelGroup: Bool) -> String {
if isTopLevelGroup, let relativePath = try? path.relativePath(from: projectDirectory ?? project.basePath).string {
return relativePath
} else {
return path.lastComponent
}
}

@discardableResult
Expand Down Expand Up @@ -286,9 +296,8 @@ class SourceGenerator {
let isTopLevelGroup = (isBaseGroup && !createIntermediateGroups) || isRootPath

let groupName = name ?? path.lastComponent
let groupPath = isTopLevelGroup ?
((try? path.relativePath(from: project.basePath)) ?? path).string :
Copy link
Collaborator Author

@giginet giginet Oct 8, 2019

Choose a reason for hiding this comment

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

The previous implementation seems to be wrong.

According to the PBXGroup's documentation, path argument must be relative path from source root.
https://github.com/tuist/XcodeProj/blob/aefcbf59cea5b0831837ed2f385e6dfd80d82cc9/Sources/XcodeProj/Objects/Files/PBXGroup.swift#L28

However, in some situations, groupPath become path.string. It is absolute path.

path.lastComponent
let groupPath = resolveGroupPath(path, isTopLevelGroup: isTopLevelGroup)

let group = PBXGroup(
children: children,
sourceTree: .group,
Expand Down
34 changes: 34 additions & 0 deletions Tests/XcodeGenKitTests/ProjectGeneratorTests.swift
Expand Up @@ -1004,4 +1004,38 @@ class ProjectGeneratorTests: XCTestCase {
}
}
}

func testGenerateXcodeProjectWithDestination() throws {
let groupName = "App_iOS"
let sourceDirectory = fixturePath + "TestProject" + groupName
let frameworkWithSources = Target(
name: "MyFramework",
type: .framework,
platform: .iOS,
sources: [TargetSource(path: sourceDirectory.string)]
)

describe("generateXcodeProject") {
$0.context("without projectDirectory") {
$0.it("generate groups") {
let project = Project(name: "test", targets: [frameworkWithSources])
let generator = ProjectGenerator(project: project)
let generatedProject = try generator.generateXcodeProject()
let group = generatedProject.pbxproj.groups.first(where: { $0.nameOrPath == groupName })
try expect(group?.path) == "App_iOS"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This value should be relative path from source root.
https://github.com/yonaskolb/XcodeGen/pull/681/files#r332485507

}
}

$0.context("with projectDirectory") {
$0.it("generate groups") {
let destinationPath = fixturePath
let project = Project(name: "test", targets: [frameworkWithSources])
let generator = ProjectGenerator(project: project)
let generatedProject = try generator.generateXcodeProject(to: destinationPath)
let group = generatedProject.pbxproj.groups.first(where: { $0.nameOrPath == groupName })
try expect(group?.path) == "TestProject/App_iOS"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Test fixing this issue.

}
}
}
}
}