Subject: [sup-talk] fixing mbox+ssh

From: [email-masked] (Andrew Pimlott)
Date: Tue, 13 Jan 2009 10:37:25 -0800

As far as I can tell, mbox+ssh access has not been maintained recently.
I wanted to use it, so I figured out how to make it work using Net::SFTP
instead of Net:SSH directly.  Below is what I have ligthly tested so
far.  I have never written Ruby before, so hints are welcome.

Using SFTP looks like it should simplify the whole module such that it
could largely be rewritten.  Would this be ok?  The thing I don't
understand since I haven't really looked at the whole sup source (and
don't know Ruby norms) is what the synchronization is for.  What
concurrency do we have, and do we expect Net:SSH to be concurrency-safe?

Andrew

diff --git a/lib/sup/mbox/ssh-file.rb b/lib/sup/mbox/ssh-file.rb
index d474636..2f3a4e6 100644
--- a/lib/sup/mbox/ssh-file.rb
+++ b/lib/sup/mbox/ssh-file.rb
@@ -1,4 +1,4 @@
-require 'net/ssh'
+require 'net/sftp'
 
 module Redwood
 module MBox
@@ -114,7 +114,7 @@ class SSHFile
   def to_s; "mbox+ssh://#@host/#@fn"; end ## TODO: remove this EVILness
 
   def connect
-    do_remote nil
+    unsafe_connect
   end
 
   def eof?; @offset >= size; end
@@ -125,9 +125,10 @@ class SSHFile
   def path; @fn end
 
   def size
+    unsafe_connect
     if @file_size.nil? || (Time.now - @last_size_check) > SIZE_CHECK_INTERVAL
       @last_size_check = Time.now
-      @file_size = do_remote("wc -c #@fn").split.first.to_i
+      @file_size = @shell.stat!(@fn).size
     end
     @file_size
   end
@@ -170,49 +171,25 @@ private
       @shell, @shell_mutex = @@shells_mutex.synchronize do
         unless @@shells.member? @key
           say "Opening SSH connection to #{@host} for #@fn..."
-          session = Net::SSH.start @host, @ssh_opts
-          say "Starting SSH shell..."
-          @@shells[@key] = [session.shell.sync, Mutex.new]
+          session = Net::SFTP.start @host, @ssh_opts
+          @@shells[@key] = [session, Mutex.new]
         end
         @@shells[@key]
       end
       
       say "Checking for #@fn..."
-      @shell_mutex.synchronize { raise Errno::ENOENT, @fn unless @shell.test("-e #@fn").status == 0 }
+      @shell_mutex.synchronize { raise Errno::ENOENT, @fn unless @shell.stat!(@fn) }
     ensure
       shutup
     end
   end
 
-  def do_remote cmd, expected_size=0
-    retries = 0
-    result = nil
-
-    begin
-      unsafe_connect
-      if cmd
-        # MBox::debug "sending command: #{cmd.inspect}"
-        result = @shell_mutex.synchronize { x = @shell.send_command cmd; sleep 0.25; x }
-        raise SSHFileError, "Failure during remote command #{cmd.inspect}: #{(result.stderr || result.stdout || "")[0 .. 100]}" unless result.status == 0
-      end
-      ## Net::SSH::Exceptions seem to happen every once in a while for
-      ## no good reason.
-    rescue Net::SSH::Exception, *RECOVERABLE_ERRORS
-      if (retries += 1) <= 3
-        @@shells_mutex.synchronize do
-          @shell = nil
-          @@shells[@key] = nil
-        end
-        retry
-      end
-      raise
-    end
-
-    result.stdout if cmd
-  end
-
   def get_bytes offset, size
-    do_remote "tail -c +#{offset + 1} #@fn | head -c #{size}", size
+    unsafe_connect
+    h = @shell.open!(@fn)
+    r = @shell.read!(h, offset, size)
+    @shell.close!(h)
+    r
   end
 
   def expand_buf_forward n=REASONABLE_TRANSFER_SIZE