Skip to content

fix: write repack launcher with LF endings so pipelines built on Windows work (v2) - #6326

Merged
mohamedzeidan2021 merged 1 commit into
aws:master-v2from
mohamedzeidan2021:fix/issue-3762-repack-launcher-crlf-v2
Sep 28, 2026
Merged

mohamedzeidan2021 merged 1 commit into
aws:master-v2from
mohamedzeidan2021:fix/issue-3762-repack-launcher-crlf-v2

Conversation

@mohamedzeidan2021

Copy link
Copy Markdown
Collaborator

Context (please read first)

This addresses #3762 (v2 backport — companion to #6313 which fixes the same bug in v3/sagemaker-mlops).

In that thread a maintainer (@qidewenwhen) confirmed the root-cause analysis is valid but noted the SDK officially supports Unix/Linux/Mac only (supported OS) and re-labeled the issue from bug → Windows-support feature request.

Opening this as a low-risk hardening change, not a claim of full Windows support: the fix is a one-argument change that is byte-for-byte identical on the supported platforms and only alters behavior when the SDK happens to run on Windows. It costs supported users nothing and unblocks the (unofficial but common) "author pipeline from a local Windows IDE" workflow. Happy to close if the team would rather keep Windows fixes out entirely.

Problem

When a pipeline is built/upserted from a Windows host, the RepackModel step fails in execution. _RepackModelStep._inject_repack_script_and_launcher writes the bash launcher _repack_script_launcher.sh from a Python string using text-mode open(..., "w"). On Windows, text mode translates every \n to os.linesep (\r\n). The resulting CRLF bash script then breaks in the Linux repack container — a trailing \r corrupts arguments (e.g. model.tar.gz\r / model.tar.gz#015), so the repack cannot find the model artifact. The same code run from Linux/mac works because os.linesep == "\n" there.

Fix

Open both launcher writes — the S3 source_dir branch and the local source_dir branch — with newline="\n", which disables newline translation so LF is written verbatim on every host. On Linux/mac the output is byte-identical to before; only Windows behavior changes.

The sibling _repack_model.py is unaffected: it's copied byte-for-byte via shutil.copy2 from an LF-only checked-in file and executed by python (universal-newline tolerant). The launcher was the only shell script written from a string.

Testing

tests/unit/sagemaker/workflow/test_utils.py:

  • New test_inject_repack_launcher_opened_with_lf_newline asserts the launcher is opened with newline="\n" — a host-independent guard that fails on the old code and passes with the fix (verified by reverting the source change).
  • New test_inject_repack_launcher_has_lf_endings reads the written launcher raw and asserts no \r\n (a Windows-only guard, labeled as such).

All tests in the module pass; black and flake8 clean.

Note: we do not have Windows CI, so this is validated via Python's documented text-mode newline semantics (translation to os.linesep under newline=None, disabled under newline="\n") rather than an end-to-end Windows run.

Backwards compatibility

_inject_repack_script_and_launcher is private; no public signature/return change. Output is unchanged on Linux/mac (already LF). No other readers/writers of the launcher exist in the module.

…ows work

_RepackModelStep._inject_repack_script_and_launcher writes the bash launcher
_repack_script_launcher.sh from a Python string using text-mode open(..., "w").
On Windows, text mode translates every \n to os.linesep (\r\n); the resulting
CRLF bash script then breaks in the Linux repack container (a trailing \r
corrupts arguments, e.g. model.tar.gz\r / model.tar.gz#015), so the repack
cannot find the model artifact. Open both launcher writes (S3 and local
source_dir branches) with newline="\n" so LF is written verbatim on every
host. Output is byte-identical on Linux/mac; only Windows behavior changes.

Adds host-independent guard tests plus a raw-bytes CRLF check.

Fixes aws#3762
@mohamedzeidan2021
mohamedzeidan2021 requested a review from a team as a code owner September 25, 2026 18:48
@mohamedzeidan2021
mohamedzeidan2021 merged commit ef40811 into aws:master-v2 Sep 28, 2026
8 of 11 checks passed

This branch was successfully deployed

1 active deployment
auto-approve — fcb7207c Deployed Sep 25, 2026 by mohamedzeidan2021 via wait-for-approval #230
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants