ci: Macos failures due to MacPorts behaviour change
Hi,
I noticed that CI tasks on the postgres' github repo fail for some branches on
macos [1]https://cirrus-ci.com/github/postgres/postgres/.
Initially I thought the problem was related to some outdated cache of the
macports installation. And indeed clearing that out does fix the issue - but
only temporarily and fixing one branch would cause some others to fail.
A fair bit of debugging later I realized that the problem is due to
src/tools/ci/ci_macports_packages.sh not actually installing missing packages
unless starting without a cache.
The cache key for the macports installation currently does not include the
major version. However, the md5 of src/tools/ci/ci_macports_packages.sh is
included in the cache key, and 17+ have a typo fix, leading to a different
cache key.
Whenever the cache was built with 15, 16 would fail, because gmake was
installed but not meson. And vice versa. It gets worse, see further down.
While we could fix the issue by including the major version in the key, that'd
only be a partial fix, because the goal is to be able to adjust the list of
packages.
The reason src/tools/ci/ci_macports_packages.sh does not "incrementally"
install new packages anymore is that port setrequested changed it's error
behaviour:
port setrequested $package errors out if $package is not installed. In the
past this was also true if multiple packages were passed in. However, now an
error is only raised if the first package is not installed:
andres@m4-dev ~ % sudo port -v setrequested bzip2 libiconv non-existing-package; echo $?
Setting requested flag for bzip2 @1.0.8_0 to 1
Setting requested flag for libiconv @1.17_0 to 1
0
andres@m4-dev ~ % sudo port -v setrequested non-existing-package bzip2 libiconv; echo $?
Error: non-existing-package is not installed
1
This means that src/tools/ci/ci_macports_packages.sh would only enter the
install-a-missing-package path if the first mentioned package was missing. As
the first package did not change between 15 and 16, we'd never install the
missing gmake/meson.
What's worse, because the remove-unneeded-packages path *does* work, we
eventually end up with a cache that has neither gmake nor meson installed
causing both 15 and 16 to fail.
The easiest fix I can see is to simply loop over the to-be-installed installed
packages and mark them as installed one-by-one. That's a few seconds slower,
but that's not too bad. Anyone got a better idea?
I'll try to report a bug to macports, but I suspect we ought to fix this for
CI before this is addressed via a new macports release.
Greetings,
Andres Freund
On Sun, Nov 17, 2024 at 2:17 PM Andres Freund <andres@anarazel.de> wrote:
Whenever the cache was built with 15, 16 would fail, because gmake was
installed but not meson. And vice versa. It gets worse, see further down.
Argh...
The easiest fix I can see is to simply loop over the to-be-installed installed
packages and mark them as installed one-by-one. That's a few seconds slower,
but that's not too bad. Anyone got a better idea?
With a loop as in the attached, it seems to take around 2.5 seconds
for that part. That's surprisingly slow for processing 12 packages,
but even "port usage" takes 0.18s on my Mac, so I guess all that tcl
code is just really slow to start up. It doesn't seem to be easy to
convince it to do more than one thing at a time without hiding
errors... for example if you pipe in commands to sudo port -F -, then
you'll have to parse the text output because $? only reports the
result of the final command. Other multi-package commands are also
too tolerant. So no, I haven't got a better idea...
Attachments:
0001-ci-Fix-MacPorts-installer-script.patchapplication/octet-stream; name=0001-ci-Fix-MacPorts-installer-script.patchDownload
From d7151045d3af912b7abf60be4cc057caa90a98af Mon Sep 17 00:00:00 2001
From: Thomas Munro <thomas.munro@gmail.com>
Date: Mon, 18 Nov 2024 21:46:38 +1300
Subject: [PATCH] ci: Fix MacPorts installer script.
The error reporting of "port setrequested list-of-packages..." changed,
hiding errors we were relying on to know if all packages in our list
were already installed. Use a loop instead.
Back-patch to 15 where CI began.
Suggested-by: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/au2uqfuy2nf43nwy2txmc5t2emhwij7kzupygto3d2ffgtrdgr%40ckvrlwyflnh2
---
src/tools/ci/ci_macports_packages.sh | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/src/tools/ci/ci_macports_packages.sh b/src/tools/ci/ci_macports_packages.sh
index b3df6d36a4e..41cb83593f7 100755
--- a/src/tools/ci/ci_macports_packages.sh
+++ b/src/tools/ci/ci_macports_packages.sh
@@ -61,9 +61,15 @@ fi
# if setting all the required packages as requested fails, we need
# to install at least one of them
-if ! sudo port setrequested $packages > /dev/null 2>&1 ; then
- echo not all required packages installed, doing so now
+echo "checking if all required packages are installed"
+for package in $packages ; do
+ if ! sudo port setrequested $package > /dev/null 2>&1 ; then
update_cached_image=1
+ fi
+done
+echo "done"
+if [ "$update_cached_image" -eq 1 ]; then
+ echo not all required packages installed, doing so now
# to keep the image small, we deleted the ports tree from the image...
sudo port selfupdate
# XXX likely we'll need some other way to force an upgrade at some
--
2.39.5 (Apple Git-154)
Oh, and yeah, we should include the branch name in the cache key.
Something like the attached. For some reason CI is not allowing me to
see the output from macOS right now (?!) so I couldn't see what
"Populate macports cache" printed out[1]https://cirrus-ci.com/task/6707217489985536, but I think this should be
right... will try again tomorrow.
I guess the alternative would be to set the package list the same
across all branches, even though they need different stuff, so they
could share the same cache without fighting over it?
Attachments:
v2-0001-ci-Fix-cached-MacPorts-installation-management.patchtext/x-patch; charset=US-ASCII; name=v2-0001-ci-Fix-cached-MacPorts-installation-management.patchDownload
From 191236b828e737de2b1a9b62140d0f9b2c9dbb70 Mon Sep 17 00:00:00 2001
From: Thomas Munro <thomas.munro@gmail.com>
Date: Mon, 18 Nov 2024 21:46:38 +1300
Subject: [PATCH v2] ci: Fix cached MacPorts installation management.
1. The error reporting of "port setrequested list-of-packages..."
changed, hiding errors we were relying on to know if all packages in our
list were already installed. Use a loop instead.
2. The cached MacPorts installation was shared between PostgreSQL
major branches, though each branch wanted different packages. Add the
branch name to the cache key, so that different branches, when tested in
one github account/repo such as postgres/postgres, stop fighting with
each other, adding and removing packages.
Back-patch to 15 where CI began.
Diagnosed-by: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/au2uqfuy2nf43nwy2txmc5t2emhwij7kzupygto3d2ffgtrdgr%40ckvrlwyflnh2
---
.cirrus.tasks.yml | 2 ++
src/tools/ci/ci_macports_packages.sh | 10 ++++++++--
2 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/.cirrus.tasks.yml b/.cirrus.tasks.yml
index fc413eb11ef..cd8e1b5eb0b 100644
--- a/.cirrus.tasks.yml
+++ b/.cirrus.tasks.yml
@@ -466,6 +466,8 @@ task:
# Include the OS major version in the cache key. If the OS image changes
# to a different major version, we need to reinstall.
sw_vers -productVersion | sed 's/\..*//'
+ # Include the branch name, because branches want different packages
+ echo ${CIRRUS_BRANCH}
# Also start afresh if we change our MacPorts install script.
md5 src/tools/ci/ci_macports_packages.sh
reupload_on_changes: true
diff --git a/src/tools/ci/ci_macports_packages.sh b/src/tools/ci/ci_macports_packages.sh
index b3df6d36a4e..41cb83593f7 100755
--- a/src/tools/ci/ci_macports_packages.sh
+++ b/src/tools/ci/ci_macports_packages.sh
@@ -61,9 +61,15 @@ fi
# if setting all the required packages as requested fails, we need
# to install at least one of them
-if ! sudo port setrequested $packages > /dev/null 2>&1 ; then
- echo not all required packages installed, doing so now
+echo "checking if all required packages are installed"
+for package in $packages ; do
+ if ! sudo port setrequested $package > /dev/null 2>&1 ; then
update_cached_image=1
+ fi
+done
+echo "done"
+if [ "$update_cached_image" -eq 1 ]; then
+ echo not all required packages installed, doing so now
# to keep the image small, we deleted the ports tree from the image...
sudo port selfupdate
# XXX likely we'll need some other way to force an upgrade at some
--
2.47.0
Hi,
On 2024-11-21 14:24:26 +1300, Thomas Munro wrote:
Oh, and yeah, we should include the branch name in the cache key.
Something like the attached.
I think that'd be too granular - we'd end up with lots of copies of
effectively the same cache, but which won't exactly the same due to timestamps
and such.
I guess the alternative would be to set the package list the same
across all branches, even though they need different stuff, so they
could share the same cache without fighting over it?
I don't think that'd work well either, imagine adding a new package to the
list...
The right approach probably is to include the list of packages in the key. A
bit annoying to change, because we'd need to move the list of packages to an
environment variable or file, but doable. I think something like
env:
MACOS_PACKAGE_LIST: >-
ccache
icu
...
fingerprint_script: |
...
echo $MACOS_PACKAGE_LIST
...
setup_additional_packages_script: |
sh src/tools/ci/ci_macports_packages.sh $MACOS_PACKAGE_LIST
should work?
For some reason CI is not allowing me to
see the output from macOS right now (?!) so I couldn't see what
"Populate macports cache" printed out[1], but I think this should be
right... will try again tomorrow.
I can see it for your link at the momemnt, fwiw.
Greetings,
Andres Freund
Hi,
On Fri, 22 Nov 2024 at 20:28, Andres Freund <andres@anarazel.de> wrote:
The right approach probably is to include the list of packages in the key. A
bit annoying to change, because we'd need to move the list of packages to an
environment variable or file, but doable. I think something likeenv:
MACOS_PACKAGE_LIST: >-
ccache
icu
...
fingerprint_script: |
...
echo $MACOS_PACKAGE_LIST
...
setup_additional_packages_script: |
sh src/tools/ci/ci_macports_packages.sh $MACOS_PACKAGE_LISTshould work?
I think this is a nice solution and it worked successfully [1]First run - second run (See cache is used in the second run.) [Master] https://cirrus-ci.com/task/6398434171682816 - https://cirrus-ci.com/task/6460963865493504 [PG 16] https://cirrus-ci.com/task/5697896752873472 - https://cirrus-ci.com/task/4656279002546176 [PG 15] https://cirrus-ci.com/task/5192066743926784 - https://cirrus-ci.com/task/5033544097988608. Now,
REL_[17 | 16]_* and master branches use the same cache which is
different from the REL_15_STABLE branch's cache.
In case you want to continue with this, the patches are attached. I
merged 'using a loop in the install script' from Thomas' patch and the
change above.
[1]: First run - second run (See cache is used in the second run.) [Master] https://cirrus-ci.com/task/6398434171682816 - https://cirrus-ci.com/task/6460963865493504 [PG 16] https://cirrus-ci.com/task/5697896752873472 - https://cirrus-ci.com/task/4656279002546176 [PG 15] https://cirrus-ci.com/task/5192066743926784 - https://cirrus-ci.com/task/5033544097988608
[Master] https://cirrus-ci.com/task/6398434171682816 -
https://cirrus-ci.com/task/6460963865493504
[PG 16] https://cirrus-ci.com/task/5697896752873472 -
https://cirrus-ci.com/task/4656279002546176
[PG 15] https://cirrus-ci.com/task/5192066743926784 -
https://cirrus-ci.com/task/5033544097988608
--
Regards,
Nazir Bilal Yavuz
Microsoft
Attachments:
v3-0001-ci-PG-17-16-Fix-cached-MacPorts-installation-mana.patchtext/x-patch; charset=US-ASCII; name=v3-0001-ci-PG-17-16-Fix-cached-MacPorts-installation-mana.patchDownload
From 6420ec51c8953c9746fe2eb77a91202f5ced4ed9 Mon Sep 17 00:00:00 2001
From: Nazir Bilal Yavuz <byavuz81@gmail.com>
Date: Wed, 27 Nov 2024 14:34:43 +0300
Subject: [PATCH v3] ci: PG 17, 16 - Fix cached MacPorts installation
management
1. The error reporting of "port setrequested list-of-packages..."
changed, hiding errors we were relying on to know if all packages in our
list were already installed. Use a loop instead.
2. The cached MacPorts installation was shared between PostgreSQL
major branches, though each branch wanted different packages. Add the
list of packages to cache key, so that different branches, when tested
in one github account/repo such as postgres/postgres, stop fighting with
each other, adding and removing packages.
Back-patch to 15 where CI began.
Author: Thomas Munro <thomas.munro@gmail.com>
Author: Nazir Bilal Yavuz <byavuz81@gmail.com>
Suggested-by: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/au2uqfuy2nf43nwy2txmc5t2emhwij7kzupygto3d2ffgtrdgr%40ckvrlwyflnh2
ci-os-only: macos
---
.cirrus.tasks.yml | 34 +++++++++++++++-------------
src/tools/ci/ci_macports_packages.sh | 10 ++++++--
2 files changed, 26 insertions(+), 18 deletions(-)
diff --git a/.cirrus.tasks.yml b/.cirrus.tasks.yml
index d8b7f9d32ab..197270feeaa 100644
--- a/.cirrus.tasks.yml
+++ b/.cirrus.tasks.yml
@@ -424,6 +424,20 @@ task:
CCACHE_DIR: ${HOME}/ccache
MACPORTS_CACHE: ${HOME}/macports-cache
+ MACOS_PACKAGE_LIST: >-
+ ccache
+ icu
+ kerberos5
+ lz4
+ meson
+ openldap
+ openssl
+ p5.34-io-tty
+ p5.34-ipc-run
+ python312
+ tcl
+ zstd
+
CC: ccache cc
CXX: ccache c++
CFLAGS: -Og -ggdb
@@ -457,26 +471,14 @@ task:
macports_cache:
folder: ${MACPORTS_CACHE}
fingerprint_script: |
- # Include the OS major version in the cache key. If the OS image changes
- # to a different major version, we need to reinstall.
+ # Reinstall packages if the OS major version, the list of the packages
+ # to install or the MacPorts install script changes.
sw_vers -productVersion | sed 's/\..*//'
- # Also start afresh if we change our MacPorts install script.
+ echo $MACOS_PACKAGE_LIST
md5 src/tools/ci/ci_macports_packages.sh
reupload_on_changes: true
setup_additional_packages_script: |
- sh src/tools/ci/ci_macports_packages.sh \
- ccache \
- icu \
- kerberos5 \
- lz4 \
- meson \
- openldap \
- openssl \
- p5.34-io-tty \
- p5.34-ipc-run \
- python312 \
- tcl \
- zstd
+ sh src/tools/ci/ci_macports_packages.sh $MACOS_PACKAGE_LIST
# system python doesn't provide headers
sudo /opt/local/bin/port select python3 python312
# Make macports install visible for subsequent steps
diff --git a/src/tools/ci/ci_macports_packages.sh b/src/tools/ci/ci_macports_packages.sh
index 692fa88c325..fb6164f49c3 100755
--- a/src/tools/ci/ci_macports_packages.sh
+++ b/src/tools/ci/ci_macports_packages.sh
@@ -61,9 +61,15 @@ fi
# if setting all the required packages as requested fails, we need
# to install at least one of them
-if ! sudo port setrequested $packages > /dev/null 2>&1 ; then
- echo not all required packages installed, doing so now
+echo "checking if all required packages are installed"
+for package in $packages ; do
+ if ! sudo port setrequested $package > /dev/null 2>&1 ; then
update_cached_image=1
+ fi
+done
+echo "done"
+if [ "$update_cached_image" -eq 1 ]; then
+ echo not all required packages installed, doing so now
# to keep the image small, we deleted the ports tree from the image...
sudo port selfupdate
# XXX likely we'll need some other way to force an upgrade at some
--
2.45.2
v3-0001-ci-PG-15-Fix-cached-MacPorts-installation-managem.patchtext/x-patch; charset=US-ASCII; name=v3-0001-ci-PG-15-Fix-cached-MacPorts-installation-managem.patchDownload
From c68d4d177823041b37172ba81647d6bcbb23c26f Mon Sep 17 00:00:00 2001
From: Nazir Bilal Yavuz <byavuz81@gmail.com>
Date: Wed, 27 Nov 2024 15:47:21 +0300
Subject: [PATCH v3] ci: PG 15 - Fix cached MacPorts installation management
1. The error reporting of "port setrequested list-of-packages..."
changed, hiding errors we were relying on to know if all packages in our
list were already installed. Use a loop instead.
2. The cached MacPorts installation was shared between PostgreSQL
major branches, though each branch wanted different packages. Add the
list of packages to cache key, so that different branches, when tested
in one github account/repo such as postgres/postgres, stop fighting with
each other, adding and removing packages.
Back-patch to 15 where CI began.
Author: Thomas Munro <thomas.munro@gmail.com>
Author: Nazir Bilal Yavuz <byavuz81@gmail.com>
Suggested-by: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/au2uqfuy2nf43nwy2txmc5t2emhwij7kzupygto3d2ffgtrdgr%40ckvrlwyflnh2
ci-os-only: macos
---
.cirrus.tasks.yml | 34 +++++++++++++++-------------
src/tools/ci/ci_macports_packages.sh | 10 ++++++--
2 files changed, 26 insertions(+), 18 deletions(-)
diff --git a/.cirrus.tasks.yml b/.cirrus.tasks.yml
index d75cbc004ee..3d344327452 100644
--- a/.cirrus.tasks.yml
+++ b/.cirrus.tasks.yml
@@ -223,6 +223,20 @@ task:
CCACHE_DIR: ${HOME}/ccache
MACPORTS_CACHE: ${HOME}/macports-cache
+ MACOS_PACKAGE_LIST: >-
+ ccache
+ gmake
+ icu
+ kerberos5
+ lz4
+ openldap
+ openssl
+ p5.34-io-tty
+ p5.34-ipc-run
+ python312
+ tcl
+ zstd
+
<<: *macos_task_template
only_if: $CIRRUS_CHANGE_MESSAGE !=~ '.*\nci-os-only:.*' || $CIRRUS_CHANGE_MESSAGE =~ '.*\nci-os-only:[^\n]*(macos|darwin|osx).*'
@@ -250,26 +264,14 @@ task:
macports_cache:
folder: ${MACPORTS_CACHE}
fingerprint_script: |
- # Include the OS major version in the cache key. If the OS image changes
- # to a different major version, we need to reinstall.
+ # Reinstall packages if the OS major version, the list of the packages
+ # to install or the MacPorts install script changes.
sw_vers -productVersion | sed 's/\..*//'
- # Also start afresh if we change our MacPorts install script.
+ echo $MACOS_PACKAGE_LIST
md5 src/tools/ci/ci_macports_packages.sh
reupload_on_changes: true
setup_additional_packages_script: |
- sh src/tools/ci/ci_macports_packages.sh \
- ccache \
- gmake \
- icu \
- kerberos5 \
- lz4 \
- openldap \
- openssl \
- p5.34-io-tty \
- p5.34-ipc-run \
- python312 \
- tcl \
- zstd
+ sh src/tools/ci/ci_macports_packages.sh $MACOS_PACKAGE_LIST
# system python doesn't provide headers
sudo /opt/local/bin/port select python3 python312
# Make macports install visible for subsequent steps
diff --git a/src/tools/ci/ci_macports_packages.sh b/src/tools/ci/ci_macports_packages.sh
index 692fa88c325..fb6164f49c3 100755
--- a/src/tools/ci/ci_macports_packages.sh
+++ b/src/tools/ci/ci_macports_packages.sh
@@ -61,9 +61,15 @@ fi
# if setting all the required packages as requested fails, we need
# to install at least one of them
-if ! sudo port setrequested $packages > /dev/null 2>&1 ; then
- echo not all required packages installed, doing so now
+echo "checking if all required packages are installed"
+for package in $packages ; do
+ if ! sudo port setrequested $package > /dev/null 2>&1 ; then
update_cached_image=1
+ fi
+done
+echo "done"
+if [ "$update_cached_image" -eq 1 ]; then
+ echo not all required packages installed, doing so now
# to keep the image small, we deleted the ports tree from the image...
sudo port selfupdate
# XXX likely we'll need some other way to force an upgrade at some
--
2.45.2
Hi,
On 2024-11-27 16:33:02 +0300, Nazir Bilal Yavuz wrote:
I think this is a nice solution and it worked successfully [1]. Now,
REL_[17 | 16]_* and master branches use the same cache which is
different from the REL_15_STABLE branch's cache.In case you want to continue with this, the patches are attached. I
merged 'using a loop in the install script' from Thomas' patch and the
change above.
Thanks! I added one comment and pushed it after giving it a spin myself.
Greetings,
Andres