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

Conversation

giginet
Copy link
Collaborator

@giginet giginet commented Oct 8, 2019

Motivation and Context

In the situation generating with --project option different with spec's directory, All containing files are missing on the generated projects.

$ xcodegen -p ./GeneratedProjects

Screen Shot 2019-10-08 at 21 19 41

There is following directory structure.

.
├── GeneratedProjects
│   └── MyFramework.xcodeproj
├── MyFramework
│   ├── SubGroup
│   │   └── A.swift
│   └── main.swift
└── project.yml

Description

This PR solve this issue. Top Level group's location must be the relative path from the generated project.

Before After
Screen Shot 2019-10-08 at 21 19 41 Screen Shot 2019-10-08 at 21 20 08

Before : MyFramework group's location was MyFramework. This value comes from a relative path from the project spec. It should be a relative path from PROJECT_ROOT.

After: MyFramework group's location chages to ../MyFramework. It's a relative path from PROJECT_ROOT.

@@ -286,9 +288,16 @@ 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.

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

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.

case .some(let relativePath) where isTopLevelGroup:
groupPath = relativePath
default:
groupPath = path.lastComponent
Copy link
Owner

Choose a reason for hiding this comment

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

Could this be simplified to use an if let

if isTopLevelGroup, let relativePath = try? path.relativePath(from: projectDirectory ?? project.basePath).string {
    groupPath = relativePath
} else {
    groupPath = path.lastComponent
}

Copy link
Owner

Choose a reason for hiding this comment

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

There might be other places using project.basePath in the file. Perhaps we can have an instance property called projectBasePath that gets set in the initializer, which everything can read

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@yonaskolb I introduce resolveGroupPath instead. It may help us to get a good group path from projectBasePath relatively.

e63fe6b

@@ -13,10 +13,11 @@ public class ProjectGenerator {
self.project = project
}

public func generateXcodeProject() throws -> XcodeProj {
public func generateXcodeProject(to projectDirectory: Path? = nil) throws -> XcodeProj {
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 {

@yonaskolb
Copy link
Owner

Looks good apart from a minor param name comment. Would be good to to know if this fixes this issue #260

@giginet giginet mentioned this pull request Oct 22, 2019
@giginet
Copy link
Collaborator Author

giginet commented Oct 22, 2019

@yonaskolb Merging is blocked because of minor changes after approval....
80808d6

Could you approve this again?

@giginet giginet merged commit 5b5d0e9 into yonaskolb:master Oct 22, 2019
@giginet giginet deleted the project-path branch October 22, 2019 19:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants