Deduplicate incompatible target_compatible_with labels

[philsc]

The latest refactor unintentionally made it so you can list the same
incompatible constraint multiple times in the target_compatible_with
attribute.

It was unintentional, but actually greatly simplifies a discussion
point on bazelbuild/bazel-skylib#381. That skylib patch aims to make
it easier to write non-trivial target_compatible_with expressions.
For example, to express that something is compatible with everything
except for Windows, one can use the following:

foo_binary(
    name = "bin",
    target_compatible_with = select({
        "@platforms//os:windows": ["@platforms//:incomaptible"],
        "//conditions:default: [],
    }),
)

The skylib patch aims to reduce that to:

foo_binary(
    name = "bin",
    target_compatible_with = compatibility.none_of("@platforms//os:windows"),
)

This works fine on its own, but a problem arises when these
expressions are composed. For example, a macro author might write the
following:

def foo_wrapped_binary(name, target_compatible_with = [], **kwargs):
    foo_binary(
        name = name,
        target_compatible_with = target_compatible_with + select({
            "@platforms//os:none": ["@platforms//:incompatible"],
            "//conditions:default": [],
        }),
    )

A macro author should be able to express their own incompatibility
while also honouring user-defined incompatibility.

It turns out that my latest refactor (#14096) unintentionally made
this work. This happened because we now check for incompatibility
before we perform a lot of sanity checks. That's also one of the
reasons that visibility restrictions are not honoured for incomaptible
targets at the moment (#16044).

Without the deduplicating behaviour, macro authors are stuck with
either not allowing composition, or having to create unique
incompatible constraints for every piece in a composed
target_compatible_with expression.

This patch adds a test to validate the deduplicating behaviour to
cement it as a feature.

Unfortunately, this means that target_compatible_with behaves
differently from other label list attributes. For other label list
attributes, bazel complains when labels are duplicated. For example,
the following BUILD file:

py_library(
    name = "lib",
)

py_binary(
    name = "bin",
    srcs = ["bin.py"],
    deps = [
    |   ":lib",
    |   ":lib",
    ],
)

will result in the following error:

$ bazel build :bin
INFO: Invocation ID: 3d8345a4-2e76-493e-8343-c96a36185b52
ERROR: /home/jenkins/repos/test_repo/BUILD:41:10: Label '//:lib' is duplicated in the 'deps' attribute of rule 'bin'
ERROR: error loading package '': Package '' contains errors
INFO: Elapsed time: 2.514s
INFO: 0 processes.
FAILED: Build did NOT complete successfully (1 packages loaded)

[philsc]

@gregestren , @katre , at the risk of disappointing you with my poor decision making, what do you think of this patch?

[katre]

I think this makes a lot of sense, and there are probably more places in Bazel where we should deduplicate instead of erroring.

[gregestren]

2570fe1 is a recent change recognizing some subtlety with select(). It's not the same purpose as yours, but a reminder that dedupe logic can't be exactly the same everwyhere.

This all reads reasonable to me but my main concern is API consistency. Surely this kind of theme has come up before w.r.t. macros? Let me run this by @comius and/or @brandjon , both of who I'll chat with tomorrow.

[katre]

Looks good to me, waiting for @gregestren, @comius, and @brandjon to weigh in.

[gregestren]

Sounds like no concerns.