{"thread":{"id":"62609","subject":"[PATCH 0/4] Random improvements to GitLab CI","startedAt":"2024-12-06T11:10:32Z","lastAt":"2024-12-12T06:47:58Z","messageCount":20,"participants":["Patrick Steinhardt","karthik nayak","Oswald Buddenhagen","Toon Claes"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"508735","messageId":"20241206-pks-ci-section-fixes-v1-0-7ab1b69e3648@pks.im","threadId":"62609","inReplyTo":null,"subject":"[PATCH 0/4] Random improvements to GitLab CI","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-12-06T11:10:12Z","receivedAt":"2024-12-06T11:10:32Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Hi,\n\nthis small patch series includes a couple of more-or-less random\nimprovements to GitLab CI.\n\nThanks!\n\nPatrick\n\n---\nPatrick Steinhardt (4):\n      gitlab-ci: update macOS images to Sonoma\n      ci/lib: remove duplicate trap to end \"CI setup\" group\n      ci/lib: use echo instead of printf to set up sections\n      ci/lib: fix \"CI setup\" sections with GitLab CI\n\n .gitlab-ci.yml | 4 ++--\n ci/lib.sh      | 9 ++++-----\n 2 files changed, 6 insertions(+), 7 deletions(-)\n\n\n---\nbase-commit: e66fd72e972df760a53c3d6da023c17adfc426d6\nchange-id: 20241206-pks-ci-section-fixes-1bb91ceb50b8\n\n"},{"id":"508736","messageId":"20241206-pks-ci-section-fixes-v1-2-7ab1b69e3648@pks.im","threadId":"62609","inReplyTo":"20241206-pks-ci-section-fixes-v1-0-7ab1b69e3648@pks.im","subject":"[PATCH 2/4] ci/lib: remove duplicate trap to end \"CI setup\" group","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-12-06T11:10:14Z","receivedAt":"2024-12-06T11:10:33Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"We exlicitly trap on EXIT in order to end the \"CI setup\" group. This\nisn't necessary though given that `begin_group ()` already sets up the\ntrap for us.\n\nRemove the duplicate trap.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n ci/lib.sh | 1 -\n 1 file changed, 1 deletion(-)\n\ndiff --git a/ci/lib.sh b/ci/lib.sh\nindex 930f98d7228166c37c236beb062b14675fb68ef3..a54601be923bf475ba1a9cafd98bb1cb71a10255 100755\n--- a/ci/lib.sh\n+++ b/ci/lib.sh\n@@ -56,7 +56,6 @@ group () {\n }\n \n begin_group \"CI setup\"\n-trap \"end_group 'CI setup'\" EXIT\n \n # Set 'exit on error' for all CI scripts to let the caller know that\n # something went wrong.\n\n-- \n2.47.0.366.g5daf58cba8.dirty\n\n"},{"id":"508737","messageId":"20241206-pks-ci-section-fixes-v1-1-7ab1b69e3648@pks.im","threadId":"62609","inReplyTo":"20241206-pks-ci-section-fixes-v1-0-7ab1b69e3648@pks.im","subject":"[PATCH 1/4] gitlab-ci: update macOS images to Sonoma","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-12-06T11:10:13Z","receivedAt":"2024-12-06T11:10:33Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The macOS Ventura images we use for GitLab CI runners have been\ndeprecated. Update them to macOS 14, aka Sonoma.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n .gitlab-ci.yml | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/.gitlab-ci.yml b/.gitlab-ci.yml\nindex 526ecfe030a43e0a5a83ddd35cb7c96d46ab2485..61c56ccac8fdc940075d91dd4cb0b54ee33d5199 100644\n--- a/.gitlab-ci.yml\n+++ b/.gitlab-ci.yml\n@@ -99,10 +99,10 @@ test:osx:\n   parallel:\n     matrix:\n       - jobname: osx-clang\n-        image: macos-13-xcode-14\n+        image: macos-14-xcode-15\n         CC: clang\n       - jobname: osx-reftable\n-        image: macos-13-xcode-14\n+        image: macos-14-xcode-15\n         CC: clang\n   artifacts:\n     paths:\n\n-- \n2.47.0.366.g5daf58cba8.dirty\n\n"},{"id":"508738","messageId":"20241206-pks-ci-section-fixes-v1-3-7ab1b69e3648@pks.im","threadId":"62609","inReplyTo":"20241206-pks-ci-section-fixes-v1-0-7ab1b69e3648@pks.im","subject":"[PATCH 3/4] ci/lib: use echo instead of printf to set up sections","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-12-06T11:10:15Z","receivedAt":"2024-12-06T11:10:34Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"We use printf to set up sections with GitLab CI even though we could\ntrivially use echo. This may cause problems in case the argument passed\nto `begin_group ()` or `end_group ()` contains formatting directives as\nwe use them as part of the formatting string.\n\nSimplify the code to instead use echo.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n ci/lib.sh | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/ci/lib.sh b/ci/lib.sh\nindex a54601be923bf475ba1a9cafd98bb1cb71a10255..ba8f4da39caf29db5edaffde160bc81a7c58c329 100755\n--- a/ci/lib.sh\n+++ b/ci/lib.sh\n@@ -18,7 +18,7 @@ elif test true = \"$GITLAB_CI\"\n then\n \tbegin_group () {\n \t\tneed_to_end_group=t\n-\t\tprintf \"\\e[0Ksection_start:$(date +%s):$(echo \"$1\" | tr ' ' _)[collapsed=true]\\r\\e[0K$1\\n\"\n+\t\techo \"\\e[0Ksection_start:$(date +%s):$(echo \"$1\" | tr ' ' _)[collapsed=true]\\r\\e[0K$1\"\n \t\ttrap \"end_group '$1'\" EXIT\n \t\tset -x\n \t}\n@@ -27,7 +27,7 @@ then\n \t\ttest -n \"$need_to_end_group\" || return 0\n \t\tset +x\n \t\tneed_to_end_group=\n-\t\tprintf \"\\e[0Ksection_end:$(date +%s):$(echo \"$1\" | tr ' ' _)\\r\\e[0K\\n\"\n+\t\techo \"\\e[0Ksection_end:$(date +%s):$(echo \"$1\" | tr ' ' _)\\r\\e[0K\"\n \t\ttrap - EXIT\n \t}\n else\n\n-- \n2.47.0.366.g5daf58cba8.dirty\n\n"},{"id":"508739","messageId":"20241206-pks-ci-section-fixes-v1-4-7ab1b69e3648@pks.im","threadId":"62609","inReplyTo":"20241206-pks-ci-section-fixes-v1-0-7ab1b69e3648@pks.im","subject":"[PATCH 4/4] ci/lib: fix \"CI setup\" sections with GitLab CI","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-12-06T11:10:16Z","receivedAt":"2024-12-06T11:10:35Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Whenever we source \"ci/lib.sh\" we wrap the directives in a separate\ngroup so that they can easily be collapsed in the web UI. And as we\nsource the script multiple times during a single CI run we thus end up\nwith the same section name reused multiple times, as well.\n\nThis is broken on GitLab CI though, where reusing the same group name is\nnot supported. The consequence is that only the last of these sections\ncan be collapsed.\n\nFix this issue by including the name of the sourcing script in the\ngroup's name.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n ci/lib.sh | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/ci/lib.sh b/ci/lib.sh\nindex ba8f4da39caf29db5edaffde160bc81a7c58c329..2cdc99e7fd05650ef80715b621b42d15d6b13a12 100755\n--- a/ci/lib.sh\n+++ b/ci/lib.sh\n@@ -55,7 +55,7 @@ group () {\n \treturn $res\n }\n \n-begin_group \"CI setup\"\n+begin_group \"CI setup via $(basename $0)\"\n \n # Set 'exit on error' for all CI scripts to let the caller know that\n # something went wrong.\n@@ -393,5 +393,5 @@ esac\n \n MAKEFLAGS=\"$MAKEFLAGS CC=${CC:-cc}\"\n \n-end_group \"CI setup\"\n+end_group \"CI setup via $(basename $0)\"\n set -x\n\n-- \n2.47.0.366.g5daf58cba8.dirty\n\n"},{"id":"508745","messageId":"CAOLa=ZRUZC9KDj9F6iLWWU+CBHz_DWKA_8znM=7RVSCVGSeD9w@mail.gmail.com","threadId":"62609","inReplyTo":"20241206-pks-ci-section-fixes-v1-1-7ab1b69e3648@pks.im","subject":"Re: [PATCH 1/4] gitlab-ci: update macOS images to Sonoma","fromName":"karthik nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2024-12-06T11:39:59Z","receivedAt":"2024-12-06T11:40:01Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> The macOS Ventura images we use for GitLab CI runners have been\n> deprecated. Update them to macOS 14, aka Sonoma.\n>\n> Signed-off-by: Patrick Steinhardt <ps@pks.im>\n> ---\n>  .gitlab-ci.yml | 4 ++--\n>  1 file changed, 2 insertions(+), 2 deletions(-)\n>\n> diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml\n> index 526ecfe030a43e0a5a83ddd35cb7c96d46ab2485..61c56ccac8fdc940075d91dd4cb0b54ee33d5199 100644\n> --- a/.gitlab-ci.yml\n> +++ b/.gitlab-ci.yml\n> @@ -99,10 +99,10 @@ test:osx:\n>    parallel:\n>      matrix:\n>        - jobname: osx-clang\n> -        image: macos-13-xcode-14\n> +        image: macos-14-xcode-15\n>          CC: clang\n>        - jobname: osx-reftable\n> -        image: macos-13-xcode-14\n> +        image: macos-14-xcode-15\n>          CC: clang\n>    artifacts:\n>      paths:\n\nThe changes look good, the documentation also states that if no image is\nmentioned, it'll use `macos-14-xcode-15` [1]. I wonder if this means\nthat if the image is unspecified, it will always use the first\nnon-deprecated version. That'd allow us to not have to keep updating\nthis.\n\n[1]: https://docs.gitlab.com/ee/ci/runners/hosted_runners/macos.html#supported-macos-images\n"},{"id":"508746","messageId":"CAOLa=ZTdpmfKfuLj9PcXuO6_EqqR-2V4y8s2mpR3iXAS0sT32A@mail.gmail.com","threadId":"62609","inReplyTo":"20241206-pks-ci-section-fixes-v1-4-7ab1b69e3648@pks.im","subject":"Re: [PATCH 4/4] ci/lib: fix \"CI setup\" sections with GitLab CI","fromName":"karthik nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2024-12-06T11:46:30Z","receivedAt":"2024-12-06T11:46:32Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> Whenever we source \"ci/lib.sh\" we wrap the directives in a separate\n> group so that they can easily be collapsed in the web UI. And as we\n> source the script multiple times during a single CI run we thus end up\n> with the same section name reused multiple times, as well.\n>\n> This is broken on GitLab CI though, where reusing the same group name is\n> not supported. The consequence is that only the last of these sections\n> can be collapsed.\n>\n> Fix this issue by including the name of the sourcing script in the\n> group's name.\n>\n> Signed-off-by: Patrick Steinhardt <ps@pks.im>\n> ---\n>  ci/lib.sh | 4 ++--\n>  1 file changed, 2 insertions(+), 2 deletions(-)\n>\n> diff --git a/ci/lib.sh b/ci/lib.sh\n> index ba8f4da39caf29db5edaffde160bc81a7c58c329..2cdc99e7fd05650ef80715b621b42d15d6b13a12 100755\n> --- a/ci/lib.sh\n> +++ b/ci/lib.sh\n> @@ -55,7 +55,7 @@ group () {\n>  \treturn $res\n>  }\n>\n> -begin_group \"CI setup\"\n> +begin_group \"CI setup via $(basename $0)\"\n>\n>  # Set 'exit on error' for all CI scripts to let the caller know that\n>  # something went wrong.\n> @@ -393,5 +393,5 @@ esac\n>\n>  MAKEFLAGS=\"$MAKEFLAGS CC=${CC:-cc}\"\n>\n> -end_group \"CI setup\"\n> +end_group \"CI setup via $(basename $0)\"\n>  set -x\n\nNice. Before, only the last instance of \"CI setup\" would be collapsible.\nNow that should be fixed.\n\nHere are the links for without and with this series.\n\nWithout: https://gitlab.com/gitlab-org/git/-/jobs/8565481567#L20\nWith: https://gitlab.com/gitlab-org/git/-/jobs/8567031049\n"},{"id":"508747","messageId":"CAOLa=ZQeRWHog4Fjkb4MgK_34kV5JVEp4yAQ7Obgx6cnDJgyAA@mail.gmail.com","threadId":"62609","inReplyTo":"20241206-pks-ci-section-fixes-v1-0-7ab1b69e3648@pks.im","subject":"Re: [PATCH 0/4] Random improvements to GitLab CI","fromName":"karthik nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2024-12-06T11:47:25Z","receivedAt":"2024-12-06T11:47:26Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> Hi,\n>\n> this small patch series includes a couple of more-or-less random\n> improvements to GitLab CI.\n>\n\nApart from one suggestion(?) in the first commit, I think the series\nlooks great! Thanks\n\n> Thanks!\n>\n> Patrick\n>\n> ---\n> Patrick Steinhardt (4):\n>       gitlab-ci: update macOS images to Sonoma\n>       ci/lib: remove duplicate trap to end \"CI setup\" group\n>       ci/lib: use echo instead of printf to set up sections\n>       ci/lib: fix \"CI setup\" sections with GitLab CI\n>\n>  .gitlab-ci.yml | 4 ++--\n>  ci/lib.sh      | 9 ++++-----\n>  2 files changed, 6 insertions(+), 7 deletions(-)\n>\n>\n> ---\n> base-commit: e66fd72e972df760a53c3d6da023c17adfc426d6\n> change-id: 20241206-pks-ci-section-fixes-1bb91ceb50b8\n"},{"id":"508749","messageId":"Z1Ln2ObkKB3ZnoGD@pks.im","threadId":"62609","inReplyTo":"CAOLa=ZQeRWHog4Fjkb4MgK_34kV5JVEp4yAQ7Obgx6cnDJgyAA@mail.gmail.com","subject":"Re: [PATCH 0/4] Random improvements to GitLab CI","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-12-06T12:02:32Z","receivedAt":"2024-12-06T12:02:50Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Fri, Dec 06, 2024 at 03:47:25AM -0800, karthik nayak wrote:\n> Patrick Steinhardt <ps@pks.im> writes:\n> \n> > Hi,\n> >\n> > this small patch series includes a couple of more-or-less random\n> > improvements to GitLab CI.\n> >\n> \n> Apart from one suggestion(?) in the first commit, I think the series\n> looks great! Thanks\n\nThanks for your review!\n\nPatrick\n"},{"id":"508750","messageId":"Z1Ln2492OCTwbII1@pks.im","threadId":"62609","inReplyTo":"CAOLa=ZRUZC9KDj9F6iLWWU+CBHz_DWKA_8znM=7RVSCVGSeD9w@mail.gmail.com","subject":"Re: [PATCH 1/4] gitlab-ci: update macOS images to Sonoma","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-12-06T12:02:35Z","receivedAt":"2024-12-06T12:02:51Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Fri, Dec 06, 2024 at 06:39:59AM -0500, karthik nayak wrote:\n> Patrick Steinhardt <ps@pks.im> writes:\n> \n> > The macOS Ventura images we use for GitLab CI runners have been\n> > deprecated. Update them to macOS 14, aka Sonoma.\n> >\n> > Signed-off-by: Patrick Steinhardt <ps@pks.im>\n> > ---\n> >  .gitlab-ci.yml | 4 ++--\n> >  1 file changed, 2 insertions(+), 2 deletions(-)\n> >\n> > diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml\n> > index 526ecfe030a43e0a5a83ddd35cb7c96d46ab2485..61c56ccac8fdc940075d91dd4cb0b54ee33d5199 100644\n> > --- a/.gitlab-ci.yml\n> > +++ b/.gitlab-ci.yml\n> > @@ -99,10 +99,10 @@ test:osx:\n> >    parallel:\n> >      matrix:\n> >        - jobname: osx-clang\n> > -        image: macos-13-xcode-14\n> > +        image: macos-14-xcode-15\n> >          CC: clang\n> >        - jobname: osx-reftable\n> > -        image: macos-13-xcode-14\n> > +        image: macos-14-xcode-15\n> >          CC: clang\n> >    artifacts:\n> >      paths:\n> \n> The changes look good, the documentation also states that if no image is\n> mentioned, it'll use `macos-14-xcode-15` [1]. I wonder if this means\n> that if the image is unspecified, it will always use the first\n> non-deprecated version. That'd allow us to not have to keep updating\n> this.\n> \n> [1]: https://docs.gitlab.com/ee/ci/runners/hosted_runners/macos.html#supported-macos-images\n\nIt does, but at the cost of potential breakage whenever GitLab decides\nto update these images. It shouldn't happen all that frequently, but\nwhen we notice that it does become annoying we can iterate in the past\nand experiment with setting no image at all.\n\nPatrick\n"},{"id":"508816","messageId":"Z1QRp8jFJPYdNmQk@ugly","threadId":"62609","inReplyTo":"20241206-pks-ci-section-fixes-v1-3-7ab1b69e3648@pks.im","subject":"Re: [PATCH 3/4] ci/lib: use echo instead of printf to set up sections","fromName":"Oswald Buddenhagen","fromEmail":"oswald.buddenhagen@gmx.de","sentAt":"2024-12-07T09:13:11Z","receivedAt":"2024-12-07T09:13:16Z","isPatch":true,"sender":{"key":"oswald.buddenhagen@gmx.de","avatar":"https://avatars.githubusercontent.com/u/812380?v=4"},"body":"On Fri, Dec 06, 2024 at 12:10:15PM +0100, Patrick Steinhardt wrote:\n>We use printf to set up sections with GitLab CI even though we could\n>trivially use echo.\n>\nprobably not a good idea, because the file is also included from plain\nsh scripts, which may invoke an `echo` that cannot deal with \\escapes.\nat least potentially.\n\n>This may cause problems in case the argument passed to `begin_group ()`\n>or `end_group ()` contains formatting directives as we use them as part\n>of the formatting string.\n>\nthat can be fixed properly by using the %s expando.\n\n>+++ b/ci/lib.sh\n>-\t\tprintf \"\\e[0Ksection_start:$(date +%s):$(echo \"$1\" | tr ' ' _)[collapsed=true]\\r\\e[0K$1\\n\"\n>+\t\techo \"\\e[0Ksection_start:$(date +%s):$(echo \"$1\" | tr ' ' _)[collapsed=true]\\r\\e[0K$1\"\n"},{"id":"508817","messageId":"CAOLa=ZSExxSOna9Py+5Gx_dpLgom-fX48TGvcsi71f4xBXyoTA@mail.gmail.com","threadId":"62609","inReplyTo":"Z1Ln2492OCTwbII1@pks.im","subject":"Re: [PATCH 1/4] gitlab-ci: update macOS images to Sonoma","fromName":"karthik nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2024-12-07T09:49:58Z","receivedAt":"2024-12-07T09:50:00Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> On Fri, Dec 06, 2024 at 06:39:59AM -0500, karthik nayak wrote:\n>> Patrick Steinhardt <ps@pks.im> writes:\n>>\n>> > The macOS Ventura images we use for GitLab CI runners have been\n>> > deprecated. Update them to macOS 14, aka Sonoma.\n>> >\n>> > Signed-off-by: Patrick Steinhardt <ps@pks.im>\n>> > ---\n>> >  .gitlab-ci.yml | 4 ++--\n>> >  1 file changed, 2 insertions(+), 2 deletions(-)\n>> >\n>> > diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml\n>> > index 526ecfe030a43e0a5a83ddd35cb7c96d46ab2485..61c56ccac8fdc940075d91dd4cb0b54ee33d5199 100644\n>> > --- a/.gitlab-ci.yml\n>> > +++ b/.gitlab-ci.yml\n>> > @@ -99,10 +99,10 @@ test:osx:\n>> >    parallel:\n>> >      matrix:\n>> >        - jobname: osx-clang\n>> > -        image: macos-13-xcode-14\n>> > +        image: macos-14-xcode-15\n>> >          CC: clang\n>> >        - jobname: osx-reftable\n>> > -        image: macos-13-xcode-14\n>> > +        image: macos-14-xcode-15\n>> >          CC: clang\n>> >    artifacts:\n>> >      paths:\n>>\n>> The changes look good, the documentation also states that if no image is\n>> mentioned, it'll use `macos-14-xcode-15` [1]. I wonder if this means\n>> that if the image is unspecified, it will always use the first\n>> non-deprecated version. That'd allow us to not have to keep updating\n>> this.\n>>\n>> [1]: https://docs.gitlab.com/ee/ci/runners/hosted_runners/macos.html#supported-macos-images\n>\n> It does, but at the cost of potential breakage whenever GitLab decides\n> to update these images. It shouldn't happen all that frequently, but\n> when we notice that it does become annoying we can iterate in the past\n> and experiment with setting no image at all.\n>\n\nYeah, I get that. All good then!\n\nKarthik\n"},{"id":"508916","messageId":"Z1gscPivAADEvMGI@pks.im","threadId":"62609","inReplyTo":"Z1QRp8jFJPYdNmQk@ugly","subject":"Re: [PATCH 3/4] ci/lib: use echo instead of printf to set up sections","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-12-10T11:56:38Z","receivedAt":"2024-12-10T11:56:56Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Sat, Dec 07, 2024 at 10:13:11AM +0100, Oswald Buddenhagen wrote:\n> On Fri, Dec 06, 2024 at 12:10:15PM +0100, Patrick Steinhardt wrote:\n> > We use printf to set up sections with GitLab CI even though we could\n> > trivially use echo.\n> > \n> probably not a good idea, because the file is also included from plain\n> sh scripts, which may invoke an `echo` that cannot deal with \\escapes.\n> at least potentially.\n\nAh, true.\n\n> > This may cause problems in case the argument passed to `begin_group ()`\n> > or `end_group ()` contains formatting directives as we use them as part\n> > of the formatting string.\n> > \n> that can be fixed properly by using the %s expando.\n\nYup, will do. Thanks!\n\nPatrick\n"},{"id":"508917","messageId":"20241210-pks-ci-section-fixes-v2-0-e087cfd174f4@pks.im","threadId":"62609","inReplyTo":"20241206-pks-ci-section-fixes-v1-0-7ab1b69e3648@pks.im","subject":"[PATCH v2 0/4] Random improvements to GitLab CI","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-12-10T12:01:45Z","receivedAt":"2024-12-10T12:02:06Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Hi,\n\nthis small patch series includes a couple of more-or-less random\nimprovements to GitLab CI.\n\nChanges in v2:\n- Use \"%s\" to inject variable data into section headers instead of\n  converting to echo.\n- Link to v1: https://lore.kernel.org/r/20241206-pks-ci-section-fixes-v1-0-7ab1b69e3648@pks.im\n\nThanks!\n\nPatrick\n\n---\nPatrick Steinhardt (4):\n      gitlab-ci: update macOS images to Sonoma\n      ci/lib: remove duplicate trap to end \"CI setup\" group\n      ci/lib: do not interpret escape sequences in `group ()` arguments\n      ci/lib: fix \"CI setup\" sections with GitLab CI\n\n .gitlab-ci.yml | 4 ++--\n ci/lib.sh      | 9 ++++-----\n 2 files changed, 6 insertions(+), 7 deletions(-)\n\nRange-diff versus v1:\n\n1:  c52dfd96a6 = 1:  c98da734b2 gitlab-ci: update macOS images to Sonoma\n2:  6214c8b6d5 = 2:  d36bafc387 ci/lib: remove duplicate trap to end \"CI setup\" group\n3:  b56c6ec0a2 ! 3:  16882b0033 ci/lib: use echo instead of printf to set up sections\n    @@ Metadata\n     Author: Patrick Steinhardt <ps@pks.im>\n     \n      ## Commit message ##\n    -    ci/lib: use echo instead of printf to set up sections\n    +    ci/lib: do not interpret escape sequences in `group ()` arguments\n     \n    -    We use printf to set up sections with GitLab CI even though we could\n    -    trivially use echo. This may cause problems in case the argument passed\n    -    to `begin_group ()` or `end_group ()` contains formatting directives as\n    -    we use them as part of the formatting string.\n    +    We use printf to set up sections with GitLab CI, which requires us to\n    +    print a bunch of escape sequences via printf. The group name is\n    +    controlled by the user and is expanded directly into the formatting\n    +    string, which may cause problems in case the argument controls escape\n    +    sequences or formatting directives.\n     \n    -    Simplify the code to instead use echo.\n    +    Fix this potential issue by using formatting directives to pass variable\n    +    data.\n     \n         Signed-off-by: Patrick Steinhardt <ps@pks.im>\n     \n    @@ ci/lib.sh: elif test true = \"$GITLAB_CI\"\n      \tbegin_group () {\n      \t\tneed_to_end_group=t\n     -\t\tprintf \"\\e[0Ksection_start:$(date +%s):$(echo \"$1\" | tr ' ' _)[collapsed=true]\\r\\e[0K$1\\n\"\n    -+\t\techo \"\\e[0Ksection_start:$(date +%s):$(echo \"$1\" | tr ' ' _)[collapsed=true]\\r\\e[0K$1\"\n    ++\t\tprintf '\\e[0Ksection_start:%s:%s[collapsed=true]\\r\\e[0K%s\\n' \"$(date +%s)\" \"$(echo \"$1\" | tr ' ' _)\" \"$1\"\n      \t\ttrap \"end_group '$1'\" EXIT\n      \t\tset -x\n      \t}\n    @@ ci/lib.sh: then\n      \t\tset +x\n      \t\tneed_to_end_group=\n     -\t\tprintf \"\\e[0Ksection_end:$(date +%s):$(echo \"$1\" | tr ' ' _)\\r\\e[0K\\n\"\n    -+\t\techo \"\\e[0Ksection_end:$(date +%s):$(echo \"$1\" | tr ' ' _)\\r\\e[0K\"\n    ++\t\tprintf '\\e[0Ksection_end:%s:%s\\r\\e[0K\\n' \"$(date +%s)\" \"$(echo \"$1\" | tr ' ' _)\"\n      \t\ttrap - EXIT\n      \t}\n      else\n4:  a2bf5ac44f = 4:  6b67c4c238 ci/lib: fix \"CI setup\" sections with GitLab CI\n\n---\nbase-commit: e66fd72e972df760a53c3d6da023c17adfc426d6\nchange-id: 20241206-pks-ci-section-fixes-1bb91ceb50b8\n\n"},{"id":"508918","messageId":"20241210-pks-ci-section-fixes-v2-1-e087cfd174f4@pks.im","threadId":"62609","inReplyTo":"20241210-pks-ci-section-fixes-v2-0-e087cfd174f4@pks.im","subject":"[PATCH v2 1/4] gitlab-ci: update macOS images to Sonoma","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-12-10T12:01:46Z","receivedAt":"2024-12-10T12:02:07Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"The macOS Ventura images we use for GitLab CI runners have been\ndeprecated. Update them to macOS 14, aka Sonoma.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n .gitlab-ci.yml | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/.gitlab-ci.yml b/.gitlab-ci.yml\nindex 526ecfe030a43e0a5a83ddd35cb7c96d46ab2485..61c56ccac8fdc940075d91dd4cb0b54ee33d5199 100644\n--- a/.gitlab-ci.yml\n+++ b/.gitlab-ci.yml\n@@ -99,10 +99,10 @@ test:osx:\n   parallel:\n     matrix:\n       - jobname: osx-clang\n-        image: macos-13-xcode-14\n+        image: macos-14-xcode-15\n         CC: clang\n       - jobname: osx-reftable\n-        image: macos-13-xcode-14\n+        image: macos-14-xcode-15\n         CC: clang\n   artifacts:\n     paths:\n\n-- \n2.47.1.447.ga7e8429e30.dirty\n\n"},{"id":"508919","messageId":"20241210-pks-ci-section-fixes-v2-2-e087cfd174f4@pks.im","threadId":"62609","inReplyTo":"20241210-pks-ci-section-fixes-v2-0-e087cfd174f4@pks.im","subject":"[PATCH v2 2/4] ci/lib: remove duplicate trap to end \"CI setup\" group","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-12-10T12:01:47Z","receivedAt":"2024-12-10T12:02:08Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"We exlicitly trap on EXIT in order to end the \"CI setup\" group. This\nisn't necessary though given that `begin_group ()` already sets up the\ntrap for us.\n\nRemove the duplicate trap.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n ci/lib.sh | 1 -\n 1 file changed, 1 deletion(-)\n\ndiff --git a/ci/lib.sh b/ci/lib.sh\nindex 930f98d7228166c37c236beb062b14675fb68ef3..a54601be923bf475ba1a9cafd98bb1cb71a10255 100755\n--- a/ci/lib.sh\n+++ b/ci/lib.sh\n@@ -56,7 +56,6 @@ group () {\n }\n \n begin_group \"CI setup\"\n-trap \"end_group 'CI setup'\" EXIT\n \n # Set 'exit on error' for all CI scripts to let the caller know that\n # something went wrong.\n\n-- \n2.47.1.447.ga7e8429e30.dirty\n\n"},{"id":"508920","messageId":"20241210-pks-ci-section-fixes-v2-3-e087cfd174f4@pks.im","threadId":"62609","inReplyTo":"20241210-pks-ci-section-fixes-v2-0-e087cfd174f4@pks.im","subject":"[PATCH v2 3/4] ci/lib: do not interpret escape sequences in `group ()` arguments","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-12-10T12:01:48Z","receivedAt":"2024-12-10T12:02:09Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"We use printf to set up sections with GitLab CI, which requires us to\nprint a bunch of escape sequences via printf. The group name is\ncontrolled by the user and is expanded directly into the formatting\nstring, which may cause problems in case the argument controls escape\nsequences or formatting directives.\n\nFix this potential issue by using formatting directives to pass variable\ndata.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n ci/lib.sh | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/ci/lib.sh b/ci/lib.sh\nindex a54601be923bf475ba1a9cafd98bb1cb71a10255..f15f77f03a06120afbee438cee76ddc2683e1fa2 100755\n--- a/ci/lib.sh\n+++ b/ci/lib.sh\n@@ -18,7 +18,7 @@ elif test true = \"$GITLAB_CI\"\n then\n \tbegin_group () {\n \t\tneed_to_end_group=t\n-\t\tprintf \"\\e[0Ksection_start:$(date +%s):$(echo \"$1\" | tr ' ' _)[collapsed=true]\\r\\e[0K$1\\n\"\n+\t\tprintf '\\e[0Ksection_start:%s:%s[collapsed=true]\\r\\e[0K%s\\n' \"$(date +%s)\" \"$(echo \"$1\" | tr ' ' _)\" \"$1\"\n \t\ttrap \"end_group '$1'\" EXIT\n \t\tset -x\n \t}\n@@ -27,7 +27,7 @@ then\n \t\ttest -n \"$need_to_end_group\" || return 0\n \t\tset +x\n \t\tneed_to_end_group=\n-\t\tprintf \"\\e[0Ksection_end:$(date +%s):$(echo \"$1\" | tr ' ' _)\\r\\e[0K\\n\"\n+\t\tprintf '\\e[0Ksection_end:%s:%s\\r\\e[0K\\n' \"$(date +%s)\" \"$(echo \"$1\" | tr ' ' _)\"\n \t\ttrap - EXIT\n \t}\n else\n\n-- \n2.47.1.447.ga7e8429e30.dirty\n\n"},{"id":"508921","messageId":"20241210-pks-ci-section-fixes-v2-4-e087cfd174f4@pks.im","threadId":"62609","inReplyTo":"20241210-pks-ci-section-fixes-v2-0-e087cfd174f4@pks.im","subject":"[PATCH v2 4/4] ci/lib: fix \"CI setup\" sections with GitLab CI","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-12-10T12:01:49Z","receivedAt":"2024-12-10T12:02:09Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"Whenever we source \"ci/lib.sh\" we wrap the directives in a separate\ngroup so that they can easily be collapsed in the web UI. And as we\nsource the script multiple times during a single CI run we thus end up\nwith the same section name reused multiple times, as well.\n\nThis is broken on GitLab CI though, where reusing the same group name is\nnot supported. The consequence is that only the last of these sections\ncan be collapsed.\n\nFix this issue by including the name of the sourcing script in the\ngroup's name.\n\nSigned-off-by: Patrick Steinhardt <ps@pks.im>\n---\n ci/lib.sh | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/ci/lib.sh b/ci/lib.sh\nindex f15f77f03a06120afbee438cee76ddc2683e1fa2..8ae8c0f77461d5bedc24e47be75711f2da2ade4f 100755\n--- a/ci/lib.sh\n+++ b/ci/lib.sh\n@@ -55,7 +55,7 @@ group () {\n \treturn $res\n }\n \n-begin_group \"CI setup\"\n+begin_group \"CI setup via $(basename $0)\"\n \n # Set 'exit on error' for all CI scripts to let the caller know that\n # something went wrong.\n@@ -393,5 +393,5 @@ esac\n \n MAKEFLAGS=\"$MAKEFLAGS CC=${CC:-cc}\"\n \n-end_group \"CI setup\"\n+end_group \"CI setup via $(basename $0)\"\n set -x\n\n-- \n2.47.1.447.ga7e8429e30.dirty\n\n"},{"id":"508982","messageId":"874j3ajsym.fsf@iotcl.com","threadId":"62609","inReplyTo":"20241210-pks-ci-section-fixes-v2-3-e087cfd174f4@pks.im","subject":"Re: [PATCH v2 3/4] ci/lib: do not interpret escape sequences in `group ()` arguments","fromName":"Toon Claes","fromEmail":"toon@iotcl.com","sentAt":"2024-12-11T12:37:53Z","receivedAt":"2024-12-11T12:38:13Z","isPatch":true,"sender":{"key":"toon@iotcl.com","avatar":"https://avatars.githubusercontent.com/u/121621?v=4"},"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> We use printf to set up sections with GitLab CI, which requires us to\n> print a bunch of escape sequences via printf. The group name is\n> controlled by the user and is expanded directly into the formatting\n> string, which may cause problems in case the argument controls escape\n> sequences or formatting directives.\n\nCould it be you mean \"contains\" instead of \"controls\"?\n\n>\n> Fix this potential issue by using formatting directives to pass variable\n> data.\n>\n> Signed-off-by: Patrick Steinhardt <ps@pks.im>\n> ---\n>  ci/lib.sh | 4 ++--\n>  1 file changed, 2 insertions(+), 2 deletions(-)\n>\n> diff --git a/ci/lib.sh b/ci/lib.sh\n> index a54601be923bf475ba1a9cafd98bb1cb71a10255..f15f77f03a06120afbee438cee76ddc2683e1fa2 100755\n> --- a/ci/lib.sh\n> +++ b/ci/lib.sh\n> @@ -18,7 +18,7 @@ elif test true = \"$GITLAB_CI\"\n>  then\n>  \tbegin_group () {\n>  \t\tneed_to_end_group=t\n> -\t\tprintf \"\\e[0Ksection_start:$(date +%s):$(echo \"$1\" | tr ' ' _)[collapsed=true]\\r\\e[0K$1\\n\"\n> +\t\tprintf '\\e[0Ksection_start:%s:%s[collapsed=true]\\r\\e[0K%s\\n' \"$(date +%s)\" \"$(echo \"$1\" | tr ' ' _)\" \"$1\"\n\nPersonally I find this line rather lengthy and hard to read with all the\nsingle and double quotes. So I would suggest to split the line with a\nbackslash and put the arguments on a separate line. But I don't think\nthere's a general guideline on this, so feel free to ignore.\n\n>  \t\ttrap \"end_group '$1'\" EXIT\n>  \t\tset -x\n>  \t}\n> @@ -27,7 +27,7 @@ then\n>  \t\ttest -n \"$need_to_end_group\" || return 0\n>  \t\tset +x\n>  \t\tneed_to_end_group=\n> -\t\tprintf \"\\e[0Ksection_end:$(date +%s):$(echo \"$1\" | tr ' ' _)\\r\\e[0K\\n\"\n> +\t\tprintf '\\e[0Ksection_end:%s:%s\\r\\e[0K\\n' \"$(date +%s)\" \"$(echo \"$1\" | tr ' ' _)\"\n\nSame here.\n\nBut that's all I've got on this patch series. Looking good!\n\n--\nToon\n"},{"id":"509016","messageId":"Z1qGmp4NC21XYC4a@pks.im","threadId":"62609","inReplyTo":"874j3ajsym.fsf@iotcl.com","subject":"Re: [PATCH v2 3/4] ci/lib: do not interpret escape sequences in `group ()` arguments","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2024-12-12T06:47:41Z","receivedAt":"2024-12-12T06:47:58Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Wed, Dec 11, 2024 at 01:37:53PM +0100, Toon Claes wrote:\n> Patrick Steinhardt <ps@pks.im> writes:\n> \n> > We use printf to set up sections with GitLab CI, which requires us to\n> > print a bunch of escape sequences via printf. The group name is\n> > controlled by the user and is expanded directly into the formatting\n> > string, which may cause problems in case the argument controls escape\n> > sequences or formatting directives.\n> \n> Could it be you mean \"contains\" instead of \"controls\"?\n\nOh, yeah.\n\n> > Fix this potential issue by using formatting directives to pass variable\n> > data.\n> >\n> > Signed-off-by: Patrick Steinhardt <ps@pks.im>\n> > ---\n> >  ci/lib.sh | 4 ++--\n> >  1 file changed, 2 insertions(+), 2 deletions(-)\n> >\n> > diff --git a/ci/lib.sh b/ci/lib.sh\n> > index a54601be923bf475ba1a9cafd98bb1cb71a10255..f15f77f03a06120afbee438cee76ddc2683e1fa2 100755\n> > --- a/ci/lib.sh\n> > +++ b/ci/lib.sh\n> > @@ -18,7 +18,7 @@ elif test true = \"$GITLAB_CI\"\n> >  then\n> >  \tbegin_group () {\n> >  \t\tneed_to_end_group=t\n> > -\t\tprintf \"\\e[0Ksection_start:$(date +%s):$(echo \"$1\" | tr ' ' _)[collapsed=true]\\r\\e[0K$1\\n\"\n> > +\t\tprintf '\\e[0Ksection_start:%s:%s[collapsed=true]\\r\\e[0K%s\\n' \"$(date +%s)\" \"$(echo \"$1\" | tr ' ' _)\" \"$1\"\n> \n> Personally I find this line rather lengthy and hard to read with all the\n> single and double quotes. So I would suggest to split the line with a\n> backslash and put the arguments on a separate line. But I don't think\n> there's a general guideline on this, so feel free to ignore.\n\nFair, will do.\n\n> >  \t\ttrap \"end_group '$1'\" EXIT\n> >  \t\tset -x\n> >  \t}\n> > @@ -27,7 +27,7 @@ then\n> >  \t\ttest -n \"$need_to_end_group\" || return 0\n> >  \t\tset +x\n> >  \t\tneed_to_end_group=\n> > -\t\tprintf \"\\e[0Ksection_end:$(date +%s):$(echo \"$1\" | tr ' ' _)\\r\\e[0K\\n\"\n> > +\t\tprintf '\\e[0Ksection_end:%s:%s\\r\\e[0K\\n' \"$(date +%s)\" \"$(echo \"$1\" | tr ' ' _)\"\n> \n> Same here.\n> \n> But that's all I've got on this patch series. Looking good!\n\nThanks!\n\nPatrick\n"}]}