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.

Sign up for GitHub

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

Jump to bottom

Fix known issues for cache #1260

Merged
merged 11 commits into from
Dec 12, 2022
Merged

Fix known issues for cache #1260

merged 11 commits into from
Dec 12, 2022

Conversation

Phantsure
Copy link
Contributor

@Phantsure Phantsure commented Dec 9, 2022 •
edited
Loading

Fix known issue with gzip fallback and bsd tar + zstd combination.

  • gzip fallback was failing as getCacheEntry return null when cache not found and throw no error.
  • bsd tar + zstd was failing as exec.exec doesn't support && so passed commands through cmd for that case only as suggested here

@Phantsure Phantsure force-pushed the phantsure/cache-testing branch from 9a0cb81 to 0c23c38 Compare December 9, 2022 11:10
@Phantsure Phantsure changed the title [IGNORE] testing PR Fix known issues with gzip fallback and bsd + zstd option for cache Dec 9, 2022
@Phantsure Phantsure changed the title Fix known issues with gzip fallback and bsd + zstd option for cache Fix known issues for cache Dec 9, 2022
@Phantsure Phantsure marked this pull request as ready for review December 9, 2022 11:24
@Phantsure Phantsure requested a review from a team as a code owner December 9, 2022 11:24
@Phantsure Phantsure requested a review from a team as a code owner December 12, 2022 06:54
bishal-pdMSFT
bishal-pdMSFT reviewed Dec 12, 2022
View reviewed changes
packages/cache/src/internal/tar.ts
Comment on lines +157 to +159
if (BSD_TAR_ZSTD) {
return args
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of this, just return directly from L152 and L154

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It returns only in case of bsd tar. Else it will go down

Phantsure and others added 2 commits December 12, 2022 17:44
@Phantsure @bishal-pdMSFT
Update packages/cache/src/internal/cacheHttpClient.ts
e7e1984 
Co-authored-by: Bishal Prasad <bishal-pdmsft@github.com>
@Phantsure
Address review comments
2d3c79e
@Phantsure Phantsure merged commit 8b695c1 into releases/cache-v3-beta Dec 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bishal-pdMSFT bishal-pdMSFT bishal-pdMSFT approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Phantsure @bishal-pdMSFT