Turn dependency dupes into errors

Review Request #1332 — Created Nov. 14, 2014 and submitted

ity, jsirois, patricklaw, stuhood

We see a bunch of dupes pop up in dependency lists in our internal repos. Often they go unnoticed by whoever should be fixing them. They also make each parsing of the build graph quite noisy for the rest of our users.

This branch turns those warnings into errors. That ought to ensure they're caught early by whoever added the duplicate dependency and they won't hit master.

ci.sh passes
tested against a dependency list with dupes

  • 1
  • 0
  • 2
  • 0
  • 3
Description From Last Updated
nit: The short description should be a single line and one sentence. The rest of the descriptions (that could exceed ... PA patricklaw
  2. src/python/pants/base/build_graph.py (Diff revision 1)

    This should be its own error type, in case the caller wants to suppress the error for whatever reason.

    The change should also include a test that exercises the new error path and type.

  3. src/python/pants/base/build_graph.py (Diff revision 1)

    Indentation is off.

  1. lgtm minus nit

  2. nit: The short description should be a single line and one sentence. The rest of the descriptions (that could exceed a line) then goes after with a blank line between them.

  1. Ship It!

Review request changed

Status: Closed (submitted)

  1. FYI, this change breaks my repo and I'm not sure what the extent of the damage is yet. In the future, I would like to land changes like this behind a flag and/or with a deprecation message to give me a grace period to fix things up.

    1. The error that I get from this is kind of cryptic:

      Exception message: SyntheticAddress(.pants.d/gen/protoc/gen-java:lighthouse.api.src.main.proto.proto) already depends on BuildFileAddress(/Users/zundel/Development/java/3rdparty/BUILD.gen, com.google.protobuf.protobuf-java)

      It would be better if the error message included the path to the BUILD file that had the problem.

      What's happening in my build is that I have java_protobuf_library() targets with

      dependencies = [ '3rdparty:protobuf-java', ... ]

      in them. I think the issue its complaining about is that the protobuf-gen logic already injected this implicit dependency.

      I think the intention is to keep out duplicate dependencies declared in a single BUILD target. Maybe there is another way to do this? I'm thinking checking the dependencies [] attribute when parse the BUILD file. e.g. in target_addresable.py. I'll spin up an RB.

    2. see https://rbcommons.com/s/twitter/r/1362/