Skip to content

Conversation

@alexanderwe
Copy link
Contributor

@alexanderwe alexanderwe commented Mar 1, 2025

📝 Description

This PR fixes a small issue in the recursive build up on child projects in a XCWorkpace. When running extractProjectPaths on the XcodeGraphMapper and encountering a .group the groups location was not appended to the projects path. This results into incorrect paths for any child projects in the graph.

🔎 Detailed explanation

An example contents.xcworkspacedata which contains nested groups:

<?xml version="1.0" encoding="UTF-8"?>
<Workspace
   version = "1.0">
   <FileRef
      location = "group:MyProject/MyProject.xcodeproj">
   </FileRef>
   <Group
      location = "group:Modules"
      name = "Modules">
      <FileRef
         location = "group:ModuleA/ModuleA.xcodeproj">
      </FileRef>
   </Group>
</Workspace>

With the current implementation, the ModuleA project path ends up to be <root>/ModuleA even though it is part of a Modules directory.

With the fix the path of the ModuleA project is correctly at <root>/Modules/ModuleA

@dosubot dosubot bot added size:XS This PR changes 0-9 lines, ignoring generated files. bug Something isn't working labels Mar 1, 2025
Copy link
Member

@fortmarek fortmarek left a comment

Choose a reason for hiding this comment

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

I'm aligned with the fix, thanks @alexanderwe! Could you also write a small unit test for this? 🙏

@dosubot dosubot bot added size:M This PR changes 30-99 lines, ignoring generated files. and removed size:XS This PR changes 0-9 lines, ignoring generated files. labels Mar 3, 2025
@alexanderwe
Copy link
Contributor Author

I'm aligned with the fix, thanks @alexanderwe! Could you also write a small unit test for this? 🙏

Hey @fortmarek, good point, sorry that I forgot. I have added a small unit test with a workspace and two groups below it. Just let me know if this what you thought of or if I should enhance or slim it down.

Copy link
Member

@fortmarek fortmarek left a comment

Choose a reason for hiding this comment

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

Thanks @alexanderwe!

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Mar 4, 2025
@fortmarek fortmarek merged commit c011c75 into tuist:main Mar 4, 2025
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working lgtm This PR has been approved by a maintainer size:M This PR changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants