rework Preprocess builder to consider deep dependencies - fix #21

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
 

build_tests/preprocess/bar.h

@@ -0,0 +1 @@
+#define BAR xyz42abc

build_tests/preprocess/foo.h

@@ -0,0 +1,2 @@
+#include "bar.h"
+BAR

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

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<String>] 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

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

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