10BC0 Fix cache missing by xfangfang · Pull Request #149 · flatpak/flatpak-github-actions · GitHub
[go: up one dir, main page]
Skip to content

Fix cache missing #149

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
Merged
merged 1 commit into from
Aug 27, 2023
Merged

Fix cache missing #149

merged 1 commit into from
Aug 27, 2023

Conversation

xfangfang
Copy link
Contributor

Due to an oversight in actions/toolkit, the CACHE_PATH was modified after running cache.restoreCache function. As a result, an incorrect version was generated during cache upload, leading to cache missing.

This PR independently passes the CACHE_PATH to avoid issues caused by s 8000 hallow copying.

Fix issue：#142

Refer to : actions/toolkit#1378 (comment)

@bilelmoussaoui
Copy link
Member

Oh no lol

bilelmoussaoui
bilelmoussaoui reviewed Jul 24, 2023
View reviewed changes
flatpak-builder/index.js Outdated
Comment on lines 243 to 245
[
'.flatpak-builder'
],
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
[
'.flatpak-builder'
],
[...CACHE_PATH], // TODO: drop once https://github.com/actions/toolkit/pull/1378 is merged

is better for me

also you will have to rebuild the index.js, see contributing guidelines

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bilelmoussaoui
I made the modifications as required. However, when I compiled using ncc, the generated files differed significantly from the previous ones. I checked the previous pull requests, but didn't find this issue. Since I'm not very familiar with node.js and related things, I manually edited the dist/index.js.
I hope it works fine now.

Copy link
Member

Choose a reason for hiding this comment

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

No, please just run ncc & push your changes. The file should never be modified manually

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Update flatpak-builder/dist/index.js compiled by nodes 18.17.0 / yarn 1.22.19 / ncc 0.36.1

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, I know what to do now. Please wait a moment

@xfangfang xfangfang force-pushed the fix/cache branch 3 times, most recently from c916ac0 to c886bfd Compare July 24, 2023 08:59
@xfangfang
Copy link
Contributor Author

@bilelmoussaoui Now it's ready to merge, Thanks for the help~

@xfangfang
Copy link
Contributor Author

Rebased to fix conflicts

@bilelmoussaoui
Copy link
Member

Would you mind rebasing again please? Thank you

@xfangfang
Copy link
Contributor Author

Rebased again

@bilelmoussaoui bilelmoussaoui merged commit 1283416 into flatpak:master Aug 27, 2023
@bilelmoussaoui
Copy link
Member

Thank you !!!

@bbhtt bbhtt mentioned this pull request Jul 29, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bilelmoussaoui bilelmoussaoui bilelmoussaoui left review comments

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
@xfangfang @bilelmoussaoui
0