Bazel: Updated build file referenced by new_http_archive doesn't get used or trigger rebuild

[pbos]

Description of the problem / feature request / question:

My WORKSPACE contains a new_http_archive entry:

new_http_archive(
    name = "foo",
    url = "http://path.to.foo/foo-1.3.0.zip",
    sha256 = "deadbeefdeadbeef",
    strip_prefix = "Foo-1.3.0",
    build_file = "third_party/foo.BUILD",
)

When I first fetched this, my foo.BUILD file was broken (contained foo/include/bar.h instead of include/bar.h). I updated foo.BUILD to contain the new path, nothing, same error. Looking into bazel-<workspace>/external/foo/BUILD.bazel I see that this contains the old build file, the updated one doesn't get copied over when the target builds. I updated BUILD.bazel here just to test my changes, but whenever I build this file gets overwritten (not by my updated foo.BUILD but rather a cached version from somewhere else). I had to clear my tmpdir for this to actually pull in the new file.

Once this actually built, I added a syntax error, "asdf", inside my foo.BUILD, which should break the build completely, but changes in this file doesn't trigger a rebuild. If I nuke the tmpdir and external/foo, I force a rebuild which breaks. FWIW blaze clean isn't enough to trigger a rebuild in external/ either.

If possible, provide a minimal example to reproduce the problem:

Pull down an external dependency with new_http_archive, use a custom build file. After building a target that depends on this target, break the target build file by inserting garbage.

Expected: Build failure, build file contains garbage.
Actual: Silent success, no rebuild triggers.

Environment info

• Operating System:

Windows 10

• Bazel version (output of bazel info release):

release 0.5.2

[laszlocsomor]

Oh no, this is a nasty correctness bug...

Here's a simple repro:

"c:\tempdir\scratchWORKSPACE":

new_http_archive(
    name = "sfml",
    # Original URL: url = "https://www.sfml-dev.org/files/SFML-2.4.2-windows-vc14-64-bit.zip",
    # Had to download to work around issue #3343
    url = "http://localhost:8000/SFML-2.4.2-windows-vc14-64-bit.zip",
    sha256 = "93f372582f7216a7f41c18b8f46cd1c6f26881c695b05747aa08b1fd06072dd7",
    build_file = "C:/tempdir/scratch/BUILD.blah",
)

"c:\tempdir\scratch\BUILD":

cc_library(
    name = "foo",
    deps = ["@sfml//:foo"],
)

"c:\tempdir\scratch\BUILD.blah":

cc_library(
  name = "foo",
  visibility = ["//visibility:public"],
  hdrs = ["include/SFML/Main.hpp"],
)

Then:

c:\tempdir\scratch>c:\work\bazel-0.5.2\bazel.exe build //:foo
...
(13:30:39) INFO: Build completed successfully, 1 total action

c:\tempdir\scratch>echo blah > BUILD.blah

c:\tempdir\scratch>c:\work\bazel-0.5.2\bazel.exe build //:foo
...
(13:30:51) INFO: Build completed successfully, 1 total action

[laszlocsomor]

I can't repro on Linux, only on Windows. There however not even bazel shutdown nor bazel clean helps -- i must use a different --output_user_root altogether, or update the rule definition.

[kchodorow]

@meteorcloudy fixed this in #3093, I think. (And you can do a bazel clean --expunge to reset external repositories, although obviously you shouldn't have to!)

[laszlocsomor]

Unfortunately I can still repro while using bazel built at head.

[meteorcloudy]

@laszlocsomor Can you repo after bazel clean --expunge? Perhaps Bazel@HEAD was reading wrong marker data generated by previous Bazel.

[laszlocsomor]

Yay! That did the trick. The bug is fixed at head indeed.

[kchodorow]

Phew!

[pbos]

Will choco install do the equivalent of bazel clean --expunge or otherwise remove invalid markers, or does an upgrading user need to be aware of this command to have a correct build environment even in a future release?

[meteorcloudy]

@pbos Yes, you are right. choco install won't remove invalid markers. Users should run bazel clean --expunge first to get a correct build environment after updating to the next release.

[pbos]

This means that this is a latent bug until users happen to run it (e.g. not fixed by updating to HEAD). Since this isn't the last time this kind of bug will happen, would it be good to have some mechanism to inform future versions of bazel that they need to clean themselves?

Chrome has a concept called landmines, so if anyone checks out a revision past a newer landmine, it'll clobber the build and do a full rebuild (to be able to fully reset bad build states).

If Bazel had some "landmine marker" in its output dir, that marker content could be updated in the build whenever something requiring a full clean is submitted, and if output_dir/landmine content doesn't equal the build landmine, do a full clean. That way bazel doesn't have to be format-compatible with earlier output dirs. Whenever fixing a bug that corrects a broken build state, update the landmine string, so future versions don't have to take previous broken build states into account.

[meteorcloudy]

@pbos Thanks for your proposal.
@damienmg pointed out to me that we just need to update MARKER_VERSION_NUMBER in https://github.com/bazelbuild/bazel/blob/d02aabf07892ccd58d8a3ec08ba9ae805507be22/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorFunction.java#L62

[pbos]

Perfect, nice that there's already a method for it. Thanks for fixing that, it makes upgrade paths easier. :)