Skip to content

Conversation

cniethammer
Copy link
Contributor

Updated macros as follows (all replacements available in autoconf 2.60):
AC_HELP_STRING --> AS_HELP_STRING
AC_ERROR --> AC_MSG_ERROR
AC_VERBOSE --> AC_MSG_NOTICE
AC_FD_CC --> AS_MESSAGE_LOG_FD
AC_CONFIG_HEADER --> AC_CONFIG_HEADERS

Signed-off-by: Christoph Niethammer niethammer@hlrs.de

@rhc54
Copy link
Contributor

rhc54 commented Jan 20, 2021

EC2 failure

bot:aws:retest

@rhc54
Copy link
Contributor

rhc54 commented Jan 20, 2021

@cniethammer Nice job! I appreciate you identifying the required changes as that will help a great deal in updating PMIx and PRRTE.

@rhc54
Copy link
Contributor

rhc54 commented Jan 20, 2021

@cniethammer I'm following your guide and trying to update PMIx, yet I'm getting a lot of warnings when running autogen.pl with autoconf 2.70:

configure.ac:251: warning: The macro `AC_TRY_LINK' is obsolete.
configure.ac:251: You should run autoupdate.

configure.ac:251: warning: The macro `AC_TRY_COMPILE' is obsolete.
configure.ac:251: You should run autoupdate.

configure.ac:251: warning: The macro `AC_PROG_NM' is obsolete.
configure.ac:251: You should run autoupdate.

configure.ac:251: warning: The macro `_AC_COMPUTE_INT' is obsolete.
configure.ac:251: You should run autoupdate.

