| Message ID | 06919b3e-86f4-8821-e3cc-e800c2c521d9@aracnet.com |
|---|---|
| State | New |
| Headers | show |
Note that in case it wasn't clear, I confirmed that this patch works by running make on Linux and examining both the generated text file and the generated HTML file in a Web browser. Aaron On 4/21/2017 4:48 PM, Aaron Levinson wrote: > On 4/21/2017 4:17 PM, Lou Logan wrote: >> On Fri, 21 Apr 2017 16:05:16 -0700 >> Aaron Levinson <alevinsn@aracnet.com> wrote: >> >>> From 7ea6455cd52ffe561f94bf03f48c4c2cf61b33c5 Mon Sep 17 00:00:00 2001 >>> From: Aaron Levinson <alevinsn@aracnet.com> >>> Date: Fri, 21 Apr 2017 15:55:11 -0700 >>> Subject: [PATCH] Added additional note to fate.texi to describe the importance >>> of not checking out files from git with core.autocrlf set to true >>> >>> Purpose: Added additional note to fate.texi to describe the importance >>> of not checking out files from git with core.autocrlf set to true. >>> When CRLF is used for line endings, this causes the source FATE test, >>> which uses check-source.sh, to fail. The issue has to do with the use >>> of git grep, which doesn't appear to be able to handle CRLF line >>> endings. Older versions of check-source.sh used grep directly, which >>> seems to do a better job of handling CRLF line endings. >>> >>> Note: thanks to Hendrik Leppkes for suggestion that the issue with >>> FATE failing was caused by CRLF line endings. >>> --- >>> doc/fate.texi | 9 +++++++++ >>> 1 file changed, 9 insertions(+) >>> >>> diff --git a/doc/fate.texi b/doc/fate.texi >>> index 7a96c25..80fac0a 100644 >>> --- a/doc/fate.texi >>> +++ b/doc/fate.texi >>> @@ -77,6 +77,15 @@ FATE_SAMPLES=fate-suite/ make fate >>> @float NOTE >>> Do not put a '~' character in the samples path to indicate a home >>> directory. Because of shell nuances, this will cause FATE to fail. >>> + >>> +In addition, FATE will fail if files are checked out from git such >>> +that CRLF is used for line endings. This will occur on Windows when >>> +git is installed using the default options, because that causes git's >>> +core.autocrlf setting to be set to 'true'. Make sure to set >>> +core.autocrlf to 'input' or 'false', or, in the case that the >>> +repository has already been cloned, it is possible to get past this by >>> +executing the following command in the top-level ffmpeg directory: >>> +@command{find -name '*.h' -type f | xargs dos2unix}. >>> @end float >> >> This is a non-blocking minor comment, but it would be nice to add some >> additional markup to core.autocrlf and true/false. For a list see: >> >> <https://www.gnu.org/software/texinfo/manual/texinfo/html_node/Indicating.html#Indicating> > > How about the following? > > Thanks, > Aaron Levinson > > ------------------------------------------------------------------------------------ > > From 707b61cfcc648eb228a157ce2c9ed0f61a2ba5ca Mon Sep 17 00:00:00 2001 > From: Aaron Levinson <alevinsn@aracnet.com> > Date: Fri, 21 Apr 2017 16:45:28 -0700 > Subject: [PATCH] Added additional note to fate.texi to describe the importance > of not checking out files from git with core.autocrlf set to true > > Purpose: Added additional note to fate.texi to describe the importance > of not checking out files from git with core.autocrlf set to true. > When CRLF is used for line endings, this causes the source FATE test, > which uses check-source.sh, to fail. The issue has to do with the use > of git grep, which doesn't appear to be able to handle CRLF line > endings. Older versions of check-source.sh used grep directly, which > seems to do a better job of handling CRLF line endings. > > Note: thanks to Hendrik Leppkes for suggestion that the issue with > FATE failing was caused by CRLF line endings. > --- > doc/fate.texi | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/doc/fate.texi b/doc/fate.texi > index 7a96c25..f3b8c0c8 100644 > --- a/doc/fate.texi > +++ b/doc/fate.texi > @@ -77,6 +77,16 @@ FATE_SAMPLES=fate-suite/ make fate > @float NOTE > Do not put a '~' character in the samples path to indicate a home > directory. Because of shell nuances, this will cause FATE to fail. > + > +In addition, FATE will fail if files are checked out from git such > +that @kbd{@key{CR}@key{LF}} is used for line endings. This will occur > +on Windows when git is installed using the default options, because > +that causes git's @var{core.autocrlf} setting to be set to > +@option{true}. Make sure to set @var{core.autocrlf} to @option{input} > +or @option{false}, or, in the case that the repository has already > +been cloned, it is possible to get past this by executing the > +following command in the top-level ffmpeg directory: @command{find > +-name '*.h' -type f | xargs dos2unix}. > @end float > > To use a custom wrapper to run the test, pass @option{--target-exec} to >
On Fri, Apr 21, 2017 at 04:48:27PM -0700, Aaron Levinson wrote: [...] > diff --git a/doc/fate.texi b/doc/fate.texi > index 7a96c25..f3b8c0c8 100644 > --- a/doc/fate.texi > +++ b/doc/fate.texi > @@ -77,6 +77,16 @@ FATE_SAMPLES=fate-suite/ make fate > @float NOTE > Do not put a '~' character in the samples path to indicate a home > directory. Because of shell nuances, this will cause FATE to fail. > + > +In addition, FATE will fail if files are checked out from git such > +that @kbd{@key{CR}@key{LF}} is used for line endings. This will occur > +on Windows when git is installed using the default options, because > +that causes git's @var{core.autocrlf} setting to be set to > +@option{true}. Make sure to set @var{core.autocrlf} to @option{input} > +or @option{false}, or, in the case that the repository has already > +been cloned, it is possible to get past this by executing the > +following command in the top-level ffmpeg directory: @command{find > +-name '*.h' -type f | xargs dos2unix}. > @end float > This is documented in doc/git-howto.texi, isn't it?
On 4/22/2017 2:16 AM, Clément Bœsch wrote: > On Fri, Apr 21, 2017 at 04:48:27PM -0700, Aaron Levinson wrote: > [...] >> diff --git a/doc/fate.texi b/doc/fate.texi >> index 7a96c25..f3b8c0c8 100644 >> --- a/doc/fate.texi >> +++ b/doc/fate.texi >> @@ -77,6 +77,16 @@ FATE_SAMPLES=fate-suite/ make fate >> @float NOTE >> Do not put a '~' character in the samples path to indicate a home >> directory. Because of shell nuances, this will cause FATE to fail. >> + >> +In addition, FATE will fail if files are checked out from git such >> +that @kbd{@key{CR}@key{LF}} is used for line endings. This will occur >> +on Windows when git is installed using the default options, because >> +that causes git's @var{core.autocrlf} setting to be set to >> +@option{true}. Make sure to set @var{core.autocrlf} to @option{input} >> +or @option{false}, or, in the case that the repository has already >> +been cloned, it is possible to get past this by executing the >> +following command in the top-level ffmpeg directory: @command{find >> +-name '*.h' -type f | xargs dos2unix}. >> @end float >> > > This is documented in doc/git-howto.texi, isn't it? Yes it is, but just because it can be found elsewhere in the documentation doesn't mean there isn't benefit to having a fallback elsewhere in the documentation. Aaron Levinson
On Sat, Apr 22, 2017 at 9:07 PM, Aaron Levinson <alevinsn@aracnet.com> wrote: > > > On 4/22/2017 2:16 AM, Clément Bœsch wrote: >> >> On Fri, Apr 21, 2017 at 04:48:27PM -0700, Aaron Levinson wrote: >> [...] >>> >>> diff --git a/doc/fate.texi b/doc/fate.texi >>> index 7a96c25..f3b8c0c8 100644 >>> --- a/doc/fate.texi >>> +++ b/doc/fate.texi >>> @@ -77,6 +77,16 @@ FATE_SAMPLES=fate-suite/ make fate >>> @float NOTE >>> Do not put a '~' character in the samples path to indicate a home >>> directory. Because of shell nuances, this will cause FATE to fail. >>> + >>> +In addition, FATE will fail if files are checked out from git such >>> +that @kbd{@key{CR}@key{LF}} is used for line endings. This will occur >>> +on Windows when git is installed using the default options, because >>> +that causes git's @var{core.autocrlf} setting to be set to >>> +@option{true}. Make sure to set @var{core.autocrlf} to @option{input} >>> +or @option{false}, or, in the case that the repository has already >>> +been cloned, it is possible to get past this by executing the >>> +following command in the top-level ffmpeg directory: @command{find >>> +-name '*.h' -type f | xargs dos2unix}. >>> @end float >>> >> >> This is documented in doc/git-howto.texi, isn't it? > > > Yes it is, but just because it can be found elsewhere in the documentation > doesn't mean there isn't benefit to having a fallback elsewhere in the > documentation. Can't we just make it point there, instead of explaining it again? - Hendrik
On 4/22/2017 12:09 PM, Hendrik Leppkes wrote: > On Sat, Apr 22, 2017 at 9:07 PM, Aaron Levinson <alevinsn@aracnet.com> wrote: >> >> >> On 4/22/2017 2:16 AM, Clément Bœsch wrote: >>> >>> On Fri, Apr 21, 2017 at 04:48:27PM -0700, Aaron Levinson wrote: >>> [...] >>>> >>>> diff --git a/doc/fate.texi b/doc/fate.texi >>>> index 7a96c25..f3b8c0c8 100644 >>>> --- a/doc/fate.texi >>>> +++ b/doc/fate.texi >>>> @@ -77,6 +77,16 @@ FATE_SAMPLES=fate-suite/ make fate >>>> @float NOTE >>>> Do not put a '~' character in the samples path to indicate a home >>>> directory. Because of shell nuances, this will cause FATE to fail. >>>> + >>>> +In addition, FATE will fail if files are checked out from git such >>>> +that @kbd{@key{CR}@key{LF}} is used for line endings. This will occur >>>> +on Windows when git is installed using the default options, because >>>> +that causes git's @var{core.autocrlf} setting to be set to >>>> +@option{true}. Make sure to set @var{core.autocrlf} to @option{input} >>>> +or @option{false}, or, in the case that the repository has already >>>> +been cloned, it is possible to get past this by executing the >>>> +following command in the top-level ffmpeg directory: @command{find >>>> +-name '*.h' -type f | xargs dos2unix}. >>>> @end float >>>> >>> >>> This is documented in doc/git-howto.texi, isn't it? >> >> >> Yes it is, but just because it can be found elsewhere in the documentation >> doesn't mean there isn't benefit to having a fallback elsewhere in the >> documentation. > > Can't we just make it point there, instead of explaining it again? OK, I'll do that.
On 4/22/2017 12:22 PM, Aaron Levinson wrote: > On 4/22/2017 12:09 PM, Hendrik Leppkes wrote: >> On Sat, Apr 22, 2017 at 9:07 PM, Aaron Levinson <alevinsn@aracnet.com> >> wrote: >>> >>> >>> On 4/22/2017 2:16 AM, Clément Bœsch wrote: >>>> >>>> On Fri, Apr 21, 2017 at 04:48:27PM -0700, Aaron Levinson wrote: >>>>> [...] >>>> >>>> This is documented in doc/git-howto.texi, isn't it? >>> >>> >>> Yes it is, but just because it can be found elsewhere in the >>> documentation >>> doesn't mean there isn't benefit to having a fallback elsewhere in the >>> documentation. >> >> Can't we just make it point there, instead of explaining it again? > > OK, I'll do that. I'm abandoning this patch and will submit a new one shortly with a new title. Aaron Levinson
diff --git a/doc/fate.texi b/doc/fate.texi index 7a96c25..f3b8c0c8 100644 --- a/doc/fate.texi +++ b/doc/fate.texi @@ -77,6 +77,16 @@ FATE_SAMPLES=fate-suite/ make fate @float NOTE Do not put a '~' character in the samples path to indicate a home directory. Because of shell nuances, this will cause FATE to fail. + +In addition, FATE will fail if files are checked out from git such +that @kbd{@key{CR}@key{LF}} is used for line endings. This will occur +on Windows when git is installed using the default options, because +that causes git's @var{core.autocrlf} setting to be set to +@option{true}. Make sure to set @var{core.autocrlf} to @option{input} +or @option{false}, or, in the case that the repository has already +been cloned, it is possible to get past this by executing the +following command in the top-level ffmpeg directory: @command{find +-name '*.h' -type f | xargs dos2unix}. @end float To use a custom wrapper to run the test, pass @option{--target-exec} to