Skip to content

Fix to addArchiveTask only removes suffix at the end#32639

Merged
blindpirate merged 2 commits intogradle:releasefrom
NaMinhyeok:issue-32621
Mar 12, 2025
Merged

Fix to addArchiveTask only removes suffix at the end#32639
blindpirate merged 2 commits intogradle:releasefrom
NaMinhyeok:issue-32621

Conversation

@NaMinhyeok
Copy link
Contributor

minus() method incorrectly removed the first occurrence of a given suffix so make new method, removeLast() in TextUtil, which correctly removes only the last occurrence of a given suffix if it appears at the end of the string.

Context

The previous implementation of TextUtil.minus() incorrectly removed the first occurrence of a given suffix in a string, causing unintended modifications when the suffix appeared in the middle of the string.

To fix this, I introduced a new method, TextUtil.removeLast(), which ensures that only the last occurrence of a given suffix is removed if it appears at the end of the string.

Additionally, I updated DistributionBasePlugin.java to use removeLast() instead of minus().

Contributor Checklist

  • Review Contribution Guidelines.
  • Make sure that all commits are signed off to indicate that you agree to the terms of Developer Certificate of Origin.
  • Make sure all contributed code can be distributed under the terms of the Apache License 2.0, e.g. the code was written by yourself or the original code is licensed under a license compatible to Apache License 2.0.
  • Check "Allow edit from maintainers" option in pull request so that additional changes can be pushed by Gradle team.
  • Provide integration tests (under <subproject>/src/integTest) to verify changes from a user perspective.
  • Provide unit tests (under <subproject>/src/test) to verify logic.
  • Update User Guide, DSL Reference, and Javadoc for public-facing changes.
  • Ensure that tests pass sanity check: ./gradlew sanityCheck.
  • Ensure that tests pass locally: ./gradlew <changed-subproject>:quickTest.

Reviewing cheatsheet

Before merging the PR, comments starting with

  • ❌ ❓must be fixed
  • 🤔 💅 should be fixed
  • 💭 may be fixed
  • 🎉 celebrate happy things

@NaMinhyeok NaMinhyeok requested a review from a team as a code owner March 8, 2025 15:14
@NaMinhyeok NaMinhyeok requested review from a team and octylFractal March 8, 2025 15:14
@bot-gradle bot-gradle added from:contributor PR by an external contributor to-triage labels Mar 8, 2025
minus() method incorrectly removed the first occurrence of a given suffix
so make new method, removeLast() in TextUtil, which correctly removes only the last
occurrence of a given suffix if it appears at the end of the string.

Signed-off-by: NaMinhyeok <[email protected]>
@big-guy

This comment has been minimized.

@bot-gradle

This comment has been minimized.

@bot-gradle
Copy link
Collaborator

The following builds have failed:

@ghale ghale changed the base branch from master to release March 11, 2025 14:05
@ghale
Copy link
Member

ghale commented Mar 11, 2025

Thanks for the PR @NaMinhyeok. I think this looks good. Let's just rename the method (for clarity) and we'll get this tested and merged into 8.14.

@ghale ghale removed the request for review from octylFractal March 11, 2025 14:07
@ghale
Copy link
Member

ghale commented Mar 11, 2025

@bot-gradle test this

@bot-gradle

This comment has been minimized.

@bot-gradle
Copy link
Collaborator

The following builds have passed:

@ghale ghale enabled auto-merge March 11, 2025 17:01
@ghale ghale added this pull request to the merge queue Mar 11, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Mar 11, 2025
@ghale ghale added this pull request to the merge queue Mar 11, 2025
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Mar 11, 2025
@blindpirate blindpirate added this pull request to the merge queue Mar 12, 2025
Merged via the queue into gradle:release with commit 7a6197e Mar 12, 2025
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

from:contributor PR by an external contributor in:application-plugin

Projects

None yet

Development

Successfully merging this pull request may close these issues.

distZip top level directory has wrong name when version has substring ".zip"

6 participants

Comments