build-runfiles: remove temporary file prior to creating it

When building with Remote Output Service, bb_clientd has the ability to
restore snapshots of previous bazel-out/ directories. Though the file
contents of these snapshots are identical to what's created in the past,
the files will be read-only. This is because the files may be shared by
multiple snapshots.

We have noticed that most of Bazel is fine with that. Most of the times
Bazel is a good citizen, where it removes any files before recreating
them. We did notice a very rare case where build-runfiles tries to make
in-place modifications to a temporary file that it maintains. This
change ensures that build-runfiles stops doing this.

src/main/tools/build-runfiles.cc

@@ -117,6 +117,10 @@ class RunfilesCreator {
 
  void ReadManifest(const std::string &manifest_file, bool allow_relative,
  bool use_metadata) {
+ if (unlink(temp_filename_.c_str()) != 0 && errno != ENOENT) {
+ PDIE("removing temporary file at '%s/%s'", output_base_.c_str(),
+ temp_filename_.c_str());
+ }
  FILE *outfile = fopen(temp_filename_.c_str(), "w");
  if (!outfile) {
  PDIE("opening '%s/%s' for writing", output_base_.c_str(),

src/test/shell/integration/runfiles_test.sh

@@ -427,5 +427,32 @@ EOF
  expect_log_once "Runfiles must not contain middleman artifacts"
 }
 
+function test_removal_of_old_tempfiles() {
+ cat > BUILD << EOF
+sh_binary(
+ name = "foo",
+ srcs = ["foo.sh"],
+)
+EOF
+ touch foo.sh
+ chmod +x foo.sh
+
+ # Build once to create a runfiles directory.
+ bazel build //:foo $EXTRA_BUILD_FLAGS >&$TEST_log || fail "build failed"
+
+ # Remove the MANIFEST file that was created by the previous build.
+ # Create an inaccessible file in the place where build-runfiles writes
+ # its temporary results.
+ rm ${PRODUCT_NAME}-bin/foo.runfiles/MANIFEST
+ touch ${PRODUCT_NAME}-bin/foo.runfiles/MANIFEST.tmp
+ chmod 0 ${PRODUCT_NAME}-bin/foo.runfiles/MANIFEST.tmp
+
+ # Even with the inaccessible temporary file in place, build-runfiles
+ # should complete successfully. The MANIFEST file should be recreated.
+ bazel build //:foo $EXTRA_BUILD_FLAGS >&$TEST_log || fail "build failed"
+ [[ -f ${PRODUCT_NAME}-bin/foo.runfiles/MANIFEST ]] \
+ || fail "MANIFEST file not recreated"
+}
+
 
 run_suite "runfiles"