Archive of RubyForge sup-devel mailing list
 help / color / mirror / Atom feed
* [sup-devel] [PATCH 1/2] ensure sources.yaml gets flushed to disk
@ 2011-01-18 18:28 Sascha Silbe
  2011-01-18 18:28 ` [sup-devel] [PATCH 2/2] {config, sources}.yaml: preserve symlinks and permissions Sascha Silbe
  0 siblings, 1 reply; 10+ messages in thread
From: Sascha Silbe @ 2011-01-18 18:28 UTC (permalink / raw)
  To: sup-devel

Before renaming sources.yaml we need to fsync() it, otherwise we could end up
with an empty file in case of a crash [1].

[1] http://thunk.org/tytso/blog/2009/03/12/delayed-allocation-and-the-zero-length-file-problem/

Signed-off-by: Sascha Silbe <sascha-pgp@silbe.org>
---
 lib/sup.rb |   10 ++++++++--
 1 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/lib/sup.rb b/lib/sup.rb
index edd23c8..09744b4 100644
--- a/lib/sup.rb
+++ b/lib/sup.rb
@@ -98,10 +98,16 @@ module Redwood
     if safe
       safe_fn = "#{File.dirname fn}/safe_#{File.basename fn}"
       mode = File.stat(fn).mode if File.exists? fn
-      File.open(safe_fn, "w", mode) { |f| f.puts o.to_yaml }
+      File.open(safe_fn, "w", mode) do |f|
+        f.puts o.to_yaml
+        f.fsync
+      end
       FileUtils.mv safe_fn, fn
     else
-      File.open(fn, "w") { |f| f.puts o.to_yaml }
+      File.open(fn, "w") do |f|
+        f.puts o.to_yaml
+        f.fsync
+      end
     end
   end
 
-- 
1.7.2.3

_______________________________________________
Sup-devel mailing list
Sup-devel@rubyforge.org
http://rubyforge.org/mailman/listinfo/sup-devel


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [sup-devel] [PATCH 2/2] {config, sources}.yaml: preserve symlinks and permissions
  2011-01-18 18:28 [sup-devel] [PATCH 1/2] ensure sources.yaml gets flushed to disk Sascha Silbe
@ 2011-01-18 18:28 ` Sascha Silbe
  2011-01-19  3:41   ` Rich Lane
  2011-01-19 15:38   ` William Morgan
  0 siblings, 2 replies; 10+ messages in thread
From: Sascha Silbe @ 2011-01-18 18:28 UTC (permalink / raw)
  To: sup-devel

If config.yaml resp. sources.yaml already exists, preserve the permissions.
Also alter the save algorithm to overwrite the file in-place, thus leaving
symlinks intact.

Signed-off-by: Sascha Silbe <sascha-pgp@silbe.org>
---
All invocations of save_yaml_obj now use the "backup" mode, so maybe we
should just hardcode it in save_yaml_obj, getting rid of the (existing)
safe and (new) backup parameters.

 bin/sup-config    |    2 +-
 lib/sup.rb        |   27 ++++++++++++++++++++++-----
 lib/sup/source.rb |    8 +-------
 3 files changed, 24 insertions(+), 13 deletions(-)

diff --git a/bin/sup-config b/bin/sup-config
index 132fd77..c4a64a3 100755
--- a/bin/sup-config
+++ b/bin/sup-config
@@ -191,7 +191,7 @@ else
   end
 end
 
-Redwood::save_yaml_obj $config, Redwood::CONFIG_FN
+Redwood::save_yaml_obj $config, Redwood::CONFIG_FN, false, true
 
 say "Ok, I've saved you up a nice lil' #{Redwood::CONFIG_FN}."
 
diff --git a/lib/sup.rb b/lib/sup.rb
index 09744b4..21bddb2 100644
--- a/lib/sup.rb
+++ b/lib/sup.rb
@@ -86,7 +86,7 @@ module Redwood
   module_function :reporting_thread, :record_exception, :exceptions
 
 ## one-stop shop for yamliciousness
-  def save_yaml_obj o, fn, safe=false
+  def save_yaml_obj o, fn, safe=false, backup=false
     o = if o.is_a?(Array)
       o.map { |x| (x.respond_to?(:before_marshal) && x.before_marshal) || x }
     elsif o.respond_to? :before_marshal
@@ -95,16 +95,33 @@ module Redwood
       o
     end
 
-    if safe
+    mode = if File.exists? fn
+      File.stat(fn).mode
+    else
+      0600
+    end
+
+    if backup
+      backup_fn = fn + '.bak'
+      unless File.exists?(backup_fn) && File.size(fn) == 0
+        File.open(backup_fn, "w", mode) do |f|
+          File.open(fn, "r") { |old_f| FileUtils.copy_stream old_f, f }
+          f.fsync
+        end
+      end
+      File.open(fn, "w") do |f|
+        f.puts o.to_yaml
+        f.fsync
+      end
+    elsif safe
       safe_fn = "#{File.dirname fn}/safe_#{File.basename fn}"
-      mode = File.stat(fn).mode if File.exists? fn
       File.open(safe_fn, "w", mode) do |f|
         f.puts o.to_yaml
         f.fsync
       end
       FileUtils.mv safe_fn, fn
     else
-      File.open(fn, "w") do |f|
+      File.open(fn, "w", mode) do |f|
         f.puts o.to_yaml
         f.fsync
       end
@@ -292,7 +309,7 @@ EOS
         :col_jump => 2
       }
       begin
-        Redwood::save_yaml_obj config, filename
+        Redwood::save_yaml_obj config, filename, false, true
       rescue StandardError => e
         $stderr.puts "warning: #{e.message}"
       end
diff --git a/lib/sup/source.rb b/lib/sup/source.rb
index 9c398f7..f2379fb 100644
--- a/lib/sup/source.rb
+++ b/lib/sup/source.rb
@@ -212,13 +212,7 @@ class SourceManager
   def save_sources fn=Redwood::SOURCE_FN
     @source_mutex.synchronize do
       if @sources_dirty
-        bakfn = fn + ".bak"
-        if File.exists? fn
-          File.chmod 0600, fn
-          FileUtils.mv fn, bakfn, :force => true unless File.exists?(bakfn) && File.size(fn) == 0
-        end
-        Redwood::save_yaml_obj sources, fn, true
-        File.chmod 0600, fn
+        Redwood::save_yaml_obj sources, fn, false, true
       end
       @sources_dirty = false
     end
-- 
1.7.2.3

_______________________________________________
Sup-devel mailing list
Sup-devel@rubyforge.org
http://rubyforge.org/mailman/listinfo/sup-devel


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [sup-devel] [PATCH 2/2] {config, sources}.yaml: preserve symlinks and permissions
  2011-01-18 18:28 ` [sup-devel] [PATCH 2/2] {config, sources}.yaml: preserve symlinks and permissions Sascha Silbe
@ 2011-01-19  3:41   ` Rich Lane
  2011-01-19 10:05     ` Sascha Silbe
  2011-01-19 15:38   ` William Morgan
  1 sibling, 1 reply; 10+ messages in thread
From: Rich Lane @ 2011-01-19  3:41 UTC (permalink / raw)
  To: Sascha Silbe; +Cc: sup-devel

Both patches applied to master.
_______________________________________________
Sup-devel mailing list
Sup-devel@rubyforge.org
http://rubyforge.org/mailman/listinfo/sup-devel


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [sup-devel] [PATCH 2/2] {config, sources}.yaml: preserve symlinks and permissions
  2011-01-19  3:41   ` Rich Lane
@ 2011-01-19 10:05     ` Sascha Silbe
  0 siblings, 0 replies; 10+ messages in thread
From: Sascha Silbe @ 2011-01-19 10:05 UTC (permalink / raw)
  To: Rich Lane; +Cc: sup-devel


[-- Attachment #1.1: Type: text/plain, Size: 242 bytes --]

Excerpts from Rich Lane's message of Wed Jan 19 04:41:53 +0100 2011:

> Both patches applied to master.
[as well as a whole bunch of other patches from me]

Thanks!

Sascha

-- 
http://sascha.silbe.org/
http://www.infra-silbe.de/

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 494 bytes --]

[-- Attachment #2: Type: text/plain, Size: 143 bytes --]

_______________________________________________
Sup-devel mailing list
Sup-devel@rubyforge.org
http://rubyforge.org/mailman/listinfo/sup-devel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [sup-devel] [PATCH 2/2] {config, sources}.yaml: preserve symlinks and permissions
  2011-01-18 18:28 ` [sup-devel] [PATCH 2/2] {config, sources}.yaml: preserve symlinks and permissions Sascha Silbe
  2011-01-19  3:41   ` Rich Lane
@ 2011-01-19 15:38   ` William Morgan
  2011-01-19 16:42     ` Sascha Silbe
  2011-01-19 17:19     ` William Morgan
  1 sibling, 2 replies; 10+ messages in thread
From: William Morgan @ 2011-01-19 15:38 UTC (permalink / raw)
  To: sup-devel

Reformatted excerpts from Sascha Silbe's message of 2011-01-18:
> +      unless File.exists?(backup_fn) && File.size(fn) == 0

This line is giving me some problems when fn doesn't exist (e.g. when
running sup-sync the first time without a config.yaml). Should it be

  File.exists?(backup_fn) && File.exists?(fn) && File.size(fn) == 0

instead?
-- 
William <wmorgan-sup@masanjin.net>
_______________________________________________
Sup-devel mailing list
Sup-devel@rubyforge.org
http://rubyforge.org/mailman/listinfo/sup-devel


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [sup-devel] [PATCH 2/2] {config, sources}.yaml: preserve symlinks and permissions
  2011-01-19 15:38   ` William Morgan
@ 2011-01-19 16:42     ` Sascha Silbe
  2011-01-19 17:17       ` William Morgan
  2011-01-19 17:19     ` William Morgan
  1 sibling, 1 reply; 10+ messages in thread
From: Sascha Silbe @ 2011-01-19 16:42 UTC (permalink / raw)
  To: William Morgan; +Cc: sup-devel


[-- Attachment #1.1: Type: text/plain, Size: 667 bytes --]

Excerpts from William Morgan's message of Wed Jan 19 16:38:11 +0100 2011:

> Reformatted excerpts from Sascha Silbe's message of 2011-01-18:
> > +      unless File.exists?(backup_fn) && File.size(fn) == 0
> 
> This line is giving me some problems when fn doesn't exist (e.g. when
> running sup-sync the first time without a config.yaml). Should it be
> 
>   File.exists?(backup_fn) && File.exists?(fn) && File.size(fn) == 0
> 
> instead?

Yes, you're right. We shouldn't try to do a copy if the original doesn't
exist. Sorry for not testing that case.
Shall I send a fix-up patch?

Sascha

-- 
http://sascha.silbe.org/
http://www.infra-silbe.de/

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 500 bytes --]

[-- Attachment #2: Type: text/plain, Size: 143 bytes --]

_______________________________________________
Sup-devel mailing list
Sup-devel@rubyforge.org
http://rubyforge.org/mailman/listinfo/sup-devel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [sup-devel] [PATCH 2/2] {config, sources}.yaml: preserve symlinks and permissions
  2011-01-19 16:42     ` Sascha Silbe
@ 2011-01-19 17:17       ` William Morgan
  2011-01-19 18:44         ` Rich Lane
  0 siblings, 1 reply; 10+ messages in thread
From: William Morgan @ 2011-01-19 17:17 UTC (permalink / raw)
  To: sup-devel

Reformatted excerpts from Sascha Silbe's message of 2011-01-19:
> Shall I send a fix-up patch?

I can descend from the heavens and apply my fix to master directly,
unless Rich objects.
-- 
William <wmorgan-sup@masanjin.net>
_______________________________________________
Sup-devel mailing list
Sup-devel@rubyforge.org
http://rubyforge.org/mailman/listinfo/sup-devel


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [sup-devel] [PATCH 2/2] {config, sources}.yaml: preserve symlinks and permissions
  2011-01-19 15:38   ` William Morgan
  2011-01-19 16:42     ` Sascha Silbe
@ 2011-01-19 17:19     ` William Morgan
  2011-01-19 18:41       ` Ben Walton
  1 sibling, 1 reply; 10+ messages in thread
From: William Morgan @ 2011-01-19 17:19 UTC (permalink / raw)
  To: sup-devel

Reformatted excerpts from William Morgan's message of 2011-01-19:
> Reformatted excerpts from Sascha Silbe's message of 2011-01-18:
> > +      unless File.exists?(backup_fn) && File.size(fn) == 0
> 
> This line is giving me some problems when fn doesn't exist (e.g. when
> running sup-sync the first time without a config.yaml). Should it be
> 
>   File.exists?(backup_fn) && File.exists?(fn) && File.size(fn) == 0
> 
> instead?

