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

Automated ICU update workflow failed #48496

Closed
targos opened this issue Jun 19, 2023 · 2 comments · Fixed by #48499
Closed

Automated ICU update workflow failed #48496

targos opened this issue Jun 19, 2023 · 2 comments · Fixed by #48499
Labels
tools Issues and PRs related to the tools directory.

Comments

@targos
Copy link
Member

targos commented Jun 19, 2023

https://github.com/nodejs/node/actions/runs/5300865700/jobs/9594690597

CleanShot 2023-06-19 at 09 07 01
@targos targos added the tools Issues and PRs related to the tools directory. label Jun 19, 2023
@richardlau
Copy link
Member

node/tools/dep_updaters/update-icu.sh

Lines 62 to 64 in 8e710c9

sed -i '' -e "s|\"url\": \"\(.*\)\".*|\"url\": \"$NEW_VERSION_TGZ_URL\",|" "$TOOLS_DIR/icu/current_ver.dep"
sed -i '' -e "s|\"md5\": \"\(.*\)\".*|\"md5\": \"$CHECKSUM\"|" "$TOOLS_DIR/icu/current_ver.dep"

I'm guessing this was written on macOS as it's using sed -i '' which won't work in GNU sed -- see https://stackoverflow.com/questions/5694228/sed-in-place-flag-that-works-both-on-mac-bsd-and-linux/5694430#5694430

@lpinca lpinca mentioned this issue Jun 19, 2023
Merged
lpinca added a commit to lpinca/node that referenced this issue Jun 19, 2023
@lpinca
tools: replace sed with perl
2a862e9 
For cross-platform compatibility use perl instead of sed.

Fixes: nodejs#48496
lpinca added a commit to lpinca/node that referenced this issue Jun 19, 2023
@lpinca
tools: replace sed with perl
0524a7a 
For cross-platform compatibility use perl instead of sed.

Fixes: nodejs#48496
lpinca added a commit to lpinca/node that referenced this issue Jun 19, 2023
@lpinca
tools: replace sed with perl
a28f37b 
For cross-platform compatibility use perl instead of sed.

Fixes: nodejs#48496
@marco-ippolito
Copy link
Member

node/tools/dep_updaters/update-icu.sh

Lines 62 to 64 in 8e710c9

sed -i '' -e "s|\"url\": \"\(.*\)\".*|\"url\": \"$NEW_VERSION_TGZ_URL\",|" "$TOOLS_DIR/icu/current_ver.dep"
sed -i '' -e "s|\"md5\": \"\(.*\)\".*|\"md5\": \"$CHECKSUM\"|" "$TOOLS_DIR/icu/current_ver.dep"

I'm guessing this was written on macOS as it's using sed -i '' which won't work in GNU sed -- see https://stackoverflow.com/questions/5694228/sed-in-place-flag-that-works-both-on-mac-bsd-and-linux/5694430#5694430

Correct 🙂

nodejs-github-bot pushed a commit that referenced this issue Jun 20, 2023
@lpinca
tools: replace sed with perl
e7646a5 
For cross-platform compatibility use perl instead of sed.

Fixes: #48496
PR-URL: #48499
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
Reviewed-By: Mestery <mestery@protonmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
RafaelGSS pushed a commit that referenced this issue Jul 3, 2023
@lpinca @RafaelGSS
tools: replace sed with perl
4345526 
For cross-platform compatibility use perl instead of sed.

Fixes: #48496
PR-URL: #48499
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
Reviewed-By: Mestery <mestery@protonmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Ceres6 pushed a commit to Ceres6/node that referenced this issue Aug 14, 2023
@lpinca @Ceres6
tools: replace sed with perl
ebd7318 
For cross-platform compatibility use perl instead of sed.

Fixes: nodejs#48496
PR-URL: nodejs#48499
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
Reviewed-By: Mestery <mestery@protonmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Ceres6 pushed a commit to Ceres6/node that referenced this issue Aug 14, 2023
@lpinca @Ceres6
tools: replace sed with perl
00424af 
For cross-platform compatibility use perl instead of sed.

Fixes: nodejs#48496
PR-URL: nodejs#48499
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
Reviewed-By: Mestery <mestery@protonmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
ruyadorno pushed a commit that referenced this issue Sep 10, 2023
@lpinca @ruyadorno
tools: replace sed with perl
38fb350 
For cross-platform compatibility use perl instead of sed.

Fixes: #48496
PR-URL: #48499
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
Reviewed-By: Mestery <mestery@protonmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
ruyadorno pushed a commit that referenced this issue Sep 13, 2023
@lpinca @ruyadorno
tools: replace sed with perl
9711bc2 
For cross-platform compatibility use perl instead of sed.

Fixes: #48496
PR-URL: #48499
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
Reviewed-By: Mestery <mestery@protonmail.com>
Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
tools Issues and PRs related to the tools directory.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

tools: replace sed with perl lpinca/node
3 participants
@targos @richardlau @marco-ippolito