Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
---
@Andrea: if you have a good explanation you'd like to put in the commit message,
I'd me glad to add it (or you can do that as well).  Thanks

 .gitlab-ci.yml      | 2 ++
 ci/cirrus/build.yml | 1 +
 2 files changed, 3 insertions(+)

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index b5930a0a46d5..6097047d9215 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -115,6 +115,7 @@ stages:
           -e "s|[@]CIRRUS_VM_IMAGE_SELECTOR@|$CIRRUS_VM_IMAGE_SELECTOR|g"
           -e "s|[@]CIRRUS_VM_IMAGE_NAME@|$CIRRUS_VM_IMAGE_NAME|g"
           -e "s|[@]UPDATE_COMMAND@|$UPDATE_COMMAND|g"
+          -e "s|[@]UPGRADE_COMMAND@|$UPGRADE_COMMAND|g"
           -e "s|[@]INSTALL_COMMAND@|$INSTALL_COMMAND|g"
           -e "s|[@]PATH@|$PATH_EXTRA${PATH_EXTRA:+:}\$PATH|g"
           -e "s|[@]PKG_CONFIG_PATH@|$PKG_CONFIG_PATH|g"
@@ -443,6 +444,7 @@ x64-macos-11-build:
     CIRRUS_VM_IMAGE_SELECTOR: image
     CIRRUS_VM_IMAGE_NAME: big-sur-base
     UPDATE_COMMAND: brew update
+    UPGRADE_COMMAND: brew upgrade
     INSTALL_COMMAND: brew install
     PATH_EXTRA: /usr/local/opt/ccache/libexec:/usr/local/opt/gettext/bin:/usr/local/opt/libpcap/bin:/usr/local/opt/libxslt/bin:/usr/local/opt/rpcgen/bin
     PKG_CONFIG_PATH: /usr/local/opt/curl/lib/pkgconfig:/usr/local/opt/libpcap/lib/pkgconfig:/usr/local/opt/libxml2/lib/pkgconfig:/usr/local/opt/ncurses/lib/pkgconfig:/usr/local/opt/readline/lib/pkgconfig
diff --git a/ci/cirrus/build.yml b/ci/cirrus/build.yml
index 39c17dc08a43..867d5f297b7e 100644
--- a/ci/cirrus/build.yml
+++ b/ci/cirrus/build.yml
@@ -15,6 +15,7 @@ env:
 build_task:
   install_script:
     - @UPDATE_COMMAND@
+    - @UPGRADE_COMMAND@
     - @INSTALL_COMMAND@ @PKGS@
     - if test -n "@PYPI_PKGS@" ; then @PIP3@ install @PYPI_PKGS@ ; fi
   clone_script:
-- 
2.31.1

On Tue, Jun 15, 2021 at 12:43:39PM +0200, Martin Kletzander wrote:
> ci: Also perform `brew upgrade` on MacOS

s/MacOS/macOS/

But see below for why we might have to change the subject even
further.

> @Andrea: if you have a good explanation you'd like to put in the commit message,
> I'd me glad to add it (or you can do that as well).  Thanks

I think something like

  The base OS image might include outdated contents, and we don't
  want to get spurious failures caused by bugs that have already been
  fixed in the respective packages.

  This is particularly important on macOS, because 'brew install foo'
  will fail if 'foo' is already installed but outdated: upgrading all
  packages first ensures we never run into this scenario.

would about sum it up.

> @@ -443,6 +444,7 @@ x64-macos-11-build:
>      CIRRUS_VM_IMAGE_SELECTOR: image
>      CIRRUS_VM_IMAGE_NAME: big-sur-base
>      UPDATE_COMMAND: brew update
> +    UPGRADE_COMMAND: brew upgrade

I believe you also need to add

  UPGRADE_COMMAND: pkg upgrade -y

to the FreeBSD jobs: I don't think Cirrus CI would appreciate having
a completely empty string in the list of commands it's supposed to
run.

