From 0ea842fffe71de85d9eeba0614addc2a6032a2ec Mon Sep 17 00:00:00 2001 From: Josh Holtrop Date: Tue, 20 Jan 2015 21:16:21 -0500 Subject: [PATCH] rework Preprocess builder to consider deep dependencies - fix #21 --- README.md | 1 + build_tests/preprocess/bar.h | 1 + build_tests/preprocess/foo.h | 2 ++ lib/rscons/builders/preprocess.rb | 36 +++++++++++++++++------ lib/rscons/environment.rb | 5 ++-- spec/build_tests_spec.rb | 20 +++++++++++++ spec/rscons/builders/preprocess_spec.rb | 38 ++++++++++++++++++++++--- 7 files changed, 89 insertions(+), 14 deletions(-) create mode 100644 build_tests/preprocess/bar.h create mode 100644 build_tests/preprocess/foo.h diff --git a/README.md b/README.md index f465ccd..27bbfa3 100644 --- a/README.md +++ b/README.md @@ -386,6 +386,7 @@ http://rubydoc.info/github/holtrop/rscons/frames. ### v1.9.0 - fix Rscons.set_suffix to append the given suffix if the filename has none +- rework Preprocess builder to consider deep dependencies ### v1.8.1 diff --git a/build_tests/preprocess/bar.h b/build_tests/preprocess/bar.h new file mode 100644 index 0000000..c83f9fd --- /dev/null +++ b/build_tests/preprocess/bar.h @@ -0,0 +1 @@ +#define BAR xyz42abc diff --git a/build_tests/preprocess/foo.h b/build_tests/preprocess/foo.h new file mode 100644 index 0000000..6475078 --- /dev/null +++ b/build_tests/preprocess/foo.h @@ -0,0 +1,2 @@ +#include "bar.h" +BAR diff --git a/lib/rscons/builders/preprocess.rb b/lib/rscons/builders/preprocess.rb index 39a4065..b12dc57 100644 --- a/lib/rscons/builders/preprocess.rb +++ b/lib/rscons/builders/preprocess.rb @@ -1,7 +1,10 @@ +require "fileutils" + module Rscons module Builders # The Preprocess builder invokes the C preprocessor class Preprocess < Builder + # Return default construction variables for the builder. # # @param env [Environment] The Environment using the builder. @@ -9,7 +12,9 @@ module Rscons # @return [Hash] Default construction variables for the builder. def default_variables(env) { - "CPP_CMD" => ["${_PREPROCESS_CC}", "-E", "-o", "${_TARGET}", "-I${CPPPATH}", "${CPPFLAGS}", "${CFLAGS}", "${_SOURCES}"], + "CPP_CMD" => %w[ + ${_PREPROCESS_CC} -E ${_PREPROCESS_DEPGEN} + -o ${_TARGET} -I${CPPPATH} ${CPPFLAGS} ${_SOURCES}], } end @@ -24,17 +29,32 @@ module Rscons # @return [String,false] # Name of the target file on success or false on failure. def run(target, sources, cache, env, vars) - pp_cc = if sources.find {|s| s.end_with?(*env.expand_varref("${CXXSUFFIX}", vars))} - "${CXX}" - else - "${CC}" - end + if sources.find {|s| s.end_with?(*env.expand_varref("${CXXSUFFIX}", vars))} + pp_cc = "${CXX}" + depgen = "${CXXDEPGEN}" + else + pp_cc = "${CC}" + depgen = "${CCDEPGEN}" + end vars = vars.merge("_PREPROCESS_CC" => pp_cc, + "_PREPROCESS_DEPGEN" => depgen, "_TARGET" => target, - "_SOURCES" => sources) + "_SOURCES" => sources, + "_DEPFILE" => Rscons.set_suffix(target, env.expand_varref("${DEPFILESUFFIX}", vars))) command = env.build_command("${CPP_CMD}", vars) - standard_build("Preprocess #{target}", target, command, sources, env, cache) + unless cache.up_to_date?(target, command, sources, env) + cache.mkdir_p(File.dirname(target)) + return false unless env.execute("Preprocess #{target}", command) + deps = sources + if File.exists?(vars["_DEPFILE"]) + deps += Environment.parse_makefile_deps(vars["_DEPFILE"], nil) + FileUtils.rm_f(vars["_DEPFILE"]) + end + cache.register_build(target, command, deps.uniq, env) + end + target end + end end end diff --git a/lib/rscons/environment.rb b/lib/rscons/environment.rb index b2021c9..808393e 100644 --- a/lib/rscons/environment.rb +++ b/lib/rscons/environment.rb @@ -696,7 +696,8 @@ module Rscons # This method is used internally by Rscons builders. # # @param mf_fname [String] File name of the Makefile to read. - # @param target [String] Name of the target to gather dependencies for. + # @param target [String, nil] + # Name of the target to gather dependencies for, nil for any/all. # # @return [Array] Paths of dependency files. def self.parse_makefile_deps(mf_fname, target) @@ -709,7 +710,7 @@ module Rscons buildup += ' ' + line if buildup =~ /^(.*): (.*)$/ mf_target, mf_deps = $1.strip, $2 - if mf_target == target + if target.nil? or mf_target == target deps += mf_deps.split(' ').map(&:strip) end end diff --git a/spec/build_tests_spec.rb b/spec/build_tests_spec.rb index 80e3de7..6fb7ee1 100644 --- a/spec/build_tests_spec.rb +++ b/spec/build_tests_spec.rb @@ -734,4 +734,24 @@ EOF expect(result.include?(%{CPPPATH => []})).to be_truthy end + it "considers deep dependencies when deciding whether to rerun Preprocess builder" do + test_dir("preprocess") + env = Rscons::Environment.new do |env| + env.Preprocess("pp", "foo.h") + end + expect(File.read("pp")).to match(%r{xyz42abc}m) + expect(lines).to eq(["Preprocess pp"]) + env.Preprocess("pp", "foo.h") + env.process + expect(lines).to eq([]) + File.open("bar.h", "w") do |fh| + fh.puts "#define BAR abc88xyz" + end + $ttt = true + env.Preprocess("pp", "foo.h") + env.process + expect(lines).to eq(["Preprocess pp"]) + expect(File.read("pp")).to match(%r{abc88xyz}m) + end + end diff --git a/spec/rscons/builders/preprocess_spec.rb b/spec/rscons/builders/preprocess_spec.rb index bc07115..0f56936 100644 --- a/spec/rscons/builders/preprocess_spec.rb +++ b/spec/rscons/builders/preprocess_spec.rb @@ -1,18 +1,48 @@ module Rscons module Builders describe Preprocess do + let(:env) {Environment.new} subject {Preprocess.new} it "supports overriding CC construction variable" do - expect(subject).to receive(:standard_build).with("Preprocess module.pp", "module.pp", ["my_cpp", "-E", "-o", "module.pp", "module.c"], ["module.c"], env, :cache) - subject.run("module.pp", ["module.c"], :cache, env, "CC" => "my_cpp") + cache = double(Cache) + command = %w[my_cpp -E -MMD -MF module.mf -o module.pp module.c] + expect(cache).to receive(:up_to_date?).with("module.pp", command, %w[module.c], env).and_return(false) + expect(cache).to receive(:mkdir_p).with(".") + expect(env).to receive(:execute).with("Preprocess module.pp", command).and_return(true) + expect(File).to receive(:exists?).with("module.mf").and_return(true) + expect(Environment).to receive(:parse_makefile_deps).with("module.mf", nil).and_return(%w[module.c one.h two.h]) + expect(FileUtils).to receive(:rm_f).with("module.mf") + expect(cache).to receive(:register_build).with("module.pp", command, %w[module.c one.h two.h], env) + + expect(subject.run("module.pp", ["module.c"], cache, env, "CC" => "my_cpp")).to eq("module.pp") end it "supports overriding CPP_CMD construction variable" do - expect(subject).to receive(:standard_build).with("Preprocess module.pp", "module.pp", ["my_cpp", "module.c"], ["module.c"], env, :cache) - subject.run("module.pp", ["module.c"], :cache, env, "CPP_CMD" => ["my_cpp", "${_SOURCES}"]) + cache = double(Cache) + command = %w[my_cpp module.c] + expect(cache).to receive(:up_to_date?).with("module.pp", command, %w[module.c], env).and_return(false) + expect(cache).to receive(:mkdir_p).with(".") + expect(env).to receive(:execute).with("Preprocess module.pp", command).and_return(true) + expect(File).to receive(:exists?).with("module.mf").and_return(true) + expect(Environment).to receive(:parse_makefile_deps).with("module.mf", nil).and_return(%w[module.c one.h two.h]) + expect(FileUtils).to receive(:rm_f).with("module.mf") + expect(cache).to receive(:register_build).with("module.pp", command, %w[module.c one.h two.h], env) + + expect(subject.run("module.pp", ["module.c"], cache, env, "CPP_CMD" => ["my_cpp", "${_SOURCES}"])).to eq("module.pp") end + + it "returns false if executing the preprocessor fails" do + cache = double(Cache) + command = %w[gcc -E -MMD -MF module.mf -o module.pp module.c] + expect(cache).to receive(:up_to_date?).with("module.pp", command, %w[module.c], env).and_return(false) + expect(cache).to receive(:mkdir_p).with(".") + expect(env).to receive(:execute).with("Preprocess module.pp", command).and_return(false) + + expect(subject.run("module.pp", ["module.c"], cache, env, {})).to eq(false) + end + end end end