On 02/02/18 07:32, Steffan Karger wrote:
> 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.I've just done a quick check where/how ax_check_compile_flag.m4 can be found on Fedora 27, RHEL-7 and RHEL-6. For Fedora 27 and RHEL-7, it is found in a package named "autoconf-archive". For RHEL-6, no such package exists and I could also not find any other packages which provides that .m4 file. We could check this on other platforms as well (*BSD, Solaris, AIX) to see how they handle this. But if there's a similar situation there, I think it makes perfect sense to ship this macro file. What I don't like about that, is that we should have some process to ensure we keep these m4 files up-to-date. Which we have not really done so far :/ -- kind regards, David Sommerseth OpenVPN Inc
signature.asc
Description: OpenPGP digital signature
------------------------------------------------------------------------------ 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