With that squashed in,

  Reviewed-by: Andrea Bolognani <abologna@redhat.com>

and thanks for taking care of this :)

-- 
Andrea Bolognani / Red Hat / Virtualization

On Wed, Jun 16, 2021 at 06:21:00AM -0700, Andrea Bolognani wrote:
>On Tue, Jun 15, 2021 at 12:43:39PM +0200, Martin Kletzander wrote:
>> ci: Also perform `brew upgrade` on MacOS
>
>s/MacOS/macOS/
>
>But see below for why we might have to change the subject even
>further.
>
>> @Andrea: if you have a good explanation you'd like to put in the commit message,
>> I'd me glad to add it (or you can do that as well).  Thanks
>
>I think something like
>
>  The base OS image might include outdated contents, and we don't
>  want to get spurious failures caused by bugs that have already been
>  fixed in the respective packages.
>
>  This is particularly important on macOS, because 'brew install foo'
>  will fail if 'foo' is already installed but outdated: upgrading all
>  packages first ensures we never run into this scenario.
>
>would about sum it up.
>
>> @@ -443,6 +444,7 @@ x64-macos-11-build:
>>      CIRRUS_VM_IMAGE_SELECTOR: image
>>      CIRRUS_VM_IMAGE_NAME: big-sur-base
>>      UPDATE_COMMAND: brew update
>> +    UPGRADE_COMMAND: brew upgrade
>
>I believe you also need to add
>
>  UPGRADE_COMMAND: pkg upgrade -y
>
>to the FreeBSD jobs: I don't think Cirrus CI would appreciate having
>a completely empty string in the list of commands it's supposed to
>run.
>

It does not cause any issues on libnbd setup where the upgrade is run
only on macOS.

>With that squashed in,
>
>  Reviewed-by: Andrea Bolognani <abologna@redhat.com>
>

Are you suggesting that I add the `pkg upgrade -y` to FreeBSDs as well
here?  Because then the commit message would not fit the patch.

>and thanks for taking care of this :)
>

and sorry for forgetting about this =)

>--
>Andrea Bolognani / Red Hat / Virtualization
>

On Tue, Jun 22, 2021 at 12:50:33PM +0200, Martin Kletzander wrote:
> On Wed, Jun 16, 2021 at 06:21:00AM -0700, Andrea Bolognani wrote:
> > On Tue, Jun 15, 2021 at 12:43:39PM +0200, Martin Kletzander wrote:
> > > ci: Also perform `brew upgrade` on MacOS
> >
> > s/MacOS/macOS/
> >
> > But see below for why we might have to change the subject even
> > further.
> >
> > > @Andrea: if you have a good explanation you'd like to put in the commit message,
> > > I'd me glad to add it (or you can do that as well).  Thanks
> >
> > I think something like
> >
> >  The base OS image might include outdated contents, and we don't
> >  want to get spurious failures caused by bugs that have already been
> >  fixed in the respective packages.
> >
> >  This is particularly important on macOS, because 'brew install foo'
> >  will fail if 'foo' is already installed but outdated: upgrading all
> >  packages first ensures we never run into this scenario.
> >
> > would about sum it up.
> >
> > > @@ -443,6 +444,7 @@ x64-macos-11-build:
> > >      CIRRUS_VM_IMAGE_SELECTOR: image
> > >      CIRRUS_VM_IMAGE_NAME: big-sur-base
> > >      UPDATE_COMMAND: brew update
> > > +    UPGRADE_COMMAND: brew upgrade
> >
> > I believe you also need to add
> >
> >  UPGRADE_COMMAND: pkg upgrade -y
> >
> > to the FreeBSD jobs: I don't think Cirrus CI would appreciate having
> > a completely empty string in the list of commands it's supposed to
> > run.
>
> It does not cause any issues on libnbd setup where the upgrade is run
> only on macOS.

Good that Cirrus CI is handling the situation gracefully! Still it
doesn't feel quite right to perform a full system upgrade on macOS,
as well as part of the Linux container build process, but not on
FreeBSD, does it?

