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

linter is allowing whitespaces in subspecs #177

Closed
mmacvicar opened this issue Sep 29, 2014 · 6 comments
Closed

linter is allowing whitespaces in subspecs #177

mmacvicar opened this issue Sep 29, 2014 · 6 comments

Comments

@mmacvicar
Copy link

This commit 17aaae0 recently changed a validation to make the version optional in the string representation. However, it also started enforcing something that was not being checked there.

The old regexp was matching anything that ended in non-whitespaces and a version (ignoring the a any prefix that included whitespaces):
/(\S_) ((._))/
The new one does not allow that (which is the right thing):
/\A(\S*)(?: ((.+)))?\Z/

The problem is that the current linter checks for whitespaces in the spec name but it does not check that for the subspecs.

For example:
https://github.com/CocoaPods/Specs/blame/4a2a7d017cddfdec782da5bbdd2abee353549280/Specs/mopub-ios-sdk/2.2.0/mopub-ios-sdk.podspec.json

That podspec passes linter validations and works fine during a fresh pod install but an exception is raised in the next pod install or pod update:

[!] Invalid string representation for a specification: mopub-ios-sdk/MoPubSDK Internal (2.2.0). The string representation should include the name and optionally the version of the Pod.

I would be happy to send a patch but first I wanted to verify that having "MoPubSDK Internal" as a subspec name is indeed not allowed.

@segiddins
Copy link
Member

@kylef / @fabiopelosin please confirm that subspecs can't include spaces?

@alloy
Copy link
Member

alloy commented Sep 29, 2014

@segiddins I think it should be consistent with root specs. Is there a good reason to not have spaces in those names? If so, then let’s apply the same to subspecs.

@mmacvicar
Copy link
Author

@alloy I understand spaces are not allowed in root specs, there is a linter spec called "'fails a specification whose name contains whitespace".

@fabiopelosin
Copy link
Member

White space was not supported because it could create issues with the integration (mostly with XCConfigs). Those areas of CocoaPods have been greatly improved so it might be feasible to allow specs with space in the root of the Pod. However I'm not sure wether that is a desirable thing.

Subspecs should just follow the policy for root specs. Accordingly the regex change naively backed on this assumption without realising how many subspecs relied on this behaviour.

@segiddins
Copy link
Member

@kapin would appreciate hearing your thoughts on this

@joshkalpin
Copy link
Member

Is there any possible way to make a subspec a "spec" object and just pass it to the linter? That might be the way to do it. I need some time to properly look at how subspecs are handled though because it has been a while.

mrackwitz added a commit that referenced this issue Apr 8, 2015
The spec is based off of the changes by @segiddins in #178 and @kaplin in #202. So this supersedes and closes #178, #202. This fixes #177.
mrackwitz added a commit that referenced this issue Apr 8, 2015
mrackwitz added a commit that referenced this issue Apr 8, 2015
The spec is based off of the changes by @segiddins in #178 and @kaplin in #202. So this supersedes and closes #178, #202. This fixes #177.
mrackwitz added a commit that referenced this issue Apr 8, 2015
mrackwitz added a commit that referenced this issue Apr 9, 2015
mrackwitz added a commit that referenced this issue Apr 12, 2015
The spec is based off of the changes by @segiddins in #178 and @kaplin in #202. So this supersedes and closes #178, #202. This fixes #177.
mrackwitz added a commit that referenced this issue Apr 12, 2015
mrackwitz added a commit that referenced this issue Apr 12, 2015
Ashton-W pushed a commit to Ashton-W/Core that referenced this issue Nov 2, 2015
The spec is based off of the changes by @segiddins in CocoaPods#178 and @kaplin in CocoaPods#202. So this supersedes and closes CocoaPods#178, CocoaPods#202. This fixes CocoaPods#177.
Ashton-W pushed a commit to Ashton-W/Core that referenced this issue Nov 2, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants