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

xcode4: Fix missing link of sibling project with custom targetextension #1654

Merged

Conversation

noresources
Copy link
Contributor

When a node category cannot be determined by extension,
use the configuration kind when available.

Add unit test to illustrate the case.

What does this PR do?
Project with a custom targetextension were not linked to other projects
because the strategy used to determine Xocde node type was uniquely based
on node name extension.
This patch adds a "last chance" fallback based on attached configuration kind

How does this PR change Premake's behavior?
Fix a edge case bug. Should not change the behavior for most common cases.

Anything else we should know?
I kept the previous behavior as the main strategy to avoid breaking changes.

Did you check all the boxes?

  • Focus on a single fix or feature; remove any unrelated formatting or code changes
  • Add unit tests showing fix or feature works; all tests pass
  • Mention any related issues (put closes #XXXX in comment to auto-close issue when PR is merged)
  • Follow our coding conventions
  • Minimize the number of commits
  • Align documentation to your changes

modules/xcode/xcode_common.lua Outdated Show resolved Hide resolved
modules/xcode/xcode_common.lua Outdated Show resolved Hide resolved
@noresources noresources force-pushed the fix-xcode-sibling-targetextension-links branch from 2cd7cf6 to 3751d15 Compare June 24, 2021 07:56
@noresources
Copy link
Contributor Author

Code updated with early return and a single-line "if"

Copy link
Member

@starkos starkos left a comment

Choose a reason for hiding this comment

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

This looks good to me. Would you mind rebasing it against or merging it with the latest main branch? It should be trivial merge, but a big line ending fix just landed which might cause issues.

@noresources noresources force-pushed the fix-xcode-sibling-targetextension-links branch from 3751d15 to 4c58958 Compare June 25, 2021 16:52
When a node category cannot be determined by extension,
use the configuration kind when available.

Add unit test to illustrate the case.
@noresources noresources force-pushed the fix-xcode-sibling-targetextension-links branch from 4c58958 to 1ce82af Compare June 28, 2021 13:54
@noresources
Copy link
Contributor Author

noresources commented Jun 28, 2021

This looks good to me. Would you mind rebasing it against or merging it with the latest main branch? It should be trivial merge, but a big line ending fix just landed which might cause issues.

Branch was rebased (twice) without issues

@starkos starkos dismissed samsinsane’s stale review June 28, 2021 20:41

Changes made as requested

@starkos starkos merged commit 0c42fb8 into premake:master Jun 28, 2021
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.

3 participants