search

tmspzz / Rome

Carthage cache for S3, Minio, Ceph, Google Storage, Artifactory and many others
MIT License
817 stars 57 forks source link

Rome uses error output for non-errors #109

Open marcpalmer opened 6 years ago

marcpalmer commented 6 years ago

Enhancement Suggestion

Currently if Rome does not find a dependency or bcsymbolmap in the cache it outputs this as an error, and this is particularly ugly when used with Fastlane.

It is just noise in almost all cases I can think of, and is particularly noisy when you do your first Rome download and when you build only for one platform e.g. iOS where you get all the symbol map errors.

Steps which explain the enhancement

  1. Suppress output of these messages unless using a "verbose" mode, or at least change them to warnings instead of errors. OR... collect them up and at the end and output a message like "4 dependencies and 37 symbol maps could not be found in the cache. This simply means some items are not yet in your cache. Run with --verbose for details"

Current and suggested behavior

Running Rome for the first time on a project that has an empty cache chokes with an error on every single dependency. e.g:

[13:36:41]: ▸ Error: could not find OnePasswordExtension.6B87C426-B309-3394-A31D-8883DC6C4D6A.bcsymbolmap in local cache at : /Users/marc/Library/Caches/Rome/XXXXXX/onepassword-extension/iOS/6B87C426-B309-3394-A31D-8883DC6C4D6A.bcsymbolmap-04f146abd3bd5e6f7ba8b18b916c55f3b3c85aed.zip
[13:36:41]: ▸ Error: could not find OnePasswordExtension.F4A8C455-19F7-35F3-96D2-7DD4CAC5040A.bcsymbolmap in local cache at : /Users/marc/Library/Caches/Rome/XXXXXX/onepassword-extension/iOS/F4A8C455-19F7-35F3-96D2-7DD4CAC5040A.bcsymbolmap-04f146abd3bd5e6f7ba8b18b916c55f3b3c85aed.zip
[13:36:41]: ▸ Error: could not find OnePasswordExtension.7D3E2BB4-DE64-321B-994C-A69BD45877C7.bcsymbolmap in local cache at : /Users/marc/Library/Caches/Rome/XXXXXX/onepassword-extension/iOS/7D3E2BB4-DE64-321B-994C-A69BD45877C7.bcsymbolmap-04f146abd3bd5e6f7ba8b18b916c55f3b3c85aed.zip
[13:36:41]: ▸ Error: could not find OnePasswordExtension.5E20B41A-A888-3783-AE7B-F45278581988.bcsymbolmap in local cache at : /Users/marc/Library/Caches/Rome/XXXXXX/onepassword-extension/iOS/5E20B41A-A888-3783-AE7B-F45278581988.bcsymbolmap-04f146abd3bd5e6f7ba8b18b916c55f3b3c85aed.zip

and running Rome again later for a single platform after the cache has been populated for a single platform results in an error for every dependency for every platform that was not build, e.g. at least two error lines per dependency if you built dependencies only for iOS:

[13:48:41]: ▸ Error: could not find OnePasswordExtension.6B87C426-B309-3394-A31D-8883DC6C4D6A.bcsymbolmap in local cache at : /Users/marc/Library/Caches/Rome/XXXXXX/onepassword-extension/iOS/6B87C426-B309-3394-A31D-8883DC6C4D6A.bcsymbolmap-04f146abd3bd5e6f7ba8b18b916c55f3b3c85aed.zip
[13:48:41]: ▸ Error: could not find OnePasswordExtension.F4A8C455-19F7-35F3-96D2-7DD4CAC5040A.bcsymbolmap in local cache at : /Users/marc/Library/Caches/Rome/XXXXXX/onepassword-extension/iOS/F4A8C455-19F7-35F3-96D2-7DD4CAC5040A.bcsymbolmap-04f146abd3bd5e6f7ba8b18b916c55f3b3c85aed.zip

Why would the enhancement be useful to most users

This output is not generally useful to users. Items not being in a cache is not an error, it is expected behaviour for any cache that can be invalidated.

Rome version: 0.13.0.33 OS and version: maxOS 10.12.6

tmspzz commented 6 years ago

Hi and thanks for opening an issue.

First off, I certainly agree that the output can be improved. Are you ware that you can ask rome to cache by platform using the --platform iOS|tvOS|mac|watchOS switch? This should limit the amount of cache misses. This however is not possible with bcsymbolmaps. If you know of a way to pair the bcsymbolmap with a particular platform I'd be happy to implement it.

The assumption I made is that cache misses are errors. When I fist started the project it would only cache frameworks so a miss would be a big deal. Then I added dsyms and all the rest.

I can certainly change the wording to "Warning" rater than error. However I would keep the output to standard error.

Hiding the cache miss error behind the --verbose switch is an attractive idea. I'm not sure about the effort that creating a summary involves but it might require some redesign.

I'll consider this for the next big refactoring. Please @marcpalmer let me know if you are experiencing other issues.