rules_jvm_external icon indicating copy to clipboard operation
rules_jvm_external copied to clipboard

Remove SNAPSHOT suffix in version number (#914)

Open jianxx opened this issue 1 year ago • 6 comments

This is a fix for #914 .

SNAPSHOT suffix should not be included in version number. For maven release repository, the artifact path is something like {group}/{artifact}/{version}/{artifact}-{version}. For maven snapshot repository, the artifact path is something like {group}/{artifact}/{version}-SNAPSHOT/{artifact}-{version}-{timestamp}-{buildNumber}.

jianxx avatar Jun 08 '23 06:06 jianxx

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 Jun 08 '23 06:06 google-cla[bot]

Is there a test that can be added for this case?

cheister avatar Jun 19 '23 23:06 cheister

sure, I’ll add a function verification test for this scenario.

cheister @.***>于2023年6月20日 周二上午7:02写道:

Is there a test that can be added for this case?

— Reply to this email directly, view it on GitHub https://github.com/bazelbuild/rules_jvm_external/pull/915#issuecomment-1597858622, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAUG6Y3B6LZST6GSRL6QSSLXMDK67ANCNFSM6AAAAAAY6Z465M . You are receiving this because you modified the open/close state.Message ID: @.***>

jianxx avatar Jun 19 '23 23:06 jianxx

I just find that coursier uses different file name for SNAPSHOT and this different behavior causes the problem. Need more investigation into coursier to clarify whether it is a bug or a feature.

jianxx avatar Jun 21 '23 08:06 jianxx

@jianxx I'm not sure from your last comment whether you'd like us to review this patch, or leave it. Could you let us know, please?

shs96c avatar Jun 22 '23 09:06 shs96c

@jianxx I'm not sure from your last comment whether you'd like us to review this patch, or leave it. Could you let us know, please?

After I did more tests with different maven repo implementations, I can describe this issue more precisely. My previous statement with courier is not correct, and here is the detailed root cause of this issue and how my patch works.

  1. The issue could be reproduced with nexus3 (I have not tested nexus2 yet )and could not be reproduced with latest nexus build and jfrog.
  2. The root cause is how the maven repo handles snapshot arfifacts. Let's take org.springframework.boot:spring-boot-cli:3.2.0-SNAPSHOT as an example. When we use coursier to fetch this artifact from the latest nexus or jfrog, the result will be "v1/https/repo.spring.io/snapshot/org/springframework/boot/spring-boot-cli/3.2.0-SNAPSHOT/spring-boot-cli-3.2.0-SNAPSHOT.jar". But when we fetch from nexus 3.x, the result would be something like "v1/https/repo.spring.io/snapshot/org/springframework/boot/spring-boot-cli/3.2.0-SNAPSHOT/spring-boot-cli-3.2.0-20230622.132711-54.jar"
  3. In the maven repo, spring-boot-cli-3.2.0-SNAPSHOT.jar does not really exist. It is an alias to latest build like "spring-boot-cli-3.2.0-20230622.132711-54.jar".

What I did in the patch is using version string without 'SNAPSHOT' to match the artifact file, so it can work for both cases. I'm adding a test case with mocking data to simulate nexus 3.x behavior. It is a little expensive to make a full testing scenario.

jianxx avatar Jun 22 '23 13:06 jianxx