Actually, I think it should be:

  if File.exists?(fn) && File.size(fn) > 0

Which has the advantage of actually working...
-- 
William <wmorgan-sup@masanjin.net>
_______________________________________________
Sup-devel mailing list
Sup-devel@rubyforge.org
http://rubyforge.org/mailman/listinfo/sup-devel


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [sup-devel] [PATCH 2/2] {config, sources}.yaml: preserve symlinks and permissions
  2011-01-19 17:19     ` William Morgan
@ 2011-01-19 18:41       ` Ben Walton
  0 siblings, 0 replies; 10+ messages in thread
From: Ben Walton @ 2011-01-19 18:41 UTC (permalink / raw)
  To: sup-devel

Excerpts from William Morgan's message of Wed Jan 19 12:19:11 -0500 2011:

> Actually, I think it should be:
> 
>   if File.exists?(fn) && File.size(fn) > 0

How about:

begin
   fninf = File.stat(fn)
   if fninf.size? ...

rescue Errno::ENOENT => e
...
end
       
I didn't look at the surrounding code so I'm not sure how to best
integrate this.  The advantage is that you save one user->kernel space
traversal.  Calling .exists? and .size via File results in two stat()
calls.  Grabbing and saving the actual stat result on the first call
and then using them for further tests saves this.  Not a huge
overhead, I admit...

HTH.
-Ben
--
Ben Walton
Systems Programmer - CHASS
University of Toronto
C:416.407.5610 | W:416.978.4302

_______________________________________________
Sup-devel mailing list
Sup-devel@rubyforge.org
http://rubyforge.org/mailman/listinfo/sup-devel


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [sup-devel] [PATCH 2/2] {config, sources}.yaml: preserve symlinks and permissions
  2011-01-19 17:17       ` William Morgan
@ 2011-01-19 18:44         ` Rich Lane
  0 siblings, 0 replies; 10+ messages in thread
From: Rich Lane @ 2011-01-19 18:44 UTC (permalink / raw)
  To: William Morgan; +Cc: sup-devel

Excerpts from William Morgan's message of Wed Jan 19 12:17:23 -0500 2011:
> Reformatted excerpts from Sascha Silbe's message of 2011-01-19:
> > Shall I send a fix-up patch?
> 
> I can descend from the heavens and apply my fix to master directly,
> unless Rich objects.

Please do.
_______________________________________________
Sup-devel mailing list
Sup-devel@rubyforge.org
http://rubyforge.org/mailman/listinfo/sup-devel


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2011-01-19 18:53 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-01-18 18:28 [sup-devel] [PATCH 1/2] ensure sources.yaml gets flushed to disk Sascha Silbe
2011-01-18 18:28 ` [sup-devel] [PATCH 2/2] {config, sources}.yaml: preserve symlinks and permissions Sascha Silbe
2011-01-19  3:41   ` Rich Lane
2011-01-19 10:05     ` Sascha Silbe
2011-01-19 15:38   ` William Morgan
2011-01-19 16:42     ` Sascha Silbe
2011-01-19 17:17       ` William Morgan
2011-01-19 18:44         ` Rich Lane
2011-01-19 17:19     ` William Morgan
2011-01-19 18:41       ` Ben Walton

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox