Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cmake: make sure sha1 is computed without filters #5920

Merged
merged 1 commit into from
Jun 16, 2022

Conversation

plbossart
Copy link
Member

Local filters in ~/gitconfig, such as

[core]
autocrlf = input

can impact the result of git hash-object. Make sure no filters are
used so that the hash value remains unmodified across user setups.

BugLink: thesofproject/rimage#99
Signed-off-by: Pierre-Louis Bossart [email protected]

Local filters in ~/gitconfig, such as

[core]
	autocrlf = input

can impact the result of git hash-object. Make sure no filters are
used so that the hash value remains unmodified across user setups.

BugLink: https://github.com/thesofproject/sof/issues/5917
Signed-off-by: Pierre-Louis Bossart <[email protected]>
Copy link
Collaborator

@marc-hb marc-hb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's a less invasive way to reproduce the same issue:

git hash-object src/arch/xtensa/hal/set_region_translate.c
b1b53ed4ab216f6a0c8e7c628d93de627ac370b1

printf '* text eol=crlf\n' > src/arch/xtensa/hal/.gitattributes

git hash-object src/arch/xtensa/hal/set_region_translate.c
27ed6b80a50b1b89f7f8b7653355f25ad0cb9932

git hash-object --no-filters src/arch/xtensa/hal/set_region_translate.c
b1b53ed4ab216f6a0c8e7c628d93de627ac370b1

rm src/arch/xtensa/hal/.gitattributes

We have two files with Windows EOL

find * -exec dos2unix {} \; # DONT DO THIS AT HOME
git diff --stat
 src/arch/xtensa/hal/set_region_translate.c  | 1068 +++++++++++++--------------
 src/arch/xtensa/include/xtensa/c6x-compat.h | 3516 +++++++++++++++++++++++++++++++++++++++++++--------------------------------------------
 2 files changed, 2292 insertions(+), 2292 deletions(-)

@lgirdwood
Copy link
Member

SOFCI TEST

@lgirdwood
Copy link
Member

Rerun QB CI as it looks like a DUT failed.

@marc-hb
Copy link
Collaborator

marc-hb commented Jun 15, 2022

The IPC timeout in https://sof-ci.01.org/sofpr/PR5920/build669/devicetest/?model=BYT_MB_NOCODEC&testcase=multiple-pause-resume-5 looks worryingly new to me but totally unrelated to this PR. Everything else is green.

SOFCI TEST

I don't think QB reacts to any magic comment, AFAIK it is only sof-ci/jenkins that does. Looking at the internal QB site it seems https://sof-ci.01.org/sof-pr-viewer/#/build/PR5920/build9618153 (which is all green now) was manually restarted by @wszypelt , @wszypelt can you confirm?

@lgirdwood and everyone else please, please when reporting an error please always, always copy the URL to the corresponding test run. If you notice someone forgot to do that, please add the URL yourself before the submitter force pushes again. I know it takes a few extra seconds but it took me several MINUTES to find a previous run here: https://sof-ci.01.org/sof-pr-viewer/#/build/PR5920/build9609898

Rerun QB CI as it looks like a DUT failed.

This previous run got stuck but I don't see any DUT failure.

@marc-hb
Copy link
Collaborator

marc-hb commented Jun 15, 2022

Rerun QB CI as it looks like a DUT failed.

I just spotted "Copy build to DUT: FAIL" in https://sof-ci.01.org/sof-pr-viewer/#/build/PR5910/build9591313 for a different PR. Mystery solved?

@lgirdwood lgirdwood merged commit 3315681 into thesofproject:main Jun 16, 2022
marc-hb added a commit to marc-hb/sof that referenced this pull request Jan 11, 2023
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]>
lgirdwood pushed a commit that referenced this pull request Jan 18, 2023
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, #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]>
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.

5 participants