pg.ddx.io pgsql-hackers@postgresql.org mailing list archive
help / color / mirror / Atom feedWhy do we define HAVE_GSSAPI_EXT_H?
4+ messages / 2 participants
[nested] [flat]
Commits from this thread:
2024-07-20 5b707bb507 cc50694c8d c3dafaaac3 meson: Add missing argument to gssapi.h check
* Why do we define HAVE_GSSAPI_EXT_H?
@ 2024-07-08 22:56 Andres Freund <andres@anarazel.de>
0 siblings, 2 replies; 4+ messages in thread
From: Andres Freund @ 2024-07-08 22:56 UTC (permalink / raw)
To: pgsql-hackers@postgresql.org, Stephen Frost <sfrost@snowman.net>; +Cc: Jonathan S. Katz <jkatz@postgresql.org>
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: download all (2 files, .tar.gz)
v1-0001-meson-Add-missing-argument-to-gssapi.h-check.patch (text/x-diff, 1.8K) download
From 4e036f77bad5264cda625222068b706555d7f85a Mon Sep 17 00:00:00 2001
From: Andres Freund <andres@anarazel.de>
Date: Mon, 8 Jul 2024 15:28:04 -0700
Subject: [PATCH v1 1/2] meson: Add missing argument to gssapi.h check
These were missing since the initial introduction of the meson based build, in
e6927270cd18. As-is this is unlikely to cause an issue, but a future commit
will add support for detecting gssapi without use of dependency(), which could
fail due to this.
Backpatch: 16-, where the meson based build was added
---
meson.build | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/meson.build b/meson.build
index 5387bb6d5fd..102b1e98a27 100644
--- a/meson.build
+++ b/meson.build
@@ -621,7 +621,8 @@ if not gssapiopt.disabled()
elif cc.check_header('gssapi/gssapi.h', dependencies: gssapi, required: false,
args: test_c_args, include_directories: postgres_inc)
cdata.set('HAVE_GSSAPI_GSSAPI_H', 1)
- elif cc.check_header('gssapi.h', args: test_c_args, dependencies: gssapi, required: gssapiopt)
+ elif cc.check_header('gssapi.h', dependencies: gssapi, required: gssapiopt,
+ args: test_c_args, include_directories: postgres_inc)
cdata.set('HAVE_GSSAPI_H', 1)
else
have_gssapi = false
@@ -631,7 +632,8 @@ if not gssapiopt.disabled()
elif cc.check_header('gssapi/gssapi_ext.h', dependencies: gssapi, required: false,
args: test_c_args, include_directories: postgres_inc)
cdata.set('HAVE_GSSAPI_GSSAPI_EXT_H', 1)
- elif cc.check_header('gssapi_ext.h', args: test_c_args, dependencies: gssapi, required: gssapiopt)
+ elif cc.check_header('gssapi_ext.h', dependencies: gssapi, required: gssapiopt,
+ args: test_c_args, include_directories: postgres_inc)
cdata.set('HAVE_GSSAPI_EXT_H', 1)
else
have_gssapi = false
--
2.44.0.279.g3bd955d269
v1-0002-Don-t-define-HAVE_-GSSAPI_-GSSAPI_EXT_H.patch (text/x-diff, 4.3K) download
From 9f7c96dfab4d807e668c9d32b44db5f4ff122e15 Mon Sep 17 00:00:00 2001
From: Andres Freund <andres@anarazel.de>
Date: Mon, 8 Jul 2024 15:55:56 -0700
Subject: [PATCH v1 2/2] Don't define HAVE_[GSSAPI_]GSSAPI_EXT_H
The check for gssapi_ext.h was added in f7431bca8b0. As we require
gssapi_ext.h to be present, there's no point in defining symbols for the
header presence.
While at it, use cc.has_header() instead of cc.check_header(), that's a bit
cheaper and it seems improbably that gssapi.h would compile while gssapi_ext.h
would not.
Author:
Reviewed-by:
Discussion: https://postgr.es/m/
Backpatch:
---
meson.build | 15 ++++-----------
configure.ac | 4 ++--
src/include/pg_config.h.in | 6 ------
configure | 12 ------------
4 files changed, 6 insertions(+), 31 deletions(-)
diff --git a/meson.build b/meson.build
index 102b1e98a27..f1912d83ce6 100644
--- a/meson.build
+++ b/meson.build
@@ -620,25 +620,18 @@ if not gssapiopt.disabled()
if not have_gssapi
elif cc.check_header('gssapi/gssapi.h', dependencies: gssapi, required: false,
args: test_c_args, include_directories: postgres_inc)
+ cc.has_header('gssapi/gssapi_ext.h', dependencies: gssapi, required: true,
+ args: test_c_args, include_directories: postgres_inc)
cdata.set('HAVE_GSSAPI_GSSAPI_H', 1)
elif cc.check_header('gssapi.h', dependencies: gssapi, required: gssapiopt,
args: test_c_args, include_directories: postgres_inc)
+ cc.has_header('gssapi_ext.h', dependencies: gssapi, required: true,
+ args: test_c_args, include_directories: postgres_inc)
cdata.set('HAVE_GSSAPI_H', 1)
else
have_gssapi = false
endif
- if not have_gssapi
- elif cc.check_header('gssapi/gssapi_ext.h', dependencies: gssapi, required: false,
- args: test_c_args, include_directories: postgres_inc)
- cdata.set('HAVE_GSSAPI_GSSAPI_EXT_H', 1)
- elif cc.check_header('gssapi_ext.h', dependencies: gssapi, required: gssapiopt,
- args: test_c_args, include_directories: postgres_inc)
- cdata.set('HAVE_GSSAPI_EXT_H', 1)
- else
- have_gssapi = false
- endif
-
if not have_gssapi
elif cc.has_function('gss_store_cred_into', dependencies: gssapi,
args: test_c_args, include_directories: postgres_inc)
diff --git a/configure.ac b/configure.ac
index ab2d51c21ce..7cd7e95ae49 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1532,8 +1532,8 @@ fi
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])])])
+ AC_CHECK_HEADER(gssapi/gssapi_ext.h, [],
+ [AC_CHECK_HEADER(gssapi_ext.h, [], [AC_MSG_ERROR([gssapi_ext.h header file is required for GSSAPI])])])
fi
PGAC_PATH_PROGS(OPENSSL, openssl)
diff --git a/src/include/pg_config.h.in b/src/include/pg_config.h.in
index f8d3e3b6b84..e0c0b51c5c1 100644
--- a/src/include/pg_config.h.in
+++ b/src/include/pg_config.h.in
@@ -180,12 +180,6 @@
/* Define to 1 if you have the `getpeerucred' function. */
#undef HAVE_GETPEERUCRED
-/* Define to 1 if you have the <gssapi_ext.h> header file. */
-#undef HAVE_GSSAPI_EXT_H
-
-/* Define to 1 if you have the <gssapi/gssapi_ext.h> header file. */
-#undef HAVE_GSSAPI_GSSAPI_EXT_H
-
/* Define to 1 if you have the <gssapi/gssapi.h> header file. */
#undef HAVE_GSSAPI_GSSAPI_H
diff --git a/configure b/configure
index 76f06bd8fda..2a9d467be04 100755
--- a/configure
+++ b/configure
@@ -13677,32 +13677,20 @@ fi
done
- for ac_header in gssapi/gssapi_ext.h
-do :
ac_fn_c_check_header_mongrel "$LINENO" "gssapi/gssapi_ext.h" "ac_cv_header_gssapi_gssapi_ext_h" "$ac_includes_default"
if test "x$ac_cv_header_gssapi_gssapi_ext_h" = xyes; then :
- cat >>confdefs.h <<_ACEOF
-#define HAVE_GSSAPI_GSSAPI_EXT_H 1
-_ACEOF
else
- for ac_header in gssapi_ext.h
-do :
ac_fn_c_check_header_mongrel "$LINENO" "gssapi_ext.h" "ac_cv_header_gssapi_ext_h" "$ac_includes_default"
if test "x$ac_cv_header_gssapi_ext_h" = xyes; then :
- cat >>confdefs.h <<_ACEOF
-#define HAVE_GSSAPI_EXT_H 1
-_ACEOF
else
as_fn_error $? "gssapi_ext.h header file is required for GSSAPI" "$LINENO" 5
fi
-done
fi
-done
fi
--
2.44.0.279.g3bd955d269
* Re: Why do we define HAVE_GSSAPI_EXT_H?
@ 2024-07-08 23:05 Tom Lane <tgl@sss.pgh.pa.us>
parent: Andres Freund <andres@anarazel.de>
1 sibling, 1 reply; 4+ messages in thread
From: Tom Lane @ 2024-07-08 23:05 UTC (permalink / raw)
To: Andres Freund <andres@anarazel.de>; +Cc: pgsql-hackers@postgresql.org, Stephen Frost <sfrost@snowman.net>; Jonathan S. Katz <jkatz@postgresql.org>
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
^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: Why do we define HAVE_GSSAPI_EXT_H?
@ 2024-07-09 00:05 Andres Freund <andres@anarazel.de>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 0 replies; 4+ messages in thread
From: Andres Freund @ 2024-07-09 00:05 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: pgsql-hackers@postgresql.org, Stephen Frost <sfrost@snowman.net>; Jonathan S. Katz <jkatz@postgresql.org>
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
^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: Why do we define HAVE_GSSAPI_EXT_H?
@ 2024-07-20 20:57 Andres Freund <andres@anarazel.de>
parent: Andres Freund <andres@anarazel.de>
1 sibling, 0 replies; 4+ messages in thread
From: Andres Freund @ 2024-07-20 20:57 UTC (permalink / raw)
To: pgsql-hackers@postgresql.org, Stephen Frost <sfrost@snowman.net>; +Cc: Jonathan S. Katz <jkatz@postgresql.org>
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.
^ permalink raw reply [nested|flat] 4+ messages in thread
end of thread, other threads:[~2024-07-20 20:57 UTC | newest]
Thread overview: 4+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2024-07-08 22:56 Why do we define HAVE_GSSAPI_EXT_H? Andres Freund <andres@anarazel.de>
2024-07-08 23:05 ` Tom Lane <tgl@sss.pgh.pa.us>
2024-07-09 00:05 ` Andres Freund <andres@anarazel.de>
2024-07-20 20:57 ` Andres Freund <andres@anarazel.de>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox