User defined artifacts get overridden by transitive dependencies

[dgarzon]

If a user explicitly references a dependency, we should use that version and not ones brought through transitive dependencies. Basically, prioritize user defined dependencies over anything else. For example:

maven_install(
    artifacts = [
        "com.google.cloud:google-cloud-storage:1.66.0",
        "com.google.guava:guava:27.1-jre",
    ]
)

Should result in a dependency tree that contains com.google.guava:guava:27.1-jre, and not the one brought by com.google.cloud:google-cloud-storage:1.66.0.

[jin]

Unless I'm missing something, this particular configuration brings in guava 27.1-jre?

$ bazel query @maven//:all | grep guava
Loading: 1 packages loaded
@maven//:com_google_guava_listenablefuture
@maven//:com_google_guava_guava
@maven//:com_google_guava_failureaccess
@maven//:com_google_guava_guava_27_1_jre
@maven//:com_google_guava_listenablefuture_9999_0_empty_to_avoid_conflict_with_guava

[dgarzon]

Ugh, I should have mentioned that this is a dummy example, not a real one 👎 It was just there to illustrate the problem!

[dgarzon]

It seems the resolution is using the highest version, in my project at least.

[jin]

Yes, I believe that's the default for the Coursier CLI. The CLI supports the --force-version flag:

  --force-version | -V  <organization:name:forcedVersion>
        Force module version

I think we can thread this information into the rule using a new attribute in the maven.artifact(..., force_version = True) spec helper.

[dgarzon]

@jin I was going to propose using that, but on a higher level as it is a flag that applies globally (all dependencies that are going to be fetched). So it would be something like:

maven_install(
    ...,
    prioritize_pinned_versions = True | False
)

What do you think? Should be a simple change to make! I can take it if you approve!

[jin]

I like that. I think we can map this API design from bazel-dep's versionConflictPolicy option as well: https://github.com/johnynek/bazel-deps#options

maven_install(
    version_conflict_policy = "pinned | highest" # defaults to highest
)

@dkelmer what do you think about this?

[davidsantiago]

Does this scheme you guys are talking about still allow for the situation where you want most things to be done with version_conflict_policy = highest, but then there's this one piece of software that has a bug in the latest version when you use it with [fill in the blank], so you need to pin that one dep's specific version to be an earlier artifact? I don't see how you'd do this with prioritize_pinned_versions as you guys are describing it, so the force_version arg on the artifact seems more flexible to me. Of course, the two are not incompatible.

I've had that situation come up, and would advocate for an approach that maintains that as an option.

[dgarzon]

@davidsantiago I don't think the coursier-cli supports granular force_version per dependency. I believe our proposal would still work as you expect: force versions for artifacts defined, and use highest on those that are not explicitly defined (transitive).

[jin]

To make it more concrete, here are examples of the current and intended behavior.

1. Pulling in guava transitively via google-cloud-storage resolves to guava-26.0-android.

maven_install(
    name = "pinning",
    artifacts = [
        "com.google.cloud:google-cloud-storage:1.66.0",
    ],
    repositories = [
        "https://repo1.maven.org/maven2",
    ]
)

$ bazel query @pinning//:all | grep guava_guava
@pinning//:com_google_guava_guava
@pinning//:com_google_guava_guava_26_0_android

2. Pulling in guava-27.0-android directly works as expected.

maven_install(
    name = "pinning",
    artifacts = [
        "com.google.cloud:google-cloud-storage:1.66.0",
        "com.google.guava:guava:27.0-android",
    ],
    repositories = [
        "https://repo1.maven.org/maven2",
    ]
)

$ bazel query @pinning//:all | grep guava_guava
@pinning//:com_google_guava_guava
@pinning//:com_google_guava_guava_27_0_android

3. Pulling in guava-25.0-android (a lower version), resolves to guava-26.0-android. This is the default version conflict policy in action, where artifacts are resolved to the highest version.

maven_install(
    name = "pinning",
    artifacts = [
        "com.google.cloud:google-cloud-storage:1.66.0",
        "com.google.guava:guava:25.0-android",
    ],
    repositories = [
        "https://repo1.maven.org/maven2",
    ]
)

$ bazel query @pinning//:all | grep guava_guava
@pinning//:com_google_guava_guava
@pinning//:com_google_guava_guava_26_0_android

4. Now, if we add version_conflict_policy = "pinned", we should see guava-25.0-android getting pulled instead. The rest of non-specified artifacts still resolve to the highest version in the case of version conflicts.

maven_install(
    name = "pinning",
    artifacts = [
        "com.google.cloud:google-cloud-storage:1.66.0",
        "com.google.guava:guava:25.0-android",
    ],
    repositories = [
        "https://repo1.maven.org/maven2",
    ]
    version_conflict_policy = "pinned",
)

$ bazel query @pinning//:all | grep guava_guava
@pinning//:com_google_guava_guava
@pinning//:com_google_guava_guava_25_0_android

[davidsantiago]

Hm, yeah. I guess this still doesn't make sense to me, but I think it's all really driven by what coursier lets you do anyways, and I'll definitely admit I'm new to it since rules_jvm_external. I feel like I must be confused.

What doesn't make sense to me is that there are several things I could mean when I specify "com.google.guava:guava:25.0-android". I could mean, on the one hand, "I want at least this version, and you can upgrade to a later one that is semver compatible if needed." On the other hand, I could mean "I want exactly this version." So then it seems like there's no way to have it be the case that most of the time you mean "at least this version," and only sometimes "exactly this version." But "at least this version except when I say to make it actually that version" seems like a reasonable situation to have arise.

But as I said, if coursier can't handle this combination of constraints, then there's nothing to be done here anyways.