Disallow duplicate cxx library names

[cormacrelf]

There is a trap for fixups like this with two cxx_library targets of the same name:

[['cfg(custom_config = "abc")'.cxx_library]]
headers = ["bzip2-1.0.8/*.h"]
name = "bzip2"
preprocessor_flags = ["-D_FILE_OFFSET_BITS=64", "-DBZ_NO_STDIO", "-DABC"]
srcs = [
  "bzip2-1.0.8/huffman.c",
]

[['cfg(custom_config = "def")'.cxx_library]]
headers = ["bzip2-1.0.8/*.h"]
name = "bzip2"
preprocessor_flags = ["-D_FILE_OFFSET_BITS=64", "-DBZ_NO_STDIO", "-DDEF"]
srcs = [
  "bzip2-1.0.8/blocksort.c",
]

This produces a single cxx_library

cxx_library(
    name = "bzip2-sys-0.1.13+1.0.8-bzip2",
    srcs = [":bzip2-sys-0.1.13+1.0.8.crate[bzip2-1.0.8/blocksort.c]"],
    headers = [
        ":bzip2-sys-0.1.13+1.0.8.crate[bzip2-1.0.8/bzlib.h]",
        ":bzip2-sys-0.1.13+1.0.8.crate[bzip2-1.0.8/bzlib_private.h]",
    ],
    preprocessor_flags = [
        "-D_FILE_OFFSET_BITS=64",
        "-DBZ_NO_STDIO",
        "-DDEF",
    ],
    visibility = [],
)

Which is used by both platforms linux-abc and linux-def. Note that both fixup entries have the same name = "bzip2" and therefore target name, so there can be only one actually emitted. If the user tries this they'll typically find windows flags all over their build unconditionally, because the last write wins alphabetically by platform name and windows starts with "w".

Reindeer's behaviour in this case is to silently clobber the abc one. I would have expected an error or warning. I specified two different cxx_libraries and ended up with just one, which was a bad outcome.

What users should do instead

[['cfg(custom_config = "abc")'.cxx_library]]
name = "bzip2-abc"
...

[['cfg(custom_config = "def")'.cxx_library]]
name = "bzip2-def"
...

Which allows reindeer to select a different cxx_library for each platform.

This PR: detect when users use duplicate names

This PR adds an error for any time we are emitting a cxx library or prebuilt cxx library more than once with the same target name. The example above produces an error with this PR:

[ERROR reindeer] Multiple cxx_library targets named 'bzip2' in package bzip2-sys-0.1.13+1.0.8. The first was emitted for platform cfg(custom_config = "abc"), the second for platform cfg(custom_config = "def")). Cxx library names must be unique per-package, except to override the base fixup with a cfg().

Unresolved questions

How to allow versioning use case for silent clobbering

For example the ring fixups from Meta under this PR produce this error:

[ERROR reindeer] Multiple cxx_library targets named 'ring-c-asm-elf-aarch64' in package ring-0.17.5. The first was emitted for platform cfg(all(target_arch = "aarch64", target_os = "linux")), the second for platform cfg(all(version = "=0.17.5", target_arch = "aarch64", target_os = "linux")). Cxx library names must be unique per-package, except to override the base fixup with a cfg().

This PR could possibly be adjusted to allow clobbering within any one reindeer platform's fixup evaluations, so that this kind of versioning shenanigans can still work. The goal would be more specific, instead detecting multiple platforms producing conflicting (!=) versions of the same cxx_library and clobbering each other.

[dtolnay]

Would it be feasible and a better user experience to collapse multiple identically named cxx_library into one Buck target using platform for the attributes that diverge?

It would still be necessary to check that one platform does not depend on multiple of the libraries, but this solves the ability to reuse a cxx library name across multiple version-specific fixups, and now also across distinct platforms.

cxx_library(
    name = "bzip2-sys-0.1.13+1.0.8-bzip2",
    headers = [
        ":bzip2-sys-0.1.13+1.0.8.crate[bzip2-1.0.8/bzlib.h]",
        ":bzip2-sys-0.1.13+1.0.8.crate[bzip2-1.0.8/bzlib_private.h]",
    ],
    platform = {
        "custom_config=abc": dict(
            preprocessor_flags = [
                "-D_FILE_OFFSET_BITS=64",
                "-DBZ_NO_STDIO",
                "-DABC",
            ],
            srcs = [":bzip2-sys-0.1.13+1.0.8.crate[bzip2-1.0.8/huffman.c]"],
        ),
        "custom_config=def": dict(
            preprocessor_flags = [
                "-D_FILE_OFFSET_BITS=64",
                "-DBZ_NO_STDIO",
                "-DDEF",
            ],
            srcs = [":bzip2-sys-0.1.13+1.0.8.crate[bzip2-1.0.8/blocksort.c]"],
        ),
    },
    visibility = [],
)

[cormacrelf]

Yes, that would be nicest. What I described at the end (one last-write-winner per platform, error if incompatible dupes cross platform) would be a step in that direction, and would be less work for now, I think.