Preserve line endings and trailing newline when updating image tags (SF-3581) - #2162
Merged
Merged
Conversation
ReplaceNodeContent rebuilt the file with StringBuilder.AppendLine, which emits Environment.NewLine, so a file whose convention differed from the agent's had every line rewritten and a one-line tag change became a whole-file diff. It now appends the line ending detected from the file itself. The trailing-newline check compared against Environment.NewLine too. On a Windows worker an LF file therefore looked as though it had no trailing newline and TrimEnd removed it, which together with the rewrite matches the reported symptom of CRLF endings and no trailing newline. It now tests for an actual line break. That second fix cannot be covered on Linux, where Environment.NewLine is already "\n" and the old comparison happens to work. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
SerializeDocuments applied the detected line ending only to the separator between documents; each document body still came out of a plain StringWriter as LF. It also had no callers, while the three inline patch replacers each did their own Save(writer) plus TrimEnd, losing the file's line endings and trailing newline. SerializeDocuments now reapplies the line ending after emitting (the emitter writes '\n' directly and ignores TextWriter.NewLine) and restores the trailing newline when the original had one. The three replacers call it instead of serializing themselves. This removes the line-ending and trailing-newline noise only. These replacers still round-trip the document through YamlDotNet, so quoting, indentation and comments are still reflowed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The trailing-newline check had two copies, one in each replacer path. It now sits next to DetectLineEnding in StringExtensions, which is where the line-ending handling already lives, and tolerates null so callers holding optional content do not need their own guard. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
flin-8
force-pushed
the
franklin/sf-3581-line-endings-minimal
branch
from
September 16, 2026 21:59
3de30e3 to
1743cd5
Compare
TwoImagesWithSameTag_OnlyUpdatesConfiguredPath asserted the output contained Environment.NewLine. Because .gitattributes checks .cs files out with native endings, the YAML literal's endings always equalled Environment.NewLine, so the assertion held on every platform and could not tell "preserved the file's endings" from "rewrote them to the agent's" — the two only differ for content that does not match the platform, which is exactly the reported case of an LF file on a Windows agent. It now spells the line endings out, runs for both LF and CRLF, and compares the whole result, so only the configured tag may change. A second case covers the reported symptom directly: CRLF with no trailing newline. Reverting the parser fix now fails both, where previously it failed neither. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Five tests compared both sides through ReplaceLineEndings, so they passed whatever the replacer did to line endings. They now spell the endings out, run for both LF and CRLF, and compare the whole result, which keeps their original subjects — quote preservation and trailing whitespace — while also pinning the endings. UpdatesTag_PreservingCrlfAndTheAbsenceOfATrailingNewline now mirrors TwoImagesWithSameTag_OnlyUpdatesConfiguredPath without the trailing newline, so the pair differs in one variable rather than in shape, and covers both endings instead of only CRLF. Reverting the parser fix fails nine of these, where before this branch it failed none. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
octonautcal
approved these changes
Sep 16, 2026
BuildDockerImages launched PowerShell for exactly one thing: running
`gzip -k -9 -f` on the OCI tar. That made the step depend on whichever .NET
runtime the agent's `pwsh` global tool was built against, and on main's build
agent those no longer line up:
App: /root/.dotnet/tools/pwsh
Framework: 'Microsoft.NETCore.App', version '10.0.0'
.NET location: .../.nuke/temp/dotnet-unix
The following frameworks were found:
8.0.30
The agent's `pwsh` needs .NET 10, and the only runtime on offer is the .NET 8
SDK that build.sh bootstraps into .nuke/temp and puts on PATH. Calling gzip
directly removes pwsh from the equation. This was the build's only use of
PowerShellTasks, so nothing else in the build cares about the agent's pwsh now.
Failures also surface properly. `pwsh -Command` exits 0 regardless of the
native exit code, so a failed gzip used to show up later as a confusing
missing-artifact error from PublishArtifacts. A Nuke Tool asserts a zero exit
code, so it now fails at the gzip call with gzip's stderr attached.
Verified with a throwaway target: resolves /usr/bin/gzip from PATH, arguments
pass through intact, -k keeps the .tar alongside the .gz, and a deliberate
failure raises `ProcessException: Process 'gzip' exited with code 1`.
Not addressed here: reaching the SDK bootstrap at all means `dotnet --version`
failed, so the agent no longer satisfies global.json's 8.0.419 pin. That costs
every build a full SDK download and belongs with the .NET 10 work.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
(cherry picked from commit edee954)
Reads the YAML fixtures from raw string literals with ReplaceLineEndings rather than string.Join, so the documents read as YAML and the quote delimiters need no escaping. Also splits the line-ending cases out of TwoImagesWithSameTag_OnlyUpdatesConfiguredPath, which no longer varies them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Minimal fix for SF-3581(OctopusDeploy/Issues#10247) onto
release/2026.3: a one-line tag change came back as a whole-file diff.HelmYamlParserrebuilt values files withAppendLine, emittingEnvironment.NewLineon every line. It now appends the line ending detected from the file. The trailing-newline check compared againstEnvironment.NewLinetoo, so on a Windows worker an LF file lost its trailing newline — matching the reported CRLF-and-no-trailing-newline symptom.SerializeDocuments, which reapplies the file's line ending and keeps the trailing newline (and drops a spurious...marker the old path appended).Scope is line endings and the trailing newline only. Inline patches in
kustomization.yamlare still re-emitted, so comments and indentation there are still reflowed; Helm values files come out unchanged apart from the tag.#2161 targets
mainand carries these commits plus the fix for the reformatting.🤖 Generated with Claude Code