src/arch/xtensa: normalize two odd files from CRLF to LF #6920
Conversation
|
Please use Github's "Hide whitespace" button and perform the fastest code review ever (zero change)
|
marc-hb
requested review from
lgirdwood,
plbossart,
mmaka1,
lbetlej and
dbaluta
as code owners
marc-hb
requested review from
aborisovich,
ktrzcinx,
mengdonglin,
ranj063,
aiChaoSONG,
andyross,
greg-intel,
keqiaozhang and
kv2019i
|
With one additional, last PR that I will submit later this finally makes Linux and Windows builds strictly identical to each other, demo in |
|
Interesting, that explains. These files indirect affect the Zephyr binary through the infamous source hash for sof-logger 3315681 (which is still used by Zephyr outside Intel).
|
Commit 3315681 ("cmake: make sure sha1 is computed without filters") added the `--no-filters` option to the `git hash-object` invocation used to produce a `src/` checksum embedded in all firmware binaries and .ldc files. This fixed a small mismatch for the (very few) people using the _asymmetric_ `autocrlf = input`. However it created a huge mismatch for the numerous people using the _symetric_ `autocrlf = true`. So revert that commit, remove `--no-filters` and replace it with a build time warning. `--no-filters` produces a checksum of the files as they are on the filesystem, as opposed to how they would be input into git after optional CRLF -> LF conversion. When `--no-filters` was added, no one was compiling in Windows. Most developers were not performing any conversion because there is no `autocrlf` conversion by default on Linux. Files were identical inside and outside of git and life was simple. Simple life except for at least one indomitable Linux developer who had the _asymmetric_ `autocrlf = input` setting. Also, the SOF git repo always had two exceptional (and generated) files with CRLF end of lines; see issue #5917 for details (all other files have LF end of lines). So for that "asymmetric" developer only and these two files only, `autocrlf = input` converted these 2 files to LF before hashing and this caused a different checksum. `--no-filters` stopped that conversion and "solved" that mismatch. But now Windows has entered the stage. On Windows, the default is usually (and unfortunately) the symmetric `autocrlf = true`. It is the default for Github's Windows runner. This symmetric default converts ALL other files to CRLF when extracting them out of git. This causes `git hash-object --no-filters` to produce a _different hash for all but 2 files on Windows_! The only solution is to: 1. Drop the `--no-filters` to align everything on what is _stored in git_. What is stored in git is the only thing we are sure all users have in common. 2. Request users to use only _symmetric_ `autocrlf` settings so any optional input+output conversion cancels itself out. 3. Convert odd files and make sure they all have LF end of lines. 4. In the future, drop this entire git hash-object technique which is flawed in multiple ways and has been causing multiple issues. Much, much more work than this very small revert though. Signed-off-by: Marc Herbert <[email protected]>
These two generated files are the only ones stored with CRLF in git end of lines for no reason. Probably because they were generated on Windows, then copied to Linux before being checked into git. If they had been added to git running on Windows then the default `autocrlf=true` would have likely converted them to LF. These CRLF EOLs have caused a `git hash-object` mismatch before as seen in #5917). Now that Windows is compiling SOF, thesofproject#5920 must be reverted (see previous commit) and they are causing mismatches again. While this normalization unfortunately causes some large git noise, that noise can be easily filtered out with [ git ] diff -b or -w or --ignore-space-at-eol. Github and many others also have similar filtering options. Temporarily converting them locally with `unix2dos` or any editor is another option. As they are generated, comparisons should most often be performed from the source(s) they come from anyway. Signed-off-by: Marc Herbert <[email protected]>
marc-hb
force-pushed
the
normalize-2-crlf
branch
2 times, most recently
from
5e7b673 to
f2de92a
Compare
|
If you scroll down to the end of the "Build" steps and look at the checksums, you can see that the Linux and Windows builds are creating strictly identical binaries at last: https://github.com/thesofproject/sof/actions/runs/3888597997/jobs/6636086765 Building with cutting-edge Zephyr fails, probably just because SOF is lagging a bit behind |
|
Forgot to mention this: per @lgirdwood's request I added very detailed commit messages. |
|
https://sof-ci.01.org/sofpr/PR6920/build3315/devicetest/index.html had 3 unrelated failures across 3 different platforms: 1. NTP glitch, 2. ALSA setting issue, 3. IPC timeout in pause/resume. All other tests passed. Looking at quickbuild now. |
|
One of the tests timed out in quick build/11211139. No obvious cause in the logs. All the builds were successful and most tests passed so this can't be caused by this PR. |
2 commits
Revert "cmake: make sure sha1 is computed without filters"
Commit 3315681 ("cmake: make sure sha1 is computed without
filters") added the
--no-filtersoption to thegit hash-objectinvocation used to produce a
src/checksum embedded in all firmwarebinaries and .ldc files.
This fixed a small mismatch for the (very few) people using the
asymmetric
autocrlf = input. However it created a huge mismatch forthe numerous people using the symetric
autocrlf = true.So revert that commit, remove
--no-filtersand replace it with a buildtime warning.
--no-filtersproduces a checksum of the files as they are on thefilesystem, as opposed to how they would be input into git after
optional CRLF -> LF conversion.
When
--no-filterswas added, no one was compiling in Windows. Mostdevelopers were not performing any conversion because there is no
autocrlfconversion by default on Linux. Files were identical insideand outside of git and life was simple.
Simple life except for at least one indomitable Linux developer who had
the asymmetric
autocrlf = inputsetting. Also, the SOF git repoalways had two exceptional (and generated) files with CRLF end of lines;
see issue #5917 for details (all other files have LF end of lines). So
for that "asymmetric" developer only and these two files only,
autocrlf = inputconverted these 2 files to LF before hashing and this caused adifferent checksum.
--no-filtersstopped that conversion and "solved"that mismatch.
But now Windows has entered the stage. On Windows, the default is
usually (and unfortunately) the symmetric
autocrlf = true. It is thedefault for Github's Windows runner. This symmetric default converts ALL
other files to CRLF when extracting them out of git. This causes
git hash-object --no-filtersto produce a different hash for all but 2files on Windows!
The only solution is to:
Drop the
--no-filtersto align everything on what is stored ingit. What is stored in git is the only thing we are sure all users have
in common.
Request users to use only symmetric
autocrlfsettings so anyoptional input+output conversion cancels itself out.
Convert odd files and make sure they all have LF end of lines.
In the future, drop this entire git hash-object technique which is
flawed in multiple ways and has been causing multiple issues. Much,
much more work than this very small revert though.
src/arch/xtensa: normalize two odd files from CRLF to LF
These two generated files are the only ones stored with CRLF in git end
of lines for no reason. Probably because they were generated on Windows,
then copied to Linux before being checked into git. If they had been
added to git running on Windows then the default
autocrlf=truewouldhave likely converted them to LF.
These CRLF EOLs have caused a
git hash-objectmismatch before as seenin #5917). Now that Windows is compiling SOF, #5920 must be
reverted (see previous commit) and they are causing mismatches again.
While this normalization unfortunately causes some large git noise, that
noise can be easily filtered out with [ git ] diff -b or -w or
--ignore-space-at-eol. Github and many others also have similar
filtering options. Temporarily converting them locally with
unix2dosor any editor is another option.
As they are generated, comparisons should most often be performed from
the source(s) they come from anyway.