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

Add sourceRootPath to the Project model #1559

Merged
merged 6 commits into from
Jul 14, 2020
Merged

Add sourceRootPath to the Project model #1559

merged 6 commits into from
Jul 14, 2020

Conversation

pepicrft
Copy link
Contributor

Short description 📝

Before this change, we used to pass down the sourceRootPath attribute to tell generators about the directory that contains the generated project. With the change in this PR that information is now part of the Project model.

But why? 🧐

This allows the mappers change the location where projects are generated. For example, let's say that users are using the new cache feature with the focus command. In that case, we don't want to override the projects that contain the targets with the source code. Instead, we want to create a temporary directory that contains all the Xcode projects with the direct and transitive dependencies as .xcframeworks.

@pepicrft pepicrft requested a review from a team July 13, 2020 16:45
@pepicrft pepicrft self-assigned this Jul 13, 2020
@pepicrft pepicrft requested review from natanrolnik and ollieatkinson and removed request for a team July 13, 2020 16:45
Copy link
Collaborator

@kwridan kwridan left a comment

Choose a reason for hiding this comment

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

Thanks @pepibumur - moving some of those properties to the Project model makes sense as it will allow transforming / mapping them on a per project basis 👍

I believe we'll need to also add xcodeProjPath to get the editor working as it did before 🤐 and support the proposed feature in general.

May help for us to write down what each of those paths represents:

  • Project.path the logical path of the project within the graph
  • Project.sourceRootPath the root path of the sources (Determines the path of the main group in Xcode)
  • xcodeProjPath the physical location of the .xcodeproj file on disk

The main group in Xcode has a path that is:

let projectRelativePath = sourceRootPath.relative(to: xcodeprojPath.parentDirectory).pathString

Sources/TuistGenerator/Generator/Generator.swift Outdated Show resolved Hide resolved
Sources/TuistGenerator/Generator/DescriptorGenerator.swift Outdated Show resolved Hide resolved
Sources/TuistGenerator/Generator/DescriptorGenerator.swift Outdated Show resolved Hide resolved
@pepicrft
Copy link
Contributor Author

Thanks for the review @kwridan. I followed your suggestions and made the following changes:

  • Added xcodeProjPath attribute to Project and use it from the descriptor generator to get the path where the Xcode projects are generated.
  • Removed fileName from Project and update the enriched method in GeneratorModelLoader that changes the project name based on the config. This makes me think we should move this into a mapper soon.
  • Fixed the edit command to generate the project in the right directory.
  • Remove the generation options model because it's no longer necessary.

Copy link
Collaborator

@kwridan kwridan left a comment

Choose a reason for hiding this comment

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

Thanks for the updates @pepibumur - changes look 👍

enrichedModel = enrichedModel.replacing(fileName: xcodeFileName)
var xcodeProjPath: AbsolutePath = enrichedModel.xcodeProjPath
if let xcodeFileName = xcodeFileNameOverride(from: config, for: model) {
xcodeProjPath = enrichedModel.xcodeProjPath.parentDirectory.appending(component: xcodeFileName)
Copy link
Collaborator

Choose a reason for hiding this comment

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

minor: I believe this may need to be:

enrichedModel.xcodeProjPath.parentDirectory.appending(component: "\(xcodeFileName).xcodeproj")

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ouch, good catch.

@pepicrft pepicrft merged commit 952846c into master Jul 14, 2020
@pepicrft pepicrft deleted the source-root-path branch July 14, 2020 12:34
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.

2 participants