fix(#1941): convert IccMatrixMath.cpp to LF so a touched line stops tripping diff --check - #1945
Merged
Merged
Conversation
…ripping diff --check IccMatrixMath.cpp is the only CRLF file among the 259 tracked C/C++ sources in the tree. .gitattributes carries rules for *.sh and .githooks/* only, and git's cr-at-eol whitespace rule is off by default, so git counts the CR itself as trailing whitespace. The consequence is that every line a change touches in this one file is reported by `git diff --check`, whatever that line actually contains. Red-tested on master 8b66b43 by adding a single line carrying no trailing whitespace at all: $ git diff --check -- IccProfLib/IccMatrixMath.cpp IccProfLib/IccMatrixMath.cpp:78: trailing whitespace. +// probe (exit 2) After this change the same probe edit exits 0. The issue proposed stripping the trailing space/tab runs: perl -0pi -e 's/[ \t]+\r\n/\r\n/g; s/[ \t]+\n/\n/g' That is necessary but not sufficient — it leaves the CRLF in place, so diff --check still fires on every touched line for the reason above. This change does both: it strips the 32 trailing-whitespace runs and converts the line terminators to LF, which also brings the file into line with its 258 siblings. All 32 trailing-whitespace runs are in the licence header and the function banner comments; none are in code. Every CR in the file was a line terminator — none appeared inside a string literal or anywhere else that would make the conversion meaningful. No semantic change, and that is verified rather than asserted: compiling the file before and after with the project's own flags (clang-18, ASAN+UBSAN, -Wall -Wextra and the rest of the IccProfLib2 command line) yields a byte-identical object, sha256 e51a30c0532344b71539b8bc1102d9952791afc5bcd4bbeb0ac136a0066dde68 both times. The line count is unchanged at 516, so no debug line numbers move either. For review, `git show -w --ignore-cr-at-eol` renders this as an empty diff. The file has been CRLF since 1f0a9dd (Sep 2015). Preventing recurrence across the repo would mean adding a *.cpp / *.h rule to .gitattributes; that has a much wider blast radius and is a maintainer call, so it is deliberately not part of this change.
colourbill-ctrl
requested review from
ChrisCoxArt,
maxderhak and
xsscx
as code owners
August 1, 2026 22:29
xsscx
enabled auto-merge (squash)
August 1, 2026 22:33
Member
Pre Merge Report2026-08-01 22:42:29 UTC
|
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
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.
Fixes #1941
What the issue reported, and what is actually wrong
The issue reports that
IccMatrixMath.cpphas CRLF characters thatgit diff --checktreats as trailing whitespace, and proposes:
That is necessary but not sufficient. It strips the trailing space/tab runs but leaves the
CRLF in place, and the CR is itself what
diff --checkis reporting:.gitattributescarries rules for
*.shand.githooks/*only, and git'scr-at-eolwhitespace rule isoff by default, so git counts the carriage return as trailing whitespace. Every line a
change touches in this file is flagged regardless of what that line contains.
Red-tested on
8b66b43a, adding a single line with no trailing whitespace at all:After this change the same probe edit exits 0.
This is not a style change
IccMatrixMath.cppis the only CRLF file among the 259 tracked C/C++ sources in thetree:
It has been CRLF since
1f0a9dd(Sep 2015). Converting it brings it into line with its258 siblings rather than imposing a new convention on any of them.
What changed
Both halves, in one pass:
function banner comments, none in code;
Every CR in the file was a line terminator; none appeared inside a string literal or
anywhere else that would make the conversion meaningful (
grep -cP '\r(?!$)'→ 0).Verification: byte-identical object, not an assertion
The change is provably semantics-free. Compiling the file before and after with the
project's own
IccProfLib2command line (clang-18, ASAN+UBSAN,-Wall -Wextra -Wshadow -Wnull-dereference -Werror=uninitialized,-std=gnu++17 -g) produces anidentical object both times:
The line count is unchanged at 516, so no debug line numbers move either.
For review:
git show -w --ignore-cr-at-eolrenders this commit as an empty diff.Pre-flight
grep -cE 'warning:' build.log→ 0).unmodified tree:
iccdev.spectral-tiff-preview—ModuleNotFoundError: No module named 'imagecodecs',a missing local Python dependency that CI installs.
iccdev.tool-coverage— the known parallel-load flake; passes standalone (51.4s).iccdev.issue-1781-applytolink-qa-matrix— same family, newly observed: 123.9s under-j4, 42.9s standalone on the identical tree. It regenerates CMYK fixtures in theshared
Testing/dir, so it races withtool-coverage/create-profilesthe same way.No CTest is added: there is no behaviour to assert, and the byte-identical object is the
stronger evidence.
Deliberately not included
Preventing recurrence repo-wide would mean adding a
*.cpp/*.hrule to.gitattributes. That has a far wider blast radius than this one file and is amaintainer call, so it is left out. Happy to follow up with it if wanted.