sup

commit a7c254f315328a2160f2a9a105e29b06cb19d83b
parent be93a14806472849414fe38565cc1bdd31d03d6a
Author: Rich Lane <rlane@club.cc.cmu.edu>
Date:   Tue, 23 Mar 2010 21:48:18 -0700

remove mutable source state

Diffstat:

7 files changed, 67 insertions(+), 173 deletions(-)
diff --git a/bin/sup-add b/bin/sup-add
@@ -97,9 +97,9 @@ begin
     source = 
       case parsed_uri.scheme
       when "maildir"
-        Redwood::Maildir.new uri, nil, !$opts[:unusual], $opts[:archive], nil, labels
+        Redwood::Maildir.new uri, !$opts[:unusual], $opts[:archive], nil, labels
       when "mbox"
-        Redwood::MBox::Loader.new uri, nil, !$opts[:unusual], $opts[:archive], nil, labels
+        Redwood::MBox::Loader.new uri, !$opts[:unusual], $opts[:archive], nil, labels
       when nil
         Trollop::die "Sources must be specified with an URI"
       else
diff --git a/bin/sup-sync b/bin/sup-sync
@@ -53,16 +53,6 @@ where <source>* is zero or more source URIs. If no sources are given,
 sync from all usual sources. Supported source URI schemes can be seen
 by running "sup-add --help".
 
-Options controlling WHICH messages sup-sync operates on:
-EOS
-  opt :new, "Operate on new messages only. Don't scan over the entire source. (Default.)", :short => :none
-  opt :changed, "Scan over the entire source for messages that have been deleted, altered, or moved from another source."
-  opt :restored, "Operate only on those messages included in a dump file as specified by --restore which have changed state."
-  opt :all, "Operate on all messages in the source, regardless of newness or changedness."
-  opt :start_at, "For --changed, --restored and --all, start at a particular offset.", :type => :int
-
-text <<EOS
-
 Options controlling HOW message state is altered:
 EOS
   opt :asis, "If the message is already in the index, preserve its state. Otherwise, use default source state. (Default.)", :short => :none
@@ -82,16 +72,9 @@ EOS
   opt :dry_run, "Don't actually modify the index. Probably only useful with --verbose.", :short => "-n"
   opt :version, "Show version information", :short => :none
 
-  conflicts :changed, :all, :new, :restored
   conflicts :asis, :restore, :discard
 end
-Trollop::die :restored, "requires --restore" if opts[:restored] unless opts[:restore]
-if opts[:start_at]
-  Trollop::die :start_at, "must be non-negative" if opts[:start_at] < 0
-  Trollop::die :start_at, "requires either --changed, --restored or --all" unless opts[:changed] || opts[:restored] || opts[:all]
-end
 
-target = [:new, :changed, :all, :restored].find { |x| opts[x] } || :new
 op = [:asis, :restore, :discard].find { |x| opts[x] } || :asis
 
 Redwood::start
@@ -126,18 +109,6 @@ begin
     end
   end
 
-  ## for all target specifications except for only-new messages, reset the
-  ## source to the beginning (or to the user-specified starting point.)
-  unless target == :new
-    if opts[:start_at]
-      Trollop::die :start_at, "can only be used on one source" unless sources.size == 1
-      sources.first.seek_to! opts[:start_at]
-      sources.first.correct_offset! if sources.first.respond_to?(:correct_offset!)
-    else
-      sources.each { |s| s.reset! }
-    end
-  end
-
   sources.each do |source|
     puts "Scanning #{source}..."
     num_added = num_updated = num_scanned = num_restored = 0
@@ -148,24 +119,6 @@ begin
       seen[m.id] = true
       old_m = index.build_message m.id
 
-      case target
-      when :changed
-        ## skip this message if we're operating only on changed messages, the
-        ## message is in the index, and it's unchanged from what the source is
-        ## reporting.
-        next if old_m && old_m.locations.member?([m.source, m.source_info])
-      when :restored
-        ## skip if we're operating on restored messages, and this one
-        ## ain't (or we wouldn't be making a change)
-        next unless old_m && restored_state[m.id] && restored_state[m.id] != old_m.labels
-      when :new
-        ## nothing to do; we'll consider all messages starting at the start offset, which
-        ## hasn't been changed.
-      when :all
-        ## nothing to do; we'll consider all messages starting at the start offset, which
-        ## was reset to the beginning above.
-      end
-
       ## tweak source labels according to commandline arguments if necessary
       m.labels.delete :inbox if opts[:archive]
       m.labels.delete :unread if opts[:read]
@@ -226,9 +179,9 @@ begin
       if Time.now - last_info_time > PROGRESS_UPDATE_INTERVAL
         last_info_time = Time.now
         elapsed = last_info_time - start_time
-        pctdone = source.respond_to?(:pct_done) ? source.pct_done : 100.0 * (source.cur_offset.to_f - source.start_offset).to_f / (source.end_offset - source.start_offset).to_f
+        pctdone = source.pct_done
         remaining = (100.0 - pctdone) * (elapsed.to_f / pctdone)
-        printf "## read %dm (~%.0f%%) @ %.1fm/s. %s elapsed, ~%s remaining, offset #{source.cur_offset}\n", num_scanned, pctdone, num_scanned / elapsed, elapsed.to_time_s, remaining.to_time_s
+        printf "## read %dm (~%.0f%%) @ %.1fm/s. %s elapsed, ~%s remaining\n", num_scanned, pctdone, num_scanned / elapsed, elapsed.to_time_s, remaining.to_time_s
       end
     end
 
@@ -236,26 +189,6 @@ begin
     puts "Restored state on #{num_restored} (#{100.0 * num_restored / num_scanned}%) messages." if num_restored > 0
   end
 
-  ## delete any messages in the index that claim they're from one of
-  ## these sources, but that we didn't see.
-  if (target == :all || target == :changed)
-    puts "Deleting missing messages from the index..."
-    num_del, num_scanned = 0, 0
-    sources.each do |source|
-      raise "no source id for #{source}" unless source.id
-      index.each_message :source_id => source.id, :load_spam => true, :load_deleted => true, :load_killed => true do |m|
-        num_scanned += 1
-        unless seen[m.id]
-          next unless m.source_info >= opts[:start_at] if opts[:start_at]
-          puts "Deleting #{m.id}" if opts[:verbose]
-          index.delete m.id unless opts[:dry_run]
-          num_del += 1
-        end
-      end
-    end
-    puts "Deleted #{num_del} / #{num_scanned} messages"
-  end
-
   index.save
 
   if opts[:optimize]
diff --git a/lib/sup/maildir.rb b/lib/sup/maildir.rb
@@ -14,9 +14,9 @@ class Maildir < Source
   MYHOSTNAME = Socket.gethostname
 
   ## remind me never to use inheritance again.
-  yaml_properties :uri, :cur_offset, :usual, :archived, :id, :labels, :mtimes
-  def initialize uri, last_date=nil, usual=true, archived=false, id=nil, labels=[], mtimes={}
-    super uri, last_date, usual, archived, id
+  yaml_properties :uri, :usual, :archived, :id, :labels
+  def initialize uri, usual=true, archived=false, id=nil, labels=[]
+    super uri, usual, archived, id
     uri = URI(Source.expand_filesystem_uri(uri))
 
     raise ArgumentError, "not a maildir URI" unless uri.scheme == "maildir"
@@ -32,7 +32,7 @@ class Maildir < Source
     #the mtime from the subdirs in the maildir with the unix epoch as default.
     #these are used to determine whether scanning the directory for new mail
     #is a worthwhile effort
-    @mtimes = { 'cur' => Time.at(0), 'new' => Time.at(0) }.merge(mtimes || {})
+    @mtimes = { 'cur' => Time.at(0), 'new' => Time.at(0) }
     @dir_ids = { 'cur' => [], 'new' => [] }
   end
 
@@ -40,13 +40,6 @@ class Maildir < Source
   def self.suggest_labels_for path; [] end
   def is_source_for? uri; super || (URI(Source.expand_filesystem_uri(uri)) == URI(self.uri)); end
 
-  def check
-    scan_mailbox
-    return unless start_offset
-
-    start = @ids.index(cur_offset || start_offset) or raise OutOfSyncSourceError, "Unknown message id #{cur_offset || start_offset}." # couldn't find the most recent email
-  end
-
   def store_message date, from_email, &block
     stored = false
     new_fn = new_maildir_basefn + ':2,S'
@@ -150,13 +143,7 @@ class Maildir < Source
 
   def each
     scan_mailbox
-    return unless start_offset
-
-    start = @ids.index(cur_offset || start_offset) or raise OutOfSyncSourceError, "Unknown message id #{cur_offset || start_offset}." # couldn't find the most recent email
-
-    start.upto(@ids.length - 1) do |i|
-      id = @ids[i]
-      self.cur_offset = id
+    @ids.each do |id|
       yield id, @labels + (seen?(id) ? [] : [:unread]) + (trashed?(id) ? [:deleted] : []) + (flagged?(id) ? [:starred] : [])
     end
   end
@@ -171,7 +158,7 @@ class Maildir < Source
     @ids.last + 1
   end
 
-  def pct_done; 100.0 * (@ids.index(cur_offset) || 0).to_f / (@ids.length - 1).to_f; end
+  def pct_done; 100.0 * (0).to_f / (@ids.length - 1).to_f; end
 
   def draft? msg; maildir_data(msg)[2].include? "D"; end
   def flagged? msg; maildir_data(msg)[2].include? "F"; end
diff --git a/lib/sup/mbox.rb b/lib/sup/mbox.rb
@@ -8,12 +8,12 @@ class MBox < Source
   BREAK_RE = /^From \S+ (.+)$/
 
   include SerializeLabelsNicely
-  yaml_properties :uri, :cur_offset, :usual, :archived, :id, :labels
+  yaml_properties :uri, :usual, :archived, :id, :labels
 
   attr_reader :labels
 
   ## uri_or_fp is horrific. need to refactor.
-  def initialize uri_or_fp, start_offset=nil, usual=true, archived=false, id=nil, labels=nil
+  def initialize uri_or_fp, usual=true, archived=false, id=nil, labels=nil
     @mutex = Mutex.new
     @labels = Set.new((labels || []) - LabelManager::RESERVED_LABELS)
 
@@ -30,8 +30,8 @@ class MBox < Source
       @path = uri_or_fp.path
     end
 
-    start_offset ||= 0
-    super uri_or_fp, start_offset, usual, archived, id
+    @offset = 0
+    super uri_or_fp, usual, archived, id
   end
 
   def file_path; @path end
@@ -47,15 +47,6 @@ class MBox < Source
     end
   end
 
-  def check
-    if (cur_offset ||= start_offset) > end_offset
-      raise OutOfSyncSourceError, "mbox file is smaller than last recorded message offset. Messages have probably been deleted by another client."
-    end
-  end
-
-  def start_offset; 0; end
-  def end_offset; File.size @f; end
-
   def load_header offset
     header = nil
     @mutex.synchronize do
@@ -88,12 +79,12 @@ class MBox < Source
   ## scan forward until we're at the valid start of a message
   def correct_offset!
     @mutex.synchronize do
-      @f.seek cur_offset
+      @f.seek @offset
       string = ""
       until @f.eof? || MBox::is_break_line?(l = @f.gets)
         string << l
       end
-      self.cur_offset += string.length
+      @offset += string.length
     end
   end
 
@@ -139,41 +130,51 @@ class MBox < Source
     end
   end
 
-  def next
-    returned_offset = nil
-    next_offset = cur_offset
+  def pct_done
+    (@offset.to_f / File.size(@f)) * 100
+  end
 
-    begin
-      @mutex.synchronize do
-        @f.seek cur_offset
-
-        ## cur_offset could be at one of two places here:
-
-        ## 1. before a \n and a mbox separator, if it was previously at
-        ##    EOF and a new message was added; or,
-        ## 2. at the beginning of an mbox separator (in all other
-        ##    cases).
-
-        l = @f.gets or return nil
-        if l =~ /^\s*$/ # case 1
-          returned_offset = @f.tell
-          @f.gets # now we're at a BREAK_RE, so skip past it
-        else # case 2
-          returned_offset = cur_offset
-          ## we've already skipped past the BREAK_RE, so just go
-        end
+  def each
+    end_offset = File.size @f
+    while @offset < end_offset
+      returned_offset = nil
+      next_offset = @offset
 
-        while(line = @f.gets)
-          break if MBox::is_break_line? line
-          next_offset = @f.tell
+      begin
+        @mutex.synchronize do
+          @f.seek @offset
+
+          ## @offset could be at one of two places here:
+
+          ## 1. before a \n and a mbox separator, if it was previously at
+          ##    EOF and a new message was added; or,
+          ## 2. at the beginning of an mbox separator (in all other
+          ##    cases).
+
+          l = @f.gets or break
+          if l =~ /^\s*$/ # case 1
+            returned_offset = @f.tell
+            @f.gets # now we're at a BREAK_RE, so skip past it
+          else # case 2
+            returned_offset = @offset
+            ## we've already skipped past the BREAK_RE, so just go
+          end
+
+          while(line = @f.gets)
+            break if MBox::is_break_line? line
+            next_offset = @f.tell
+          end
         end
+      rescue SystemCallError, IOError => e
+        raise FatalSourceError, "Error reading #{@f.path}: #{e.message}"
       end
-    rescue SystemCallError, IOError => e
-      raise FatalSourceError, "Error reading #{@f.path}: #{e.message}"
+
+      @offset = next_offset
+      yield returned_offset, (labels + [:unread])
     end
+  end
 
