rules_jvm_external icon indicating copy to clipboard operation
rules_jvm_external copied to clipboard

Add workspace_prefix_for_pinned_deps

Open tgeng opened this issue 2 years ago • 7 comments

When artifacts are pinned, maven_install would create individual external repos for each dependency. The name of such an external repo is simply the maven coordinate with all punctuations replaced by _. This can cause name conflicts for code bases that are already using http_archive to download thousands of dependencies through some other mechanism but would like to migrate to use maven_install.

This PR tries to fix this by specifying a prefix for intermediate external repos so that name collision becomes manageable.

tgeng avatar Aug 02 '22 17:08 tgeng

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

google-cla[bot] avatar Aug 02 '22 17:08 google-cla[bot]

Hi @cheister , can you take a look at this change? Thanks!

tgeng avatar Aug 10 '22 22:08 tgeng

Hi @shs96c can you take a look at this?

tgeng avatar Aug 22 '22 20:08 tgeng

Apologies for the slow review on this. Here now!

First comment: if you want a prefix, then I'd suggest defaulting to the repository rule's name. Secondly, if the versions pointed to are the same (down to the sha256), then it shouldn't matter which one is being used or where it's being pulled from. If this is the case, then it would be better to use a maybe to load the http_file, rather than a prefix.

Would that approach work for you?

shs96c avatar Aug 30 '22 10:08 shs96c

Apologies for the slow review on this. Here now!

First comment: if you want a prefix, then I'd suggest defaulting to the repository rule's name. Secondly, if the versions pointed to are the same (down to the sha256), then it shouldn't matter which one is being used or where it's being pulled from. If this is the case, then it would be better to use a maybe to load the http_file, rather than a prefix.

Would that approach work for you?

Hi, maybe would not solve our problem because in our current code base we are creating external repos with a different mechanism, which could result in artifacts that are of the same version but loaded from different sources which are built differently (and hence may have different SHA). My purpose of this PR is to allow easier adoption of maven_install so that it will not interfere with existing code.

Defaulting the prefix to repo name would be a breaking change for existing users. Also, moving forward, with our use case (and typical use cases of others), we actually want to share the same intermediate artifacts across multiple pinned maven install. So it seems defaulting to "" is still the desirable behavior for most use cases.

BTW, regardless of this change, it does seem that without a prefix, the same http_file could be registered multiple times with multiple maven_install. So it seems all http_file calls in defs.bzl should be wrapped inside maybe. I can also update this PR to include such change if desired.

What do you think?

tgeng avatar Sep 01 '22 21:09 tgeng

gentile ping @shs96c

tgeng avatar Sep 06 '22 17:09 tgeng

Any updates? @shs96c

tgeng avatar Nov 07 '22 18:11 tgeng