> Are you suggesting that I add the `pkg upgrade -y` to FreeBSDs as well
> here?

Yes.

> Because then the commit message would not fit the patch.

How so? AFAICT you just need to tweak the subject - the commit
message I suggested still applies just fine after you add the 'pkg
upgrade' call.

-- 
Andrea Bolognani / Red Hat / Virtualization

On Wed, Jun 23, 2021 at 02:46:18AM -0700, Andrea Bolognani wrote:
>On Tue, Jun 22, 2021 at 12:50:33PM +0200, Martin Kletzander wrote:
>> On Wed, Jun 16, 2021 at 06:21:00AM -0700, Andrea Bolognani wrote:
>> > On Tue, Jun 15, 2021 at 12:43:39PM +0200, Martin Kletzander wrote:
>> > > ci: Also perform `brew upgrade` on MacOS
>> >
>> > s/MacOS/macOS/
>> >
>> > But see below for why we might have to change the subject even
>> > further.
>> >
>> > > @Andrea: if you have a good explanation you'd like to put in the commit message,
>> > > I'd me glad to add it (or you can do that as well).  Thanks
>> >
>> > I think something like
>> >
>> >  The base OS image might include outdated contents, and we don't
>> >  want to get spurious failures caused by bugs that have already been
>> >  fixed in the respective packages.
>> >
>> >  This is particularly important on macOS, because 'brew install foo'
>> >  will fail if 'foo' is already installed but outdated: upgrading all
>> >  packages first ensures we never run into this scenario.
>> >
>> > would about sum it up.
>> >
>> > > @@ -443,6 +444,7 @@ x64-macos-11-build:
>> > >      CIRRUS_VM_IMAGE_SELECTOR: image
>> > >      CIRRUS_VM_IMAGE_NAME: big-sur-base
>> > >      UPDATE_COMMAND: brew update
>> > > +    UPGRADE_COMMAND: brew upgrade
>> >
>> > I believe you also need to add
>> >
>> >  UPGRADE_COMMAND: pkg upgrade -y
>> >
>> > to the FreeBSD jobs: I don't think Cirrus CI would appreciate having
>> > a completely empty string in the list of commands it's supposed to
>> > run.
>>
>> It does not cause any issues on libnbd setup where the upgrade is run
>> only on macOS.
>
>Good that Cirrus CI is handling the situation gracefully! Still it
>doesn't feel quite right to perform a full system upgrade on macOS,
>as well as part of the Linux container build process, but not on
>FreeBSD, does it?
>

The intricacies of what "brew upgrade" means are beyond me, so I did not compare
it to anything else we do.

>> Are you suggesting that I add the `pkg upgrade -y` to FreeBSDs as well
>> here?
>
>Yes.
>
>> Because then the commit message would not fit the patch.
>
>How so? AFAICT you just need to tweak the subject - the commit
>message I suggested still applies just fine after you add the 'pkg
>upgrade' call.
>

OK, but I'll send it as a v2 just for the sake of sane-checking.

Thanks.

On Wed, Jun 23, 2021 at 12:38:00PM +0200, Martin Kletzander wrote:
> On Wed, Jun 23, 2021 at 02:46:18AM -0700, Andrea Bolognani wrote:
> > On Tue, Jun 22, 2021 at 12:50:33PM +0200, Martin Kletzander wrote:
> > > Are you suggesting that I add the `pkg upgrade -y` to FreeBSDs as well
> > > here?
> >
> > Yes.
> >
> > > Because then the commit message would not fit the patch.
> >
> > How so? AFAICT you just need to tweak the subject - the commit
> > message I suggested still applies just fine after you add the 'pkg
> > upgrade' call.
>
> OK, but I'll send it as a v2 just for the sake of sane-checking.

Sounds good! Feel free to CC me when you do for a swift reply :)

-- 
Andrea Bolognani / Red Hat / Virtualization