Allow pinning transitive artifacts via new pinned_artifacts attribute

[dmivankov]

Using artifacts= + version_conflict_policy=pinned is possible to set versions
of transitive artifacts but that affects visibility, required policy change and
makes artifacts= bigger than it needs to be.

[shs96c]

I thought coursier supported exact version pinning using the [version] maven syntax. If so, you can get the equivalent of pinned_artifacts using coordinates like com.google.guava:guava:[31.0.1-jre].

Please let me know if I am mistaken, and I'll do a proper review of this PR.

[dmivankov]

It's possible to use

maven_install(
    artifacts = ["com.google.guava:guava:31.0.1-jre"],
    version_conflict_policy = "pinned",
)

But the downside is

• conflict policy applies to all artifacts
• artifact items become visible even if not necessarily directly used by project

With new attribute one can do something like this

maven_install(
    artifacts = ["com.google.guava:guava:31.0.1-jre", ...].
    version_conflict_policy = "pinned",  # or maybe use the default one
    pinned_artifacts = [
        # 4.1.67.Final has CVE https://github.com/advisories/GHSA-grg4-wf29-r9vv
        "io.netty:netty-codec-http2:4.1.68.Final",
        "io.netty:netty-codec-http:4.1.68.Final",
        "io.netty:netty-codec-socks:4.1.68.Final",
        "io.netty:netty-codec:4.1.68.Final",
        "io.netty:netty-handler-proxy:4.1.68.Final",
        "io.netty:netty-transport-native-epoll:jar:linux-x86_64:4.1.68.Final",
        "io.netty:netty-transport-native-kqueue:jar:osx-x86_64:4.1.68.Final",
        "io.netty:netty-transport-native-unix-common:4.1.68.Final",
    ],  
)

transitive dependency version is bumped, but directly used dependencies vs transitive dependency versions lists are still separated

[jin]

I don't see how the new attribute is necessarily simpler than putting the new list of pinned_artifacts in artifacts, and using pinned version_conflict_policy?

[dmivankov]

Mainly two reasons:

• esthetic separation of direct dependencies + not providing public visibility for indirect artifacts
• makes it simpler to write a check for directly unused dependencies (which don't need their version to be manually maintained/pinned generally, except for when they do and this is unblocked by this PR). Script along such lines

bazel query 'kind(jvm_import, @maven//... - deps(//..., 1)) intersect attr(visibility, public, @maven//...)'

[jin]

esthetic separation of direct dependencies + not providing public visibility for indirect artifacts

I don't think adding a new API for this makes it necessarily simpler. A new attribute is an attribute that we'd have to support ~forever. For aesthetics, it could be a project-side concatenation of lists.

re: public visibility for indirect artifacts is needed, given #147 (comment)

makes it simpler to write a check for directly unused dependencies (which don't need their version to be manually maintained/pinned generally, except for when they do and this is unblocked by this PR). Script along such lines

Why can't this be an rdeps query?

In general, I'm not too comfortable with adding another core attribute like this, especially if it doesn't bring significant benefit over the existing mechanisms.

[dmivankov]

main thing that isn't possible without this PR is having version_conflict_policy != "pinned" and setting specific single dependency version. But maybe that's a rare case as often either pinned is used or default picks same or newer (iirc) version of all specified dependencies (downgrade pinning should be more rare)

will close then