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

About install node command #3075

Open
Buguin opened this issue Mar 31, 2023 · 1 comment · May be fixed by #3079
Open

About install node command #3075

Buguin opened this issue Mar 31, 2023 · 1 comment · May be fixed by #3079
Labels
installing node Issues with installing node/io.js versions. performance This relates to anything regarding the speed of using nvm. pull request wanted This is a great way to contribute! Help us out :-D

Comments

@Buguin
Copy link

Buguin commented Mar 31, 2023

Background
When I use "nvm install v19.0.0" to install Node, if the corresponding version of Node is not found in the repository, the file cannot be downloaded locally. However, the checksum is still verified.

image

Although a check was made using grep before the verification started, the nvm_compare_checksum function was still executed later. It is recommended to modify it to verify whether the downloaded file exists after downloading the file, and return if it does not exist. There is no need to execute the function further.

  nvm_err "Downloading ${TARBALL_URL}..."
  nvm_download -L -C - "${PROGRESS_BAR}" "${TARBALL_URL}" -o "${TARBALL}" || (
    command rm -rf "${TARBALL}" "${tmpdir}"
    nvm_err "Binary download from ${TARBALL_URL} failed, trying source."
    return 4
  )

 # Check if the file is successfully downloaded, the previous "return 4" actually did not return
  if [ ! -f "${TARBALL}" ]; then
      nvm_err 'Download file not found, skip checksum.'
      return 7
  fi

  if nvm_grep '404 Not Found' "${TARBALL}" >/dev/null; then
    command rm -rf "${TARBALL}" "${tmpdir}"
    nvm_err "HTTP 404 at URL ${TARBALL_URL}"
    return 5
  fi

  nvm_compare_checksum "${TARBALL}" "${CHECKSUM}" || (
    command rm -rf "${tmpdir}/files"
    return 6
  )
@ljharb
Copy link
Member

ljharb commented Mar 31, 2023

That sounds like a great improvement. Would you like to make a PR?

@ljharb ljharb added pull request wanted This is a great way to contribute! Help us out :-D performance This relates to anything regarding the speed of using nvm. installing node Issues with installing node/io.js versions. labels Mar 31, 2023
@Buguin Buguin linked a pull request Apr 6, 2023 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
installing node Issues with installing node/io.js versions. performance This relates to anything regarding the speed of using nvm. pull request wanted This is a great way to contribute! Help us out :-D
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants