Ensuring the correct platform SDK dependencies path is set #419
Conversation
Resolves tuist#417 - Updating SDK dependency paths to be based on the developer directory and the platform SDK root path Test Plan: - generate the fixture `fixtures/ios_app_with_sdk` via `tuist generate` - Verify the linked system frameworks and libraries have the correct path set based on the platform
Generated by π« Danger |
Codecov Report
@@ Coverage Diff @@
## master #419 +/- ##
==========================================
+ Coverage 92.24% 92.27% +0.02%
==========================================
Files 293 293
Lines 15267 15318 +51
==========================================
+ Hits 14083 14134 +51
Misses 1184 1184
Continue to review full report at Codecov.
|
| status: SDKStatus) throws { | ||
| let sdk = AbsolutePath("/\(name)") | ||
|
|
||
| guard let sdkExtension = sdk.extension, |
There was a problem hiding this comment.
Instead of throwing here, I'd add a new linting rule that verifies that the extension is supported. That way developers can see all the validation issues at once and fix them before running tuist generate again.
There was a problem hiding this comment.
That means then that this Error.unsupported should be reported as a bug because we should never reach this point if the extension is not supported.
There was a problem hiding this comment.
Moving this to a lint rule would be much better - I did attempt that in the previous PR, sadly was unable to as the graph is created before any linting is done.
One idea is to always construct an SDKNode even if an invalid name is passed in (e.g. Foo without the extension, or Foo.bar not a .framework and .tbd) and only work out the paths and type later in the pipeline - that way we're able to lint the names. Can play around with this see if it works.
Another is possibly to be more explicit to avoid user error in the first place (will be a breaking change - but maybe ok if we feel its a better choice).
dependencies: [
.sdk(name: "CloudKit", type: .framework),
.sdk(name: "ARKit", type: .framework, status: .optional),
.sdk(name: "libc++", type: .tbd),
]There was a problem hiding this comment.
Another is possibly to be more explicit to avoid user error in the first place (will be a breaking change - but maybe ok if we feel its a better choice).
I like this one π, even though it's a breaking change.
There was a problem hiding this comment.
Would you be ok for that being a follow up PR to reduce the scope of changes in this one?
Resolves #417
Short description π
When the project doesn't have the
SDKROOTsetting set at the project level,.sdkRootbased paths always assume they are for macOS!Solution π¦
Implementation π©βπ»π¨βπ»
Test Plan π
fixtures/ios_app_with_sdkviatuist generate