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.

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

wpyoga
Copy link
Contributor

@wpyoga wpyoga commented Feb 2, 2022

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.

Comment on lines +43 to +68
# 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
Copy link

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"

Copy link
Collaborator

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?

Copy link
Contributor

@airtonix airtonix Feb 24, 2023

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 and machine, 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.

Comment on lines +43 to +68
# 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
Copy link
Contributor

@airtonix airtonix Feb 24, 2023

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 and machine, 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.

Copy link
Contributor

@airtonix airtonix left a 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"

@jthegedus
Copy link
Collaborator

@airtonix Sorry for the delay, if you:

  • update to your proposed solution to my request for get_machine_os() in utils
  • rebase this PR

I will re-review and most likely merge.

@yordis
Copy link

yordis commented Oct 25, 2023

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants