Skip to content

fix(release): remove GNUism#54

Merged
multani merged 2 commits into
mainfrom
remove-gnuism
May 26, 2026
Merged

fix(release): remove GNUism#54
multani merged 2 commits into
mainfrom
remove-gnuism

Conversation

@multani

@multani multani commented Apr 23, 2026

Copy link
Copy Markdown
Member

grep -p is GNU specific and doesn't work on MacOS

On MacOS, the following steps are not working due to the -P option used in the grep command:

make .release.generate-notes
make .release.bump-chart-version

@multani multani requested review from Copilot and pranjalg13 April 23, 2026 14:04
@multani multani added the bug Something isn't working label Apr 23, 2026
@multani multani self-assigned this Apr 23, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Updates release helper scripts to avoid GNU-specific grep -P/-o usage (which breaks on macOS) by switching YAML field extraction to yq.

Changes:

  • Replace grep -Po parsing of Chart.yaml fields with yq lookups in release note generation and version bump scripts.
  • Minor formatting/no-op change in download-chart-docker-images.sh.

Reviewed changes

Copilot reviewed 2 out of 3 changed files in this pull request and generated 3 comments.

File Description
scripts/generate-release-notes.sh Use yq to read chart name and version instead of GNU grep -P.
scripts/bump-chart-version.sh Use yq to read chart version instead of GNU grep -P.
scripts/download-chart-docker-images.sh Formatting-only change (no functional impact).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread scripts/generate-release-notes.sh Outdated
Comment thread scripts/bump-chart-version.sh Outdated
Comment thread scripts/bump-chart-version.sh Outdated

@pranjalg13 pranjalg13 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM 👍 , I guess we might need to manually verify this? on both macos and linux 😄

@multani

multani commented Apr 24, 2026

Copy link
Copy Markdown
Member Author

LGTM 👍 , I guess we might need to manually verify this? on both macos and linux 😄

Can you run:

make .release.generate-notes .release.bump-chart-version

It should modify local files and that should work with MacOS now

@ChrisKujawa

Copy link
Copy Markdown
Member

@pranjalg13 can you run this on mac?

@ChrisKujawa ChrisKujawa requested a review from pranjalg13 May 26, 2026 06:49
@pranjalg13 pranjalg13 requested a review from a team as a code owner May 26, 2026 07:37
@pranjalg13

Copy link
Copy Markdown
Contributor

@pranjalg13 can you run this on mac?

I rebased with main, still getting this error:

cmd: make .release.generate-notes .release.bump-chart-version

WARNING: The requested image's platform (linux/amd64) does not match the detected host platform (linux/arm64/v8) and no specific platform was requested
fetch https://dl-cdn.alpinelinux.org/alpine/v3.14/main/x86_64/APKINDEX.tar.gz
fetch https://dl-cdn.alpinelinux.org/alpine/v3.14/community/x86_64/APKINDEX.tar.gz
(1/7) Installing ncurses-terminfo-base (6.2_p20210612-r1)
(2/7) Installing ncurses-libs (6.2_p20210612-r1)
(3/7) Installing readline (8.1.0-r0)
(4/7) Installing bash (5.1.16-r0)
Executing bash-5.1.16-r0.post-install
(5/7) Installing pcre (8.44-r0)
(6/7) Installing grep (3.7-r0)
(7/7) Installing yq (4.6.3-r3)
Executing busybox-1.33.1-r3.trigger
OK: 27 MiB in 28 packages
⌚  Generating changelog ...
 ERROR  "camunda-load-tests-0.1.11" tag already exists
make: *** [.release.generate-notes] Error 1

It is not bumping the version automatically

cc: @multani @ChrisKujawa

@multani

multani commented May 26, 2026

Copy link
Copy Markdown
Member Author

It is not bumping the version automatically

Actually, it also doesn't work on main at the moment 🤦

multani added 2 commits May 26, 2026 11:00
`grep -p` is GNU specific and doesn't work on MacOS
@multani

multani commented May 26, 2026

Copy link
Copy Markdown
Member Author

@pranjalg13 can you try again from the latest version of this branch with:

make .release.bump-chart-version .release.generate-notes

(I changed the order of the make targets above ⬆️, to bump first, then generate the release notes)


# Generate new version based on the old one.
chart_version_old=$(grep -Po "(?<=^version: ).+" charts/${chart_name}/Chart.yaml)
chart_version_old=$(yq eval .version charts/${chart_name}/Chart.yaml)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think the problem with this is that we need yq installed 🤔 Not sure whether we can expect this to be the default. Do we have configured/documented as requirement?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

yq is already used in other scripts and installed by the Docker builder too.

I guess it's reasonable to assume releaser would have yq already installed.

@pranjalg13

pranjalg13 commented May 26, 2026

Copy link
Copy Markdown
Contributor

@pranjalg13 can you try again from the latest version of this branch with:

make .release.bump-chart-version .release.generate-notes

(I changed the order of the make targets above ⬆️, to bump first, then generate the release notes)

I earlier tried that as well but this was the error:

pranjal.goyal@Pranjals-MacBook-Pro camunda-load-tests-helm % make .release.bump-chart-version .release.generate-notes
sed: 1: "charts/camunda-load-tes ...": command c expects \ followed by text
make: *** [.release.bump-chart-version] Error 1

I guess it is fine, not a big issue 😄 , I will use GitHub action

@multani multani added this pull request to the merge queue May 26, 2026
Merged via the queue into main with commit 2eed1c5 May 26, 2026
2 checks passed
@multani multani deleted the remove-gnuism branch May 26, 2026 13:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants