-
Notifications
You must be signed in to change notification settings - Fork 287
Fix Debian GitHub build & zstd CMake error #200
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
Conversation
It fixed the issue above, but the Debian build now fails on the cmake/zstd issue. zstd 1.5.4 was released in Jan-23 but preceding v1.5.2 was in Jan-22 (a whole year in-between), so not sure when we might see the fix (on dev branch) merged into release. We could do what is done by this repo, i.e., cherrypicking the commit:
Or pin it at 1.5.2 or that commit with the fix (con: need to manually update it later), or use the dev branch (con: potentially less stable than release). |
Decided to pin zstd to the commit that has the cmake fix. Pinning it might be a good idea anyway for stability. Doing it in general and not just for Debian since it affects all systems with an older cmake. |
Debian uses gcc-8.3 and thus now fails on the issue fixed by #201. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for working on this.
Updated as suggested. Thanks! Also, I got a successful Debian build on my fork after I cherrypicked #201: https://github.com/wonglkd/CacheLib-1/actions/runs/4298872004/jobs/7493484190 |
Made requested changes and rebased. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for fixing this.
Somehow I cannot import and merge this. @wonglkd Just in case, could you rebase? |
Workaround for Debian Docker bug.
Pin zstd at commit that fixes cmake error (after 1.5.4)
Used external_git_tag instead, added comments, fixed a typo.
2d74276
to
c97b8e7
Compare
@jaesoo-fb rebased! |
@jaesoo-fb has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
@jaesoo-fb merged this pull request in 185bbe6. |
Context for 1: In latest Debian Docker image , there is a regression that affects the checkout action.
From actions/checkout#1169:
The suggested workaround was to use --system instead of --global.
Test Plan: See if GitHub Action Debian build is fixed.