| Message ID | 20200518100356.29292-6-dtucker@dtucker.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/7] Redirect grep's stderr top null too. | expand |
Darren Tucker <dtucker@dtucker.net> writes: > Signed-off-by: Darren Tucker <dtucker@dtucker.net> > --- > configure.ac | 10 ++++++++-- > 1 file changed, 8 insertions(+), 2 deletions(-) > > diff --git a/configure.ac b/configure.ac > index 14e09b04b6..87a39c5ae0 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -1164,10 +1164,16 @@ GIT_CHECK_FUNC(strtoull, > [NO_STRTOULL=YesPlease]) > GIT_CONF_SUBST([NO_STRTOULL]) > # > -# Define NO_STRTOUMAX if you don't have strtoumax in the C library. > +# Define NO_STRTOUMAX if you don't have strtoumax in the C library > +# or as a macro in inttypes.h. > GIT_CHECK_FUNC(strtoumax, > [NO_STRTOUMAX=], > -[NO_STRTOUMAX=YesPlease]) > +[ > + AC_CHECK_DECL(strtoumax, > + [NO_STRTOUMAX=], > + [NO_STRTOUMAX=YesPlease], > + [#include <inttypes.h>]) > +]) It is kind of surprising that we got away with GIT_CHECK_FUNC() that misses an implementation by macro without having problems with other symbols. I don't mind taking this patch as-is, but it makes me wonder if we need to devise a more systematic approach to the issue than "oh, I found GIT_CHECK_FUNC() does not work for X on system Y, so let's add an AC_CHECK_DECL() for it, too" approach, which this patch is its first step. Thanks.
On Tue, 19 May 2020 at 03:17, Junio C Hamano <gitster@pobox.com> wrote: [...] > It is kind of surprising that we got away with GIT_CHECK_FUNC() that > misses an implementation by macro without having problems with other > symbols. I suspect that in most cases you'll go to the fallback and be fine. In this case, it falls back to strtoull which it doesn't have. (Adding the strtoull test in patch 3/7 makes it fall back to strtoul which it does have). The other annoyance is that it produces a macro redefinition warning on each file. An alternative solution to that would be to undef strtoumax in git-compat-util.h before redefining it. > I don't mind taking this patch as-is, but it makes me wonder if we > need to devise a more systematic approach to the issue than "oh, I > found GIT_CHECK_FUNC() does not work for X on system Y, so let's add > an AC_CHECK_DECL() for it, too" approach, which this patch is its > first step. The caveat with putting it in GIT_CHECK_FUNC() is that you'll need to include the union of all of the header files that might have all of the things you're looking for instead of the hopefully small set of them that might have the one specific thing you're looking for. I suspect that would be more hassle than it's worth.
Darren Tucker <dtucker@dtucker.net> writes: > The caveat with putting it in GIT_CHECK_FUNC() is that you'll need to > include the union of all of the header files that might have all of > the things you're looking for instead of the hopefully small set of > them that might have the one specific thing you're looking for. I > suspect that would be more hassle than it's worth. Oh, what I had in mind was to replace GIT_CHECK_FUNC() with a macro that takes not just function name but also include file. Not "the union of all the include files".
diff --git a/configure.ac b/configure.ac index 14e09b04b6..87a39c5ae0 100644 --- a/configure.ac +++ b/configure.ac @@ -1164,10 +1164,16 @@ GIT_CHECK_FUNC(strtoull, [NO_STRTOULL=YesPlease]) GIT_CONF_SUBST([NO_STRTOULL]) # -# Define NO_STRTOUMAX if you don't have strtoumax in the C library. +# Define NO_STRTOUMAX if you don't have strtoumax in the C library +# or as a macro in inttypes.h. GIT_CHECK_FUNC(strtoumax, [NO_STRTOUMAX=], -[NO_STRTOUMAX=YesPlease]) +[ + AC_CHECK_DECL(strtoumax, + [NO_STRTOUMAX=], + [NO_STRTOUMAX=YesPlease], + [#include <inttypes.h>]) +]) GIT_CONF_SUBST([NO_STRTOUMAX]) # # Define NO_SETENV if you don't have setenv in the C library.
Signed-off-by: Darren Tucker <dtucker@dtucker.net> --- configure.ac | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)