bazel: better support for temporarily using a local dependency repository

[jsedgwick]

Description:
I was working with envoy in #2771 and simultaneously with data-plane-api in this PR. I didn't want to have to push a new branch to dpapi (sorry, my fingers will eventually fall off from typing that out) and then point bazel at it every time I made a change just to build envoy.

Local repos are supported-ish with the current git_repository() rules but bazel does have a local_repository() rule for this purpose. This PR adds support for it. Now, as documented, you can stick a local_path w/ absolute path in the dep and you'll build against it.

local_path is the only parameter for local_repository(), you can't point at a specific commit or branch; it just uses whatever is currently checked out. I thought this was surprising enough to put a warning in when you mix local_path and other parameters.

I tried (like, tried hard) to make local overrides live in a separate gitignored file. Unfortunately, Bazel's load stage is so totally hermetic that I just couldn't make it depend on an untracked file that might not exist. It was sad. Instead, I added a pre-push hook to catch people accidentally committing and pushing local deps.

Here is an example of the hook output when it doesn't pass:
https://gist.github.com/jsedgwick/19797d74d2a19201faf909127132f5d4

p.s. I learned a little about git's "pickaxe" feature for the hook. It's pretty sweet and you could definitely do some crazy, well, digging if you knew how to use it, which I don't yet. Take a look.

Risk Level: Low. Build system stuff. Main risk is that it's easier to push your temporary local pointer, but there's a pre-push hook to try and prevent that. Hook can be bypassed w/ --no-verify if necessary.

Testing:
I put this together while working on dependent branches in data-plane-api and envoy, and it works. Adding, removing, or changing the path triggers an envoy rebuild. Changing the contents of the local repo triggers an envoy rebuild. All in all, iterative development works.

I manually tested the hook with various scenarios (push w/ no offending commit, push w/ offending commit, etc) and it works as expected.

Docs Changes:
In code, N/A

Release Notes:
N/A

[junr03]

@jmillikin-stripe or @dnoe can you take a first pass?

[jmillikin-stripe]

I'm slightly -1 on putting the local_path logic into repository_locations.bzl, due to the complexity and extra error handling required.

For your use case, is it practical to put the local_repository statements directly into WORKSPACE? The if name in existing_rule_keys: line in _repository_impl() means that external repositories can be overridden by defining them before loading the main dependency list.

[htuch]

Thanks, this is a nice cross-repo productivity boost.

[htuch]

@jmillikin-stripe I think it's simpler for developers to just find existing defs and add an override rather than having to think about what Bazel's local_repository setup looks like. This is for quick-and-dirty local development, not long term maintenance. I know I just want a quick "override that stuff" capability. That said, I do think WORKSPACE is the technically correct place to put it in a Bazel flow.

I don't feel that strongly about this at the end of the day, so up to you and @jsedgwick.

[jsedgwick]

@jmillikin-stripe My reasoning was pretty much exactly what @htuch said. WORKSPACE is likely technically correct, but this is a productivity hack so we want the friendliest option. I'd rather the complexity be buried in repositories.bzl, which users hopefully won't have to look at, than be pushed out to the docs. We'd have to explain how to use local_repository(), WORKSPACE, etc.

FWIW, I put this in WORKSPACE at one point but decided it was confusing to have the original in one place and the override in another.

[jsedgwick]

Interesting: git update-index --assume-unchanged bazel/repository_locations.bzl is a hacky way of keeping your locations file from appearing as changed according to git. I say hacky because if you read git help update-index it definitely wasn't intended for this purpose.

[lizan]

A bit late here, I'm with @jmillikin-stripe. Unless you are overriding tons of dependencies, the --override_repository command line option does exactly what you need. We might just need to document it better as many people are working with data-plane-api at same time.

[mattklein123]

@lizan hmm. OK. I didn't realize there was a CLI option that does this. That seems better. Feel free to revert this PR and doc the CLI option and we can discuss in that PR if that is the right approach or not. Thank you.

[lizan]

I think your #2816 will also revert this PR, I will open a PR soon to document --override_repository.

[mattklein123]

@lizan no, I don't think it does?