Strip credentials from ambiguous source warnings. #3256
Conversation
|
|
||
| def source_uris_for_spec(spec) | ||
| specs.search_all(spec.name).map{|s| s.source_uri } | ||
| specs.search_all(spec.name).inject([]) do |uris, spec| | ||
| uris << uri_without_credentials(spec.source_uri) if spec.source_uri |
There was a problem hiding this comment.
probably we want to create a credential-less map of source urls and use that so that we don't have to dup and remove creds from the same source urls hundreds of times?
There was a problem hiding this comment.
Good point. I'll update.
|
@indirect any more thoughts on the change to add caching? |
| uris += source_uris_for_spec(spec) | ||
| uris.compact! | ||
| uris.uniq! | ||
| Installer.ambiguous_gems << [spec.name, *uris] if uris.length > 1 |
There was a problem hiding this comment.
Come to think of it, this probably isn't thread-safe either 😦
|
@TimMoore did you get back to this, by any chance? If not, I can probably take a stab at it sometime in the next week. |
|
@indirect not yet, still on vacation, but I was planning to work on it tomorrow |
|
👍
|
|
I rewrote this based on the earlier discussion with @indirect. The current implementation passes the tests, including a new one that reproduces #3249, but it still leaves things a little messy. I'm working on a bigger refactoring at https://github.com/TimMoore/bundler/compare/TimMoore:issue-3249-hide-auth-in-source-warning...rubygems-remote-refactor but it will take a few more days to finish, and it might end up being to big for a 1.7.x release. So assuming that everything passes once Travis is up again, I'd like to merge this (or a slight tweak to this if anyone has any feedback) to |
|
Awesome, thanks! |
…-warning Strip credentials from ambiguous source warnings.
Fixes #3249.