Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove SNAPSHOT suffix in version number (#914) #915

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jianxx
Copy link

@jianxx jianxx commented Jun 8, 2023

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 jianxx requested a review from cheister as a code owner June 8, 2023 06:09
@google-cla
Copy link

google-cla bot commented Jun 8, 2023

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.

@jianxx jianxx closed this Jun 8, 2023
@jianxx jianxx reopened this Jun 8, 2023
@cheister
Copy link
Collaborator

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

@jianxx
Copy link
Author

jianxx commented Jun 19, 2023 via email

@jianxx
Copy link
Author

jianxx commented Jun 21, 2023

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.

@shs96c
Copy link
Collaborator

shs96c commented Jun 22, 2023

@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?

@jianxx
Copy link
Author

jianxx commented Jun 22, 2023

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants