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.

Sign up for GitHub

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

Jump to bottom

[MDEP-952] Partially decouple AbstractFromConfigurationMojo from StringUtils #487

Merged
merged 7 commits into from
Nov 24, 2024
Merged

[MDEP-952] Partially decouple AbstractFromConfigurationMojo from StringUtils #487

merged 7 commits into from
Nov 24, 2024

Conversation

elharo
Copy link
Contributor

@elharo elharo commented Nov 20, 2024 •
edited
Loading

Needs a closer look. This appears to be the change that caused the bug MDEP-952

Running post-build script: /Users/elharo/maven-dependency-plugin/target/it/mdep-952-unpack-fails-if-extension-of-artifact-is-used/verify.groovy
Assertion failed: 

assert buildLog.contains( 'BUILD SUCCESS' )
       |        |
       |        false

@elharo elharo changed the title Decouple AbstractFromConfigurationMojo frm StringUtils [MDEP-952] Decouple AbstractFromConfigurationMojo frm StringUtils Nov 20, 2024
@elharo elharo changed the title [MDEP-952] Decouple AbstractFromConfigurationMojo frm StringUtils [MDEP-952] Decouple AbstractFromConfigurationMojo and UnpackMojo from StringUtils Nov 20, 2024
@michael-o michael-o removed their request for review November 20, 2024 15:16
@elharo elharo changed the title [MDEP-952] Decouple AbstractFromConfigurationMojo and UnpackMojo from StringUtils [MDEP-952] Decouple AbstractFromConfigurationMojo from StringUtils Nov 20, 2024
elharo
elharo commented Nov 20, 2024
View reviewed changes
...ava/org/apache/maven/plugins/dependency/fromConfiguration/AbstractFromConfigurationMojo.java Outdated
@@ -367,7 +367,7 @@ public void setArtifact(String artifact) throws MojoFailureException {
if (artifact != null) {
String packaging = "jar";
String classifier;
String[] tokens = StringUtils.split(artifact, ":");
String[] tokens = artifact.split(":");
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is likely the issue. I think the split methods may not be quite the same.

@elharo
Copy link
Contributor Author

elharo commented Nov 20, 2024

Hooray for tests. I was able to narrow down the problem to the change from StringUtils.split(foo to foo.split.

I'm not immediately sure why/how those two split methods differ but they do. So this PR now removes usages of StringUtils except for StringUtils.split. I'll address that carefully in a subsequent PR after more investigation.

@elharo elharo changed the title [MDEP-952] Decouple AbstractFromConfigurationMojo from StringUtils [MDEP-952] Partilly ecouple AbstractFromConfigurationMojo from StringUtils Nov 20, 2024
@elharo elharo marked this pull request as ready for review November 20, 2024 20:10
@elharo elharo changed the title [MDEP-952] Partilly ecouple AbstractFromConfigurationMojo from StringUtils [MDEP-952] Partilly decouple AbstractFromConfigurationMojo from StringUtils Nov 20, 2024
@elharo elharo changed the title [MDEP-952] Partilly decouple AbstractFromConfigurationMojo from StringUtils [MDEP-952] Partially decouple AbstractFromConfigurationMojo from StringUtils Nov 20, 2024
@elharo elharo merged commit eceab09 into master Nov 24, 2024
51 checks passed
@elharo elharo deleted the from branch November 24, 2024 14:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@slachiewicz slachiewicz slachiewicz approved these changes

@khmarbaise khmarbaise Awaiting requested review from khmarbaise

@slawekjaranowski slawekjaranowski Awaiting requested review from slawekjaranowski

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@elharo @slachiewicz