Hi,
Thanks for the review!
On 10/1/22 14:12, Peter Eisentraut wrote:
This patch could use some more in-code comments. For example, this
+# get prefix for kerberos executables and try to find them at this path
+sub test_krb5_paths
is not helpful. What does it "get", where does it put it, how does it
"try", and what does it do if it fails? What are the inputs and
outputs of this function?
+ # remove '\n' since 'krb5-config --prefix' returns path ends with
'\n'
+ $krb5_path =~ s/\n//g;
use chomp
I updated patch regarding these comments.
I have a question about my logic:
+ elsif ($^O eq 'linux')
+ {
+ test_krb5_paths('/usr/');
+ }
}
Before that, test could use krb5kdc, kadmin and kdb5_util from
'/usr/sbin/'; krb5_config and kinit from $PATH. However, now it will try
to use all of them from $PATH or from '/usr/sbin/' and '/usr/bin/'. Does
that cause a problem?
Ci run after fix is applied:
https://cirrus-ci.com/build/5359971746447360
Regards,
Nazir Bilal Yavuz
From 5aa95409db8146a076255e00be399a0d9ce1efe8 Mon Sep 17 00:00:00 2001
From: Nazir Bilal Yavuz <byavu...@gmail.com>
Date: Mon, 26 Sep 2022 10:56:51 +0300
Subject: [PATCH 1/2] ci: Add arm CPU for darwin
.
ci-os-only: darwin.
---
.cirrus.yml | 20 +++++++++++++++-----
1 file changed, 15 insertions(+), 5 deletions(-)
diff --git a/.cirrus.yml b/.cirrus.yml
index 9f2282471a..c23be7363c 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -293,7 +293,6 @@ task:
name: macOS - Monterey - Meson
env:
- CPUS: 12 # always get that much for cirrusci macOS instances
BUILD_JOBS: $CPUS
# Test performance regresses noticably when using all cores. 8 seems to
# work OK. See
@@ -313,15 +312,26 @@ task:
only_if: $CIRRUS_CHANGE_MESSAGE !=~ '.*\nci-os-only:.*' || $CIRRUS_CHANGE_MESSAGE =~ '.*\nci-os-only:[^\n]*(macos|darwin|osx).*'
- osx_instance:
- image: monterey-base
+ matrix:
+ - name: macOS - Monterey - Meson - ARM CPU
+ macos_instance:
+ image: ghcr.io/cirruslabs/macos-monterey-base:latest
+ env:
+ BREW_PATH: /opt/homebrew
+ CPUS: 4
+
+ - name: macOS - Monterey - Meson - Intel CPU
+ osx_instance:
+ image: monterey-base
+ env:
+ BREW_PATH: /usr/local
+ CPUS: 12 # always get that much for cirrusci macOS instances
sysinfo_script: |
id
uname -a
ulimit -a -H && ulimit -a -S
export
-
setup_core_files_script:
- mkdir ${HOME}/cores
- sudo sysctl kern.corefile="${HOME}/cores/core.%P"
@@ -360,7 +370,7 @@ task:
ccache_cache:
folder: $CCACHE_DIR
configure_script: |
- brewpath="/usr/local"
+ brewpath=${BREW_PATH}
PKG_CONFIG_PATH="${brewpath}/lib/pkgconfig:${PKG_CONFIG_PATH}"
for pkg in icu4c krb5 openldap openssl zstd ; do
--
2.37.3
From 5ad2357ff7f3ddcb24754847f542952143ccafa7 Mon Sep 17 00:00:00 2001
From: Nazir Bilal Yavuz <byavu...@gmail.com>
Date: Fri, 23 Sep 2022 14:30:06 +0300
Subject: [PATCH 2/2] fix: darwin: ARM CPU darwin krb5 path fix
---
src/test/kerberos/t/001_auth.pl | 103 ++++++++++++++++++++++++--------
1 file changed, 79 insertions(+), 24 deletions(-)
diff --git a/src/test/kerberos/t/001_auth.pl b/src/test/kerberos/t/001_auth.pl
index a2bc8a5351..9587698df3 100644
--- a/src/test/kerberos/t/001_auth.pl
+++ b/src/test/kerberos/t/001_auth.pl
@@ -30,39 +30,94 @@ elsif ($ENV{PG_TEST_EXTRA} !~ /\bkerberos\b/)
plan skip_all => 'Potentially unsafe test GSSAPI/Kerberos not enabled in PG_TEST_EXTRA';
}
-my ($krb5_bin_dir, $krb5_sbin_dir);
-
-if ($^O eq 'darwin')
-{
- $krb5_bin_dir = '/usr/local/opt/krb5/bin';
- $krb5_sbin_dir = '/usr/local/opt/krb5/sbin';
-}
-elsif ($^O eq 'freebsd')
-{
- $krb5_bin_dir = '/usr/local/bin';
- $krb5_sbin_dir = '/usr/local/sbin';
-}
-elsif ($^O eq 'linux')
-{
- $krb5_sbin_dir = '/usr/sbin';
-}
-
my $krb5_config = 'krb5-config';
my $kinit = 'kinit';
my $kdb5_util = 'kdb5_util';
my $kadmin_local = 'kadmin.local';
my $krb5kdc = 'krb5kdc';
-if ($krb5_bin_dir && -d $krb5_bin_dir)
+# control variable for checking if all executables are found
+my $is_krb5_found = 0;
+
+# Given $krb5_path_prefix (possible paths of kerberos executables),
+# first check value of $is_krb5_found:
+# if it is truthy value this means executables are already found so
+# don't try to locate further, else add '/bin/' and '/sbin/'
+# to the $krb5_path_prefix and try to locate all required kerberos executables
+# ($krb5_config, $kinit, $kdb5_util, $kadmin_local, $krb5kdc).
+# If all executables are found, set $is_krb5_found to 1 and use executables
+# from this path; even if one of the executables is not found, skip this path.
+# If all paths are searched and executables are still not found,
+# test will try to use kerberos executables from $PATH.
+sub test_krb5_paths
{
- $krb5_config = $krb5_bin_dir . '/' . $krb5_config;
- $kinit = $krb5_bin_dir . '/' . $kinit;
+ my ($krb5_path_prefix) = (@_);
+
+ # if executables are already found, return
+ if ($is_krb5_found)
+ {
+ return;
+ }
+
+ my ($kdb5_util_path, $kadmin_local_path, $krb5kdc_path, $krb5_config_path, $kinit_path, @krb5_file_paths, $bin, $sbin);
+
+ # remove '\n' since 'krb5-config --prefix' returns path ends with '\n'
+ chomp($krb5_path_prefix);
+ # remove trailing slashes
+ $krb5_path_prefix =~ s{(.+)/\z}{$1};
+ $bin = '/bin/';
+ $sbin = '/sbin/';
+ $kdb5_util_path = $krb5_path_prefix . $sbin . $kdb5_util;
+ $kadmin_local_path = $krb5_path_prefix . $sbin . $kadmin_local;
+ $krb5kdc_path = $krb5_path_prefix . $sbin . $krb5kdc;
+ $krb5_config_path = $krb5_path_prefix . $bin . $krb5_config;
+ $kinit_path = $krb5_path_prefix . $bin . $kinit;
+ @krb5_file_paths = ($kdb5_util_path, $kadmin_local_path, $krb5kdc_path, $krb5_config_path, $kinit_path);
+
+ # check if all of the executables exist, even if one of the executables
+ # is not found; return
+ foreach (@krb5_file_paths)
+ {
+ if(! -e $_)
+ {
+ return;
+ }
+ }
+ # all executables are found
+ $krb5_config = $krb5_config_path;
+ $kinit = $kinit_path;
+ $kdb5_util = $kdb5_util_path;
+ $kadmin_local = $kadmin_local_path;
+ $krb5kdc = $krb5kdc_path;
+
+ # set $is_krb5_found to the 1 to not check other possible paths
+ $is_krb5_found = 1;
}
-if ($krb5_sbin_dir && -d $krb5_sbin_dir)
+
+# first try to use 'krb5-config --prefix', then try hardcoded paths.
+# if executables are still not found after trying these paths,
+# try to use them from $PATH.
+my $krb5_config_prefix_cmd = $krb5_config . ' --prefix';
+test_krb5_paths(`$krb5_config_prefix_cmd`);
+if (! $is_krb5_found)
{
- $kdb5_util = $krb5_sbin_dir . '/' . $kdb5_util;
- $kadmin_local = $krb5_sbin_dir . '/' . $kadmin_local;
- $krb5kdc = $krb5_sbin_dir . '/' . $krb5kdc;
+ if ($^O eq 'darwin')
+ {
+ # krb5 is placed at /usr/local/opt for Intel CPU darwin
+ test_krb5_paths('/usr/local/opt/krb5/');
+ # krb5 is placed at /opt/homebrew/opt/ for arm CPU darwin
+ test_krb5_paths('/opt/homebrew/opt/krb5/');
+ # krb5 is placed at /opt/local/ for MacPorts
+ test_krb5_paths('/opt/local/');
+ }
+ elsif ($^O eq 'freebsd')
+ {
+ test_krb5_paths('/usr/local/');
+ }
+ elsif ($^O eq 'linux')
+ {
+ test_krb5_paths('/usr/');
+ }
}
my $host = 'auth-test-localhost.postgresql.example.com';
--
2.37.3
