-
Notifications
You must be signed in to change notification settings - Fork 53
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
feat: add example for os & arch detection #46
base: main
Are you sure you want to change the base?
Conversation
# TODO: Adapt the OS & Architecture naming convention for <YOUR TOOL> | ||
# See the release flavours in the /releases page of <YOUR TOOL> | ||
#local uname_s="$(uname -s)" | ||
#local uname_m="$(uname -m)" | ||
#local os arch | ||
|
||
#case "$uname_s" in | ||
#FreeBSD) os="freebsd" ;; | ||
#Darwin) os="darwin" ;; | ||
#Linux) os="linux" ;; | ||
#*) fail "OS not supported: $uname_s" ;; | ||
#esac | ||
|
||
#case "$uname_m" in | ||
#i?86) arch="386" ;; | ||
#x86_64) arch="amd64" ;; | ||
#aarch64) arch="arm64" ;; | ||
#armv8l) arch="arm64" ;; | ||
#arm64) arch="arm64" ;; | ||
#armv7l) arch="arm" ;; | ||
#mips) arch="mips" ;; | ||
#mipsel) arch="mipsle" ;; | ||
#mips64) arch="mips64" ;; | ||
#mips64el) arch="mips64le" ;; | ||
#*) fail "Architecture not supported: $uname_m" ;; | ||
#esac |
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 created a plugin using this template. This PR comment line would indeed be helpful 👍
However, I believe it is more appropriate to have it in template/bin/download
. This is because many plugins detected the OS and architecture before calling download_release()
. Required to extract the command file itself.
e.g.)
# File generated based on template/bin/download
# Download tar.gz file to the download directory
release_file="${TOOL_NAME}-${ASDF_INSTALL_VERSION}-${arch}-${os}.tar.gz"
# Download tar.gz file to the download directory
download_release "$ASDF_INSTALL_VERSION" "$release_file"
# Extract contents of tar.gz file into the download directory
tar -xzf "$release_file" -C "$ASDF_DOWNLOAD_PATH" --strip-components=1 || fail "Could not extract $release_file"
# Remove the tar.gz file since we don't need to keep it
rm "$release_file"
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 would prefer a separate function in the lib/utils.bash
file that has a call commented out in the bin/download
file. Can someone please update this to 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.
like this :
get_machine_os() {
local OS
OS=$(uname -s | tr '[:upper:]' '[:lower:]')
case "${OS}" in
darwin*) echo "darwin" ;;
linux*) echo "linux" ;;
*) fail "OS not supported: ${OS}" ;;
esac
}
get_machine_arch() {
local ARCH
ARCH=$(uname -m | tr '[:upper:]' '[:lower:]')
case "${ARCH}" in
i?86) echo "386" ;;
x86_64) echo "amd64" ;;
aarch64) echo "arm64" ;;
armv8l) echo "arm64" ;;
arm64) echo "arm64" ;;
*) fail "Architecture not supported: $ARCH" ;;
esac
}
then later on :
download_release() {
local version filename url os arch
version="$1"
filename="$2"
// Optional, if your TOOL requires os and arch
// os=$(get_machine_os)
// arch=$(get_machine_arch)
[ -f "${filename}" ] || {
// TODO: If your tool requires OS and ARCH
// url="${GH_REPO}/releases/download/v${version}/${TOOL_NAME}_${version}_${os}_${arch}.tar.gz"
url="${GH_REPO}/releases/download/v${version}/${TOOL_NAME}_${version}.tar.gz"
echo "* Downloading ${TOOL_NAME} release ${version}"
curl "${curl_opts[@]}" -o "${filename}" -C - "${url}" || fail "Could not download ${url}"
}
}
- You can add more cases in
os
andmachine
, but people will remove or add other cases in order to guard against the supported aspects of their particular release policy. - the
${release_file}
is not describing the source fileurl to download, but the filepath where the download should be saved.
# TODO: Adapt the OS & Architecture naming convention for <YOUR TOOL> | ||
# See the release flavours in the /releases page of <YOUR TOOL> | ||
#local uname_s="$(uname -s)" | ||
#local uname_m="$(uname -m)" | ||
#local os arch | ||
|
||
#case "$uname_s" in | ||
#FreeBSD) os="freebsd" ;; | ||
#Darwin) os="darwin" ;; | ||
#Linux) os="linux" ;; | ||
#*) fail "OS not supported: $uname_s" ;; | ||
#esac | ||
|
||
#case "$uname_m" in | ||
#i?86) arch="386" ;; | ||
#x86_64) arch="amd64" ;; | ||
#aarch64) arch="arm64" ;; | ||
#armv8l) arch="arm64" ;; | ||
#arm64) arch="arm64" ;; | ||
#armv7l) arch="arm" ;; | ||
#mips) arch="mips" ;; | ||
#mipsel) arch="mipsle" ;; | ||
#mips64) arch="mips64" ;; | ||
#mips64el) arch="mips64le" ;; | ||
#*) fail "Architecture not supported: $uname_m" ;; | ||
#esac |
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.
like this :
get_machine_os() {
local OS
OS=$(uname -s | tr '[:upper:]' '[:lower:]')
case "${OS}" in
darwin*) echo "darwin" ;;
linux*) echo "linux" ;;
*) fail "OS not supported: ${OS}" ;;
esac
}
get_machine_arch() {
local ARCH
ARCH=$(uname -m | tr '[:upper:]' '[:lower:]')
case "${ARCH}" in
i?86) echo "386" ;;
x86_64) echo "amd64" ;;
aarch64) echo "arm64" ;;
armv8l) echo "arm64" ;;
arm64) echo "arm64" ;;
*) fail "Architecture not supported: $ARCH" ;;
esac
}
then later on :
download_release() {
local version filename url os arch
version="$1"
filename="$2"
// Optional, if your TOOL requires os and arch
// os=$(get_machine_os)
// arch=$(get_machine_arch)
[ -f "${filename}" ] || {
// TODO: If your tool requires OS and ARCH
// url="${GH_REPO}/releases/download/v${version}/${TOOL_NAME}_${version}_${os}_${arch}.tar.gz"
url="${GH_REPO}/releases/download/v${version}/${TOOL_NAME}_${version}.tar.gz"
echo "* Downloading ${TOOL_NAME} release ${version}"
curl "${curl_opts[@]}" -o "${filename}" -C - "${url}" || fail "Could not download ${url}"
}
}
- You can add more cases in
os
andmachine
, but people will remove or add other cases in order to guard against the supported aspects of their particular release policy. - the
${release_file}
is not describing the source fileurl to download, but the filepath where the download should be saved.
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.
changing it to "just a suggestion".
😭 can't remove "changes requested"
@airtonix Sorry for the delay, if you:
I will re-review and most likely merge. |
hey folks, any opportunity on taking this to the finish line? I recently tried to create a plugin (and I need to create another one) and this addition is definitely really valuable! |
Add an example for OS & Architecture detection. This is commonly needed for compiled tools. Provide this as an example, instead of letting the plugin developer reinvent the wheel each time.
I realize that there are multiple ways to achieve this, please let me know if you would like me to take another approach.
P.S. I refactored the original code a bit -- it seems that in other places, local variables are initialized at the point of declaration, whenever possible.