[FFmpeg-devel] [PATCH 0/3] tests/fate: Improvements for running FATE on Windows/MSYS2

[softworkz .]

> -----Original Message-----
> From: Kacper Michajlow <kasper93 at gmail.com>
> Sent: Tuesday, June 17, 2025 3:18 PM
> To: softworkz . <softworkz at hotmail.com>
> Cc: FFmpeg development discussions and patches <ffmpeg-
> devel at ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH 0/3] tests/fate: Improvements for running
> FATE on Windows/MSYS2
> 
> On Tue, 17 Jun 2025 at 03:46, softworkz . <softworkz at hotmail.com> wrote:
> >
> >
> >
> > > -----Original Message-----
> > > From: Kacper Michajlow <kasper93 at gmail.com>
> > > Sent: Tuesday, June 17, 2025 3:00 AM
> > > To: softworkz . <softworkz at hotmail.com>
> > > Cc: FFmpeg development discussions and patches <ffmpeg-
> > > devel at ffmpeg.org>
> > > Subject: Re: [FFmpeg-devel] [PATCH 0/3] tests/fate: Improvements for
> > > running FATE on Windows/MSYS2
> > >
> > > On Tue, 17 Jun 2025 at 01:05, softworkz . <softworkz at hotmail.com>
> wrote:
> > > >
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Kacper Michajlow <kasper93 at gmail.com>
> > > > > Sent: Tuesday, June 17, 2025 12:44 AM
> > > > > To: FFmpeg development discussions and patches <ffmpeg-
> > > > > devel at ffmpeg.org>
> > > > > Cc: softworkz <softworkz at hotmail.com>
> > > > > Subject: Re: [FFmpeg-devel] [PATCH 0/3] tests/fate: Improvements
> > > > > for running FATE on Windows/MSYS2
> > > > >
> > > > > On Mon, 12 May 2025 at 12:00, ffmpegagent
> > > > > <ffmpegagent at gmail.com>
> > > > > wrote:
> > > > > >
> > > > > > When setting up the new Patchword builders I noticed some
> > > > > > issues when running FATE tests on Windows. Initially I had
> > > > > > them suppressed on the builders, but this patchset should finally fix
> it.
> > > > > >
> > > > > > softworkz (3):
> > > > > >   tests/fate: Fix subtitle fate tests on Windows
> > > > > >   tests/source-check: Fix make inclusion-guard check
> > > > > > EOL-agnostic
> > > > >
> > > > > I think ffmpeg repositories should always be checked out with LF
> > > > > line endings, there is nothing that expects those sources to
> > > > > have CRLF. If you like you can set attributes to all files to LF
> > > > > (not only subs)
> > > >
> > > > FATE already fails when setting LF for all subtitle ref files.
> > > >
> > >
> > > What do you mean? Everything is LF based. I don't see any failures.
> >
> > Hi Kasper,
> >
> > A common misconception is that autocrlf=off would mean that you are
> > dealing just with LF line endings in Git - but that's not the case,
> > even the opposite is true: Disabling autocrlf is what actually enables
> > check-in of files with CRLF - often accidentally. But it's not always
> > accidental. Some of the FATE reference files for subtitle tests
> > actually _do_ have CRLF line endings. By specifying LF in
> > .gitattributes, CRLF would get replaced by LF and the tests will fail.
> > Setting LF in .gitattributes is something totally different from autocrlf=off.
> > The latter means "do not change line endings back-and-forth when
> > checking In and out", but what you set in .gitattributes trumps
> > autocrlf - i.e. autocrlf doesn't act on files with a .gitattributes entry about
> line endings.
> 
> I know how autocrlf works. I only said that imposing default logic that
> "Windows always must use CRLF" and git implicitly will break your committed
> line endings by default was a short sighted design decision.

I believe the original intention was simply to avoid that Windows users would
make commits with CRLF. I see it having both advantages as well as 
disadvantages. I never had major trouble with it, but I've experienced trouble
in several cases in the past, when Windows users had set autocrlf to off
(different projects, not FFmpeg).

> It's good to have options to do the conversion IF the user wants that or is
> configured as needed for a certain repository/platform.

Absolutely, yes!

> 
> > The patch may look like as if I had forgotten some of the subtitle ref
> > files, but no: I had to carefully choose the ones who need it and
> > which must not be changed.
> 
> Could you be more specific? Which ones? I think rcombs and astiob changed
> all remaining CRLF sometime in last year or even earlier.

I know, but that was about ASS primarily. 
Which ones? All the subtitle refs that I've not included in the patch in the
.gitattributes files. You can easily try be adding the remaining ones in the 
same way and running FATE.

> Either way, files are committed in the proper way in the repository, so just
> make your git client not break that on checkout.

It's not about myself.

It's that autocrlf is on by default on Windows and we cannot change that.
In this regard, I also do not accept the narrative that this would be an 
"improper" way. It is not - it is a common work pattern on Windows and
nothing is wrong about it. It's working fine and only those few FATE tests
are causing errors. It is in our best interest that people are running 
FATE before submitting patches and when FATE is failing already even 
without any changes, then it discourages people to even deal with FATE.

The fix for this is easy and simple, that's why I don't think it makes sense
to argue about what people should do and how they should work when
the reality is different. Especially, when people are getting to the point 
when they see FATE failing, it's already too late because they already
have their working setup.

> > > It works fine as is, MSYS2 won't convert this because it's separated with |.
> >
> > The fix is only needed when you are running make fate interactively
> > from a command line in an MSYS2 shell.
> 
> Hmm, I see.

Normally, I don't run FATE on MSYS2 like this myself. I came across that
issue when preparing the CI builds where I tested the script commands
locally.  It's safe in a way that it doesn't affect any other environments.

Thanks and best regards,
sw