Hi,

On 02-02-18 05:43, Selva Nair wrote:
> On Thu, Feb 1, 2018 at 10:45 AM, Steffan Karger <stef...@karger.me> wrote:
>> [...]
>>
>> +AX_CHECK_COMPILE_FLAG([-Wall], [CFLAGS="-Wall ${CFLAGS}"])
> 
> The three options could have checked together, but this is fine too.
> 
> All said and done, the check is not as useful as I had imagined..
> AX_CHECK_COMPLIE_FLAG only catches errors, not warnings.
> For instance, IBM's xlc, at least on Linux, only warns about unknown options.
> So nothing gained -- in fact 4 warning lines per source file caused by
> these options.
> Hopefully, its still useful on some other never used build envs..
> 
> Anyway, the check protects against probable build failures on some unknown
> systems, and I won't waste any more time dwelling on this.
> 
>> diff --git a/m4/ax_check_compile_flag.m4 b/m4/ax_check_compile_flag.m4
>> new file mode 100644
>> [...]
> 
> This macro is a part of autoconf archive which, I thought, is
> prerequisite for building from the repo. One anyway needs autotools
> installed. Can't we not insist on having these common macros be present
> as well? After all this is for building from git sources, not from a
> dist tarball
> which needs no autotools.
> 
> Also the notes in the macro file says "Please keep this macro in sync with
> AX_CHECK_{PREPROC,LINK}_FLAG." which is easier to do if we use the
> system's installed copy.
> 
> Anyway, if including such m4 macros in the repo is the policy,  this
> is fine.

We previously did not depend on it (I didn't have the archive installed)
and thought we didn't want to pull in another dependency.  Also, the
error when the archive is missing is quite terrible and will cost a lot
of people time to understand.  Autoconf happily continues, then
configure fails with:

./configure: line 17861: syntax error near unexpected token `newline'
./configure: line 17861: `AX_CHECK_COMPILE_FLAG('

That said, if we prefer to not include the macro, it can simply be
removed from the commit.  I could even rewrite then to directly use
AC_COMPILE_IFELSE, which is bulkier and less clear but doesn't need the
include.

> Having done the self-satisfying nitpicking:
> 
> Acked-by: Selva Nair <selva.n...@gmail.com>

Thanks :)

-Steffan

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Reply via email to