-
Notifications
You must be signed in to change notification settings - Fork 79
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
RUBY_DESCRIPTION, minor formatting, width #8
base: master
Are you sure you want to change the base?
Conversation
Below is a output from current:
|
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.
Only issue I found was using exit_code = 1
then never referencing the variable. I made a suggestion for a change.
@MSP-Greg if you're happy with this lmk and I can just apply the |
Actually, that would be great, as my internet connection is down... EDIT: Got a slow (but working connection), so I eliminated the |
1. Remove abort, as console output is sometimes scrambled in CI. 2. Only show OpenSSL cert location info if net/http failure, as it often contains user name. 3. Misc formatting re output width.
Let me make a few more changes to this. I think I used the exit_code with the idea of showing as much 'system info' as possible, but must have been interrupted as I never finished. Loading OpenSSL can fail if the ruby code doesn't exist, but it can also fails if Ruby's OpenSSL can't find the system's OpenSSL files (depending on how it's compiled). It would be helpful to show info about that if it happens. Let me look at that... |
I've thought this could provide better messages when OpenSSL doesn't load. With MRI Ruby, OpenSSL loads as follows: openssl.rb => openssl. => system openssl files I've added errors messages for whether 'system openssl files' can't load, whether 'openssl.' can't load or doesn't exist. What I still need to do is create/modify messages for if a user has installed the openssl gem... |
@MSP-Greg this is looking really good still, and I like your idea on better OpenSSL feedback. lmk when it's ready and I'll take another look! ^^ |
Thanks. I think I'm going to turn it into a module (with a main So, I'm trying to get it providing more diagnostics. I've also added checking for OpenSSL gems, etc. For example, maybe someone tried to install a newer Ruby OpenSSL and that's broken, but it's the version that loaded. Or, maybe their DNS is bad. Presently, for someone with connection issues, there may be a need for additional 'try this' back and forth that more/better diagnostics might eliminate. Hence, it's getting more involved. Locally, I can test all major/minor versions back to Ruby 1.9.3 and OpenSSL 1.0.0. I'll ping you when done... |
Sorry, putting out fires here and there. What it looks like now (for a working Ruby):
Both Bundler & RG use a combination of their own certs and OpenSSL's 'system certs'. If either are current, connections can be made, as long as TLSv1_2 is supported. Typically, that would include Ruby 2.0. Below is output from a 2.0 build with current 'system certs', but Bundler & RG with outdated certs:
Anyway, what do you think? There's a lot of combinations for error reporting when 'system certs' are taken into account... |
@MSP-Greg that's looks really good! |
Thanks. Adding system cert info to the test is opening a rather large rabbit hole. I don't want to confuse new users, but years ago I had to support a large legacy app. It should be clear that older Rubies' Bundler & RG may work fine if the system certs are up to date. 'May work fine' is hard to test, but at least this verifies whether they can connect. Adding another variable (system certs) makes the reporting (what does the user need to fix) a bit larger. I'll keep working at it... |
Using Ruby 2.7 and later,
RUBY_REVISION
is the full Git commit SHA. UseRUBY_DESCRIPTION
if defined.Also, since cert files don't affect RubyGems & Bundler connections, info about them is only shown when they cause an error with net/http.
Made additional changes to format long output lines into shorter separate lines. Passing example output: