Permalink
Browse files

Build on #623 adding todos and some code style changes

Also added a test to cover hardlink support for FPM::Util.copy_entry
  • Loading branch information...
1 parent 32c6be5 commit 4958b8f711fe51da8b3a5bb7b85fbcf6581755fa @jordansissel committed Mar 7, 2014
Showing with 46 additions and 8 deletions.
  1. +18 −8 lib/fpm/util.rb
  2. +28 −0 spec/fpm/util_spec.rb
View
@@ -168,17 +168,27 @@ def copy_entry(src, dst)
else
# if the file with the same dev and inode has been copied already -
# hard link it's copy to `dst`, otherwise make an actual copy
- st = File.stat(src)
- if copied_entry = copied_entries[[st.dev, st.ino]]
- FileUtils.ln(copied_entry, dst)
+ st = File.lstat(src)
+ known_entry = copied_entries[[st.dev, st.ino]]
+ if known_entry
+ FileUtils.ln(known_entry, dst)
else
FileUtils.copy_entry(src, dst)
- copied_entries.store([st.dev, st.ino], dst)
+ copied_entries[[st.dev, st.ino]] = dst
end
- end
- end
+ end # else...
+ end # def copy_entry
def copied_entries
- @copied_entries ||= {}
- end
+ # TODO(sissel): I wonder that this entry-copy knowledge needs to be put
+ # into a separate class/module. As is, calling copy_entry the same way
+ # in slightly different contexts will result in weird or bad behavior.
+ # What I mean is if we do:
+ # pkg = FPM::Package::Dir...
+ # pkg.output()...
+ # pkg.output()...
+ # The 2nd output call will fail or behave weirdly because @copied_entries
+ # is already populated. even though this is anew round of copying.
+ return @copied_entries ||= {}
+ end # def copied_entries
end # module FPM::Util
View
@@ -1,5 +1,8 @@
require "spec_setup"
require "fpm" # local
+require "fpm/util" # local
+require "stud/temporary"
+
describe FPM::Util do
subject do
@@ -11,6 +14,31 @@ def initialize
end.new
end
+ context "#copy_entry" do
+ context "when given files that are hardlinks" do
+ it "should keep those files as hardlinks" do
+ Stud::Temporary.directory do |path|
+ a = File.join(path, "a")
+ b = File.join(path, "b")
+ File.write(a, "hello")
+ File.link(a, b)
+
+ Stud::Temporary.directory do |target|
+ ta = File.join(target, "a")
+ tb = File.join(target, "b")
+ subject.copy_entry(a, ta)
+ subject.copy_entry(b, tb)
+
+ # This seems to work to compare file stat calls.
+ # target 'a' and 'b' should have the same stat result because
+ # they are linked to the same file.
+ insist { File.lstat(ta) } == File.lstat(tb)
+ end
+ end
+ end
+ end
+ end # #copy_entry
+
describe "#safesystem" do
context "with a missing $SHELL" do
before do

0 comments on commit 4958b8f

Please sign in to comment.