Re: [FFmpeg-devel] [PATCH v2 2/3] tests/source-check: Fix make inclusion-guard check EOL-agnostic

["softworkz ." <softworkz-at-hotmail.com@ffmpeg.org>]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Andreas
> Rheinhardt
> Sent: Donnerstag, 22. Mai 2025 12:42
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH v2 2/3] tests/source-check: Fix make
> inclusion-guard check EOL-agnostic
> 
> softworkz:
> > From: softworkz <softworkz@hotmail.com>
> >
> > ..to make it work when checked out with autocrlf=on,
> > which is Git default on Windows.
> >
> > Signed-off-by: softworkz <softworkz@hotmail.com>
> > ---
> >  tests/fate/source-check.sh | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/tests/fate/source-check.sh b/tests/fate/source-check.sh
> > index 4d7e175784..99e869e869 100755
> > --- a/tests/fate/source-check.sh
> > +++ b/tests/fate/source-check.sh
> > @@ -28,7 +28,7 @@ for f in `git ls-files | grep '\.h$'` ; do
> >          -e 's/_vaf_/_/' \
> >      | tr abcdefghijklmnopqrstuvwxyz ABCDEFGHIJKLMNOPQRSTUVWXYZ`"
> >
> > -    git grep -L "^#define $macro$" $f
> > +    git grep -L "^#define $macro\>" $f
> 
> This makes the test less strict. 

Yea, that's correct, but does that defeat the intention of the test?
It might allow whitespace at the end of the but this is something
that can happen for any line in any file, not just the guard definitions
in header files. Eventually this is guarded against by the hooks of
the Git repo when pushing. 
It might also allow more text after some whitespace, but that would 
file compilation, I think.

Do you know some regex Kung-Fu to ignore EOL and still use an end 
marker? I had found a way but that requires switching to Perl matching
(-E), but from what I've read, we cannot assume this to be available
on all platforms.


> Why don't we instead just specify that
> the repo should be checked out with lf only? Would this break something?

From my experience it can cause a lot of trouble. The following 
discussions from last year may give you an idea of these pitfalls,
even though not everything might apply to FFmpeg:

https://github.com/ffmpeginteropx/FFmpegInteropX/pull/433
https://github.com/ffmpeginteropx/FFmpegInteropX/pull/431
https://github.com/ffmpeginteropx/FFmpegInteropX/pull/430

The risk is that it causes more trouble than the problems it might
solve.
What stands on the other side is that these two patches is all that
is needed to successfully run FATE tests on Windows.

When a new subtitle test is added, the entry in .gitattributes may
be forgotten, but with the new CI builds on Windows it would also
be discovered immediately.


Thanks a lot for looking at this,
sw





_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".