configure.ac:251: warning: The macro `AC_HEADER_STDC' is obsolete.
configure.ac:251: You should run autoupdate.

configure.ac:224: warning: AC_PROG_LEX without either yywrap or noyywrap is obsolete
./lib/autoconf/programs.m4:740: _AC_PROG_LEX is expanded from...
./lib/autoconf/programs.m4:714: AC_PROG_LEX is expanded from...

Since we copy almost all of our m4 from OMPI, I checked and those macros are still in OMPI master. Are you seeing the same warnings? Note that autogen.pl completed successfully even with all those warnings.

@rhc54
Copy link
Contributor

rhc54 commented Jan 20, 2021

Take a gander at openpmix/openpmix#2029 where I have started to deal with all those warnings. I believe we'll need to do the same on OMPI.

@cniethammer
Copy link
Contributor Author

@rhc54 yes I see also a lot of left stuff as I did not fix all warnings: (a) some of these are not straight forward, e.g., the C99 detection part, and (b) fixes are dependent on the autoconf version. So far I only used alternatives compatible with 2.60, which I found to be the required min version in ompi's configure.ac.

@rhc54
Copy link
Contributor

rhc54 commented Jan 20, 2021

Yeah, I think that 2.60 is too low - this 2.70 release is going to force us to raise that minimum version. Not sure how high we have to go to find fully compatible solutions. I'm using the ones available in 2.69 for now as that has been released for many years now and seems pretty available, and I believe its changes go back a little further (something like 2.67?). We should raise this issue at the next telecon.

@cniethammer cniethammer force-pushed the autoconf-2.70-warning-fixes branch 2 times, most recently from 34db19c to 8860e57 Compare January 21, 2021 19:19
@gpaulsen
Copy link
Member

gpaulsen commented Mar 1, 2021

@cniethammer, could you please rebase this, and try again?

Great to get this in, and then talk about how much effort the autoconf 2.7 is going to be, and if we need it for v5.0.

@awlauria
Copy link
Contributor

awlauria commented Mar 1, 2021

It looks like autoconf v2.7 just came out in Dec 2020 based on these dates:
http://ftp.gnu.org/gnu/autoconf/

Since it's so new I'm of the opinion that this is not needed for v5.0, unless someone really needs the latest and greatest. FWIW RHEL 8 is still on autoconf 2.69, and I can't see them updating until at earliest RHEL 9 (but that is just a guess). Are there any distros picking up 2.7 at this time?

@awlauria
Copy link
Contributor

awlauria commented Mar 1, 2021

Checking some distros:

Ubuntu 16.04/18.04/20.4/20.10: 2.69
RHEL 7.7/8.2: 2.69
openSUSE 15.2/Tumbleweed: 2.69
Amazon Linux 1: 2.69
Amazon Linux 2: can't find data

@rhc54
Copy link
Contributor

rhc54 commented Mar 9, 2021

@cniethammer Did you have any luck finding a replacement for AC_PROG_NM? I'm striking out. I simply remo,ved it and it seems to work just fine without it.

Anyone know if we really need this?

@bwbarrett
Copy link
Member

We do still need $NM defined, because we use it in the assembly configure code. However, it looks Libtool defines it these days anyway, and it's an alias for LT_PATH_NM. So I think we can just call LT_PATH_NM and be good to go.

@rhc54
Copy link
Contributor

rhc54 commented Mar 10, 2021

@jsquyres suggested on Slack AC_PATH_PROG([NM], [nm]) - would that suffice?

@bwbarrett
Copy link
Member

I think LT_PROG_NM should work. Does it not? It actually checks more than a simple AC_PATH_PROG, in that it verifies that the nm found actually speaks BSD (which we rely on).

@rhc54
Copy link
Contributor

rhc54 commented Mar 10, 2021

Haven't tried it - will do so and report back.

@rhc54
Copy link
Contributor

rhc54 commented Mar 10, 2021

I've confirmed that LT_PATH_NM works fine (@bwbarrett provided the corrected name).

@cniethammer cniethammer force-pushed the autoconf-2.70-warning-fixes branch from 8860e57 to d52673e Compare March 10, 2021 10:58
@cniethammer
Copy link
Contributor Author

@rhc54 I included the fix replacing AC_PROG_NM with LT_PATH_NM as suggested
Works for me on my system with autoconf 2.71 and libtool 2.4.6.42

@jsquyres
Copy link
Member

I didn't look deep enough to find LT_PATH_NM; if that exists, it's probably a better solution.

It looks like we AC_REQUIRE([AC_PROG_NM]) in a few places in OMPI -- that should probably be changed to AC_REQUIRE([LT_PATH_NM]) to make sure $NM is set when we need it.

@cniethammer
Copy link
Contributor Author

@jsquyres There are no other AC_PROG_NM in OMPI as far as I saw - all I found was in 3rd-party / openmpix and prrte configure.

@jsquyres
Copy link
Member

@rhc54
Copy link
Contributor

rhc54 commented Mar 10, 2021

Just as an FYI: in searching for solutions to some of these obsolete warnings, I have found tons of projects that are doing the update to support v2.70. I think this is something we are really just going to have to complete.

@cniethammer
Copy link
Contributor Author

@jsquyres There are no other AC_PROG_NM in OMPI as far as I saw - all I found was in 3rd-party / openmpix and prrte configure.

https://github.com/open-mpi/ompi/blob/master/config/ompi_fortran_find_ext_symbol_convention.m4#L26
https://github.com/open-mpi/ompi/blob/master/config/opal_config_asm.m4#L542
https://github.com/open-mpi/ompi/blob/master/config/opal_config_asm.m4#L703

That's covered by my last commit in this PR from this morning - d52673e

@jsquyres
Copy link
Member

That's covered by my last commit in this PR from this morning - d52673e

I admittedly didn't look at your patches from this morning; I was just replying to your text. 😄

It looks like the treematch merge made a conflict with this PR. ☹️ Can you fix?

Updated macros as follows (all replacements available in autoconf 2.60):
AC_HELP_STRING --> AS_HELP_STRING
AC_ERROR --> AC_MSG_ERROR
AC_VERBOSE --> AC_MSG_NOTICE
AC_FD_CC --> AS_MESSAGE_LOG_FD
AC_CONFIG_HEADER --> AC_CONFIG_HEADERS

Signed-off-by: Christoph Niethammer <niethammer@hlrs.de>
Updated libtool related macros as follows:
AM_ENABLE_SHARED --> AC_ENABLE_SHARED
AM_DISABLE_STATIC --> AC_DISABLE_STATIC

Note: libtool renamed AM_* macros to AC_* in 1999

Signed-off-by: Christoph Niethammer <niethammer@hlrs.de>
…ssing

Signed-off-by: Christoph Niethammer <niethammer@hlrs.de>
With autoconf 2.70 AC_PROG_CC now enables C2011 by default and will handle
C99 and C89.

Signed-off-by: Christoph Niethammer <niethammer@hlrs.de>
AC_PROG_NM --> LT_PATH_NM

Signed-off-by: Christoph Niethammer <niethammer@hlrs.de>
@cniethammer cniethammer force-pushed the autoconf-2.70-warning-fixes branch from d52673e to a988a8f Compare March 10, 2021 18:48
@jsquyres
Copy link
Member

I think we all agree; I'll take the liberty of merging this in.

@jsquyres jsquyres merged commit d91e646 into open-mpi:master Mar 10, 2021
@cniethammer cniethammer deleted the autoconf-2.70-warning-fixes branch March 10, 2021 22:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants