diff --git a/src/GeneratorCache.cpp b/src/GeneratorCache.cpp index 94c023b1af89..80c8ab2ece59 100644 --- a/src/GeneratorCache.cpp +++ b/src/GeneratorCache.cpp @@ -637,6 +637,14 @@ bool GeneratorCache::try_restore(const std::string &key, if (ec) { // Fall back to a copy if rename across the tmp/dest pair failed. fs::copy_file(p.first, p.second, fs::copy_options::overwrite_existing, ec); + if (ec) { + const std::string message = ec.message(); + for (const auto &pending_file : pending) { + fs::remove(pending_file.first, ec); + } + user_error << "GeneratorCache: failed to restore " << p.second.string() + << ": " << message << "\n"; + } fs::remove(p.first, ec); } } diff --git a/src/GeneratorCache.h b/src/GeneratorCache.h index 2942b9c1be0e..f6bed084b133 100644 --- a/src/GeneratorCache.h +++ b/src/GeneratorCache.h @@ -79,8 +79,9 @@ struct GeneratorCache { /** If a complete entry for `key` exists in the cache, copy each cached * file into the destination path given by `output_files` and return - * true. Otherwise leave the filesystem untouched and return false. A - * partial or corrupt entry is treated as a miss. */ + * true only if all outputs were restored. An output replacement failure + * is reported as a user error; some outputs may already have been replaced. + * A partial or corrupt entry is treated as a miss. */ static bool try_restore(const std::string &key, const std::map &output_files); diff --git a/test/correctness/generator_cache.cpp b/test/correctness/generator_cache.cpp index b6d48ebb8ad9..c71c268c50a1 100644 --- a/test/correctness/generator_cache.cpp +++ b/test/correctness/generator_cache.cpp @@ -51,7 +51,16 @@ int generate_one(const std::string &outdir, const std::string &offset) { args.targets = {get_host_target()}; args.generator_name = "cache_add"; args.generator_params = {{"offset", offset}}; - Internal::execute_generator(args); +#ifdef HALIDE_WITH_EXCEPTIONS + try { +#endif + Internal::execute_generator(args); +#ifdef HALIDE_WITH_EXCEPTIONS + } catch (const CompileError &e) { + std::cerr << e.what(); + return 1; + } +#endif return 0; } @@ -162,6 +171,25 @@ int main(int argc, char **argv) { check(read_all(obj_c) != sentinel); check(read_all(obj_c) != real_obj); + // A complete cache entry is not a hit if its output cannot be replaced. + // A nonempty directory blocks both rename and copy on every platform, + // including when the test runs with permission to write read-only files. + const fs::path blocked_dir = tmp / "blocked"; + const fs::path blocked_obj = blocked_dir / ("cache_add" + obj_ext); + fs::create_directories(blocked_obj); + { std::ofstream marker(blocked_obj / "keep"); } + const int blocked_status = Internal::run_process( + {self, "--gen", blocked_dir.string(), "2"}); + check(blocked_status != 0); + check(fs::exists(blocked_obj / "keep")); + check(!fs::exists(blocked_obj.string() + ".hlcache.tmp")); + check(!fs::exists((blocked_dir / "cache_add.h").string() + ".hlcache.tmp")); + + // Once the obstruction is removed, the cached implementation restores. + fs::remove_all(blocked_obj); + check(Internal::run_process({self, "--gen", blocked_dir.string(), "2"}) == 0); + check(read_all(blocked_obj) == read_all(obj_c)); + // 4. Corrupt every cache entry (drop manifests): the next offset=1 run must // treat the entry as a miss and recompile the real object. for (const auto &e : fs::recursive_directory_iterator(entries)) {