Skip to content

Fix spurious "Output was trimmed" message in archive.extracted#69770

Open
ggiesen wants to merge 3 commits into
saltstack:3006.xfrom
ggiesen:fix-59570
Open

Fix spurious "Output was trimmed" message in archive.extracted#69770
ggiesen wants to merge 3 commits into
saltstack:3006.xfrom
ggiesen:fix-59570

Conversation

@ggiesen

@ggiesen ggiesen commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

archive.extracted appended . Output was trimmed to {} number of lines to the state comment every time files were extracted, regardless of whether any trimming actually happened. Trimming only occurs when the trim_output argument is set, so with its default value (trim_output=False) the message was still added with the literal value interpolated, producing the misleading comment Output was trimmed to False number of lines.

This wraps the comment append in if trim_output: -- the same condition that already gates the actual output trimming -- so the message is only shown when output was really trimmed. By that point trim_output has been normalized (True -> 100, string integers -> int), so the value shown is always the accurate line count.

What issues does this PR fix or reference?

Fixes #59570

Previous Behavior

Running archive.extracted on a source that actually extracts files, without specifying trim_output, produced:

Result: True
Comment: <archive> extracted to <dest>. Output was trimmed to False number of lines

even though no output had been trimmed.

New Behavior

With the default trim_output, the trailing "Output was trimmed to ..." sentence is no longer added:

Result: True
Comment: <archive> extracted to <dest>

When trim_output is set to a truthy value and output is actually trimmed, the message still appears with the correct line count (e.g. Output was trimmed to 1 number of lines), unchanged from before.

Note: the issue discussion also raised a larger redesign of trim_output (accepting only integers and treating "unset" as "don't trim", since it currently accepts both bool and int with True meaning 100). That is a breaking API change that would need a deprecation cycle and is intentionally left out of this fix; this PR resolves the reported symptom without changing the argument's contract.

Merge requirements satisfied?

  • Docs -- n/a (behaviour-only fix, no documented behaviour changes)
  • Changelog -- changelog/59570.fixed.md added
  • Tests written/updated -- added test_tar_bsdtar_without_trim_output_59570 (default trim_output, asserts the message is absent) and test_tar_bsdtar_with_trim_output_zero (explicit falsy trim_output=0); the existing test_tar_bsdtar_with_trim_output (trim_output=1) is retained as the must-not-regress case proving the message still appears when trimming really happens.

Commits signed with GPG?

No

archive.extracted appended ". Output was trimmed to {} number of
lines" unconditionally whenever files were extracted, even though
trimming only happens when trim_output is set. With the default
trim_output=False this produced the misleading comment "Output was
trimmed to False number of lines" while nothing was trimmed. Guard the
append with the same trim_output check that gates the actual trimming so
the message only appears when output was really trimmed.

Fixes saltstack#59570
@ggiesen ggiesen requested a review from a team as a code owner July 10, 2026 17:56
@charzl charzl self-assigned this Jul 10, 2026
@dwoz dwoz added the test:full Run the full test suite label Jul 10, 2026
Comment thread salt/states/archive.py Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

test:full Run the full test suite

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants