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 unjustified warning about casing for directory entries #155

Conversation

mthmulders
Copy link
Contributor

Follow-up for #128, where we omitted the case that an archive entry could be a directory. In that case, its name will end with /, but the canonical target path will not have the / suffix. This would be seen as a difference in casing, but that is not true.

@mthmulders mthmulders marked this pull request as draft October 24, 2020 09:14
@mthmulders mthmulders marked this pull request as ready for review October 24, 2020 12:09
@mthmulders
Copy link
Contributor Author

mthmulders commented Oct 30, 2020

Thanks, @michael-o for the review. I don't have the rights to merge this PR, I'd be grateful if someone who does have the rights can merge it for me.

@mthmulders
Copy link
Contributor Author

@michael-o I see a lot of Maven components already updating Plexus Archiver, with this little issue still present. Shouldn't we try to merge this in, too, given the false warnings that'll occur otherwise?

@plamentotev
Copy link
Member

@mthmulders would you please squash the commits into one and I'll merge it.

@mthmulders mthmulders force-pushed the fix-unjustified-warning-about-casing-for-directory-entries branch from 9feba71 to a503307 Compare January 27, 2021 19:33
@mthmulders
Copy link
Contributor Author

@mthmulders would you please squash the commits into one and I'll merge it.

Squashed & force-pushed. Thanks in advance :-)

@plamentotev plamentotev merged commit f7f90ec into codehaus-plexus:master Jan 27, 2021
@plamentotev plamentotev added this to the plexus-archiver-4.2.4 milestone Jan 27, 2021
@plamentotev
Copy link
Member

The build is failing after the merge. I'll check it later today

@mthmulders
Copy link
Contributor Author

Error: Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.8.1:compile (default-compile) on project plexus-archiver: Execution default-compile of goal org.apache.maven.plugins:maven-compiler-plugin:3.8.1:compile failed: Plugin org.apache.maven.plugins:maven-compiler-plugin:3.8.1 or one of its dependencies could not be resolved: Could not transfer artifact org.apache.maven.shared:maven-shared-incremental:jar:1.1 from/to central (https://repo.maven.apache.org/maven2): Transfer failed for https://repo.maven.apache.org/maven2/org/apache/maven/shared/maven-shared-incremental/1.1/maven-shared-incremental-1.1.jar: Connection reset -> [Help 1]

My guess is that's unrelated to the code change, but feel free to investigate further and let me know if you draw a different conclusion.

@michael-o
Copy link
Member

Error: Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.8.1:compile (default-compile) on project plexus-archiver: Execution default-compile of goal org.apache.maven.plugins:maven-compiler-plugin:3.8.1:compile failed: Plugin org.apache.maven.plugins:maven-compiler-plugin:3.8.1 or one of its dependencies could not be resolved: Could not transfer artifact org.apache.maven.shared:maven-shared-incremental:jar:1.1 from/to central (https://repo.maven.apache.org/maven2): Transfer failed for https://repo.maven.apache.org/maven2/org/apache/maven/shared/maven-shared-incremental/1.1/maven-shared-incremental-1.1.jar: Connection reset -> [Help 1]

My guess is that's unrelated to the code change, but feel free to investigate further and let me know if you draw a different conclusion.

Correct, is an ephemeral network issue. Restart or check your internet connection.

@plamentotev
Copy link
Member

@mthmulders I've released a new version with this fix. Thanks for contributing.

@mthmulders
Copy link
Contributor Author

Thank you for considering this contribution and for releasing a new version so quickly.

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

Successfully merging this pull request may close these issues.

None yet

3 participants