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

Fix downloads for @yarnpkg/cli-dist #647

Merged
merged 2 commits into from
Sep 18, 2023
Merged

Conversation

colincasey
Copy link
Contributor

@colincasey colincasey commented Sep 14, 2023

The changes to make this download script work for both npm and yarn distributions #612 has a bug where the requested distribution (yarn) differs from its package name if the version is >=2 (@yarnpkg/cli-dist) and that wasn't accounted for in the name of the download file.

The curl command that downloads the tarball should have been saving to yarn-v{x.y.z}.tar.gz but was instead saving to @yarnpkg/cli-dist-v{x.y.z}.tar.gz. This is causing automation failures (e.g.; https://github.com/heroku/buildpacks-nodejs/actions/runs/6179740345/job/16775160336)

This PR keeps the distribution_name and package_name separate so this mismatch will no longer happen.

It also:

  • adds some extra logging which would have made the error more obvious
  • DRYS up the npm_url, tarball_url, and downloaded_tarball which previously were being inlined into commands
  • adds retries and timeouts to curl

The changes to make this download script work for both `npm` and `yarn` distributions [#612](#612) has a bug where the requested distribution (`yarn`) differs from its package name if the version is `>=2` (`@yarnpkg/cli-dist`) and that wasn't accounted for in the name of the download file.

The `curl` command that downloads the tarball should have been saving to `yarn-v{x.y.z}.tar.gz` but was instead saving to `@yarnpkg/cli-dist-v{x.y.z}.tar.gz`. This is causing automation failures (e.g.; https://github.com/heroku/buildpacks-nodejs/actions/runs/6179740345/job/16775160336)

This PR keeps the `distribution_name` and `package_name` separate so this mismatch will no longer happen.

It also:
- adds some extra logging which would have made the error more obvious
- DRYS up the `npm_url`, ``tarball_url`, and `download_file` which previously were being inlined into commands
- adds retries and timeouts to `curl`
common/bin/download-verify-npm-package Outdated Show resolved Hide resolved
@colincasey colincasey enabled auto-merge (squash) September 18, 2023 19:09
@colincasey colincasey merged commit 6cbd518 into main Sep 18, 2023
15 checks passed
@colincasey colincasey deleted the fix_yarnpkg_cli-dist_downloads branch September 18, 2023 19:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants