speed up handling of registered side-effect files - #92

build_tests/custom_builder/produces.rb

@@ -7,10 +7,10 @@ build do
     env.depends("${build_root}/program.o", "${inc_h}")
     env.Program("program.exe", ["program.c", "inc.c"])
 
-    env.Command("inc.c",
+    inc_c = env.Command("inc.c",
                         [],
                         "CMD" => %w[ruby gen.rb ${_TARGET}],
                         "CMD_DESC" => "Generating")
-    env.produces("inc.c", "inc.h")
+    inc_c.produces("inc.h")
   end
 end

build_tests/custom_builder/produces_env.rb

@@ -0,0 +1,16 @@
+build do
+  Environment.new do |env|
+    env["build_root"] = env.build_root
+    env["inc_h"] = "inc.h"
+
+    env.Copy("copy_inc.h", "${inc_h}")
+    env.depends("${build_root}/program.o", "${inc_h}")
+    env.Program("program.exe", ["program.c", "inc.c"])
+
+    env.Command("inc.c",
+                [],
+                "CMD" => %w[ruby gen.rb ${_TARGET}],
+                "CMD_DESC" => "Generating")
+    env.produces("inc.c", "inc.h")
+  end
+end

build_tests/simple/error_produces_nonexistent_target.rb

@@ -0,0 +1,5 @@
+build do
+  Environment.new do |env|
+    env.produces("foo", "bar")
+  end
+end

lib/rscons/builder.rb

@@ -47,6 +47,10 @@ module Rscons
     #   Construction variables used to perform the build operation.
     attr_accessor :vars
 
+    # @return [Set<String>]
+    #   Side effect file(s) produced when this builder runs.
+    attr_accessor :side_effects
+
     # @return [Integer]
     #   Build step.
     attr_accessor :build_step
@@ -71,6 +75,7 @@ module Rscons
       @cache = options[:cache]
       @env = options[:env]
       @vars = options[:vars]
+      @side_effects = Set.new
     end
 
     # Return the name of the builder.
@@ -100,6 +105,18 @@ module Rscons
       @env.depends(@target, *user_deps)
     end
 
+    # Manually record the given side effect file(s) as being produced when the
+    # named target is produced.
+    #
+    # @return [void]
+    def produces(*side_effects)
+      side_effects.each do |side_effect|
+        side_effect_expanded = @env.expand_path(@env.expand_varref(side_effect))
+        @env.register_side_effect(side_effect_expanded)
+        @side_effects << side_effect_expanded
+      end
+    end
+
     # Run the builder to produce a build target.
     #
     # @param options [Hash]

lib/rscons/builder_set.rb

@@ -8,10 +8,10 @@ module Rscons
     #   Hash mapping targets to a set of build dependencies. A builder will not
     #   be returned as ready to run if any of its dependencies are still
     #   building.
-    # @param side_effects [Hash]
-    #   Hash mapping targets to a set of side-effect files. A builder will not
-    #   be returned as ready to run if any of its dependencies is a side-effect
-    #   of another target that has not yet been built.
+    # @param side_effects [Set]
+    #   Set of side-effect files. A builder will not be returned as ready to
+    #   run if any of its dependencies is a side-effect of another target that
+    #   has not yet been built.
     def initialize(build_dependencies, side_effects)
       super()
       @build_dependencies = build_dependencies
@@ -53,22 +53,24 @@ module Rscons
     # @return [nil, Builder]
     #   The next builder to run.
     def get_next_builder_to_run(targets_still_building)
-      not_built_yet = targets_still_building + self.keys
-      not_built_yet += not_built_yet.reduce([]) do |result, target|
-        result + (@side_effects[target] || [])
+      to_build = self.find do |target, builders|
+        deps = builders.first.sources + (@build_dependencies[target] || []).to_a
+        # All dependencies must have been built for this target to be ready to
+        # build.
+        deps.all? do |dep|
+          !(targets_still_building.include?(dep) ||
+            self.include?(dep) ||
+            @side_effects.include?(dep))
+        end
       end
 
-      target_to_build = self.keys.find do |target|
-        deps = self[target][0].sources + (@build_dependencies[target] || []).to_a
-        !deps.find {|dep| not_built_yet.include?(dep)}
-      end
-
-      if target_to_build
-        builder = self[target_to_build][0]
-        if self[target_to_build].size > 1
-          self[target_to_build].slice!(0)
+      if to_build
+        target, builders = *to_build
+        builder = builders.first
+        if builders.size > 1
+          builders.slice!(0)
         else
-          self.delete(target_to_build)
+          self.delete(target)
         end
         return builder
       end

lib/rscons/builders/mixins/object.rb

@@ -81,7 +81,7 @@ module Rscons
               end
             @cache.mkdir_p(File.dirname(@vars["_DEPFILE"]))
             command = @env.build_command(@command_template, @vars)
-            @env.produces(@target, @vars["_DEPFILE"])
+            self.produces(@vars["_DEPFILE"])
             if @vars[:direct]
               message = "#{@short_description}/Linking <source>#{Util.short_format_paths(@sources)}<reset> => <target>#{@target}<reset>"
             else

lib/rscons/builders/preprocess.rb

@@ -25,7 +25,7 @@ module Rscons
           @vars["_SOURCES"] = @sources
           @vars["_DEPFILE"] = Rscons.set_suffix(target, env.expand_varref("${DEPFILESUFFIX}", vars))
           command = @env.build_command("${CPP_CMD}", @vars)
-          @env.produces(@target, @vars["_DEPFILE"])
+          self.produces(@vars["_DEPFILE"])
           standard_command("Preprocessing <source>#{Util.short_format_paths(@sources)}<reset> => <target>#{@target}<reset>", command)
         end
       end

lib/rscons/environment.rb

@@ -72,7 +72,8 @@ module Rscons
       # Hash of Thread object => {Command} or {Builder}.
       @threads = {}
       @registered_build_dependencies = {}
-      @side_effects = {}
+      # Set of side-effect files that have not yet been built.
+      @side_effects = Set.new
       @builder_sets = []
       @build_targets = {}
       @user_deps = {}
@@ -412,11 +413,25 @@ module Rscons
     # @return [void]
     def produces(target, *side_effects)
       target = expand_path(expand_varref(target))
-      side_effects = Array(side_effects).map do |side_effect|
-        expand_path(expand_varref(side_effect))
-      end.flatten
-      @side_effects[target] ||= []
-      @side_effects[target] += side_effects
+      @builder_sets.reverse.each do |builder_set|
+        if builders = builder_set[target]
+          builders.last.produces(*side_effects)
+          return
+        end
+      end
+      raise "Could not find a registered build target #{target.inspect}"
+    end
+
+    # Register a side effect file.
+    #
+    # This is an internally used method.
+    #
+    # @api private
+    #
+    # @param side_effect [String]
+    #   Side effect fiel name.
+    def register_side_effect(side_effect)
+      @side_effects << side_effect
     end
 
     # Return the list of user dependencies for a given target.
@@ -606,8 +621,9 @@ module Rscons
       when true
         # Register side-effect files as build targets so that a Cache
         # clean operation will remove them.
-        (@side_effects[builder.target] || []).each do |side_effect_file|
-          Cache.instance.register_build(side_effect_file, nil, [], self)
+        builder.side_effects.each do |side_effect|
+          Cache.instance.register_build(side_effect, nil, [], self)
+          @side_effects.delete(side_effect)
         end
         @build_hooks[:post].each do |build_hook_block|
           build_hook_block.call(builder)

spec/build_tests_spec.rb

@@ -324,6 +324,12 @@ EOF
     expect(IO.read('foo.yml')).to eq("---\nkey: value\n")
   end
 
+  it "raises an error when a side-effect file is registered for a build target that is not registered" do
+    test_dir "simple"
+    result = run_rscons(rsconscript: "error_produces_nonexistent_target.rb")
+    expect(result.stderr).to match /Could not find a registered build target "foo"/
+  end
+
   context "clean operation" do
     it 'cleans built files' do
       test_dir("simple")
@@ -1562,12 +1568,19 @@ EOF
       expect(result.stderr).to eq ""
     end
 
-    it "allows the user to specify side-effect files produced by another builder" do
+    it "allows the user to specify side-effect files produced by another builder with Builder#produces" do
       test_dir("custom_builder")
       result = run_rscons(rsconscript: "produces.rb", rscons_args: %w[-j 4])
       expect(result.stderr).to eq ""
       expect(File.exists?("copy_inc.h")).to be_truthy
     end
+
+    it "allows the user to specify side-effect files produced by another builder with Environment#produces" do
+      test_dir("custom_builder")
+      result = run_rscons(rsconscript: "produces_env.rb", rscons_args: %w[-j 4])
+      expect(result.stderr).to eq ""
+      expect(File.exists?("copy_inc.h")).to be_truthy
+    end
   end
 
   context "CLI" do