feat: Add Target.buildableFolders attribute #266
Conversation
dosubot
bot
added
the
size:M
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Caution Review failedThe pull request is closed. WalkthroughA new BuildableFolder model was added and Target was extended with a buildableFolders property (with initializer and factory updates). Dependency pins in Package.resolved were updated. The PR lint GitHub Actions workflow was reconfigured to trigger on all PR events and switched to a different semantic-pr action. Changes
Sequence Diagram(s)sequenceDiagram
participant Dev
participant XcodeGraph
participant Target
participant BuildableFolder
Dev->>XcodeGraph: Define Target with buildableFolders
XcodeGraph->>Target: Call initializer(include buildableFolders)
Target->>BuildableFolder: Store AbsolutePath(s) in buildableFolders
Target-->>Dev: Return Target containing buildableFolders
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15 minutes Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (4)
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for buildable folders to the Target model by introducing a new BuildableFolder struct and adding a buildableFolders attribute to the Target struct. This feature maps to Xcode's PBXFileSystemSynchronizedRootGroup functionality introduced in Xcode 16.
- Introduces
BuildableFolderstruct to represent synchronized root groups - Adds
buildableFoldersproperty toTargetstruct - Updates all Target initializers and test helpers to include the new parameter
Reviewed Changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| Sources/XcodeGraph/Models/BuildableFolder.swift | New model representing a buildable folder with path property |
| Sources/XcodeGraph/Models/Target.swift | Adds buildableFolders property and updates all constructors |
Comments suppressed due to low confidence (1)
Sources/XcodeGraph/Models/Target.swift:64
- The comment '/// Package directories' on line 64 appears to be misplaced or orphaned after the removal of context. It should either be moved to the correct location above the packages property or removed if no longer applicable.
public let type: TargetType
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (2)
Sources/XcodeGraph/Models/Target.swift (2)
323-342: Critical: Include buildableFolders in Equatable implementation.The
buildableFoldersproperty is missing from the==operator implementation, which will cause incorrect equality comparisons between Target instances.Apply this diff to include the missing property:
public static func == (lhs: Target, rhs: Target) -> Bool { lhs.name == rhs.name && lhs.destinations == rhs.destinations && lhs.product == rhs.product && lhs.bundleId == rhs.bundleId && lhs.productName == rhs.productName && lhs.infoPlist == rhs.infoPlist && lhs.entitlements == rhs.entitlements && lhs.settings == rhs.settings && lhs.sources == rhs.sources && lhs.resources == rhs.resources && lhs.headers == rhs.headers && lhs.coreDataModels == rhs.coreDataModels && lhs.scripts == rhs.scripts && lhs.dependencies == rhs.dependencies && lhs.mergedBinaryType == rhs.mergedBinaryType && lhs.mergeable == rhs.mergeable && lhs.environmentVariables == rhs.environmentVariables && - lhs.type == rhs.type + lhs.type == rhs.type && + lhs.packages == rhs.packages && + lhs.buildableFolders == rhs.buildableFolders }
344-352: Critical: Include buildableFolders in hash implementation.The
buildableFoldersproperty is missing from thehash(into:)method, which will cause incorrect hashing behavior and issues with collections that rely on hashing.Apply this diff to include the missing property:
public func hash(into hasher: inout Hasher) { hasher.combine(name) hasher.combine(destinations) hasher.combine(product) hasher.combine(bundleId) hasher.combine(productName) hasher.combine(environmentVariables) hasher.combine(type) + hasher.combine(packages) + hasher.combine(buildableFolders) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/workflows/conventional-pr.yml(1 hunks)Package.resolved(3 hunks)Sources/XcodeGraph/Models/BuildableFolder.swift(1 hunks)Sources/XcodeGraph/Models/Target.swift(9 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.swift
📄 CodeRabbit Inference Engine (AGENT.md)
Use Swift 6.0.3+ with StrictConcurrency enabled
Files:
Sources/XcodeGraph/Models/BuildableFolder.swiftSources/XcodeGraph/Models/Target.swift
Sources/XcodeGraph/Models/**/*
📄 CodeRabbit Inference Engine (AGENT.md)
Core data structures like Project, Target, Scheme, and BuildConfiguration should be implemented in
Sources/XcodeGraph/Models/
Files:
Sources/XcodeGraph/Models/BuildableFolder.swiftSources/XcodeGraph/Models/Target.swift
🧠 Learnings (5)
📓 Common learnings
Learnt from: CR
PR: tuist/XcodeGraph#0
File: AGENT.md:0-0
Timestamp: 2025-08-07T14:29:08.174Z
Learning: Applies to Sources/XcodeGraph/Models/**/* : Core data structures like Project, Target, Scheme, and BuildConfiguration should be implemented in `Sources/XcodeGraph/Models/`
Learnt from: CR
PR: tuist/XcodeGraph#0
File: AGENT.md:0-0
Timestamp: 2025-08-07T14:29:08.174Z
Learning: Applies to Sources/XcodeGraph/Graph/**/* : Graph representation with dependencies and relationships should be implemented in `Sources/XcodeGraph/Graph/`
📚 Learning: 2025-08-07T14:29:08.174Z
Learnt from: CR
PR: tuist/XcodeGraph#0
File: AGENT.md:0-0
Timestamp: 2025-08-07T14:29:08.174Z
Learning: Applies to **/*.swift : Use Swift 6.0.3+ with StrictConcurrency enabled
Applied to files:
Package.resolved
📚 Learning: 2025-08-07T14:29:08.174Z
Learnt from: CR
PR: tuist/XcodeGraph#0
File: AGENT.md:0-0
Timestamp: 2025-08-07T14:29:08.174Z
Learning: Applies to Sources/XcodeGraph/Models/**/* : Core data structures like Project, Target, Scheme, and BuildConfiguration should be implemented in `Sources/XcodeGraph/Models/`
Applied to files:
Sources/XcodeGraph/Models/BuildableFolder.swiftSources/XcodeGraph/Models/Target.swift
📚 Learning: 2025-08-07T14:29:08.174Z
Learnt from: CR
PR: tuist/XcodeGraph#0
File: AGENT.md:0-0
Timestamp: 2025-08-07T14:29:08.174Z
Learning: Applies to Sources/XcodeGraph/Graph/**/* : Graph representation with dependencies and relationships should be implemented in `Sources/XcodeGraph/Graph/`
Applied to files:
Sources/XcodeGraph/Models/BuildableFolder.swift
📚 Learning: 2025-08-07T14:29:08.174Z
Learnt from: CR
PR: tuist/XcodeGraph#0
File: AGENT.md:0-0
Timestamp: 2025-08-07T14:29:08.174Z
Learning: Applies to Sources/XcodeGraphMapper/Mappers/**/* : Conversion logic from XcodeProj to XcodeGraph models should be implemented in `Sources/XcodeGraphMapper/Mappers/`
Applied to files:
Sources/XcodeGraph/Models/BuildableFolder.swift
🧬 Code Graph Analysis (1)
Sources/XcodeGraph/Models/BuildableFolder.swift (1)
Sources/XcodeGraphMapper/Extensions/XCWorkspaceDataFileRef+Extensions.swift (1)
path(9-37)
🪛 actionlint (1.7.7)
.github/workflows/conventional-pr.yml
10-10: the runner of "actions/checkout@v2" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: SPM Build (ubuntu-22.04, 6.0.3)
- GitHub Check: SPM Build (macos-15, 6.0.3)
- GitHub Check: SPM Test (ubuntu-22.04, 6.0.3)
- GitHub Check: SPM Test (macos-15, 6.0.3)
- GitHub Check: Build
🔇 Additional comments (8)
Package.resolved (1)
2-2: LGTM! Dependency updates look good.The dependency updates appear to be routine maintenance with incremental version bumps for filesystem, machokit, swift-fileio, swift-log, and swift-nio libraries.
Also applies to: 36-37, 45-46, 126-127, 135-136, 144-145
.github/workflows/conventional-pr.yml (3)
3-3: LGTM! Simplified trigger configuration.The change to trigger on all pull request events without restrictions simplifies the workflow configuration.
7-7: LGTM! Clear job naming.Adding an explicit job name improves workflow readability.
11-13: New PR linting action validatedI confirmed that
amannn/action-semantic-pull-request@v5exists (1.1 K stars) and itsaction.ymldefines therequireScopeinput as expected. This action enforces the Conventional Commits spec for PR titles, matching or exceeding the validation provided by the previous tool. No further changes are needed.Sources/XcodeGraph/Models/BuildableFolder.swift (1)
1-16: LGTM! Well-designed model struct.The
BuildableFolderstruct is well-implemented with:
- Appropriate protocol conformances (Sendable, Codable, Equatable, Hashable)
- Clear documentation explaining the Xcode 16 synchronized groups feature
- Correct placement in the Models directory per coding guidelines
- Simple, focused design with a single responsibility
The implementation follows Swift best practices and will integrate well with the existing codebase.
Sources/XcodeGraph/Models/Target.swift (3)
66-66: LGTM! New buildableFolders property well-integrated.The addition of the
buildableFoldersproperty follows the established pattern of other collection properties in the Target struct.
100-101: LGTM! Initializer properly updated.The initializer correctly includes the new
buildableFoldersparameter with appropriate default value and assignment.Also applies to: 133-133
447-447: LGTM! Test factory methods properly updated.All test factory methods (
testoverloads andempty) have been correctly updated to include the newbuildableFoldersparameter with appropriate default values.Also applies to: 475-476, 508-508, 537-538, 564-564, 587-587
| type: TargetType = .local, | ||
| packages: [AbsolutePath] = [] | ||
| packages: [AbsolutePath] = [], | ||
| buildableFolders: [BuildableFolder] = [] |
There was a problem hiding this comment.
can you move this property below sources ?, it will be better when both are next to each other. Then as a user I don't have to jump around to see which files or folder are part of target
There was a problem hiding this comment.
note that XcodeGraph is not the abstraction used in Project.swift. We have an extra layer that sits in-between.
I don't have a strong preference here, but agree that in ProjectDescription, it should be close to sources and resources.
| type: TargetType = .local, | ||
| packages: [AbsolutePath] = [] | ||
| packages: [AbsolutePath] = [], | ||
| buildableFolders: [BuildableFolder] = [] |
There was a problem hiding this comment.
note that XcodeGraph is not the abstraction used in Project.swift. We have an extra layer that sits in-between.
I don't have a strong preference here, but agree that in ProjectDescription, it should be close to sources and resources.
pepicrft
force-pushed
the
buildable-folders
branch
from
ec2e29d to
a0d77f9
Compare
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
pepicrft
force-pushed
the
buildable-folders
branch
from
a0d77f9 to
e746ecb
Compare
I'm adding
Target.buildableFoldersattribute that we can use to support buildable folders in Tuist. Note that I leaned on naming the attributebuildableFoldersbecause this is the name that it's being used publicly (.pbxprojuses synchronized root group). The breadth of options in pbxproj is much larger, which means we can't support mapping from pbxproj toXcodeGraphuntil we include those too, so I leaned on doing that in a follow-up PR.Summary by CodeRabbit
New Features
Chores