fix: pass artifact paths to rsync via --files-from #1825
Open
Paul-Goulpie wants to merge 2 commits intofirecow:masterfrom
Open
fix: pass artifact paths to rsync via --files-from #1825Paul-Goulpie wants to merge 2 commits intofirecow:masterfrom
--files-from #1825Paul-Goulpie wants to merge 2 commits intofirecow:masterfrom
Conversation
Contributor
There was a problem hiding this comment.
1 issue found across 4 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/job.ts">
<violation number="1" location="src/job.ts:1430">
P2: Unquoted `echo \$_gcl_f` can split or glob filenames when generating the rsync `--files-from` list, breaking artifact copying for paths with spaces or leading `-` characters. Quote the variable (or use printf) when writing entries.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
- Collect artifact paths into a temp file using a bash for loop instead of appending them directly as rsync CLI arguments - Use rsync --files-from to read paths from the temp file, avoiding ARG_MAX (~2 MB) exceeded errors with large artifacts - The bash for loop (a shell built-in) correctly expands glob patterns and is not subject to ARG_MAX constraints - Remove the temp file after rsync completes
- Add test case with 9000 files of 255-char names (~2.3 MB total path size) to verify rsync --files-from handles large artifact sets that would exceed ARG_MAX (~2 MB) as CLI arguments - Assert "exported artifacts" in stdout to catch silent rsync failures hidden by || true - Assert no FAIL in stderr to ensure consume-artifacts receives all 9000 files via artifact transfer - Requires --shell-isolation so artifact transfer is exercised rather than the shared cwd being used directly
b24874b to
d81fcfd
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fix: pass artifact paths to rsync via
--files-fromProblem
When copying artifacts out, each path listed under
artifacts.pathswas appended directly as a rsync CLI argument. With a large number of
files (or glob patterns expanding to many files), the total size of
the argument list could exceed
ARG_MAX(~2 MB on Linux), causingrsync to fail silently (swallowed by
|| true) and producing noartifacts.
Additionally, glob patterns such as
artifact_*were passedsingle-quoted to rsync, preventing bash from expanding them — so
rsync received a literal
artifact_*and matched nothing.Solution
forloop (a shell built-in, not subject to
ARG_MAX)forloop correctly expands glob patterns before writingindividual resolved paths to the temp file
--files-fromso the argumentlist remains constant (3 args) regardless of the number of files
Test
Added
tests/test-cases/artifacts-many-paths: creates 9 000 fileswith 255-character names (~2.3 MB of paths, exceeding
ARG_MAX),exports them via a glob pattern, and asserts in the consume stage
that all 9 000 files were transferred correctly.
Summary by cubic
Fix artifact export by passing paths to rsync via --files-from to avoid ARG_MAX errors and ensure glob patterns expand correctly. Uses a temp file and a bash for loop to list files safely.
Written for commit d81fcfd. Summary will update on new commits.