From bdfee82844f76e5879d6883f9012f613a1c65781 Mon Sep 17 00:00:00 2001 From: Corry Haines Date: Mon, 1 Aug 2011 20:35:56 -0700 Subject: [PATCH] Add "safesystem" function Basically, its a replacement for system. If the command fails, then it raises an exception and prints out the entire command that was attempted. This will prevent issues where fpm finished (with return code 0) and produces an artifact with real size (>0 bytes) but the artifact is worthless as it contains nothing. Fixes #86 --- lib/fpm/builder.rb | 7 ++++--- lib/fpm/source.rb | 5 +++-- lib/fpm/source/dir.rb | 7 ++++--- lib/fpm/source/gem.rb | 5 +++-- lib/fpm/source/npm.rb | 5 +++-- lib/fpm/source/python.rb | 11 ++++------- lib/fpm/source/rpm.rb | 5 +++-- lib/fpm/source/tar.rb | 7 ++++--- lib/fpm/target/deb.rb | 13 +++++++------ lib/fpm/target/rpm.rb | 8 +++----- lib/fpm/target/solaris.rb | 13 +++++++------ lib/fpm/util.rb | 7 +++++++ 12 files changed, 52 insertions(+), 41 deletions(-) create mode 100644 lib/fpm/util.rb diff --git a/lib/fpm/builder.rb b/lib/fpm/builder.rb index f50fb4f63b..e2c9d2432a 100644 --- a/lib/fpm/builder.rb +++ b/lib/fpm/builder.rb @@ -1,4 +1,5 @@ require "fileutils" +require "fpm/util" require "pathname" class FPM::Builder @@ -102,10 +103,10 @@ def assemble! data_tarball = File.join(builddir, "data.tar.gz") Dir.chdir(builddir) do FileUtils.mkdir_p(@package.unpack_data_to) - system("gzip -d #{data_tarball}") + safesystem("gzip -d #{data_tarball}") Dir.chdir(@package.unpack_data_to) do @source.root = Dir.pwd - system("tar -xf #{data_tarball.gsub(/\.gz$/, "")}") + safesystem("tar -xf #{data_tarball.gsub(/\.gz$/, "")}") end end end @@ -185,7 +186,7 @@ def edit_specfile # TODO(sissel): support editing multiple files for targets like # puppet which generate multiple manifests. editor = ENV['FPM_EDITOR'] || ENV['EDITOR'] || 'vi' - system("#{editor} '#{package.specfile(builddir)}'") + safesystem("#{editor} '#{package.specfile(builddir)}'") unless File.size? package.specfile(builddir) puts "Empty specfile. Aborting." exit 1 diff --git a/lib/fpm/source.rb b/lib/fpm/source.rb index 50e6e78da5..4dd1b2f9f8 100644 --- a/lib/fpm/source.rb +++ b/lib/fpm/source.rb @@ -1,4 +1,5 @@ require "fpm/namespace" +require "fpm/util" # Abstract class for a "thing to build a package from" class FPM::Source @@ -124,7 +125,7 @@ def tar(output, paths, chdir=".") + dirs ::Dir.chdir(chdir) do - system(*dir_tar) if dirs.any? + safesystem(*dir_tar) if dirs.any? end files_tar = [ tar_cmd ] \ @@ -132,7 +133,7 @@ def tar(output, paths, chdir=".") + [ "--owner=root", "--group=root", "-rf", output ] \ + paths ::Dir.chdir(chdir) do - system(*files_tar) + safesystem(*files_tar) end end # def tar diff --git a/lib/fpm/source/dir.rb b/lib/fpm/source/dir.rb index 54adb58aee..fc2476b8b9 100644 --- a/lib/fpm/source/dir.rb +++ b/lib/fpm/source/dir.rb @@ -1,6 +1,7 @@ require "fpm/source" require "fileutils" require "fpm/rubyfixes" +require "fpm/util" class FPM::Source::Dir < FPM::Source def get_metadata @@ -31,7 +32,7 @@ def make_tarball!(tar_path, builddir) ::FileUtils.mkdir_p(dest) rsync = ["rsync", "-a", path, dest] p rsync - system(*rsync) + safesystem(*rsync) # FileUtils.cp_r is pretty silly about how it copies files in some # cases (funky permissions, etc) @@ -45,7 +46,7 @@ def make_tarball!(tar_path, builddir) end ::Dir.chdir("#{builddir}/tarbuild") do - system("ls #{builddir}/tarbuild") + safesystem("ls #{builddir}/tarbuild") tar(tar_path, ".") end else @@ -53,6 +54,6 @@ def make_tarball!(tar_path, builddir) end # TODO(sissel): Make a helper method. - system(*["gzip", "-f", tar_path]) + safesystem(*["gzip", "-f", tar_path]) end # def make_tarball! end # class FPM::Source::Dir diff --git a/lib/fpm/source/gem.rb b/lib/fpm/source/gem.rb index 8199fd19e3..3f21b8e324 100644 --- a/lib/fpm/source/gem.rb +++ b/lib/fpm/source/gem.rb @@ -3,6 +3,7 @@ require "rubygems/package" require "rubygems" require "fileutils" +require "fpm/util" class FPM::Source::Gem < FPM::Source def self.flags(opts, settings) @@ -136,14 +137,14 @@ def make_tarball!(tar_path, builddir) end args << gem - system(*args) + safesystem(*args) # make paths relative (/foo becomes ./foo) tar(tar_path, @paths.collect {|p| ".#{p}"}, tmpdir) FileUtils.rm_r(tmpdir) # TODO(sissel): Make a helper method. - system(*["gzip", "-f", tar_path]) + safesystem(*["gzip", "-f", tar_path]) end end # class FPM::Source::Gem diff --git a/lib/fpm/source/npm.rb b/lib/fpm/source/npm.rb index d647481de5..2d2eb68ca8 100644 --- a/lib/fpm/source/npm.rb +++ b/lib/fpm/source/npm.rb @@ -1,5 +1,6 @@ require "fpm/namespace" require "fpm/source" +require "fpm/util" require "fileutils" class FPM::Source::Npm < FPM::Source @@ -25,11 +26,11 @@ def make_tarball!(tar_path, builddir) ::FileUtils.mkdir_p(installdir) args = ["gem", "install", "--quiet", "--no-ri", "--no-rdoc", "--install-dir", installdir, "--ignore-dependencies", @paths.first] - system(*args) + safesystem(*args) tar(tar_path, ".", tmpdir) # TODO(sissel): Make a helper method. - system(*["gzip", "-f", tar_path]) + safesystem(*["gzip", "-f", tar_path]) end end # class FPM::Source::Gem diff --git a/lib/fpm/source/python.rb b/lib/fpm/source/python.rb index cb407367a0..9b394a225c 100644 --- a/lib/fpm/source/python.rb +++ b/lib/fpm/source/python.rb @@ -1,5 +1,6 @@ require "fpm/namespace" require "fpm/source" +require "fpm/util" require "rubygems/package" require "rubygems" require "fileutils" @@ -47,11 +48,7 @@ def download(package, version=nil) want_pkg = "#{package}==#{version}" end - return_value = system(self[:settings][:easy_install], "--editable", "--build-directory", @tmpdir, want_pkg) - - if return_value.nil? - raise "The execution of #{self[:settings][:easy_install]} failed" - end + safesystem(self[:settings][:easy_install], "--editable", "--build-directory", @tmpdir, want_pkg) # easy_install will put stuff in @tmpdir/packagename/, flatten that. # That is, we want @tmpdir/setup.py, and start with @@ -101,7 +98,7 @@ def make_tarball!(tar_path, builddir) # Some setup.py's assume $PWD == current directory of setup.py, so let's # chdir first. ::Dir.chdir(dir) do - system(self[:settings][:python], "setup.py", "bdist") + safesystem(self[:settings][:python], "setup.py", "bdist") end dist_tar = ::Dir.glob(File.join(dir, "dist", "*.tar.gz")).first @@ -110,7 +107,7 @@ def make_tarball!(tar_path, builddir) @paths = [ "." ] - system("cp", dist_tar, "#{tar_path}.gz") + safesystem("cp", dist_tar, "#{tar_path}.gz") end # def make_tarball! def garbage diff --git a/lib/fpm/source/rpm.rb b/lib/fpm/source/rpm.rb index 79ea814f54..c1f8c336fb 100644 --- a/lib/fpm/source/rpm.rb +++ b/lib/fpm/source/rpm.rb @@ -1,4 +1,5 @@ require "fpm/source" +require "fpm/util" class FPM::Source::RPM < FPM::Source def get_metadata @@ -18,10 +19,10 @@ def get_metadata def make_tarball!(tar_path, builddir) tmpdir = "#{tar_path}.dir" ::Dir.mkdir(tmpdir) - system("rpm2cpio #{@rpm} | (cd #{tmpdir}; cpio -i --make-directories)") + safesystem("rpm2cpio #{@rpm} | (cd #{tmpdir}; cpio -i --make-directories)") tar(tar_path, ".", tmpdir) @paths = ["."] # TODO(sissel): Make a helper method. - system(*["gzip", "-f", tar_path]) + safesystem(*["gzip", "-f", tar_path]) end end diff --git a/lib/fpm/source/tar.rb b/lib/fpm/source/tar.rb index cff326dc2b..a574d61f37 100644 --- a/lib/fpm/source/tar.rb +++ b/lib/fpm/source/tar.rb @@ -1,6 +1,7 @@ +require "fpm/rubyfixes" require "fpm/source" +require "fpm/util" require "fileutils" -require "fpm/rubyfixes" class FPM::Source::Tar < FPM::Source def get_metadata @@ -31,7 +32,7 @@ def make_tarball!(tar_path, builddir) end #puts("tar #{flags}") #sleep 5 - system("tar #{flags}") + safesystem("tar #{flags}") if self[:prefix] @paths = [self[:prefix]] @@ -44,6 +45,6 @@ def make_tarball!(tar_path, builddir) end # TODO(sissel): Make a helper method. - system(*["gzip", "-f", tar_path]) + safesystem(*["gzip", "-f", tar_path]) end # def make_tarball! end # class FPM::Source::Dir diff --git a/lib/fpm/target/deb.rb b/lib/fpm/target/deb.rb index 145d95c2ae..d7fca9bad0 100644 --- a/lib/fpm/target/deb.rb +++ b/lib/fpm/target/deb.rb @@ -2,6 +2,7 @@ require "fpm/namespace" require "fpm/package" require "fpm/errors" +require "fpm/util" class FPM::Target::Deb < FPM::Package @@ -66,16 +67,16 @@ def build!(params) self.scripts.each do |name, path| case name when "pre-install" - system("cp #{path} ./preinst") + safesystem("cp #{path} ./preinst") control_files << "preinst" when "post-install" - system("cp #{path} ./postinst") + safesystem("cp #{path} ./postinst") control_files << "postinst" when "pre-uninstall" - system("cp #{path} ./prerm") + safesystem("cp #{path} ./prerm") control_files << "prerm" when "post-uninstall" - system("cp #{path} ./postrm") + safesystem("cp #{path} ./postrm") control_files << "postrm" else raise "Unsupported script name '#{name}' (path: #{path})" end # case name @@ -87,13 +88,13 @@ def build!(params) end # Make the control - system("tar -zcf control.tar.gz #{control_files.map{ |f| "./#{f}" }.join(" ")}") + safesystem("tar -zcf control.tar.gz #{control_files.map{ |f| "./#{f}" }.join(" ")}") # create debian-binary File.open("debian-binary", "w") { |f| f.puts "2.0" } # pack up the .deb - system("ar -qc #{params[:output]} debian-binary control.tar.gz data.tar.gz") + safesystem("ar -qc #{params[:output]} debian-binary control.tar.gz data.tar.gz") end # def build def default_output diff --git a/lib/fpm/target/rpm.rb b/lib/fpm/target/rpm.rb index 742482e9b0..140ef15423 100644 --- a/lib/fpm/target/rpm.rb +++ b/lib/fpm/target/rpm.rb @@ -1,4 +1,5 @@ require "fpm/package" +require "fpm/util" class FPM::Target::Rpm < FPM::Package def architecture @@ -44,14 +45,11 @@ def build!(params) "--define", "_sourcedir #{Dir.pwd}", "--define", "_rpmdir #{Dir.pwd}/RPMS", "#{name}.spec"] - ret = system(*args) - if !ret - raise "rpmbuild failed (exit code: #{$?.exitstatus})" - end + safesystem(*args) Dir["#{Dir.pwd}/RPMS/**/*.rpm"].each do |path| # This should only output one rpm, should we verify this? - system("mv", path, params[:output]) + safesystem("mv", path, params[:output]) end end # def build! end # class FPM::Target::RPM diff --git a/lib/fpm/target/solaris.rb b/lib/fpm/target/solaris.rb index 020a44842d..b5b101bd95 100644 --- a/lib/fpm/target/solaris.rb +++ b/lib/fpm/target/solaris.rb @@ -2,6 +2,7 @@ require "fpm/namespace" require "fpm/package" require "fpm/errors" +require "fpm/util" # TODO(sissel): Add dependency checking support. # IIRC this has to be done as a 'checkinstall' step. @@ -25,10 +26,10 @@ def build!(params) self.scripts.each do |name, path| case name when "pre-install" - system("cp #{path} ./preinstall") + safesystem("cp #{path} ./preinstall") File.chmod(0755, "./preinstall") when "post-install" - system("cp #{path} ./postinstall") + safesystem("cp #{path} ./postinstall") File.chmod(0755, "./postinstall") when "pre-uninstall" raise FPM::InvalidPackageConfiguration.new( @@ -43,9 +44,9 @@ def build!(params) # Unpack data.tar.gz so we can build a package from it. Dir.mkdir("data") - system("gzip -d data.tar.gz"); + safesystem("gzip -d data.tar.gz"); Dir.chdir("data") do - system("tar -xf ../data.tar"); + safesystem("tar -xf ../data.tar"); end #system("(echo 'i pkginfo'; pkgproto data=/) > Prototype") @@ -69,10 +70,10 @@ def build!(params) end # File prototype # Should create a package directory named by the package name. - system("pkgmk -o -d .") + safesystem("pkgmk -o -d .") # Convert the 'package directory' built above to a real solaris package. - system("pkgtrans -s . #{params[:output]} #{name}") + safesystem("pkgtrans -s . #{params[:output]} #{name}") end # def build def default_output diff --git a/lib/fpm/util.rb b/lib/fpm/util.rb new file mode 100644 index 0000000000..240732d3a5 --- /dev/null +++ b/lib/fpm/util.rb @@ -0,0 +1,7 @@ +def safesystem(*call) + return_val = system(*call) + if !return_val + raise "'#{call}' failed with error code: #{$?.exitstatus}" + end + return return_val +end # def safesystem