Why do we define HAVE_GSSAPI_EXT_H?

Started by Andres Freundalmost 2 years ago4 messageshackers
Jump to latest
#1Andres Freund
andres@anarazel.de

Hi,

Since

commit f7431bca8b0138bdbce7025871560d39119565a0
Author: Stephen Frost <sfrost@snowman.net>
Date: 2023-04-13 08:55:13 -0400

Explicitly require MIT Kerberos for GSSAPI

WHen building with GSSAPI support, explicitly require MIT Kerberos and
check for gssapi_ext.h in configure.ac and meson.build. Also add
documentation explicitly stating that we now require MIT Kerberos when
building with GSSAPI support.

configure/meson define HAVE_GSSAPI_EXT_H / HAVE_GSSAPI_GSSAPI_EXT_H - but
afaict we don't use those anywhere?

It makes sense to test for the presence of gssapi_ext.h, but given that we
require it to be present, I think it's not worth emitting HAVE_GSSAPI_EXT_H
etc.

As f7431bca8b is in 16, it seems best to just change this in 18.

While looking at this I also found an argument omission present in the commit
adding meson support. I plan to fix that with the attached commit.

Greetings,

Andres Freund

Attachments:

v1-0001-meson-Add-missing-argument-to-gssapi.h-check.patchtext/x-diff; charset=us-asciiDownload+4-3
v1-0002-Don-t-define-HAVE_-GSSAPI_-GSSAPI_EXT_H.patchtext/x-diff; charset=us-asciiDownload+6-32
#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Andres Freund (#1)
Re: Why do we define HAVE_GSSAPI_EXT_H?

Andres Freund <andres@anarazel.de> writes:

configure/meson define HAVE_GSSAPI_EXT_H / HAVE_GSSAPI_GSSAPI_EXT_H - but
afaict we don't use those anywhere?

It looks to me like it's just a byproduct of the autoconf macros
we use to verify that you have a sane installation:

if test "$with_gssapi" = yes ; then
AC_CHECK_HEADERS(gssapi/gssapi.h, [],
[AC_CHECK_HEADERS(gssapi.h, [], [AC_MSG_ERROR([gssapi.h header file is required for GSSAPI])])])
AC_CHECK_HEADERS(gssapi/gssapi_ext.h, [],
[AC_CHECK_HEADERS(gssapi_ext.h, [], [AC_MSG_ERROR([gssapi_ext.h header file is required for GSSAPI])])])
fi

There might be a variant of AC_CHECK_HEADERS that doesn't have
the default define-a-symbol action, not sure.

Maybe it's not really necessary to check both gssapi.h and
gssapi_ext.h, but I'm not very familiar with all the variants of
GSSAPI that are out there.

regards, tom lane

#3Andres Freund
andres@anarazel.de
In reply to: Tom Lane (#2)
Re: Why do we define HAVE_GSSAPI_EXT_H?

Hi,

On 2024-07-08 19:05:32 -0400, Tom Lane wrote:

Andres Freund <andres@anarazel.de> writes:

configure/meson define HAVE_GSSAPI_EXT_H / HAVE_GSSAPI_GSSAPI_EXT_H - but
afaict we don't use those anywhere?

It looks to me like it's just a byproduct of the autoconf macros
we use to verify that you have a sane installation:

if test "$with_gssapi" = yes ; then
AC_CHECK_HEADERS(gssapi/gssapi.h, [],
[AC_CHECK_HEADERS(gssapi.h, [], [AC_MSG_ERROR([gssapi.h header file is required for GSSAPI])])])
AC_CHECK_HEADERS(gssapi/gssapi_ext.h, [],
[AC_CHECK_HEADERS(gssapi_ext.h, [], [AC_MSG_ERROR([gssapi_ext.h header file is required for GSSAPI])])])
fi

There might be a variant of AC_CHECK_HEADERS that doesn't have
the default define-a-symbol action, not sure.

Yep, the singular version doesn't. That's what my attached patch uses...

Maybe it's not really necessary to check both gssapi.h and
gssapi_ext.h, but I'm not very familiar with all the variants of
GSSAPI that are out there.

Me neither. I think it's fine to check both, I am just suggesting not to
define a pg_config.h symbol for both...

Greetings,

Andres Freund

#4Andres Freund
andres@anarazel.de
In reply to: Andres Freund (#1)
Re: Why do we define HAVE_GSSAPI_EXT_H?

On 2024-07-08 15:56:59 -0700, Andres Freund wrote:

While looking at this I also found an argument omission present in the commit
adding meson support. I plan to fix that with the attached commit.

Pushed that portion.