Skip to content

fix(npm): escape backslash line continuation in tar command - #1663

Open
lengau wants to merge 1 commit into
mainfrom
work/fix-npm-plugin-backslash-escape
Open

fix(npm): escape backslash line continuation in tar command#1663
lengau wants to merge 1 commit into
mainfrom
work/fix-npm-plugin-backslash-escape

Conversation

@lengau

@lengau lengau commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Stacked on #1662.

Addresses review comment: #1662 (comment)

The f-string generating the tar shell command in npm_plugin.py had an unescaped \ at end-of-line. Since it's not a raw string, Python treated \<newline> as a string-continuation escape, silently swallowing the newline and merging the tar ... and --no-same-owner ... lines with leftover indentation whitespace baked in as literal spaces, instead of emitting a real shell line-continuation.

Escaped it as \\ so the generated script contains an actual \ followed by a newline (valid bash line continuation), and updated the corresponding test assertion.

Verified with ruff check and pytest tests/unit/plugins/test_npm_plugin.py (34 passed).

@lengau
lengau requested a review from cmatsuoka as a code owner August 12, 2026 17:57
@lengau
lengau marked this pull request as draft August 12, 2026 18:02
@lengau
lengau marked this pull request as ready for review August 12, 2026 18:20
@lengau
lengau requested review from a team and a lite review from Copilot August 12, 2026 18:20

Copilot AI 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.

Pull request overview

Fixes the npm plugin’s generated tar shell command so it emits a real bash line-continuation (\ + newline) instead of having Python treat \<newline> as a string-continuation escape inside the f-string, which previously collapsed the command onto one line with unintended whitespace.

Changes:

  • Escapes the end-of-line backslash in the tar command template in craft_parts/plugins/npm_plugin.py so the generated script contains a literal \ followed by a newline.
  • Updates the unit test assertion in tests/unit/plugins/test_npm_plugin.py to match the corrected multi-line command output.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
craft_parts/plugins/npm_plugin.py Fixes the f-string so the emitted tar command uses a proper bash line continuation.
tests/unit/plugins/test_npm_plugin.py Adjusts expected build-command output to assert the literal \ + newline behavior.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Base automatically changed from work/fix-lint to main August 12, 2026 19:43
@mr-cal
mr-cal force-pushed the work/fix-npm-plugin-backslash-escape branch from 59c9676 to f0b9935 Compare August 12, 2026 19:44
@lengau
lengau requested a review from a team August 12, 2026 20:06
@lengau
lengau force-pushed the work/fix-npm-plugin-backslash-escape branch from f0b9935 to 3a799d2 Compare August 13, 2026 16:27
The unescaped backslash in the f-string was interpreted by Python as
a string continuation, silently joining the tar and --no-same-owner
lines with leftover indentation whitespace instead of emitting a real
shell line continuation. Escape it so the generated script contains a
literal backslash-newline.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@lengau
lengau force-pushed the work/fix-npm-plugin-backslash-escape branch from 3a799d2 to 61062d6 Compare August 14, 2026 14:13
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.

4 participants