Libguestfs - Mar 2023 - [nbdkit PATCH 0/5] ci: Get to green status on FreeBSD and MacOS

On 3/1/23 17:54, Eric Blake wrote:
> I took the easy route of crippling what I couldn't get working, on the
> grounds that partial coverage is better than none now that we have
> Cirrus CI checking commits on additional platforms.
> 
> This series got me to a green checkmark:
> https://gitlab.com/ebblake/nbdkit/-/pipelines/793156983
> 
> but depends on an as-yet uncommitted patch in libvirt-ci:
> https://gitlab.com/libvirt/libvirt-ci/-/merge_requests/360
> 
> Eric Blake (5):
>   ci: Expose more env vars needed by build.sh
>   ci: Another attempt at MacOS
>   rust: Skip CI builds on MacOS
>   golang: Skip CI builds on MacOS and newer FreeBSD
>   perl: Skip CI builds on newer FreeBSD
> 
>  .gitlab-ci.yml                |  3 +++
>  ci/cirrus/build.yml           |  3 +++
>  ci/cirrus/macos-12.vars       |  4 ++--
>  ci/gitlab.yml                 |  7 +++++++
>  ci/gitlab/build-templates.yml | 31 ++++++++++++++++++++++++++++++-
>  ci/gitlab/builds.yml          | 11 ++++++++---
>  ci/manifest.yml               | 20 +++++++++++++++-----
>  7 files changed, 68 insertions(+), 11 deletions(-)
> 
series
Acked-by: Laszlo Ersek <lersek at redhat.com>

One question (for my understanding) about the context of patch#1:

           -e "s|[@]PYPI_PKGS@|$PYPI_PKGS|g"

what's this [@] notation? Why do we need to sink the at-sign into a
bracket expression?

Thanks
Laszlo

On 3/1/23 18:43, Laszlo Ersek wrote:
> On 3/1/23 17:54, Eric Blake wrote:
>> I took the easy route of crippling what I couldn't get working, on
the
>> grounds that partial coverage is better than none now that we have
>> Cirrus CI checking commits on additional platforms.
>>
>> This series got me to a green checkmark:
>> https://gitlab.com/ebblake/nbdkit/-/pipelines/793156983
>>
>> but depends on an as-yet uncommitted patch in libvirt-ci:
>> https://gitlab.com/libvirt/libvirt-ci/-/merge_requests/360
>>
>> Eric Blake (5):
>>   ci: Expose more env vars needed by build.sh
>>   ci: Another attempt at MacOS
>>   rust: Skip CI builds on MacOS
>>   golang: Skip CI builds on MacOS and newer FreeBSD
>>   perl: Skip CI builds on newer FreeBSD
>>
>>  .gitlab-ci.yml                |  3 +++
>>  ci/cirrus/build.yml           |  3 +++
>>  ci/cirrus/macos-12.vars       |  4 ++--
>>  ci/gitlab.yml                 |  7 +++++++
>>  ci/gitlab/build-templates.yml | 31 ++++++++++++++++++++++++++++++-
>>  ci/gitlab/builds.yml          | 11 ++++++++---
>>  ci/manifest.yml               | 20 +++++++++++++++-----
>>  7 files changed, 68 insertions(+), 11 deletions(-)
>>
> 
> series
> Acked-by: Laszlo Ersek <lersek at redhat.com>
> 
> One question (for my understanding) about the context of patch#1:
> 
>            -e "s|[@]PYPI_PKGS@|$PYPI_PKGS|g"
> 
> what's this [@] notation? Why do we need to sink the at-sign into a
> bracket expression?
Ugh, is it possible that the replacement covers the file *itself* that
contains these sed scripts? IOW, that we don't use plain
"@FOOBAR@" nor
"\\@FOOBAR@" because sed would then modify the command file itself?

(This seems extreme, admittedly, but I've got no other idea. Another
hack for the same purpose could have been @{1}FOOBAR@, as an extended
regex.)

Laszlo

