fix(cli): fall back to regular upload when git filtering excludes all files#1783
fix(cli): fall back to regular upload when git filtering excludes all files#1783russellb wants to merge 7 commits into
Conversation
russellb
requested review from
a team,
derekwaynecarr,
maxamillion and
mrunalp
as code owners
|
/ok to test 6d322fd |
johntmyers
added
the
gator:in-review
PR Review StatusValidation: This PR is project-valid because it directly fixes validated issue #1778 for Independent review: completed in this cycle. No critical correctness blockers were found in the core fallback or regression coverage. Review feedback still needs author or maintainer resolution before pipeline handoff:
E2E: This change affects sandbox file transfer behavior and adds an E2E regression test, so Next state: |
… files When uploading a directory whose contents are entirely excluded by .gitignore, git_sync_files returns an empty file list. The git-aware upload path silently accepted this and printed "Upload complete" without transferring any files. Now both call sites (direct CLI handler and sandbox_upload_plan) check for an empty file list and fall through to the regular, unfiltered upload path instead. Closes NVIDIA#1778 Signed-off-by: Russell Bryant <rbryant@nvidia.com>
…#1778) Reproduces the reported workflow: download a file from a sandbox into a local git repo where the target directory is gitignored, then re-upload that directory without --no-git-ignore. Before the fix the upload silently succeeded with zero files transferred. Signed-off-by: Russell Bryant <rbryant@nvidia.com>
Address PR review: print a notice when .gitignore filtering excludes all files and the CLI falls back to unfiltered upload. Add a GitFilteredEmpty variant to SandboxUploadPlan so the caller can distinguish this case and emit a visible warning. Document the default gitignore filtering and fallback behavior in manage-sandboxes.
russellb
force-pushed
the
fix/upload-empty-gitignore-fallback
branch
from
6d322fd to
7cc189a
Compare
Re-check After Author UpdateI re-evaluated latest head Disposition: partially resolved. Resolved items:
Remaining items:
Independent review: completed for the current head and found the same remaining warning-path gap; no other blocking findings were identified. Next state: |
The SandboxCommands::Upload path in main.rs had the same silent fallback as sandbox_create. Restructure the git_sync_files check to emit the same .gitignore warning before falling back to unfiltered upload. Assert the warning in the E2E regression test.
Re-check After Author UpdateI re-evaluated latest head Disposition: partially resolved. Resolved items:
Remaining items:
If `.gitignore` filtering excludes every file in the upload path, the CLI falls
back to an unfiltered upload and prints a warning. Pass `--no-git-ignore` to
opt into unfiltered uploads explicitly, upload a path outside the Git work
tree, or force-add the intended files if they should remain Git-aware.Independent review: completed for the current head and found the same docs wording issue; no other blocking findings were identified. Next state: |
Replace inaccurate "explicit file list" advice with actionable alternatives: --no-git-ignore, uploading outside the work tree, or git force-adding the files.
johntmyers
added
gator:watch-pipeline
|
Label |
Re-check After Author UpdateI re-evaluated latest head Disposition: resolved. Remaining items:
Docs: updated under E2E: applied Next state: |
|
/ok to test 1e0ce41 |
Pipeline FailureCI failed for latest head
This appears PR-caused and needs an author update. The E2E workflow is still running, but the required branch gate cannot pass until the clippy failure is fixed. Next state: |
johntmyers
added
gator:in-review
Re-check After Author UpdateI re-evaluated latest head Disposition: resolved. Remaining items:
Docs: updated under E2E: Next state: |
johntmyers
added
gator:watch-pipeline
|
/ok to test c21f8c9 |
Pipeline FailureCI failed for latest head
This appears PR-caused and needs an author update. The E2E workflow is still running, but the required branch gate cannot pass until the formatting failure is fixed. Next state: |
johntmyers
added
gator:in-review
4 participants
Summary
When uploading a directory whose contents are entirely excluded by
.gitignore,git_sync_filesreturns an empty file list. The git-aware upload path silently accepted this and printed "Upload complete" without transferring any files. Now both call sites check for an empty file list and fall through to the regular, unfiltered upload path instead.Related Issue
Closes #1778
Changes
main.rs: Added!files.is_empty()guard to the git-aware upload branch so an empty git file list falls through to the regular uploadrun.rs: Added the same guard insandbox_upload_plan()so thesandbox runupload path also falls back correctlysandbox_upload_plan_falls_back_when_all_files_gitignoredconfirming a gitignored directory produces aRegularupload planTesting
mise run pre-commitpassesChecklist