search

isabanin / mercurial-ruby

Ruby API for Mercurial DVCS.
MIT License
41 stars 14 forks source link

Ignore mercurial warnings printed to stderr as long as the exitstatus is 0 #1

Closed potatosalad closed 12 years ago

potatosalad commented 12 years ago

An error should only be raised if the process exited irregularly (non-zero).

Here's the example case I ran into:

$ hg id https://bitbucket.org/django/django
warning: bitbucket.org certificate with fingerprint 24:9c:45:8b:9c:aa:ba:55:4e:01:6d:58:ff:e4:28:7d:2a:14:ae:3b not verified (check hostfingerprints or web.cacerts config setting)
warning: bitbucket.org certificate with fingerprint 24:9c:45:8b:9c:aa:ba:55:4e:01:6d:58:ff:e4:28:7d:2a:14:ae:3b not verified (check hostfingerprints or web.cacerts config setting)
warning: bitbucket.org certificate with fingerprint 24:9c:45:8b:9c:aa:ba:55:4e:01:6d:58:ff:e4:28:7d:2a:14:ae:3b not verified (check hostfingerprints or web.cacerts config setting)
warning: bitbucket.org certificate with fingerprint 24:9c:45:8b:9c:aa:ba:55:4e:01:6d:58:ff:e4:28:7d:2a:14:ae:3b not verified (check hostfingerprints or web.cacerts config setting)
f96430a396e1
$ echo $?
0

Technically this command was successful, but when running it from the gem...

>> Mercurial::Shell.run(["id ?", "https://bitbucket.org/django/django"], append_hg: true)
Mercurial::CommandError: warning: bitbucket.org certificate with fingerprint 24:9c:45:8b:9c:aa:ba:55:4e:01:6d:58:ff:e4:28:7d:2a:14:ae:3b not verified (check hostfingerprints or web.cacerts config setting)
warning: bitbucket.org certificate with fingerprint 24:9c:45:8b:9c:aa:ba:55:4e:01:6d:58:ff:e4:28:7d:2a:14:ae:3b not verified (check hostfingerprints or web.cacerts config setting)
warning: bitbucket.org certificate with fingerprint 24:9c:45:8b:9c:aa:ba:55:4e:01:6d:58:ff:e4:28:7d:2a:14:ae:3b not verified (check hostfingerprints or web.cacerts config setting)
warning: bitbucket.org certificate with fingerprint 24:9c:45:8b:9c:aa:ba:55:4e:01:6d:58:ff:e4:28:7d:2a:14:ae:3b not verified (check hostfingerprints or web.cacerts config setting)
from ~/.rvm/gems/ruby-1.9.3-p125/gems/mercurial-ruby-0.6.1/lib/mercurial-ruby/command.rb:69:in `raise_error_if_needed'

This pull request allows the previous command to run as expected...

>> Mercurial::Shell.run(["id ?", "https://bitbucket.org/django/django"], append_hg: true)
=> "f96430a396e1\n"

...while still throwing a ruby exception if the exitstatus is not 0

>> Mercurial::Shell.run(["id ?", "https://bitbucket.org/django/i-dont-exist"], append_hg: true)
Mercurial::CommandError: warning: bitbucket.org certificate with fingerprint 24:9c:45:8b:9c:aa:ba:55:4e:01:6d:58:ff:e4:28:7d:2a:14:ae:3b not verified (check hostfingerprints or web.cacerts config setting)
abort: HTTP Error 404: NOT FOUND
from ~/.rvm/gems/ruby-1.9.3-p125@pagoda/bundler/gems/mercurial-ruby-9f0166b30d06/lib/mercurial-ruby/command.rb:70:in `raise_error_if_needed'
isabanin commented 12 years ago

Great addition, thanks.

potatosalad commented 12 years ago

Sure thing, potatsalad/mercurial-ruby@6d213bbd745987e148c0814395a8be2eab364a95 moves the exit status logic inside the raise_error_if_needed method.

isabanin commented 12 years ago

I just realized that this will not work with Ruby 1.8 as wait_thr was added to popen3 only in Ruby 1.9. I will have to revert this until we figure out a way to make both versions work.

isabanin commented 12 years ago

This actually reminded me why I didn't do the status check in a first place. There's no simple way to check process exit status in 1.8.

isabanin commented 12 years ago

I just pushed a new version of the gem that's using Open4 to check status of the executed commands on 1.8.7.