-
Notifications
You must be signed in to change notification settings - Fork 29.8k
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
tools: automate icu update #47727
Merged
nodejs-github-bot
merged 9 commits into
nodejs:main
from
marco-ippolito:feat/automate-icu-update
May 9, 2023
Merged
tools: automate icu update #47727
Changes from all commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
3d9f617
tools: automate icu-small update
marco-ippolito 503ffb7
fix: replace - with .
marco-ippolito 0299d51
fix: md5 from source
marco-ippolito c82d718
fix: comparing checksum
marco-ippolito 84c6077
fix: mention the script in the maintaining guide
marco-ippolito e794280
fix: lint
marco-ippolito b915e51
fix: update timezone.txt
marco-ippolito 12a7f50
fix: remove tz update
marco-ippolito 15ff4c5
fix: use version as whole
marco-ippolito File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,75 @@ | ||
#!/bin/sh | ||
set -e | ||
# Shell script to update icu in the source tree to a specific version | ||
|
||
BASE_DIR=$(cd "$(dirname "$0")/../.." && pwd) | ||
DEPS_DIR="$BASE_DIR/deps" | ||
TOOLS_DIR="$BASE_DIR/tools" | ||
|
||
[ -z "$NODE" ] && NODE="$BASE_DIR/out/Release/node" | ||
[ -x "$NODE" ] || NODE=$(command -v node) | ||
|
||
NEW_VERSION="$("$NODE" --input-type=module <<'EOF' | ||
const res = await fetch('https://api.github.com/repos/unicode-org/icu/releases/latest'); | ||
if (!res.ok) throw new Error(`FetchError: ${res.status} ${res.statusText}`, { cause: res }); | ||
const { tag_name } = await res.json(); | ||
console.log(tag_name.replace('release-', '').replace('-','.')); | ||
EOF | ||
)" | ||
|
||
ICU_VERSION_H="$DEPS_DIR/icu-small/source/common/unicode/uvernum.h" | ||
|
||
CURRENT_VERSION="$(grep "#define U_ICU_VERSION " "$ICU_VERSION_H" | cut -d'"' -f2)" | ||
|
||
echo "Comparing $NEW_VERSION with $CURRENT_VERSION" | ||
|
||
if [ "$NEW_VERSION" = "$CURRENT_VERSION" ]; then | ||
echo "Skipped because icu is on the latest version." | ||
exit 0 | ||
fi | ||
|
||
DASHED_NEW_VERSION=$(echo "$NEW_VERSION" | sed 's/\./-/g') | ||
|
||
LOW_DASHED_NEW_VERSION=$(echo "$NEW_VERSION" | sed 's/\./_/g') | ||
|
||
NEW_VERSION_TGZ="icu4c-${LOW_DASHED_NEW_VERSION}-src.tgz" | ||
|
||
NEW_VERSION_TGZ_URL="https://github.com/unicode-org/icu/releases/download/release-${DASHED_NEW_VERSION}/$NEW_VERSION_TGZ" | ||
|
||
NEW_VERSION_MD5="https://github.com/unicode-org/icu/releases/download/release-${DASHED_NEW_VERSION}/icu4c-${LOW_DASHED_NEW_VERSION}-src.md5" | ||
|
||
./configure --with-intl=full-icu --with-icu-source="$NEW_VERSION_TGZ_URL" | ||
|
||
"$TOOLS_DIR/icu/shrink-icu-src.py" | ||
|
||
rm -rf "$DEPS_DIR/icu" | ||
|
||
CHECKSUM=$(curl -sL "$NEW_VERSION_MD5" | grep "$NEW_VERSION_TGZ" | grep -v "\.asc$" | awk '{print $1}') | ||
|
||
GENERATED_CHECKSUM=$( curl -sL "$NEW_VERSION_TGZ_URL" | md5sum | cut -d ' ' -f1) | ||
|
||
echo "Comparing checksums: deposited $CHECKSUM with $GENERATED_CHECKSUM" | ||
|
||
if [ "$CHECKSUM" != "$GENERATED_CHECKSUM" ]; then | ||
echo "Skipped because checksums do not match." | ||
exit 0 | ||
fi | ||
|
||
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" | ||
|
||
rm -rf out "$DEPS_DIR/icu" "$DEPS_DIR/icu4c*" | ||
|
||
echo "All done!" | ||
echo "" | ||
echo "Please git add icu, commit the new version:" | ||
echo "" | ||
echo "$ git add -A deps/icu-small" | ||
echo "$ git add tools/icu/current_ver.dep" | ||
echo "$ git commit -m \"deps: update icu to $NEW_VERSION\"" | ||
marco-ippolito marked this conversation as resolved.
Show resolved
Hide resolved
|
||
echo "" | ||
|
||
# The last line of the script should always print the new version, | ||
# as we need to add it to $GITHUB_ENV variable. | ||
echo "NEW_VERSION=$NEW_VERSION" |
Empty file.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We'll likely need to update
./test/fixtures/tz-version.txt
to match the timezone version shipped in the new version of ICU.Refs: #47456 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and there are potential 'golden' value breakage, but not much to do about that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dont we have an action that does this? #47302
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@marco-ippolito The timezone data gets baked into the ICU data file, which updating ICU will overwrite. If
./test/fixtures/tz-version.txt
doesn't match the timezone data version in the ICU data file then the associated test will fail.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have managed to get the latest version from https://data.iana.org/time-zones/releases/ but its a bit hacky @srl295 do you know any api we could fetch the latest version?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think what you should do is extract the tz version from the ICU you're updating to, and put that in
tz-version.txt
.Maybe
And then in reviewing the PR make sure it's not a regression.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think that will work because the update script added here doesn't rebuild Node.js, so that will output the tz version of whatever
node
binary is on the runner instead of the one from the data. I presume the tz version must be in the updated ICU files but maybe it'll be too complicated to extract without building Node.js with the new data?I suppose we could ignore it and fix up manually if the opened PR breaks because of a different tz version in the new ICU data.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@richardlau it ought to build to make sure node works… the PR will do that, though. the tz version is deep in the gzipped binary .dat file.
Yes, it could be ignored and fixed manually . As i said it's likely there will be some manual tweaks as Node has been made more sensitive to these changes.
In that case, just don't modify tz-version.txt at all in this workflow.