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

Fix pathinfo with classifier and tar.bz2 type issue #103

Merged
merged 1 commit into from
Aug 27, 2024

Conversation

ligangty
Copy link
Member

Seems a path contains classifier and the type of tar.bz2 will cause
parse issue for pathinfo, we will treat it as special case like the
tar.gz too.
And also changes the java version to 11 and some code style issue.

  Seems a path contains classifier and the type of tar.bz2 will cause
  parse issue for pathinfo, we will treat it as special case like the
  tar.gz too.
  And also changes the java version to 11 and some code style issue.
@ligangty ligangty requested review from ruhan1 and sswguo August 27, 2024 05:55
Copy link
Contributor

@ruhan1 ruhan1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@sswguo
Copy link
Member

sswguo commented Aug 27, 2024

LGTM

@geored
Copy link
Member

geored commented Aug 27, 2024

Hey @ligangty , sorry for jumping into this PR but this is somewhat interesting...
why not use something like this pattern:
\.(?:[a-zA-Z0-9]+(\.[a-zA-Z0-9]+)*)?$

@ligangty
Copy link
Member Author

ligangty commented Aug 27, 2024

Hey @ligangty , sorry for jumping into this PR but this is somewhat interesting... why not use something like this pattern: \.(?:[a-zA-Z0-9]+(\.[a-zA-Z0-9]+)*)?$

@geored It's hard to use a dedicated pattern here because there is no limit to the classifier, like:
/io/quarkus/platform/quarkus-google-cloud-services-bom-quarkus-platform-descriptor/2.13.7.Final-redhat-00001/quarkus-google-cloud-services-bom-quarkus-platform-descriptor-2.13.7.Final-redhat-00001-2.13.7.Final-redhat-00001.json

Another case is: /org/jboss/modules/jboss-modules/1.5.0.Final-temporary-redhat-00033/jboss-modules-1.5.0.Final-temporary-redhat-00033-project-sources.tar.gz
In this case we don't know the artifact it's hard to determine which is valid classifier by pattern, because both "project-sources.tar" with type "gz" and "project-sources" with type "tar.gz" are valid classifier here based on maven rules.

@ligangty ligangty merged commit c688d29 into Commonjava:master Aug 27, 2024
1 check passed
@geored
Copy link
Member

geored commented Aug 27, 2024

@ligangty so in the case of quarkus-google-cloud-services-bom-quarkus-platform-descriptor-2.13.7.Final-redhat-00001-2.13.7.Final-redhat-00001.json is the required for .json extension to be captured?

@ligangty
Copy link
Member Author

ligangty commented Aug 27, 2024

@ligangty so in the case of quarkus-google-cloud-services-bom-quarkus-platform-descriptor-2.13.7.Final-redhat-00001-2.13.7.Final-redhat-00001.json is the required for .json extension to be captured?

@geored this is what we needed for this:
artifactId: quarkus-google-cloud-services-bom-quarkus-platform-descriptor
version: 2.13.7.Final-redhat-00001
classifier: 2.13.7.Final-redhat-00001
type: json

And for case: jboss-modules-1.5.0.Final-temporary-redhat-00033-project-sources.tar.gz
ArtifactId: jboss-modules
version: 1.5.0.Final-temporary-redhat-00033
classifier: project-sources
type: tar.gz
but this is hard to parse with your regex for the classifier, because project-sources.tar is also valid for maven, then the type will be gz

@geored
Copy link
Member

geored commented Aug 27, 2024

Yeah, it is for the one that i suggested as a example, but i you can capture only file extension then you can afterwards easily use already established code methods (in existing code) to get the artifactId, version and classifier.
Never mind, you already merged this solution, i was only interested in this implementation

@ligangty
Copy link
Member Author

ligangty commented Aug 27, 2024

@geored Maybe I can give a more edged one here: wildfly-modules-8.7.0-redhat-00001-wildfly8.7.0.jar
This means the classifier should be wildfly8.7.0 and type is jar, but for maven rules the classifier is valid for wildfly8.7 with type 0.jar, or classifier wildfly8 with type 7.0.jar is also valid. So the rule is floating and hard to give a dedicated pattern to capture the file type.

@geored
Copy link
Member

geored commented Aug 27, 2024

@ligangty hm.... i got it.... seems difficult to find proper regex for last edge case but i think if we use exclude pattern for numbers and dot's before .jar (or war) then i think we can capture those edge cases also, without compromising previous ones.
But let's leave it, i was only interested to know why we can't use regex in this situations and probably it is better if regex is really complicated pattern.

@ligangty
Copy link
Member Author

@geored the pain point here for non-regex solution is the classifier part which don't have any limitation, especially it can have dots inside. This makes it hard to decide the type(file type suffix) here if it includes dots. Maybe this PR is not looking so smoothly, but that's what I can get besides a really complicated regex pattern.

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.

4 participants