ci: run functional tests on GitHub Actions, upload functional test logs as artifacts #6583
Conversation
|
GitHub Actions run for 4c221cc, https://github.com/kwvg/dash/actions/runs/13364618784 |
WalkthroughThis update set modifies several components of the CI/CD pipeline. In the GitHub Actions workflows, new outputs and steps are added to manage build artifacts. A test workflow file named Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (10)
ci/dash/bundle-logs.sh (1)
27-28: Optional: Verify zstd Dependency
Since this script relies onzstdfor compression, consider adding an early check (e.g., using[ -z "$(command -v zstd)" ]) to ensure thatzstdis installed. This would improve the script’s robustness in environments wherezstdmight be missing.ci/dash/bundle-output.sh (1)
13-25: Comprehensive Input Validation
The series of checks ensure that the mandatory environment variableBUILD_TARGETis defined, that thezstdcompressor is available, and that a valid operation verb is provided.Suggestion:
To improve reliability, replace the check:-elif [ ! "$(command -v zstd)" ]; then +elif [ -z "$(command -v zstd)" ]; thenThis more explicitly checks for an empty result from
command -v zstd.ci/dash/bundle-build.sh (3)
12-21: Validation of Essential Environment Variables
Checking for the presence ofBUILD_TARGET,ARTIFACT_PATH, and the necessaryzstdutility ensures that the build process does not proceed under misconfigured conditions.Suggestion:
Similar to the previous script, refine the zstd check as follows:-elif [ ! "$(command -v zstd)" ]; then +elif [ -z "$(command -v zstd)" ]; thenThis change clarifies the condition when
zstdis not available.
56-58: Creative Use of Brace Expansion for Directory Creation
The script uses a subshell with brace expansion to create directories under the artifact base path. While this approach is clever, it might be a bit opaque to future maintainers.Suggestion:
Consider adding a brief comment or refactoring this logic for improved readability—possibly by assigning the expanded paths to a variable before callingmkdir.
60-68: Efficient Copying of Build Binaries
The conditional block that locates and copies the main daemon, benchmarking, and test binaries from the build directory to the artifact directory is effective.Observation:
The use of nestedbash -c "echo ..."constructs for brace expansion in thecpcommand is functionally correct but somewhat intricate. Simplifying or isolating this logic into a helper function (if appropriate) could further improve maintainability..github/workflows/test-src.yml (2)
55-65: Run Functional Tests Step: Quote Variables to Prevent Word Splitting
The functional tests are run after configuring Git settings and extracting build outputs. However, when invoking the integration tests, the variable${INTEGRATION_TESTS_ARGS}is unquoted.Suggestion:
Wrap the variable in quotes to prevent unintended word splitting:- ./ci/dash/test_integrationtests.sh ${INTEGRATION_TESTS_ARGS} + ./ci/dash/test_integrationtests.sh "${INTEGRATION_TESTS_ARGS}"🧰 Tools
🪛 actionlint (1.7.4)
57-57: shellcheck reported issue in this script: SC2086:info:6:36: Double quote to prevent globbing and word splitting
(shellcheck)
66-74: Bundling Test Logs with Conditional Logic
The step to bundle test logs correctly exportsBUILD_TARGET, writes a short SHA toGITHUB_OUTPUT, and conditionally runs the log bundling script if thetestlogsdirectory exists. The logic using a compound condition (with&&/||) is concise.Note:
Double-check that the intended precedence is achieved; while it appears correct, adding parentheses might improve clarity..github/workflows/build-src.yml (1)
39-51: Initial Setup Step: Git Configuration and Environment Preparation
The “Initial setup” step configures Git to suppress detached head advice, adjusts the safe directory, and retrieves necessary details (such asHOSTandPR_BASE_SHA) for later steps.Suggestion:
Consider quoting variables when appending to$GITHUB_OUTPUTfor added safety.🧰 Tools
🪛 actionlint (1.7.4)
41-41: shellcheck reported issue in this script: SC2086:info:8:24: Double quote to prevent globbing and word splitting
(shellcheck)
41-41: shellcheck reported issue in this script: SC2086:info:9:71: Double quote to prevent globbing and word splitting
(shellcheck)
.github/workflows/build.yml (1)
7-10: Permissions Update – Validate Minimal Required PermissionsThe addition of
actions: writein this permissions block is essential for enabling artifact uploads. Please review that granting write access to actions is scoped appropriately and does not introduce unnecessary security risks.contrib/containers/ci/Dockerfile (1)
160-166: GitHub CLI Installation BlockThis new section adds the GitHub CLI repository, fetches its GPG key, and installs
gh. This setup is aligned with the PR’s objective to manage build and test artifacts. Consider pinning a specific version ofghto avoid unexpected upgrades impacting reproducibility.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.github/workflows/build-src.yml(2 hunks).github/workflows/build.yml(2 hunks).github/workflows/test-src.yml(1 hunks)ci/dash/bundle-build.sh(1 hunks)ci/dash/bundle-logs.sh(1 hunks)ci/dash/bundle-output.sh(1 hunks)contrib/containers/ci/Dockerfile(2 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/test-src.yml
57-57: shellcheck reported issue in this script: SC2086:info:6:36: Double quote to prevent globbing and word splitting
(shellcheck)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: x86_64-w64-mingw32 / Build depends
- GitHub Check: x86_64-pc-linux-gnu_multiprocess / Build depends
- GitHub Check: arm-linux-gnueabihf / Build depends
- GitHub Check: x86_64-pc-linux-gnu / Build depends
- GitHub Check: x86_64-pc-linux-gnu_nowallet / Build depends
🔇 Additional comments (43)
ci/dash/bundle-logs.sh (6)
1-5: Header and Licensing Section Looks Good
The shebang, copyright notice, and license information are correctly set up.
6-9: Robust Environment Configuration
Setting the locale toC.UTF-8and enabling strict error handling withset -eo pipefailensures a predictable environment.
10-12: Clear Variable Initialization
AssigningSH_NAMEand definingLOG_DIRECTORYmakes the script’s purpose and logging context clear.
13-19: Well-Structured Conditional Checks
The script first checks for the existence of the${LOG_DIRECTORY}and then verifies if the requiredBUILD_TARGETvariable is defined. The use of an exit code of 0 for a missing log directory (to skip archiving) versus a 1 for a missing build target is clear and intentional.
21-25: Prevention of Archive Overwrite
By checking for the existence of the archive file (${LOG_ARCHIVE}) before proceeding, the script avoids accidental overwrites.
27-28: Efficient Archiving and Checksum Generation
The use oftarwithzstdcompression and the subsequent generation of a SHA-256 checksum provide a reliable method for archiving log files.ci/dash/bundle-output.sh (7)
1-5: Script Header is In Order
The shebang and licensing headers are properly included, establishing the correct execution environment.
6-8: Strong Environment Setup
Setting the locale and usingset -eo pipefailensures that errors are caught early, which is essential for CI scripts.
10-12: Proper Initialization of Key Variables
InitializingSH_NAMEand capturing the first argument intoVERBsets up a clear context for later operations.
27-34: Output Archive Existence Check
The validation logic that prevents archive creation when one already exists (or its absence for extraction) is clear and prevents data loss.
36-52: Handling 'create' Operation with Exclusions
Constructing an array of exclusion patterns and building the exclusion argument string fortaris a practical approach. The subsequent command to create the archive with compression is well implemented.
56-59: Proper Extraction and Cleanup for 'extract' Operation
The extraction logic correctly usesunzstdto decompress and then removes the archive, which helps in managing disk space efficiently.
60-62: Catch-All Error Handling
The generic error branch appropriately covers any unexpected verb values, ensuring the script fails safely.ci/dash/bundle-build.sh (8)
1-8: Solid Header and Environment Setup
The script’s shebang, licensing information, locale setting, and strict error mode are correctly established.
23-27: Archive Existence Check Prevents Overwrite
The logic that halts execution if the target archive (${ARTIFACT_ARCHIVE}) already exists helps avoid unintended data loss.
29-32: Adaptive File Extension and Artifact Directory Setup
Dynamically settingFILE_EXTENSIONbased on theBUILD_TARGET(e.g., appending.exefor Windows) and preparing theARTIFACT_DIRSstring ensures the script adapts to different build environments.
41-48: Dynamic Binary Name Construction
The initialization and adjustment ofBIN_NAMESbased on conditions such as wallet inclusion or multiprocess support demonstrates flexibility in handling various build configurations.
72-76: Handling of Qt Application Artifacts
The script correctly checks for the existence of the Qt binary and its corresponding test binary before copying them, ensuring that only available files are processed.
78-81: Copying of Fuzz Testing Binary
The inclusion of the fuzz testing binary in the artifact directory is handled simply and clearly.
83-86: Platform-Specific Artifact Handling for macOS
The conditional copying of theDash-Core.ziparchive for macOS targets is appropriate and maintains consistency across platforms.
88-94: Robust Archive Creation and Checksum Calculation
Changing into the artifact path before creating the archive ensures the desired directory structure. The use ofzstdfor compression and the generation of a SHA-256 checksum for integrity verification are both well implemented..github/workflows/test-src.yml (8)
1-18: Clear Workflow Trigger and Input Definitions
The workflow is correctly configured to useworkflow_callwith mandatory inputs (build-key,build-target, andcontainer-path), ensuring that downstream jobs receive the necessary context.
19-24: Appropriate Permissions and Global Environment Settings
Granting write permissions to actions and defining theINTEGRATION_TESTS_ARGSenvironment variable provide a solid foundation for the workflow’s operations.
25-32: Well-Defined Job Environment and Container Usage
Specifying the job to run onubuntu-24.04and using a container image (with the override of the user to root) ensures a controlled and predictable runtime environment for the tests.
33-38: Reliable Code Checkout
Utilizingactions/checkout@v4to fetch the repository at the specific commit referenced by the pull request ensures that the tests run on the correct code version.
39-46: Effective Build Outputs Restoration
Restoring the cached build outputs using a defined key and path ensures that necessary build artifacts are available before tests commence.
47-54: Selective Cache Management for Releases
The conditional caching of releases specific to thelinux64build target, based on hashed script files, helps maintain cache integrity.
75-83: Cache Deletion Enhances Storage Efficiency
The step to delete the build outputs cache using the GitHub CLI (gh cache delete) is a pragmatic approach to managing limited storage—along with a clear warning message for subsequent runs.
84-97: Uploading Test Logs as Artifacts Works as Intended
The final step conditionally uploads the test logs as artifacts usingactions/upload-artifact@v4. The artifact name construction incorporatingbuild-targetand the short SHA aids in traceability. The use of a zero compression level and specified retention period are well-tuned for testing artifacts..github/workflows/build-src.yml (9)
1-13: Comprehensive Workflow Input and Output Setup
The workflow’s definition, including required inputs (build-target,container-path,depends-key) and the output declaration forkey, is clear and enables downstream workflows to access build artifacts reliably.
23-32: Job Configuration for Build Source is Well-Structured
Defining thebuild-srcjob with its outputs (sourced from the bundle step) and specifying a containerized environment ensures consistent build behavior.
33-37: Reliable Code Checkout in Build Job
Usingactions/checkout@v4with the appropriate ref and fetch depth guarantees that the build process uses the correct revision.
53-61: Efficient Restoration of Dependency Caches
Restoring thedependscache using well-defined keys ensures that dependencies are retrieved quickly, which is crucial for fast builds.
62-71: Effective ccache Management
Caching the compiler cache (/cache) with multiple restore keys (including host-specific keys) is a pragmatic strategy to accelerate builds.
73-81: Combining Build and Unit Test Execution
The step that builds the source and runs unit tests is straightforward and consolidates critical build operations, ensuring early detection of issues.
83-91: Bundling Outputs and Artifacts with Randomized Key Generation
The step to bundle outputs uses thebundle-output.shand subsequently callsbundle-build.shwith theARTIFACT_PATHoverridden. The unique key generation using/dev/urandomis clever, though ensure that such randomness is acceptable for the cache key’s intended use.Note:
If reproducibility of the cache key is ever required, you might reconsider using a random key.
92-103: Saving Build Outputs to Cache
The conditional caching of build outputs for specific target strings and linking the cache key to the bundled output ensures consistency between builds and tests.
104-113: Uploading Build Artifacts with Clear Naming Conventions
The artifact upload step correctly names the artifacts with thebuild-targetand short SHA, and the specified parameters (compression level, overwrite flag, retention days) are suitable for build artifacts..github/workflows/build.yml (4)
141-150: New Test Job 'test-linux64' ConfigurationThe job correctly uses the
test-src.ymlworkflow and lists the appropriate dependencies (container,depends-linux64, andsrc-linux64). The passed input parameters (build-key, build-target, container-path) match the expected requirements for functional testing.
151-159: New Test Job 'test-linux64_nowallet' ConfigurationThis job is configured consistently with its dependencies (
container,depends-linux64_nowallet, andsrc-linux64_nowallet) and inputs. Everything appears correctly integrated for testing the nowallet build variant.
160-168: New Test Job 'test-linux64_sqlite' ConfigurationThe job reuses the
depends-linux64dependency along withsrc-linux64_sqliteand the container. Please verify that sharing the dependency with the standard linux64 build is intentional and meets the specific requirements for sqlite-based testing.
169-177: New Test Job 'test-linux64_ubsan' ConfigurationThe configuration for the ubsan variant follows the same pattern as the other test jobs, specifying the necessary dependencies and inputs. The structure is consistent and clear.
contrib/containers/ci/Dockerfile (1)
192-193: Zstd Package InstallationThe inclusion of the
zstdpackage supports the new artifact compression requirements. This addition is straightforward and meets the intended functionality.
There was a problem hiding this comment.
Seems to be working https://github.com/UdjinM6/dash/actions/runs/13461635642
light ACK 4c221cc
|
Edit: |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (7)
ci/dash/slim-workspace.sh (2)
12-18: Robust Environment Variable Checks
The script correctly validates that bothBUILD_TARGETandBUNDLE_KEYare defined before proceeding. For added usability, consider including a brief suggestion on how to define these variables in the error messages.
38-43: Effective Workspace Slimming Loop
The iteration over theTARGETSarray and removal usingrm -rfis clear and effective. Optionally, you could add logging (e.g., echoing the target being removed) to aid in debugging..github/workflows/build-src.yml (1)
98-105: Bundle Artifacts Step Integration
The new "Bundle artifacts" step effectively generates a unique key (using the build target and commit short SHA) and creates the artifact bundle. Note that a shellcheck warning was disabled here; if possible, consider refactoring the variable assignments to satisfy shellcheck guidelines.🧰 Tools
🪛 actionlint (1.7.4)
100-100: shellcheck reported issue in this script: SC2155:warning:2:8: Declare and assign separately to avoid masking return values
(shellcheck)
ci/dash/bundle-artifacts.sh (2)
39-51: Tar Exclusions Construction
Building the exclusion arguments dynamically for the tar command is effective. The use ofshellcheck disable=SC2086indicates awareness of potential quoting issues; you might consider logging the resultingEXCLUSIONS_ARGto aid in future debugging.
52-59: Archive Operations and Error Handling
Both the archive creation (using tar with zstd compression) and extraction (using unzstd) commands are correctly implemented. Adding an explicit error check post-tar execution could further enhance robustness..github/workflows/test-src.yml (2)
49-60: Run Functional Tests Step
The sequence of commands—from configuring git safety settings, extracting artifacts viabundle-artifacts.sh extract, slimming the workspace, to executing integration tests—is logically sequenced. Consider quoting${INTEGRATION_TESTS_ARGS}to prevent any unintended word splitting in cases where the variable might contain spaces.🧰 Tools
🪛 actionlint (1.7.4)
51-51: shellcheck reported issue in this script: SC2086:info:7:36: Double quote to prevent globbing and word splitting
(shellcheck)
61-69: Bundle Test Logs Step: Quoting Consideration
In the step that bundles test logs, ensure that all variable expansions are adequately quoted to avoid globbing issues. This aligns with the shellcheck hint regarding double quoting for safety.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.github/workflows/build-src.yml(2 hunks).github/workflows/build.yml(1 hunks).github/workflows/test-src.yml(1 hunks)ci/dash/bundle-artifacts.sh(1 hunks)ci/dash/bundle-logs.sh(1 hunks)ci/dash/slim-workspace.sh(1 hunks)contrib/containers/ci/Dockerfile(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- contrib/containers/ci/Dockerfile
- ci/dash/bundle-logs.sh
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/test-src.yml
51-51: shellcheck reported issue in this script: SC2086:info:7:36: Double quote to prevent globbing and word splitting
(shellcheck)
.github/workflows/build-src.yml
100-100: shellcheck reported issue in this script: SC2155:warning:2:8: Declare and assign separately to avoid masking return values
(shellcheck)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Build container / Build container
🔇 Additional comments (6)
.github/workflows/build.yml (1)
157-193: New Test Jobs Configuration
The newly added test jobs (e.g.,test-linux64,test-linux64_nowallet,test-linux64_sqlite, andtest-linux64_ubsan) are well configured with appropriate dependencies and inputs. This ensures that each test job reliably receives the correct artifact key and container path from the build jobs..github/workflows/build-src.yml (2)
18-22: Enhanced Workflow Output Declaration
Adding thekeyoutput under theworkflow_callevent streamlines downstream artifact handling. Verify that the output correctly propagates the intended artifact key.
27-28: Job Output Propagation
The configuration to retrieve thekeyfrom the bundle step in thebuild-srcjob is well implemented. This setup facilitates reliable access for subsequent jobs.ci/dash/bundle-artifacts.sh (2)
13-28: Comprehensive Pre-execution Checks
The script robustly verifies the presence ofBUILD_TARGET,BUNDLE_KEY, and the availability of thezstdcommand, as well as validating the provided operation verb. This ensures that the script fails fast on misconfiguration.
30-37: Archive Existence Verification
The logic that checks for the presence or absence of the archive (based on whether the operation iscreateorextract) is well implemented, preventing accidental overwrites or missing inputs..github/workflows/test-src.yml (1)
71-83: Upload Test Logs Step Verification
The "Upload test logs" step is well configured with the correct conditional logic and artifact naming convention, ensuring that logs are uploaded only when appropriate.
|
GitHub Actions run for d26c253, https://github.com/kwvg/dash/actions/runs/13520276093 |
kwvg
requested review from
UdjinM6 and
PastaPastaPasta
and removed request for
UdjinM6 and
PastaPastaPasta
We want logs even if tests fail, so we need some extra logic. Also, we shouldn't create artifacts if there's nothing to upload, so there's extra logic for that too.
We don't offer alternate restore keys because get_previous_releases.py bails if it already finds a 'releases' directory present, so a cache miss is desirable to allow the script to re-fetch them.
Currently, we are experiencing issues surrounding exhausting disk space during the log collection of these runs. While actions exist to free up space on the runner, they are unusable when the job runs in a container context nor are deleting files a viable way to get by, we will drop this for now until a remedy for this can be figured out.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/test-src.yml (1)
49-59: Functional Tests Step – Ensure Safe Variable ExpansionThe functional tests step is structured well:
- The git configuration (
git config --global --add safe.directory "$PWD") ensures the repository is recognized as safe.- Environment variables
BUILD_TARGETandBUNDLE_KEYare set before running the subsequent scripts.However, note the invocation:
./ci/dash/test_integrationtests.sh ${INTEGRATION_TESTS_ARGS}Static analysis (SC2086) suggests that unquoted variable expansion could lead to word splitting or globbing issues. If the intention is to allow splitting into multiple arguments, make sure that the test script handles them correctly. Otherwise, consider quoting the variable.
🧰 Tools
🪛 actionlint (1.7.4)
51-51: shellcheck reported issue in this script: SC2086:info:7:36: Double quote to prevent globbing and word splitting
(shellcheck)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.github/workflows/build.yml(1 hunks).github/workflows/test-src.yml(1 hunks)ci/dash/bundle-artifacts.sh(1 hunks)ci/dash/bundle-logs.sh(1 hunks)ci/dash/slim-workspace.sh(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- ci/dash/slim-workspace.sh
- .github/workflows/build.yml
- ci/dash/bundle-logs.sh
- ci/dash/bundle-artifacts.sh
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/test-src.yml
51-51: shellcheck reported issue in this script: SC2086:info:7:36: Double quote to prevent globbing and word splitting
(shellcheck)
🔇 Additional comments (9)
.github/workflows/test-src.yml (9)
1-1: Workflow Name is ClearThe workflow is named "Test source" which is descriptive of its purpose.
3-18: Inputs for Workflow Call are Properly ConfiguredThe required inputs (
bundle-key,build-target,container-path) are clearly defined with descriptions and proper types. This setup ensures that any downstream callers must supply the needed parameters.
19-21: Environment Variable Setup is AppropriateThe environment variable
INTEGRATION_TESTS_ARGSis defined with the intended test flags. Ensure these arguments remain consistent with test expectations in the integration scripts.
22-29: Job and Container Configuration ReviewThe job
test-srcis set to run onubuntu-24.04and uses a container image provided via inputs. The container options specify--user root; please verify that running as root is intentional and does not introduce security risks.
30-35: Checkout Step is Correctly ImplementedThe checkout step uses
actions/checkout@v4with a reference to the pull request head SHA and a fetch-depth of 1. This ensures minimal checkout time while getting the correct commit.
36-40: Artifact Download Step Looks GoodThe step to download build artifacts using the provided bundle key is implemented correctly.
41-48: Releases Cache Management is SoundUsing
actions/cache@v4to manage the releases cache with a key generated by hashing specific files is a reliable approach. Be mindful that frequent changes to the referenced files may cause cache misses.
61-69: Bundle Test Logs Step – Verify Git Repository ContextThis step bundles log files and extracts a short SHA via:
echo "short-sha=$(git rev-parse --short=8 HEAD)" >> "${GITHUB_OUTPUT}"Note from previous review (line 66): "this line fails with
fatal: not a git repository (or any parent up to mount point /)"Please ensure that this command is executed in a valid git repository context. Although the checkout step should populate the repository, verifying the working directory or explicitly setting it may help avoid repository-related errors.
71-83: Upload Test Logs Step Configuration ReviewThe artifact upload step uses conditions to determine whether logs should be uploaded. The configuration, including artifact naming and a very short retention period (1 day), appears intentional. Double-check that a 1-day retention aligns with the overall testing and storage strategy.
|
GitHub Actions for 3461c14, https://github.com/kwvg/dash/actions/runs/13522878030 Note: Test failure in |
kwvg
changed the title
Additional Information
actions/cacheallows specification of directories that want to be cached using glob expressions, which extend further into defining exclusions, needed due keep cache sizes maintainable (source).Unfortunately, the implementation of globbing with respect to exclusions is more-or-less broken (see actions/toolkit#713) with the requirement that the inclusion depth should match the depth of the exclusion. Attempting to play by these rules more or less fails (build).
Attempting to use third party actions like
tj-actions/globprovide for a much more saner experience but they enumerate individual files that match patterns, not folders. This means that when we pass them toactions/cache, we breach the arguments length limit (build).Modifying
ulimitto get around this isn't very feasible due to odd behavior surrounding it (see actions/runner#3421) and the general consensus is to save to a file and have the next action read from file (source).tj-actions/globgraciously does this with thepaths-output-fileoutput but it takes two to play andactions/cachedoes not accept files (pathmust be a newline-delimited string).The path of least resistance, it seems, is to use a script to bundle our cache into a neat input and leave
actions/cacheout of it entirely, this is the approach taken.As we aren't using self-hosted runners, we are subject to GitHub's limits for everything, runner space, total artifact storage budget, total cache storage budget.
Caches that not accessed in 7 days are evicted and there's a 10 GB budget for all caches (source) and GitHub will evict oldest (presumably by creation?) caches to make sure that limit is adhered to.
What makes this limit troubling is the immutable nature of caches as unlike GitLab, which is more conductive to shared caches (source), GitHub insists on its immutability (see actions/toolkit#505) and the only way to "update" a cache is to structure your cache key to allow the updated content to reflect in the key itself or delete the cache and create a new one, which brings race condition concerns (comment).
Sidenote, overwriting contents are allowed for artifacts (source), just not caches.
This means we need to be proactive in getting rid of caches with a short shelf life to avoid more long lasting caches (like
depends-sources) from being evicted due to old age as we breach the 10 GB limit. We cannot just set a short retention period as GitHub doesn't offer you to do that with caches like they do with artifacts (source).While doing this properly would require us to implement a cache reaper workflow, this still needed to be addressed now as the contents ofbuild-cineed to be passed onto the functional test workflow and this creates an ephemeral cache with a short lifespan that threatens longer-living (but older) caches.This is currently approached by deletingbuild-ci(output) caches when the functional test runner is successful, we let the cache stick around if the build fails to allow for rerunning failed instances.If for whatever reason a successful build has to be rerun, the build workflow would need to be rerun (though theEdit: Switched to using artifacts to mitigate cache thrashing concerns. Auto-expiration is a huge plus, too.ccachecache will speed this up significantly) to generate the output cache again for the test workflow to succeed. Failing to do this will result in a cache miss and run failure.Runners are limited to 14 GB of addressable storage space (source) and breaching this limit will cause the runners to fail.
Our TSan (build) and multiprocess (build) test variants breach this limit when collecting logs and deleting the
build-ci(see 2153b0b) cache doesn't make enough of a dent to help (build).test_logsartifacts and therefore wasn't considered.While third-party actions like
AdityaGarg8/remove-unwanted-softwarecan bring significant space savings (build), they cannot be run in jobs that utilize thecontainercontext (so, all jobs after the container creation workflow) as they'd be executed inside the container when we want to affect the runner underneath (build, notice the step being run after "Initialize containers").This has been sidestepped tentatively by omitting TSan and multiprocess builds as any attempted fixes would require precision gutting to get borderline space savings which could easily be nullified by future code changes that occupy more space and such measures are better reserved for future PRs.
Artifacts share their storage quota with GitHub Packages (source) and artifacts by default linger around for 90 days (source) and considering that each run can generate multi-gigabyte total artifacts, testing the upper limit runs the risk of being very expensive. Edit: It appears pricing is reflective of artifacts in private repos, public repos don't seem to run this risk.
All artifacts generated have an expiry of one day (compared to GitLab's three days, source, but they are self-hosted).Edit: Artifacts now have an expiry of three days, matching GitLab.Artifacts are compressed as ZIP archives and there is no way around that as of now (see actions/upload-artifact#109) and the permissions loss it entails is acknowledged by GitHub and their solution is... to put them in a tarball (source).
To keep size under control, artifacts contain
zstd-5compressed tarballs (compression level determined by benchmarks, see below) generated by bundling scripts alongside their checksum.Benchmarks:
Notes
If we add or remove binaries in terms of compile output,No longer applicable.bundle-build.shneeds to be updated. Without updating it, it will fail if it cannot find the files it was looking for and it will not include files that it wasn't told to include.Breaking Changes
None expected.
Checklist