On Wed, Mar 01, 2023 at 06:43:11PM +0100, Laszlo Ersek
wrote:
> On 3/1/23 17:54, Eric Blake wrote:
> > I took the easy route of crippling what I couldn't get working, on
the
> > grounds that partial coverage is better than none now that we have
> > Cirrus CI checking commits on additional platforms.
> > 
> > This series got me to a green checkmark:
> > https://gitlab.com/ebblake/nbdkit/-/pipelines/793156983
> > 
> > but depends on an as-yet uncommitted patch in libvirt-ci:
> > https://gitlab.com/libvirt/libvirt-ci/-/merge_requests/360
> > 
> > Eric Blake (5):
> >   ci: Expose more env vars needed by build.sh
> >   ci: Another attempt at MacOS
> >   rust: Skip CI builds on MacOS
> >   golang: Skip CI builds on MacOS and newer FreeBSD
> >   perl: Skip CI builds on newer FreeBSD
> > 
> >  .gitlab-ci.yml                |  3 +++
> >  ci/cirrus/build.yml           |  3 +++
> >  ci/cirrus/macos-12.vars       |  4 ++--
> >  ci/gitlab.yml                 |  7 +++++++
> >  ci/gitlab/build-templates.yml | 31 ++++++++++++++++++++++++++++++-
> >  ci/gitlab/builds.yml          | 11 ++++++++---
> >  ci/manifest.yml               | 20 +++++++++++++++-----
> >  7 files changed, 68 insertions(+), 11 deletions(-)
> > 
> 
> series
> Acked-by: Laszlo Ersek <lersek at redhat.com>
Thanks; now in as 99c57eef..648a7909
> 
> One question (for my understanding) about the context of patch#1:
> 
>            -e "s|[@]PYPI_PKGS@|$PYPI_PKGS|g"
> 
> what's this [@] notation? Why do we need to sink the at-sign into a
> bracket expression?
In the context of this patch, copy-and-paste from existing paradigm.
More historically, I can think of two possible sources, both from GNU
heritage:

For a project using autoconf, it is explicitly documented that any
@STR@ expression in a .in file will be substituted with the contents
of the shell variable $STR at the time that ./configure has finished
learning about the current system.  But it can be expensive to write a
lot of .in templates, so autoconf actually encourages a style where
you only need to generate Makefile.in (which DOES use @FOO@
substitution from autoconf) where the resulting Makefile in turn
applies subsequent @FOO@ substitution in other target files not under
autoconf's control.  But when the make rule to do that substitution
lives in a file that itself undergoes @ expansion, you need a way to
quote which instances get expanded at make time rather than configure
time.  For example, look under 'make install' in:
https://www.gnu.org/savannah-checkouts/gnu/autoconf/manual/autoconf-2.71/autoconf.html#Installation-Directory-Variables

| Some details are noteworthy:
| 
| ?@bindir[@]?
| 
|     The brackets prevent configure from replacing ?@bindir@? in the Sed
expression itself. Brackets are preferable to a backslash here, since Posix says
?\@? is not portable.

The other place where I've seen common use of a single-character
bracket expression as an escape for an otherwise literal regex is in
gnulib's tree-wide syntax-checking.  And of course, you don't want to
hit a false-positive of the regex itself being an otherwise forbidden
string.  For example:
https://git.savannah.gnu.org/cgit/gnulib.git/tree/top/maint.mk#n371

And since libvirt used to use gnulib's maint.mk regex-based syntax
checking, before weaning itself off gnulib, this may be why libvirt-ci
(the source of lcitool) still utilizes @FOO@ regex with [@] escaping.

Or, it could be because gitlab and/or Cirrus do their own @FOO@
expansions at job kickoff, borrowing from the earlier practice of
autoconf.  I don't see any other @FOO@ listings in .gitlab-ci.yml or
ci/gitlab.yml, but I do see that the resulting ci/cirrus/$NAME.yml
produced by the sed substitution during gitlab time DOES contain other
things like @CIRRUS_VM_INSTANCE_TYPE@ that we are NOT touching on the
gitlab side, so it seems fairly certain that Cirrus does some @FOO@
expansion, and timing which part of the job does which expansions may
matter.  At this point, erring on the side of caution and coding by
copy/paste is simpler than determining if dropping the quoting is
feasible.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org