* [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 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
* 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
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