-
-
Notifications
You must be signed in to change notification settings - Fork 3k
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
enhance(main/dart): enable auto updates #21508
base: master
Are you sure you want to change the base?
Conversation
210ae34
to
2373415
Compare
and various build script cleanup
e5126bd
to
01ac780
Compare
01ac780
to
da8b20c
Compare
Yes, I'll surely love to do the review! PS: I'm @priyanuj-gogoi (Using a different account) |
# Get latest release tag: | ||
local api_url="https://api.github.com/repos/dart-lang/sdk/git/refs/tags" |
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.
Use https://storage.googleapis.com/dart-archive/channels/stable/release/latest/VERSION
(official) instead of the GitHub API to fetch latest stable dart version.
The response of that GitHub API URL is too large and you've to filter the latest release later in which case it's not ideal.
TERMUX_PKG_SKIP_SRC_EXTRACT=true | ||
TERMUX_PKG_AUTO_UPDATE=true | ||
TERMUX_PKG_UPDATE_TAG_TYPE='newest-tag' | ||
TERMUX_PKG_UPDATE_VERSION_REGEXP='^\d+.\d+.\d+$' |
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.
Although this regex pattern is valid but it's not sed
compatible (like \d
is not supported, +
should be \+
).
Prefer [0-9]\+\.[0-9]\+\.[0-9]\+
.
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.
That check is done here.
termux-packages/scripts/updates/utils/termux_pkg_upgrade_version.sh
Lines 20 to 32 in 388cd7e
# If needed, filter version numbers using grep regexp. | |
if [[ -n "${TERMUX_PKG_UPDATE_VERSION_REGEXP:-}" ]]; then | |
# Extract version numbers. | |
local OLD_LATEST_VERSION="${LATEST_VERSION}" | |
LATEST_VERSION="$(grep -oP "${TERMUX_PKG_UPDATE_VERSION_REGEXP}" <<< "${LATEST_VERSION}" || true)" | |
if [[ -z "${LATEST_VERSION:-}" ]]; then | |
termux_error_exit <<-EndOfError | |
ERROR: failed to filter version numbers using regexp '${TERMUX_PKG_UPDATE_VERSION_REGEXP}'. | |
Ensure that it works correctly with ${OLD_LATEST_VERSION}. | |
EndOfError | |
fi | |
unset OLD_LATEST_VERSION | |
fi |
# Get latest release tag: | ||
local api_url="https://api.github.com/repos/dart-lang/sdk/git/refs/tags" | ||
local latest_refs_tags | ||
latest_refs_tags=$(curl -s "${api_url}" | jq '.[].ref' | sed -ne "s|.*${TERMUX_PKG_UPDATE_VERSION_REGEXP}\"|\1|p") |
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'm facing an error with the sed expression.
sed: -e expression #1, char 23: invalid reference \1 on `s' command's RHS
Perhaps, you've forgotten to add the group construct? i.e
"s|.*\(${TERMUX_PKG_UPDATE_VERSION_REGEXP}\)\"|\1|p"
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.
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.
Oh crap I completely forgot about this PR.
Consider removing |
So, I've tried testing the build artifact and looks like the build built 3.6.0 instead of the stable release. You can verify it by yourself:
The |
I also took the opportunity to do some miscellaneous build script cleanup,
switching to the GitHub source also allows us to enable auto updates for dart.
I would appreciate it if @priyanuj-gogoi could check this over,
reviews from anyone else are of course welcome as always.