[Openvpn-devel,3/3] travis: compile with -Werror on Linux

  Signed-off-by: Antonio Quartulli <a@unstable.cc>
---
 .travis/build-check.sh | 1 +
 1 file changed, 1 insertion(+)
  

Am 10.11.19 um 14:35 schrieb Antonio Quartulli:
> Signed-off-by: Antonio Quartulli <a@unstable.cc>
> ---
>  .travis/build-check.sh | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/.travis/build-check.sh b/.travis/build-check.sh
> index 039a7dcf..250bb454 100755
> --- a/.travis/build-check.sh
> +++ b/.travis/build-check.sh
> @@ -8,6 +8,7 @@ fi
>  
>  if [ "${TRAVIS_OS_NAME}" = "linux" ]; then
>  	export LD_LIBRARY_PATH="${PREFIX}/lib:${LD_LIBRARY_PATH:-}"
> +	export CFLAGS="${CFLAGS} -Werror"
>  fi
>  
>  if [ "${TRAVIS_OS_NAME}" = "osx"   ]; then
> 

I assume that you compile tested this, so

Acked-By: Arne Schwabe <arne@rfc2549.org>
  

Hi,

On Sun, Nov 10, 2019 at 8:36 AM Antonio Quartulli <a@unstable.cc> wrote:

> Signed-off-by: Antonio Quartulli <a@unstable.cc>
> ---
>  .travis/build-check.sh | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/.travis/build-check.sh b/.travis/build-check.sh
> index 039a7dcf..250bb454 100755
> --- a/.travis/build-check.sh
> +++ b/.travis/build-check.sh
> @@ -8,6 +8,7 @@ fi
>
>  if [ "${TRAVIS_OS_NAME}" = "linux" ]; then
>         export LD_LIBRARY_PATH="${PREFIX}/lib:${LD_LIBRARY_PATH:-}"
> +       export CFLAGS="${CFLAGS} -Werror"
>  fi
>

If this is only for linux (and travis), and works with the current code
base, probably its fine.

But it seems it may also affect mingw builds on travis. I would resist the
temptation to add -Werror in general and to Windows cross-build in
particular.

Not all warnings can be fixed easily. Often the only option is to hide the
warning  (e.g, by adding a cast) which imo is not always a good strategy.
-Werror just forces one's hand to use tricks that hide warnings.

Selva
<div dir="ltr"><div>Hi,</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Sun, Nov 10, 2019 at 8:36 AM Antonio Quartulli &lt;a@unstable.cc&gt; wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Signed-off-by: Antonio Quartulli &lt;a@unstable.cc&gt;<br>
---<br>
 .travis/build-check.sh | 1 +<br>
 1 file changed, 1 insertion(+)<br>
<br>
diff --git a/.travis/build-check.sh b/.travis/build-check.sh<br>
index 039a7dcf..250bb454 100755<br>
--- a/.travis/build-check.sh<br>
+++ b/.travis/build-check.sh<br>
@@ -8,6 +8,7 @@ fi<br>
<br>
 if [ &quot;${TRAVIS_OS_NAME}&quot; = &quot;linux&quot; ]; then<br>
        export LD_LIBRARY_PATH=&quot;${PREFIX}/lib:${LD_LIBRARY_PATH:-}&quot;<br>
+       export CFLAGS=&quot;${CFLAGS} -Werror&quot;<br>
 fi<br></blockquote><div><br></div><div>If this is only for linux (and travis), and works with the current code</div><div>base, probably its fine.</div><div><br></div><div>But it seems it may also affect mingw builds on travis. I would resist the</div><div>temptation to add -Werror in general and to Windows cross-build in particular.</div><div><br></div><div>Not all warnings can be fixed easily. Often the only option is to hide the</div><div>warning  (e.g, by adding a cast) which imo is not always a good strategy.</div><div>-Werror just forces one&#39;s hand to use tricks that hide warnings.</div><div><br></div><div>Selva</div></div></div>
  

Hi,

On Sun, Nov 10, 2019 at 11:48:16AM -0500, Selva Nair wrote:
> But it seems it may also affect mingw builds on travis. I would resist the
> temptation to add -Werror in general and to Windows cross-build in
> particular.

Yeah, we're not intending to.

The plan is to have "default builds (no configure options) on Linux with
sufficiently current C compilers" warning-free.  Which can be achieved
with the current code base.

> Not all warnings can be fixed easily. Often the only option is to hide the
> warning  (e.g, by adding a cast) which imo is not always a good strategy.
> -Werror just forces one's hand to use tricks that hide warnings.

Right.  Older systems will warn, oddball systems will warn, non-default
build options will warn (and I take one "unused global variable" warning 
vs.  "adding two lines #ifdef/#endif").

gert
  

Your patch has been applied to the master branch.

commit 1513489be632f6577021e3a0f51799b59c4ace6a
Author: Antonio Quartulli
Date:   Sun Nov 10 14:35:25 2019 +0100

     travis: compile with -Werror on Linux

     Signed-off-by: Antonio Quartulli <a@unstable.cc>
     Acked-by: Arne Schwabe <arne@rfc2549.org>
     Message-Id: <20191110133525.6069-3-a@unstable.cc>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg19094.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering
  

Forgot to hit send on this, and probably this is only partially relevant
now, but here goes.

Hi

On Sun, Nov 10, 2019 at 12:03 PM Gert Doering <gert@greenie.muc.de> wrote:

> Hi,
>
> On Sun, Nov 10, 2019 at 11:48:16AM -0500, Selva Nair wrote:
> > But it seems it may also affect mingw builds on travis. I would resist
> the
> > temptation to add -Werror in general and to Windows cross-build in
> > particular.
>
> Yeah, we're not intending to.
>

But the patch does affect cross-compilation for Windows on linux.

Unless restricted to a very small cross-section (say gcc x.y + linux) this
is going to cause a lot of grief. Note that every check run by configure
that succeeds with warnings will become mysterious error at build time.
There may be other unexpected failures too. -Werror is never fit for
production in my view. There is a reason why warnings are warnings.

Just my 2c.

Selva
<div dir="ltr"><div>Forgot to hit send on this, and probably this is only partially relevant now, but here goes.</div><div><br></div><div>Hi</div><div><br></div><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Sun, Nov 10, 2019 at 12:03 PM Gert Doering &lt;<a href="mailto:gert@greenie.muc.de" target="_blank">gert@greenie.muc.de</a>&gt; wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi,<br>
<br>
On Sun, Nov 10, 2019 at 11:48:16AM -0500, Selva Nair wrote:<br>
&gt; But it seems it may also affect mingw builds on travis. I would resist the<br>
&gt; temptation to add -Werror in general and to Windows cross-build in<br>
&gt; particular.<br>
<br>
Yeah, we&#39;re not intending to.<br></blockquote><div><br></div><div>But the patch does affect cross-compilation for Windows on linux.</div><div><br></div><div>Unless restricted to a very small cross-section (say gcc x.y + linux) this is going to cause a lot of grief. Note that every check run by configure that succeeds with warnings will become mysterious error at build time. There may be other unexpected failures too. -Werror is never fit for production in my view. There is a reason why warnings are warnings.</div><div><br></div><div>Just my 2c.</div><div><br></div><div>Selva</div></div></div>
  

Hi,

On Sun, Nov 10, 2019 at 05:03:26PM -0500, Selva Nair wrote:
> > the
> > > temptation to add -Werror in general and to Windows cross-build in
> > > particular.
> >
> > Yeah, we're not intending to.
> 
> But the patch does affect cross-compilation for Windows on linux.

That seems to have been an oversight in how to feed Travis what we want - 
I saw an update yesterday evening.

> Unless restricted to a very small cross-section (say gcc x.y + linux) this
> is going to cause a lot of grief. Note that every check run by configure
> that succeeds with warnings will become mysterious error at build time.
> There may be other unexpected failures too. -Werror is never fit for
> production in my view. There is a reason why warnings are warnings.

"gcc $current + linux" was the plan, and only do it in Travis, not do
it in configure for general consumption.

gert
  

Hi,

On 11/11/2019 08:23, Gert Doering wrote:
>> But the patch does affect cross-compilation for Windows on linux.
> 
> That seems to have been an oversight in how to feed Travis what we want - 
> I saw an update yesterday evening.
> 
>> Unless restricted to a very small cross-section (say gcc x.y + linux) this
>> is going to cause a lot of grief. Note that every check run by configure
>> that succeeds with warnings will become mysterious error at build time.
>> There may be other unexpected failures too. -Werror is never fit for
>> production in my view. There is a reason why warnings are warnings.
> 
> "gcc $current + linux" was the plan, and only do it in Travis, not do
> it in configure for general consumption.
> 

Right - this was my mistake while creating this patch.
A fix has been sent to the ml yesterday night. That should work as expected.

However, note that Travis won't be 100% happy because some compilers
don't have the -Wno-stringop-truncation we recently added and
configure.ac is failing to detect that....


Cheers,