Skip to content

Opt out of inhibit_all_warnings for specific pods #1587

Closed
hfossli opened this Issue Nov 13, 2013 · 33 comments

9 participants

@hfossli
hfossli commented Nov 13, 2013

Hi

It would be great if the :inhibit_warnings => false was respected on specific pods when used. Then warnings for internal libraries could then still be visible in xcode.

Example:

inhibit_all_warnings!

pod 'ReactiveCocoa', '~> 1.9.6'
pod 'CargoBay', :git => 'https://github.com/mattt/CargoBay.git'

pod 'IVYViewTransitioner', :path => '../transitioner-ios', :inhibit_warnings => false
@swizzlr
swizzlr commented Nov 13, 2013

Is there something wrong with writing :inhibit_warnings => true on most of them except the local ones?

@fabiopelosin
CocoaPods member

Yeah, I'm not crazy about providing facilities for suppressing warnings as I would prefer to seem them fixed in libraries.

@swizzlr
swizzlr commented Nov 13, 2013

@irrationalfab a) warnings still show up anyway on any targets that link the pod; b) some of us like to use -Werror or -Weverything, and some don't. Pods users are not working on fixing things, and suppressing warnings is advisable in the same way system headers have warnings suppressed.

@hfossli
hfossli commented Nov 13, 2013

@swizzlr glad you asked. Let me start all over. I missed an important note.

My setup, which I guess is rather common:

  • I'm linking to several pods
  • one of them, pod 'X', is a local pod I'm currently developing on
  • pod 'X' has dependencies to other pods.

In code:

pod 'ReactiveCocoa'
pod 'CargoBay'
pod 'X', :path => '../local-path'

To me it would make perfect sense to avoid having warnings from all pods except pod 'X'. And I especially won't inhibit warnings from pod 'X' since I'm most interested in those warnings.

Attempt 1
I could solve this by setting :inhibit_warnings => false on every pod except the pod 'X' like this

pod 'ReactiveCocoa', :inhibit_warnings => true
pod 'CargoBay', :inhibit_warnings => true
pod 'X', :path => '../local-path'

Problem
Any dependency from pod 'X' will emit warnings.

Attempt 2
Same as attemp 1, but this time I'm listing every pod which pod 'X' is dependent on.

pod 'ReactiveCocoa', :inhibit_warnings => true
pod 'CargoBay', :inhibit_warnings => true
pod 'X', :path => '../local-path'

# hack : listing all dependencies for pod 'X'
pod 'BlocksKit', :inhibit_warnings => true

Problem
This gets out of sync very fast. Also it is a mess to look at. And lastly, it is very tedious.

@hfossli
hfossli commented Nov 13, 2013

Just on top of my head this specific situation could be solved by adding :inhibit_dependency_warnings. And then specify :inhibit_warnings => true for all except the ones you want.

@fabiopelosin
CocoaPods member

Marked as discussion as I see some good points.

Pods users are not working on fixing things

I know, and this is partly our fault. Contributing back is too tedious. I'm really looking forward to a feature like $ pod edit NAME

@hfossli
hfossli commented Nov 14, 2013

@irrationalfab @swizzlr Well, since ruby really isn't a language I know well, I wouldn't dare to contribute without investing a considerable amount of time. So I create an issue her on github and cross fingers somebody else shares the same experience as me and want to do something about it. :)

@fabiopelosin
CocoaPods member

@hfossli I don't know if I misinterpreted the original message in turn, but I was referring to Pod users not contributing back to Objective-C libraries to fix warnings (not to CocoaPods itself).

@hfossli
hfossli commented Nov 14, 2013

@irrationalfab :D haha. ok.

@hfossli
hfossli commented Nov 14, 2013

@mneorr I see you have been involved in both implementation and discussion on this topic on other issues. Would you care to join in here as well? :)

@swizzlr
swizzlr commented Nov 14, 2013

