From 24ed649982083215e2a777eeb73fa64ac2290784 Mon Sep 17 00:00:00 2001 From: John Lindgren Date: Tue, 3 Mar 2020 17:32:52 -0500 Subject: [PATCH] Check that user dependencies exist before running builders. IMHO it's more intuitive if env.depends("my-target", "invalid-dep") causes the build to fail when "invalid-dep" does not exist, rather than having no effect (i.e. the build still succeeds with no warning). This will make it more obvious, earlier, if you made a mistake in an env.depends() call. Otherwise the mistake could sit latent for a while until you happen to notice that a build used a out-of-date copy of some file rather than rebuilding it. I've only added the check for user dependencies, not for direct sources. Listing a source file that doesn't exist will probably cause the builder to fail anyway, so I don't think the preliminary check is needed in that case. --- build_tests/simple/new_dep | 1 + lib/rscons/cache.rb | 20 ++++++++++---------- lib/rscons/environment.rb | 21 +++++++++++++++++++++ spec/build_tests_spec.rb | 10 +++++----- 4 files changed, 37 insertions(+), 15 deletions(-) create mode 100644 build_tests/simple/new_dep diff --git a/build_tests/simple/new_dep b/build_tests/simple/new_dep new file mode 100644 index 0000000..bf05ff7 --- /dev/null +++ b/build_tests/simple/new_dep @@ -0,0 +1 @@ +# This file is referenced by cache_debugging.rb diff --git a/lib/rscons/cache.rb b/lib/rscons/cache.rb index d7f3502..aee4243 100644 --- a/lib/rscons/cache.rb +++ b/lib/rscons/cache.rb @@ -282,6 +282,16 @@ module Rscons @cache["directories"].keys end + # Return a file's checksum, or the previously calculated checksum for + # the same file. + # + # @param file [String] The file name. + # + # @return [String] The file's checksum. + def lookup_checksum(file) + @lookup_checksums[file] || calculate_checksum(file) + end + private # Return a String key based on the target name to use in the on-disk cache. @@ -312,16 +322,6 @@ module Rscons @lookup_checksums = {} end - # Return a file's checksum, or the previously calculated checksum for - # the same file. - # - # @param file [String] The file name. - # - # @return [String] The file's checksum. - def lookup_checksum(file) - @lookup_checksums[file] || calculate_checksum(file) - end - # Calculate and return a file's checksum. # # @param file [String] The file name. diff --git a/lib/rscons/environment.rb b/lib/rscons/environment.rb index c1a3471..5f26a81 100644 --- a/lib/rscons/environment.rb +++ b/lib/rscons/environment.rb @@ -341,6 +341,7 @@ module Rscons cache.clear_checksum_cache! if job + validate_user_deps(job[:target], @user_deps[job[:target]], cache) result = run_builder(job[:builder], job[:target], job[:sources], @@ -1107,5 +1108,25 @@ module Rscons end deps end + + # Ensures that user dependencies exist with valid checksums in the + # cache. Raises an exception if any dependency is invalid. + # + # @param target [String] + # Target to be built + # @param deps [Array, nil] + # User dependencies of the target + # @param cache [Cache] + # Rscons cache instance + # + # @return [void] + def validate_user_deps(target, user_deps, cache) + return if user_deps.nil? + user_deps.each do |dep| + if cache.lookup_checksum(dep) == "" + raise "User dependency #{dep} of target #{target} is invalid" + end + end + end end end diff --git a/spec/build_tests_spec.rb b/spec/build_tests_spec.rb index 55ae5ec..fcd0b45 100644 --- a/spec/build_tests_spec.rb +++ b/spec/build_tests_spec.rb @@ -417,14 +417,14 @@ EOF expect(result.stderr).to eq "" expect(lines(result.stdout)).to eq(["LD simple.exe"]) - File.unlink("program.ld") - result = run_test(rsconsfile: "user_dependencies.rb") - expect(result.stderr).to eq "" - expect(lines(result.stdout)).to eq ["LD simple.exe"] - result = run_test(rsconsfile: "user_dependencies.rb") expect(result.stderr).to eq "" expect(result.stdout).to eq "" + + File.unlink("program.ld") + result = run_test(rsconsfile: "user_dependencies.rb") + expect(result.stderr).to match /User dependency program\.ld of target simple\.exe is invalid/ + expect(result.status).to_not eq 0 end unless ENV["omit_gdc_tests"]