-
-
Notifications
You must be signed in to change notification settings - Fork 507
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
Fix target linter warnings for target products that allow dots and hyphens #6290
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for fixing this @kapitoshka438. Only a small thing regarding the message and we should be good to go.
} | ||
|
||
if target.productName.unicodeScalars.allSatisfy(allowed.contains) == false { | ||
let reason = | ||
"Invalid product name '\(target.productName)'. This string must contain only alphanumeric (A-Z,a-z,0-9)\(allowsDot ? ", period (.)" : ""), and underscore (_) characters." | ||
"Invalid product name '\(target.productName)'. This string must contain only alphanumeric (A-Z,a-z,0-9)\(allowsExtraCharacters ? ", period (.), hyphen (-)" : ""), and underscore (_) characters." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This message is not true for products that are not included in productsAllowingExtraCharacters
, is it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Period and hyphen are allowed in product name for app, bundle, commandLineTool and, I believe, many other types of targets but framework.
I can check all types where Xcode automatically replaces these characters with underscore.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, I've checked almost all targets in Xcode and there's only one type where it transforms the product name by replacing dots and hyphens with underscores. It's Framework. When you add it, Xcode automatically sets its Product Name to $(TARGET_NAME:c99extidentifier)
so that, for example, a target named my.frame-work
gets Product Name as my_frame_work
.
I've also tried to add such targets (.staticFramework
and .framework
) in a Tuist project and it works differently, replacing only hyphens: My.Frame-work
-> My.Frame_work
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we need to make changes here:
self.productName = productName ?? name.sanitizedModuleName |
if let productName {
self.productName = productName
} else if self.product == .framework || self.product == .staticFramework {
self.productName = "$(TARGET_NAME:c99extidentifier)"
} else {
self.productName = "$(TARGET_NAME)"
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @kapitoshka438
I wonder if a different bug is causing the product names for Braze to get mangled and not propagated correctly - I don't have a lot of context on the SPM integration but from what I can see in the package manifest all target names seem sane with no hyphens or dots!
https://github.com/braze-inc/braze-swift-sdk/blob/main/Package.swift
if let productName { | ||
self.productName = productName | ||
} else if product == .framework || product == .staticFramework { | ||
self.productName = "$(TARGET_NAME:c99extidentifier)" | ||
} else { | ||
self.productName = "$(TARGET_NAME)" | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
productName
is used in a few different contexts within Tuist in addition to setting the underlying build setting. While Xcode can recognise variables - sadly Tuist can't and will regress a few things within it where the productName
is relied upon to derive various xcodeProj element / product names.
The logic needs to continue to be defined in tuist code as opposed to via variables.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah makes sense. I'll revert this. It's not really related to the original issue. Anyway it's a pity that we can't use such variables. But maybe someday we will. )
self.productName = productName ?? name.sanitizedModuleName | ||
if let productName { | ||
self.productName = productName | ||
} else if product == .framework || product == .staticFramework { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is the discrepancy of what constitutes of valid product names documented anywhere? I wasn't aware frameworks, libraries an applications have different rules π€
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's the result of my manual research. I created an empty project in Xcode and created all possible types of targets with names containing dots and hyphens. Framework
is the only target where Xcode transforms Product Name by setting it to "$(TARGET_NAME:c99extidentifier)". Product Name for all other targets was just "$(TARGET_NAME)"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would suggest to try to bring the same approach in Tuist someday. But, taking into account your previous comment, not in this PR.
This is a bug of the product name linter. Look at the package name, not targets names. The package name contains hyphens (braze-swift-sdk). And there are bundle products which names look like "{packageName}_{name}.bundle". Bundles product names can contain hyphens but the linter allows only dots and only for apps and commandLineTools. |
a5b3f3c
to
77cc617
Compare
77cc617
to
f92c62b
Compare
Resolves #6289
Short description π
This PR fixes the target linter to allow dot (".") and hyphen ("-") in some types of target products.
How to test the changes locally π§
Generate the
app_with_spm_dependencies
fixture. Target linter warnings for BrazeUI dependency bundles should not appear anymore.Contributor checklist β
mise run lint:fix
Reviewer checklist β
changelog:added
,changelog:fixed
, orchangelog:changed
, and the title is usable as a changelog entry