Skip to content

Conversation

AThousandShips
Copy link
Member

@AThousandShips AThousandShips commented Jul 13, 2024

This improves turnaround time on large PRs where compilation is successful but unit testing or similar fails, forcing recompilation of unchanged code

Unsure what exactly should be part of the upload but placed the save step after the artifact prepare step and before the upload step to hopefully upload as identically as possible, assuming the artifact preparation might make for a better artifact with striping down, unsure what exactly is uploaded (it seems only the .scons-cache is uploaded, so might be able to move it earlier as the cache isn't touched by other steps)

This was a source of friction for me making some PRs where the compilation worked fine, but the changes were relatively substantial and unit tests kept failing on Linux builds due to sanitizer errors, being unable to run those locally I ended up having to run a few CI builds to try and fix it and since they had to be recompiled from scratch (or rather from master) it took longer than it could have, hence this PR

uses: actions/cache/save@v4
with:
path: ${{inputs.scons-cache}}
key: ${{inputs.cache-name}}-${{env.GODOT_BASE_BRANCH}}-${{github.ref}}-${{github.sha}}
Copy link
Member Author

Choose a reason for hiding this comment

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

Skipped the restore-keys here as we're not restoring from any possible keys just exactly this one

Copy link
Contributor

@Repiteo Repiteo left a comment

Choose a reason for hiding this comment

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

Great catch! Rebasing in larger PRs will always be tedious to a certain extent, but this will absolutely make minor tweaks less of a headache

@AThousandShips
Copy link
Member Author

Can't be easily cherry picked for 4.2 so will make a dedicated commit for that if this is merged, and one for 3.x, if desired

This improves turnaround time on large PRs where compilation is
successful but unit testing or similar fails, forcing recompilation of
unchanged code
@AThousandShips
Copy link
Member Author

Moved the upload immediately after the compile step as it only involves the SCons cache

@AThousandShips AThousandShips added cherrypick:3.x Considered for cherry-picking into a future 3.x release cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release labels Jul 13, 2024
@AThousandShips AThousandShips modified the milestones: 4.x, 4.3 Jul 13, 2024
@AThousandShips AThousandShips added cherrypick:3.5 Considered for cherry-picking into a future 3.5.x release cherrypick:4.1 labels Jul 13, 2024
@akien-mga akien-mga merged commit a526445 into godotengine:master Jul 17, 2024
@akien-mga
Copy link
Member

Thanks!

@AThousandShips AThousandShips deleted the cache_improve branch July 17, 2024 10:08
@AThousandShips
Copy link
Member Author

Thank you!

@AThousandShips AThousandShips removed cherrypick:3.x Considered for cherry-picking into a future 3.x release cherrypick:3.5 Considered for cherry-picking into a future 3.5.x release cherrypick:4.1 cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release labels Jul 17, 2024
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.

3 participants