@hfossli I recommend you not fear Ruby. Cocoapods is a veritable monolith of code but it mostly makes sense (it needs refactoring IMO but hey let's get full test coverage first). If you like Objective-C, you'll like Ruby. Give it a try. Clone it, type rake bootstrap and get editing.

@swizzlr
swizzlr commented Nov 14, 2013

Our problems are these:

  • :inhibit_all_warnings! fails to act as you would expect. It does not inhibit all warnings when linking. (my pet annoyance)
  • :inhibit_warnings => true fails to inhibit warnings from implicitly linked dependencies (I assume other dependencies which are explicitly mentioned should be allowed)
  • :inhibit_warnings does not have a false equivalent.

Having spent some time in the warning inhibition code I can tell you that this is not an easy fix. We need full test coverage describing these problems and a refactor of how the installer injects source files. The only solution to inhibit_all_warnings!, truly, is ensuring that clang treats them like system headers. Since this requires modification of the source files the maintainers of Cocoapods do not wish to offer it, though I will be offering it as a plugin soon™ (when the hook exists – when I make it). (Cocoapods already nerfs the directory structure and applies flags independently, so my jimmies rustle slightly at the logic of it all)

This is going to sound really, really sucky, but I don't think warning inhibition should be supported anymore. It doesn't work, and until it does, it shouldn't be offered in the documentation.

@hfossli
hfossli commented Nov 14, 2013

Sad to hear. Yep, you are right. We would have to differentiate between implicitly and explicitly added pods.

I'll have to stick with "Attempt 2" as described above until some hero saves my day. Though it is tedious and prone to fail from time to time it actually frees me from the pollution of warnings from pods I don't maintain.

@mx4492
mx4492 commented Dec 10, 2013

@hfossli Thanks for the detailed explanation and the workaround.

@rivera-ernesto

I see your point and I agree that the inhibit_all_warnings could be more flexible.

On the other as each pod has its own target it is easy to "hide" warnings you're not interested in by collapsing them. Xcode will remember which ones you collapsed and won't expand them again.

Also I would be concerned if the pods my library uses have too many warnings anyway.

@hfossli
hfossli commented Dec 10, 2013

@mx4492 no problem. @rivera-ernesto I agree, pods emitting warnings is a concern. Though it will happen. In my experience - too often. We either have to A) adress the pod-maintainers by giving some sort of penalty for having warnings or B) solve it by extending cocoapods? I guess there will always be warnings in some pods so we either way have to do B, right? I kind of also like A. One way to go about it could be to write out on pod install: Installing SomePod 0.2 ~ This pod has 10 warnings. Contact the author at nasty@habits.com

@alloy
CocoaPods member
alloy commented Dec 10, 2013

@hfossli That would require xcodebuild to be run after pod install which would make it too slow. However, this sounds like a prime candidate for a plugin! It would be nice for debs that do hide warnings to quickly be able to see where they can apply some patches and know who to submit them to.

@hfossli
hfossli commented Dec 10, 2013

@alloy I see :)

@fabiopelosin
CocoaPods member

Today I had an idea add to the specification format a new attribute high_quality and don't allow to upload Pods which have any warning with that attribute turned on. Then we could highlight those Pods in cocoapods.org.

@rivera-ernesto

@hfossli I think that users already punish libs with warnings by trying replacing them with better maintained ones. Also bugging developers who may be doing it all for free anyway may not be the best solution.

@irrationalfab Most pods "develop" warnings over time. Also sometimes we break them.

I think what we could do is:

  • Accept only new pods with no warnings at all.
  • Periodically scan old pods for warnings/errors and flag them.
  • Show up this statistics at pod install, search, etc. Much like README.md build status badges show up for repositories (Build Status).

2013-12-11 10 52 08

2013-12-11 10 53 52

@alloy
CocoaPods member
alloy commented Dec 11, 2013

@irrationalfab Like @rivera-ernesto points out, that is not possible, as warnings might be emitted after a Xcode update, where the same code did not raise a warning before. Also, having no warnings absolutely does not mean the lib is of high quality. We should not ever try to highlight certain libs over others with such heuristics, as it will lead to bad cargo culting.

@alloy
CocoaPods member
alloy commented Dec 11, 2013

@rivera-ernesto The problem with “Accept only new pods with no warnings at all.” is that we are going to move away from testing every spec on Travis and shift the responsibility to the spec submitter. (See the https://github.com/CocoaPods/trunk.cocoapods.org app.)

@fabiopelosin
CocoaPods member

@irrationalfab Like @rivera-ernesto points out, that is not possible, as warnings might be emitted after a Xcode update, where the same code did not raise a warning before. Also, having no warnings absolutely does not mean the lib is of high quality. We should not ever try to highlight certain libs over others with such heuristics, as it will lead to bad cargo culling.

@alloy 👍

Periodically scan old pods for warnings/errors and flag them

@rivera-ernesto The idea of storing the stats on a web service is pretty nice, however I'm not sure if it is worth the non trivial effort. We could do it from the Travis on the master repo, just uploading the information as Coveralls does.

@supermarin
CocoaPods member

@irrationalfab @alloy @swizzlr so how do we move on this?

We're having the same issue at Yammer - local pods depend on pods with warnings (even AFN 1.x throws some), so we have this nasty post_install block.

Does :inhibit_dependency_warnings sound like a viable solution? For me that one makes the most sense, with a boolean option.

@CocoaPodsBot

Issue has been confirmed by @supermarin

@CocoaPodsBot CocoaPodsBot was unassigned by hfossli Mar 29, 2014
@alloy
CocoaPods member
alloy commented Mar 29, 2014

@supermarin What would happen if one of those dependent pods is also a dependency of another pod that does not have :inhibit_dependency_warnings enabled? Would it show warnings for that pod?

@supermarin
CocoaPods member

@alloy I think we need to spec it out, let's try:

  • Let's assume AFNetworking is throwing a warning
platform :ios, '7.0'

  # TurboKit is our local pod. It depends on AFNetworking.
  # In this case warnings aren't coming from AFNetworking, they're inhibited
  pod :TurboKit, :path => '../', :inhibit_dependency_warnings => true

  # Rocket depends on AFNetworking.
  # Since we inhibit warnings here as well, there's still no warnings in the project.
  pod :Rocket, :inhibit_warnings => true

  # AFIncrementalStore depends on AFNetworking. Since we don't inhibit warnings here,
  # warnings ARE BEING THROWN from AFNetworking
  pod :AFIncrementalStore
@alloy
CocoaPods member
alloy commented Mar 30, 2014

I think this looks good, but I’ll have to re-read it with a clear mind when I’m rested :)

@fabiopelosin
CocoaPods member

Sounds reasonable to me: so the rule is to inhibit the warnings as long the Pod is not pulled by another path (either directly or as a dep) which doesn't have the warnings inhibited.

@supermarin supermarin self-assigned this Apr 9, 2014
@segiddins
CocoaPods member

I'm going to close this out, as it's been open for a year and a half without being addressed -- I'd welcome a PR implementing this, but I don't think it needs to remain opening any longer.

@segiddins segiddins closed this Mar 4, 2015
@hfossli
hfossli commented Aug 5, 2015

We're having the same issue at Yammer - local pods depend on pods with warnings (even AFN 1.x throws some), so we have this nasty post_install block.

@supermarin Can I see your nasty post_install? :)

I'm trying to solve this:
http://stackoverflow.com/questions/31644959/how-can-i-silence-warnings-from-all-pods-except-local-pods

@supermarin
CocoaPods member

@hfossli unfortunately I don't work there anymore :)

@hfossli
hfossli commented Aug 5, 2015

@supermarin can you @-mention someone who does? I really would like to know if it possible.. There's a bounty of 150 on the stackoverflow question, but no traction...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.