Turn dependency dupes into errors

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

Johan Oskarsson
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
  • 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 ... Patrick Lawson Patrick Lawson
Patrick Lawson
Patrick Lawson
Ity Kaul
Johan Oskarsson
Review request changed

Status: Closed (submitted)

Eric Ayers

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/