Hotfix Zstd breaking due to version change in runners by lvpx · Pull Request #1353 · actions/toolkit

lvpx · 2023-02-20T13:13:52Z

Summary

Currently zstd check is dependent on the text output of the zstd --version to include zstd command line interface. This text changed to *** Zstandard CLI (64-bit) v1.5.4, by Yann Collet *** breaking the check and @actions/cache always falling back to gzip compression.

Implementation

Set to use gzip if the version output string is empty which should be the case if zstd is not installed.
Introduced --quiet flag to zstd so moving on the zstd --version should output machine readable version parsable via SemVer instead of text.
Removed code that checks for version less than 1.3.2 as it was not working. Now we default to using zstd without --long as that is what currently happens.

Next Steps

Will have another PR that fixes use of zstd with --long as intended. This release should unblock zstd usage.

Issues

Fixes #1341

…rking. Defaulting to zstd without long as that is what is always happening currently.

packages/cache/src/internal/cacheUtils.ts

bishal-pdMSFT · 2023-02-20T14:14:44Z

Checks are failing. Please fix the unit tests.

packages/cache/src/internal/cacheUtils.ts

…e on version for now

cdce8p · 2023-02-20T22:58:46Z

packages/cache/src/internal/cacheUtils.ts

+  core.debug(`Checking ${app} ${additionalArgs.join(' ')}`)
  try {
-    await exec.exec(`${app} --version`, [], {
+    await exec.exec(`${app}`, additionalArgs, {


Thanks again @pdotl!

Just saw this while looking at the PR. '${app}' seems a bit redundant. The first argument should be a string and app is already one. Maybe you can fix that when you address the zstd --long issue. It would be a bit much to open a separate PR for it 😅

Sure @cdce8p will take this up in the next PR.

Phantsure

Looks fine to remove --long check entirely. --quiet would supress warnings but not clear how that would help in semver readable text

cdce8p · 2023-02-21T08:50:47Z

--quiet would supress warnings but not clear how that would help in semver readable text

Found this in the documentation for zstd

V, --version: display version number and exit. Advanced : -vV also displays supported formats. -vvV also displays POSIX support. -q will only display the version number, suitable for machine reading.

lvpx added 5 commits February 20, 2023 10:05

Add debug statements

f2aa430

Test passing quiet as argument

652109d

bump version

c2d3089

Fix bug with version shortcircuiting because of version being null

3630ea6

Hotfix zstd version change only

9e06993

lvpx requested a review from a team as a code owner February 20, 2023 13:13

lvpx added 4 commits February 20, 2023 13:27

Removed code that checks for version less than 1.3.2 as it was not wo…

1d1d545

…rking. Defaulting to zstd without long as that is what is always happening currently.

Fix lint issues

83dffb7

Fix version number

4fd4259

0.0.1

e18b2d8

lvpx requested a review from a team as a code owner February 20, 2023 13:44

lvpx force-pushed the pdotl/zstd-version-fix branch from 976c8e8 to 853e85a Compare February 20, 2023 13:52

Bump version using npm

0f91c9c

lvpx force-pushed the pdotl/zstd-version-fix branch from 853e85a to 0f91c9c Compare February 20, 2023 13:53