-    self.cur_offset = next_offset
-    [returned_offset, (labels + [:unread])]
+  def next
   end
 
   def self.is_break_line? l
@@ -190,7 +191,7 @@ class MBox < Source
   end
 
   class Loader < self
-    yaml_properties :uri, :cur_offset, :usual, :archived, :id, :labels
+    yaml_properties :uri, :usual, :archived, :id, :labels
   end
 end
 end
diff --git a/lib/sup/poll.rb b/lib/sup/poll.rb
@@ -102,9 +102,8 @@ EOS
 
     @mutex.synchronize do
       @poll_sources.each do |source|
-#        yield "source #{source} is done? #{source.done?} (cur_offset #{source.cur_offset} >= #{source.end_offset})"
         begin
-          yield "Loading from #{source}... " unless source.done? || (source.respond_to?(:has_errors?) && source.has_errors?)
+          yield "Loading from #{source}... " unless source.has_errors?
         rescue SourceError => e
           warn "problem getting messages from #{source}: #{e.message}"
           Redwood::report_broken_sources :force_to_top => true
@@ -160,7 +159,7 @@ EOS
   ## this is the primary mechanism for iterating over messages from a source.
   def each_message_from source, opts={}
     begin
-      return if source.done? || source.has_errors?
+      return if source.has_errors?
 
       source.each do |offset, source_labels|
         if source.has_errors?
diff --git a/lib/sup/sent.rb b/lib/sup/sent.rb
@@ -36,12 +36,12 @@ class SentManager
 end
 
 class SentLoader < MBox::Loader
-  yaml_properties :cur_offset
+  yaml_properties
 
-  def initialize cur_offset=0
+  def initialize
     @filename = Redwood::SENT_FN
     File.open(@filename, "w") { } unless File.exists? @filename
-    super "mbox://" + @filename, cur_offset, true, true
+    super "mbox://" + @filename, true, true
   end
 
   def file_path; @filename end
diff --git a/lib/sup/source.rb b/lib/sup/source.rb
@@ -59,50 +59,29 @@ class Source
   ## Examples for you to look at: mbox/loader.rb, imap.rb, and
   ## maildir.rb.
 
-  ## let's begin!
-  ##
-  ## dirty? means cur_offset has changed, so the source info needs to
-  ## be re-saved to sources.yaml.
-  bool_reader :usual, :archived, :dirty
-  attr_reader :uri, :cur_offset
+  bool_reader :usual, :archived
+  attr_reader :uri
   attr_accessor :id
 
-  def initialize uri, initial_offset=nil, usual=true, archived=false, id=nil
+  def initialize uri, usual=true, archived=false, id=nil
     raise ArgumentError, "id must be an integer: #{id.inspect}" unless id.is_a? Fixnum if id
 
     @uri = uri
-    @cur_offset = initial_offset
     @usual = usual
     @archived = archived
     @id = id
-    @dirty = false
   end
 
   ## overwrite me if you have a disk incarnation (currently used only for sup-sync-back)
   def file_path; nil end
 
   def to_s; @uri.to_s; end
-  def seek_to! o; self.cur_offset = o; end
-  def reset!; seek_to! start_offset; end
   def == o; o.uri == uri; end
-  def done?; start_offset.nil? || (self.cur_offset ||= start_offset) >= end_offset; end
   def is_source_for? uri; uri == @uri; end
 
-  ## check should throw a FatalSourceError or an OutOfSyncSourcError
-  ## if it can detect a problem. it is called when the sup starts up
-  ## to proactively notify the user of any source problems.
-  def check; end
-
-  ## yields successive offsets and labels, starting at #cur_offset.
-  ##
-  ## when implementing a source, you can overwrite either #each or #next. the
-  ## default #each just calls next over and over.
+  ## yields successive offsets and labels
   def each
-    self.cur_offset ||= start_offset
-    until done?
-      offset, labels = self.next
-      yield offset, labels
-    end
+    unimplemented
   end
 
   ## utility method to read a raw email header from an IO stream and turn it
@@ -154,11 +133,6 @@ protected
   def Source.expand_filesystem_uri uri
     uri.gsub "~", File.expand_path("~")
   end
-
-  def cur_offset= o
-    @cur_offset = o
-    @dirty = true
-  end
 end
 
 ## if you have a @labels instance variable, include this
@@ -219,7 +193,7 @@ class SourceManager
 
   def save_sources fn=Redwood::SOURCE_FN
     @source_mutex.synchronize do
-      if @sources_dirty || @sources.any? { |id, s| s.dirty? }
+      if @sources_dirty
         bakfn = fn + ".bak"
         if File.exists? fn
           File.chmod 0600, fn