Hi,

Thanks for the reviews!


On 9/27/2022 5:21 AM, Tom Lane wrote:
Andres Freund <and...@anarazel.de> writes:
Maybe we should rely on PATH, rather than hardcoding OS dependent locations?
My suggestion to consult krb5-config first was meant to allow PATH
to influence the results.  However, if that doesn't work, it's important
IMO to have a sane list of hardwired places to look in.


I updated my patch regarding these reviews.

The current logic is it will try to find all executables in that order(if it finds all executables, it won't try remaining steps):


1 - 'krb5-config --prefix'

2 - hardcoded paths(I added arm and MacPorts paths for darwin)

3 - from PATH

Also, I tried to do some refactoring for adding another paths to search in the future and being sure about all executables are found.

Ci run after fix is applied:
https://cirrus-ci.com/build/5758254918664192

Regards,
Nazir Bilal Yavuz

From 916de33762aa72e9d9e4665ba23a1e610c585570 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 7b5cb02102..cecd978515 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -259,7 +259,6 @@ task:
   name: macOS - Monterey - Meson
 
   env:
-    CPUS: 12 # always get that much for cirrusci macOS instances
     BUILD_JOBS: $CPUS
     TEST_JOBS: $CPUS # already fast enough to not be worth tuning
 
@@ -275,15 +274,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"
@@ -322,7 +332,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.25.1

From cc3f5236a6c18d9d16c8c6faa1e2044d0610f490 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 | 89 ++++++++++++++++++++++++---------
 1 file changed, 65 insertions(+), 24 deletions(-)

diff --git a/src/test/kerberos/t/001_auth.pl b/src/test/kerberos/t/001_auth.pl
index 47169a1d1e..c1211b80dd 100644
--- a/src/test/kerberos/t/001_auth.pl
+++ b/src/test/kerberos/t/001_auth.pl
@@ -30,39 +30,80 @@ 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)
+# get prefix for kerberos executables and try to find them at this path
+sub test_krb5_paths
 {
-       $krb5_config = $krb5_bin_dir . '/' . $krb5_config;
-       $kinit       = $krb5_bin_dir . '/' . $kinit;
+       my ($is_krb5_found, $krb5_path) = (@_);
+
+       # if it is alredy found return
+       if ($is_krb5_found)
+       {
+               return 1;
+       }
+
+       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'
+       $krb5_path =~ s/\n//g;
+       # remove trailing slashes
+       $krb5_path =~ s{(.+)/\z}{$1};
+       $bin = '/bin/';
+       $sbin = '/sbin/';
+       $kdb5_util_path    = $krb5_path . $sbin . $kdb5_util;
+       $kadmin_local_path = $krb5_path . $sbin . $kadmin_local;
+       $krb5kdc_path      = $krb5_path . $sbin . $krb5kdc;
+       $krb5_config_path  = $krb5_path . $bin . $krb5_config;
+       $kinit_path        = $krb5_path . $bin . $kinit;
+       @krb5_file_paths = ($kdb5_util_path, $kadmin_local_path, $krb5kdc_path, 
$krb5_config_path, $kinit_path);
+
+       # check if all of the files exist
+       foreach (@krb5_file_paths)
+       {
+               if(! -e $_)
+               {
+                       return 0;
+               }
+       }
+       # all files are found
+       $krb5_config  = $krb5_config_path;
+       $kinit        = $kinit_path;
+       $kdb5_util    = $kdb5_util_path;
+       $kadmin_local = $kadmin_local_path;
+       $krb5kdc      = $krb5kdc_path;
+       return 1;
 }
-if ($krb5_sbin_dir && -d $krb5_sbin_dir)
+
+my $is_krb5_found = 0;
+# 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';
+$is_krb5_found = test_krb5_paths($is_krb5_found, `$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
+               $is_krb5_found = test_krb5_paths($is_krb5_found, 
'/usr/local/opt/krb5/');
+               # krb5 is placed at /opt/homebrew/opt/ for arm CPU darwin
+               $is_krb5_found = test_krb5_paths($is_krb5_found, 
'/opt/homebrew/opt/krb5/');
+               # krb5 is placed at /opt/local/ for MacPorts
+               $is_krb5_found = test_krb5_paths($is_krb5_found, '/opt/local/');
+       }
+       elsif ($^O eq 'freebsd')
+       {
+               $is_krb5_found = test_krb5_paths($is_krb5_found, '/usr/local/');
+       }
+       elsif ($^O eq 'linux')
+       {
+               $is_krb5_found = test_krb5_paths($is_krb5_found, '/usr/sbin/');
+       }
 }
 
 my $host     = 'auth-test-localhost.postgresql.example.com';
-- 
2.25.1

Reply via email to