Skip to content

Fix GitArchivePackager when repo or output paths contain spaces - #645

Merged
ko3n1g merged 1 commit into
NVIDIA-NeMo:mainfrom
rioyu123:codex/git-packager-spaced-paths
Oct 9, 2026
Merged

ko3n1g merged 1 commit into
NVIDIA-NeMo:mainfrom
rioyu123:codex/git-packager-spaced-paths

Conversation

@rioyu123

@rioyu123 rioyu123 commented Oct 9, 2026

Copy link
Copy Markdown
Contributor

Summary

GitArchivePackager fails when the repository path, job directory or archive name contains spaces. A few shell commands still interpolate these paths without quoting them.

This change uses shlex.quote for Git path discovery and the final gzip/cleanup commands, following the quoting already used elsewhere in the packager. It also expands ~ explicitly so home-relative repository paths continue to work after quoting.

Repository paths are now treated literally rather than as shell expressions; expand environment variables such as $HOME before passing them in basepath.

Archive contents and packaging options are unchanged. This is separate from #637, which fixed PatternPackager; it does not change include-pattern, subpath or submodule handling.

Testing

  • Added six cases using a real Git repository and real git/tar/gzip commands: ordinary paths, a repository path with spaces, the basepath override, output directories and archive names with spaces, and a home-relative basepath.
  • The original four space-path cases fail before the fix; the ordinary-path control passes. All six cases now pass and check archive contents and removal of temporary files.
  • Packaging suite: 30 passed.
  • Full suite with the repository's locked dependencies on Linux/Python 3.11: 1,700 passed, 1 deselected, 1 xfailed. Run in a fresh /tmp mount because existing unrelated /tmp/exp1 state on the host breaks eight NVCRE tests on both main and this branch. The original host file was left untouched.
  • Ruff check/format and git diff --check passed.

The full run exits successfully; the installed Ray package still emits a non-failing missing-ray.dag message during interpreter shutdown.

@copy-pr-bot

copy-pr-bot Bot commented Oct 9, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@ko3n1g

ko3n1g commented Oct 9, 2026

Copy link
Copy Markdown
Contributor

/ok to test 3ecf02c

@ko3n1g

ko3n1g commented Oct 9, 2026

Copy link
Copy Markdown
Contributor

/review

@svcnvidia-nemo-ci svcnvidia-nemo-ci left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No merge-blocking defects found in this light review. Repository discovery expands home-relative paths before quoting, and compression/cleanup quote complete filenames consistently. The six regression cases check archive contents and cleanup across ordinary paths, paths with spaces, and basepath/home-relative overrides.

Validation was static; tests were inspected but not executed.

Review generated by codex

@ko3n1g
ko3n1g added this pull request to the merge queue Oct 9, 2026
Merged via the queue into NVIDIA-NeMo:main with commit ce30cbc Oct 9, 2026
31 checks passed

This branch was successfully deployed

1 active deployment
public — 3ecf02cf Deployed Oct 9, 2026 by copy-pr-bot[bot] via release / finalize / publish-docs #398
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