about summary refs log tree commit homepage
diff options
context:
space:
mode:
authorEric Wong <normalperson@yhbt.net>2010-12-24 13:23:32 -0800
committerEric Wong <normalperson@yhbt.net>2010-12-24 13:28:57 -0800
commit9b46379f75f384c86e42046ab03ce55231197c92 (patch)
tree33ffd8703db04f1bc299df03f12673fda2f888c7
parent8d58b42d0255880d732ba0700597b312a8219f8f (diff)
downloadclogger-9b46379f75f384c86e42046ab03ce55231197c92.tar.gz
This lessens confusion for people configuring Clogger in
config.ru, since "File" could be mistaken for Rack::File
and "::File" needs to be specified.
-rw-r--r--README10
-rw-r--r--ext/clogger_ext/clogger.c35
-rw-r--r--lib/clogger/pure.rb9
-rw-r--r--test/test_clogger.rb18
4 files changed, 58 insertions, 14 deletions
diff --git a/README b/README
index 59a6ced..c61e0da 100644
--- a/README
+++ b/README
@@ -36,7 +36,7 @@ is customizable so you can specify exactly which fields to log.
   require "clogger"
   use Clogger,
       :format => Clogger::Format::Combined,
-      :logger => ::File.open("/path/to/log", "ab"),
+      :path => "/path/to/log",
       :reentrant => true
   run YourApplication.new
 
@@ -45,9 +45,15 @@ somewhere inside the "Rails::Initializer.run do |config|" block:
 
   config.middleware.use 'Clogger',
       :format => Clogger::Format::Combined,
-      :logger => ::File.open("/path/to/log", "ab"),
+      :path => "/path/to/log",
       :reentrant => false
 
+Instead of specifying a :path, you may also specify a :logger object
+that receives a "<<" method:
+
+  use Clogger, :logger=> $stdout, :reentrant => true
+  run YourApplication.new
+
 == VARIABLES
 
 * $http_* - HTTP request headers (e.g. $http_user_agent)
diff --git a/ext/clogger_ext/clogger.c b/ext/clogger_ext/clogger.c
index ad5bbb9..09eeffd 100644
--- a/ext/clogger_ext/clogger.c
+++ b/ext/clogger_ext/clogger.c
@@ -213,11 +213,6 @@ static VALUE obj_fileno(VALUE obj)
         return rb_funcall(obj, rb_intern("fileno"), 0);
 }
 
-static VALUE obj_enable_sync(VALUE obj)
-{
-        return rb_funcall(obj, rb_intern("sync="), 1, Qtrue);
-}
-
 /* only for writing to regular files, not stupid crap like NFS  */
 static void write_full(int fd, const void *buf, size_t count)
 {
@@ -588,12 +583,35 @@ static VALUE cwrite(struct clogger *c)
         return Qnil;
 }
 
+static void init_logger(struct clogger *c, VALUE path)
+{
+        ID id;
+
+        if (!NIL_P(path) && !NIL_P(c->logger))
+                rb_raise(rb_eArgError, ":logger and :path are independent");
+        if (!NIL_P(path)) {
+                VALUE ab = rb_str_new2("ab");
+                id = rb_intern("open");
+                c->logger = rb_funcall(rb_cFile, id, 2, path, ab);
+        }
+
+        id = rb_intern("sync=");
+        if (rb_respond_to(c->logger, id))
+                rb_funcall(c->logger, id, 1, Qtrue);
+
+        id = rb_intern("fileno");
+        if (rb_respond_to(c->logger, id))
+                c->fd = raw_fd(rb_funcall(c->logger, id, 0));
+}
+
 /**
  * call-seq:
  *   Clogger.new(app, :logger => $stderr, :format => string) => obj
  *
  * Creates a new Clogger object that wraps +app+.  +:logger+ may
  * be any object that responds to the "<<" method with a string argument.
+ * If +:logger:+ is a string, it will be treated as a path to a
+ * File that will be opened in append mode.
  */
 static VALUE clogger_init(int argc, VALUE *argv, VALUE self)
 {
@@ -609,12 +627,9 @@ static VALUE clogger_init(int argc, VALUE *argv, VALUE self)
         if (TYPE(o) == T_HASH) {
                 VALUE tmp;
 
+                tmp = rb_hash_aref(o, ID2SYM(rb_intern("path")));
                 c->logger = rb_hash_aref(o, ID2SYM(rb_intern("logger")));
-                if (!NIL_P(c->logger)) {
-                        rb_rescue(obj_enable_sync, c->logger, NULL, 0);
-                        tmp = rb_rescue(obj_fileno, c->logger, NULL, 0);
-                        c->fd = raw_fd(tmp);
-                }
+                init_logger(c, tmp);
 
                 tmp = rb_hash_aref(o, ID2SYM(rb_intern("format")));
                 if (!NIL_P(tmp))
diff --git a/lib/clogger/pure.rb b/lib/clogger/pure.rb
index 0dd5021..452e232 100644
--- a/lib/clogger/pure.rb
+++ b/lib/clogger/pure.rb
@@ -14,7 +14,12 @@ class Clogger
 
     @app = app
     @logger = opts[:logger]
-    (@logger.sync = true) rescue nil
+    path = opts[:path]
+    path && @logger and
+      raise ArgumentError, ":logger and :path are independent"
+    path and @logger = File.open(path, "ab")
+
+    @logger.sync = true if @logger.respond_to?(:sync=)
     @fmt_ops = compile_format(opts[:format] || Format::Common, opts)
     @wrap_body = need_wrap_body?(@fmt_ops)
     @reentrant = opts[:reentrant]
@@ -69,7 +74,7 @@ class Clogger
   end
 
   def fileno
-    @logger.fileno rescue nil
+    @logger.respond_to?(:fileno) ? @logger.fileno : nil
   end
 
 private
diff --git a/test/test_clogger.rb b/test/test_clogger.rb
index 1311017..9252a42 100644
--- a/test/test_clogger.rb
+++ b/test/test_clogger.rb
@@ -3,6 +3,7 @@ $stderr.sync = $stdout.sync = true
 require "test/unit"
 require "date"
 require "stringio"
+require "tempfile"
 
 require "rack"
 
@@ -629,4 +630,21 @@ class TestClogger < Test::Unit::TestCase
     end
   end if RUBY_PLATFORM =~ /linux/ && File.readable?(LINUX_PROC_PID_STATUS)
 
+  def test_path_open_file
+    tmp = Tempfile.new('test_clogger')
+    app = lambda { |env| [ 200, {}, [] ] }
+    app = Clogger.new(app, :format => '$status', :path => tmp.path)
+    assert_kind_of Integer, app.fileno
+    assert app.fileno != tmp.fileno
+    status, headers, body = app.call(@req)
+    assert_equal "200\n", tmp.read
+  end
+
+  def test_path_logger_conflict
+    tmp = Tempfile.new('test_clogger')
+    app = lambda { |env| [ 200, {}, [] ] }
+    assert_raises(ArgumentError) {
+      Clogger.new(app, :logger=> $stderr, :path => tmp.path)
+    }
+  end
 end