diff mbox series

[v8,11/27] meson: Use -b to ignore CR vs. CR-LF issues on Windows

Message ID 20200912224431.1428-12-luoyonggang@gmail.com (mailing list archive)
State New, archived
Headers show
Series W32, W64 msys2/mingw patches | expand

Commit Message

Yonggang Luo Sept. 12, 2020, 10:44 p.m. UTC
On windows, a difference in line endings causes testsuite failures
complaining that every single line in files such as
'tests/qapi-schemadoc-good.texi' is wrong.  Fix it by adding -b to diff.

Signed-off-by: Yonggang Luo <luoyonggang@gmail.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
---
 tests/qapi-schema/meson.build | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Philippe Mathieu-Daudé Sept. 13, 2020, 3:35 p.m. UTC | #1
On 9/13/20 12:44 AM, Yonggang Luo wrote:
> On windows, a difference in line endings causes testsuite failures
> complaining that every single line in files such as
> 'tests/qapi-schemadoc-good.texi' is wrong.  Fix it by adding -b to diff.

Isn't '--strip-trailing-cr' more adapted?

> 
> Signed-off-by: Yonggang Luo <luoyonggang@gmail.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  tests/qapi-schema/meson.build | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/qapi-schema/meson.build b/tests/qapi-schema/meson.build
> index c87d141417..f1449298b0 100644
> --- a/tests/qapi-schema/meson.build
> +++ b/tests/qapi-schema/meson.build
> @@ -220,6 +220,6 @@ qapi_doc = custom_target('QAPI doc',
>  
>  # "full_path()" needed here to work around
>  # https://github.com/mesonbuild/meson/issues/7585
> -test('QAPI doc', diff, args: ['-u', files('doc-good.texi'), qapi_doc[0].full_path()],
> +test('QAPI doc', diff, args: ['-b', '-u', files('doc-good.texi'), qapi_doc[0].full_path()],
>       depends: qapi_doc,
>       suite: ['qapi-schema', 'qapi-doc'])
>
Yonggang Luo Sept. 13, 2020, 4:01 p.m. UTC | #2
On Sun, Sep 13, 2020 at 11:35 PM Philippe Mathieu-Daudé <philmd@redhat.com>
wrote:

> On 9/13/20 12:44 AM, Yonggang Luo wrote:
> > On windows, a difference in line endings causes testsuite failures
> > complaining that every single line in files such as
> > 'tests/qapi-schemadoc-good.texi' is wrong.  Fix it by adding -b to diff.
>
> Isn't '--strip-trailing-cr' more adapted?
>
> er, I did that before.....

> >
> > Signed-off-by: Yonggang Luo <luoyonggang@gmail.com>
> > Reviewed-by: Eric Blake <eblake@redhat.com>
> > Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
> >  tests/qapi-schema/meson.build | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/tests/qapi-schema/meson.build
> b/tests/qapi-schema/meson.build
> > index c87d141417..f1449298b0 100644
> > --- a/tests/qapi-schema/meson.build
> > +++ b/tests/qapi-schema/meson.build
> > @@ -220,6 +220,6 @@ qapi_doc = custom_target('QAPI doc',
> >
> >  # "full_path()" needed here to work around
> >  # https://github.com/mesonbuild/meson/issues/7585
> > -test('QAPI doc', diff, args: ['-u', files('doc-good.texi'),
> qapi_doc[0].full_path()],
> > +test('QAPI doc', diff, args: ['-b', '-u', files('doc-good.texi'),
> qapi_doc[0].full_path()],
> >       depends: qapi_doc,
> >       suite: ['qapi-schema', 'qapi-doc'])
> >
>
>
Philippe Mathieu-Daudé Sept. 13, 2020, 6:49 p.m. UTC | #3
On 9/13/20 6:01 PM, 罗勇刚(Yonggang Luo) wrote:
> 
> 
> On Sun, Sep 13, 2020 at 11:35 PM Philippe Mathieu-Daudé
> <philmd@redhat.com <mailto:philmd@redhat.com>> wrote:
> 
>     On 9/13/20 12:44 AM, Yonggang Luo wrote:
>     > On windows, a difference in line endings causes testsuite failures
>     > complaining that every single line in files such as
>     > 'tests/qapi-schemadoc-good.texi' is wrong.  Fix it by adding -b to
>     diff.
> 
>     Isn't '--strip-trailing-cr' more adapted?
> 
> er, I did that before..... 

So, yes/no? We can not follow all patches, so better if you
add a note about your changes, either in the patch description
if you think it's worthwhile keeping that information in the
git history, else below the '---' separator, so that information
is stripped from the patch when applying.

Looking at the archive I see Thomas said this option is not
POSIX thus not portable.

What about adding this?

"Ideally we would use the '--strip-trailing-cr' option, but not
being POSIX is a portability problem (i.e. BSDs and Solaris
based OSes). Instead use the '-b' option which, although doing
slightly more, produce the expected result on Windows."

> 
>     >
>     > Signed-off-by: Yonggang Luo <luoyonggang@gmail.com
>     <mailto:luoyonggang@gmail.com>>
>     > Reviewed-by: Eric Blake <eblake@redhat.com <mailto:eblake@redhat.com>>
>     > Reviewed-by: Daniel P. Berrangé <berrange@redhat.com
>     <mailto:berrange@redhat.com>>
>     > ---
>     >  tests/qapi-schema/meson.build | 2 +-
>     >  1 file changed, 1 insertion(+), 1 deletion(-)
>     >
>     > diff --git a/tests/qapi-schema/meson.build
>     b/tests/qapi-schema/meson.build
>     > index c87d141417..f1449298b0 100644
>     > --- a/tests/qapi-schema/meson.build
>     > +++ b/tests/qapi-schema/meson.build
>     > @@ -220,6 +220,6 @@ qapi_doc = custom_target('QAPI doc',
>     > 
>     >  # "full_path()" needed here to work around
>     >  # https://github.com/mesonbuild/meson/issues/7585
>     > -test('QAPI doc', diff, args: ['-u', files('doc-good.texi'),
>     qapi_doc[0].full_path()],
>     > +test('QAPI doc', diff, args: ['-b', '-u', files('doc-good.texi'),
>     qapi_doc[0].full_path()],
>     >       depends: qapi_doc,
>     >       suite: ['qapi-schema', 'qapi-doc'])
>     >
> 
> 
> 
> -- 
>          此致
> 礼
> 罗勇刚
> Yours
>     sincerely,
> Yonggang Luo
Eric Blake Sept. 14, 2020, 2:01 p.m. UTC | #4
On 9/13/20 10:35 AM, Philippe Mathieu-Daudé wrote:
> On 9/13/20 12:44 AM, Yonggang Luo wrote:
>> On windows, a difference in line endings causes testsuite failures
>> complaining that every single line in files such as
>> 'tests/qapi-schemadoc-good.texi' is wrong.  Fix it by adding -b to diff.
> 
> Isn't '--strip-trailing-cr' more adapted?

No, BSD lacks that spelling.
Peter Maydell Sept. 14, 2020, 2:41 p.m. UTC | #5
On Sun, 13 Sep 2020 at 19:51, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>
> On 9/13/20 6:01 PM, 罗勇刚(Yonggang Luo) wrote:
> >
> >
> > On Sun, Sep 13, 2020 at 11:35 PM Philippe Mathieu-Daudé
> > <philmd@redhat.com <mailto:philmd@redhat.com>> wrote:
> >
> >     On 9/13/20 12:44 AM, Yonggang Luo wrote:
> >     > On windows, a difference in line endings causes testsuite failures
> >     > complaining that every single line in files such as
> >     > 'tests/qapi-schemadoc-good.texi' is wrong.  Fix it by adding -b to
> >     diff.
> >
> >     Isn't '--strip-trailing-cr' more adapted?
> >
> > er, I did that before.....
>
> So, yes/no? We can not follow all patches, so better if you
> add a note about your changes, either in the patch description
> if you think it's worthwhile keeping that information in the
> git history, else below the '---' separator, so that information
> is stripped from the patch when applying.
>
> Looking at the archive I see Thomas said this option is not
> POSIX thus not portable.
>
> What about adding this?
>
> "Ideally we would use the '--strip-trailing-cr' option, but not
> being POSIX is a portability problem (i.e. BSDs and Solaris
> based OSes). Instead use the '-b' option which, although doing
> slightly more, produce the expected result on Windows."

The other important point to note here is that because
texi output is mostly not case-sensitive, we are not
losing important test coverage by not checking for exact
whitespace matches between the known-good output and
the generated output. If we were checking the doc-good.out
text file, -b would be bad because there whitespace is
important and is part of what we need to be checking.

(Also the texi stuff is going to go away soon I hope :-))

thanks
-- PMM
Yonggang Luo Sept. 14, 2020, 3:20 p.m. UTC | #6
On Mon, Sep 14, 2020 at 10:41 PM Peter Maydell <peter.maydell@linaro.org>
wrote:

> On Sun, 13 Sep 2020 at 19:51, Philippe Mathieu-Daudé <philmd@redhat.com>
> wrote:
> >
> > On 9/13/20 6:01 PM, 罗勇刚(Yonggang Luo) wrote:
> > >
> > >
> > > On Sun, Sep 13, 2020 at 11:35 PM Philippe Mathieu-Daudé
> > > <philmd@redhat.com <mailto:philmd@redhat.com>> wrote:
> > >
> > >     On 9/13/20 12:44 AM, Yonggang Luo wrote:
> > >     > On windows, a difference in line endings causes testsuite
> failures
> > >     > complaining that every single line in files such as
> > >     > 'tests/qapi-schemadoc-good.texi' is wrong.  Fix it by adding -b
> to
> > >     diff.
> > >
> > >     Isn't '--strip-trailing-cr' more adapted?
> > >
> > > er, I did that before.....
> >
> > So, yes/no? We can not follow all patches, so better if you
> > add a note about your changes, either in the patch description
> > if you think it's worthwhile keeping that information in the
> > git history, else below the '---' separator, so that information
> > is stripped from the patch when applying.
> >
> > Looking at the archive I see Thomas said this option is not
> > POSIX thus not portable.
> >
> > What about adding this?
> >
> > "Ideally we would use the '--strip-trailing-cr' option, but not
> > being POSIX is a portability problem (i.e. BSDs and Solaris
> > based OSes). Instead use the '-b' option which, although doing
> > slightly more, produce the expected result on Windows."
>
> The other important point to note here is that because
> texi output is mostly not case-sensitive, we are not
> losing important test coverage by not checking for exact
> whitespace matches between the known-good output and
> the generated output. If we were checking the doc-good.out
> text file, -b would be bad because there whitespace is
> important and is part of what we need to be checking.
>
> (Also the texi stuff is going to go away soon I hope :-))
>
So continue with -b are accepted? or I just diable it on win32?

>
> thanks
> -- PMM
>
diff mbox series

Patch

diff --git a/tests/qapi-schema/meson.build b/tests/qapi-schema/meson.build
index c87d141417..f1449298b0 100644
--- a/tests/qapi-schema/meson.build
+++ b/tests/qapi-schema/meson.build
@@ -220,6 +220,6 @@  qapi_doc = custom_target('QAPI doc',
 
 # "full_path()" needed here to work around
 # https://github.com/mesonbuild/meson/issues/7585
-test('QAPI doc', diff, args: ['-u', files('doc-good.texi'), qapi_doc[0].full_path()],
+test('QAPI doc', diff, args: ['-b', '-u', files('doc-good.texi'), qapi_doc[0].full_path()],
      depends: qapi_doc,
      suite: ['qapi-schema', 'qapi-doc'])