[libvirt PATCH 11/33] ci: build.sh: Add a wrapper function over the 'rpmbuild' job

Erik Skultety posted 33 patches 2 years, 5 months ago
There is a newer version of this series
[libvirt PATCH 11/33] ci: build.sh: Add a wrapper function over the 'rpmbuild' job
Posted by Erik Skultety 2 years, 5 months ago
This helper is a shell function transcript of its original GitLab CI
counterpart.

Signed-off-by: Erik Skultety <eskultet@redhat.com>
---
 ci/build.sh | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/ci/build.sh b/ci/build.sh
index 6990f2d171..30f4712e4b 100644
--- a/ci/build.sh
+++ b/ci/build.sh
@@ -94,3 +94,14 @@ run_potfile() {
     run_meson_setup
     run_build
 }
+
+run_rpmbuild() {
+    local CMD="rpmbuild \
+                 --clean \
+                 --nodeps \
+                 --define "_without_mingw 1" \
+                 -ta build/meson-dist/libvirt-*.tar.xz"
+
+    run_meson_setup
+    run_dist
+}
-- 
2.41.0
Re: [libvirt PATCH 11/33] ci: build.sh: Add a wrapper function over the 'rpmbuild' job
Posted by Erik Skultety 2 years, 5 months ago
On Fri, Aug 25, 2023 at 07:55:19PM +0200, Erik Skultety wrote:
> This helper is a shell function transcript of its original GitLab CI
> counterpart.
> 
> Signed-off-by: Erik Skultety <eskultet@redhat.com>
> ---
>  ci/build.sh | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/ci/build.sh b/ci/build.sh
> index 6990f2d171..30f4712e4b 100644
> --- a/ci/build.sh
> +++ b/ci/build.sh
> @@ -94,3 +94,14 @@ run_potfile() {
>      run_meson_setup
>      run_build
>  }
> +
> +run_rpmbuild() {
> +    local CMD="rpmbuild \
> +                 --clean \
> +                 --nodeps \
> +                 --define "_without_mingw 1" \
> +                 -ta build/meson-dist/libvirt-*.tar.xz"
> +
> +    run_meson_setup
> +    run_dist
> +}
> -- 
> 2.41.0
> 

Consider the following squashed in:

diff --git a/ci/build.sh b/ci/build.sh
index 5eef53f8d1..b990f5eeac 100644
--- a/ci/build.sh
+++ b/ci/build.sh
@@ -87,6 +87,7 @@ run_rpmbuild() {

     run_meson_setup
     run_dist
+    run_cmd "$CMD"
 }


AND


diff --git a/ci/build.sh b/ci/build.sh
index b990f5eeac..a45bc2b110 100644
--- a/ci/build.sh
+++ b/ci/build.sh
@@ -82,7 +82,7 @@ run_rpmbuild() {
     local CMD="rpmbuild \
                  --clean \
                  --nodeps \
-                 --define "_without_mingw 1" \
+                 --define '_without_mingw 1' \
                  -ta build/meson-dist/libvirt-*.tar.xz"

     run_meson_setup


Regards,
Erik
Re: [libvirt PATCH 11/33] ci: build.sh: Add a wrapper function over the 'rpmbuild' job
Posted by Daniel P. Berrangé 2 years, 5 months ago
On Fri, Aug 25, 2023 at 07:55:19PM +0200, Erik Skultety wrote:
> This helper is a shell function transcript of its original GitLab CI
> counterpart.
> 
> Signed-off-by: Erik Skultety <eskultet@redhat.com>
> ---
>  ci/build.sh | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/ci/build.sh b/ci/build.sh
> index 6990f2d171..30f4712e4b 100644
> --- a/ci/build.sh
> +++ b/ci/build.sh
> @@ -94,3 +94,14 @@ run_potfile() {
>      run_meson_setup
>      run_build
>  }
> +
> +run_rpmbuild() {
> +    local CMD="rpmbuild \
> +                 --clean \
> +                 --nodeps \
> +                 --define "_without_mingw 1" \
> +                 -ta build/meson-dist/libvirt-*.tar.xz"
> +
> +    run_meson_setup

Redundant here as implied by run_dist

> +    run_dist
> +}

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
Re: [libvirt PATCH 11/33] ci: build.sh: Add a wrapper function over the 'rpmbuild' job
Posted by Erik Skultety 2 years, 5 months ago
On Thu, Aug 31, 2023 at 05:55:54PM +0100, Daniel P. Berrangé wrote:
> On Fri, Aug 25, 2023 at 07:55:19PM +0200, Erik Skultety wrote:
> > This helper is a shell function transcript of its original GitLab CI
> > counterpart.
> > 
> > Signed-off-by: Erik Skultety <eskultet@redhat.com>
> > ---
> >  ci/build.sh | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> > 
> > diff --git a/ci/build.sh b/ci/build.sh
> > index 6990f2d171..30f4712e4b 100644
> > --- a/ci/build.sh
> > +++ b/ci/build.sh
> > @@ -94,3 +94,14 @@ run_potfile() {
> >      run_meson_setup
> >      run_build
> >  }
> > +
> > +run_rpmbuild() {
> > +    local CMD="rpmbuild \
> > +                 --clean \
> > +                 --nodeps \
> > +                 --define "_without_mingw 1" \
> > +                 -ta build/meson-dist/libvirt-*.tar.xz"
> > +
> > +    run_meson_setup
> 
> Redundant here as implied by run_dist

While I can drop all these redundant setup calls, the reason why I put them
there was simply readability. Let's face it Shell isn't a particularly nice
structured language to look at especially when it comes to functions, so rather
than having everyone go and look what run_dist/run_build, etc. do, I instead
opted for transparent naming and explicit function calls, IOW so that when you
look at run_rpmbuild you immediately know we run meson setup followed by a
project build, etc., but then if you call run_dist alone in a local interactive
environment then run_dist would also call meson_setup because it can't go
without it, so hence the redundancy in all the functions.
If you still feel like it's undesirable even in ^this case, I will drop the
calls.

Erik

Re: [libvirt PATCH 11/33] ci: build.sh: Add a wrapper function over the 'rpmbuild' job
Posted by Daniel P. Berrangé 2 years, 5 months ago
On Fri, Sep 01, 2023 at 09:29:13AM +0200, Erik Skultety wrote:
> On Thu, Aug 31, 2023 at 05:55:54PM +0100, Daniel P. Berrangé wrote:
> > On Fri, Aug 25, 2023 at 07:55:19PM +0200, Erik Skultety wrote:
> > > This helper is a shell function transcript of its original GitLab CI
> > > counterpart.
> > > 
> > > Signed-off-by: Erik Skultety <eskultet@redhat.com>
> > > ---
> > >  ci/build.sh | 11 +++++++++++
> > >  1 file changed, 11 insertions(+)
> > > 
> > > diff --git a/ci/build.sh b/ci/build.sh
> > > index 6990f2d171..30f4712e4b 100644
> > > --- a/ci/build.sh
> > > +++ b/ci/build.sh
> > > @@ -94,3 +94,14 @@ run_potfile() {
> > >      run_meson_setup
> > >      run_build
> > >  }
> > > +
> > > +run_rpmbuild() {
> > > +    local CMD="rpmbuild \
> > > +                 --clean \
> > > +                 --nodeps \
> > > +                 --define "_without_mingw 1" \
> > > +                 -ta build/meson-dist/libvirt-*.tar.xz"
> > > +
> > > +    run_meson_setup
> > 
> > Redundant here as implied by run_dist
> 
> While I can drop all these redundant setup calls, the reason why I put them
> there was simply readability. Let's face it Shell isn't a particularly nice
> structured language to look at especially when it comes to functions, so rather
> than having everyone go and look what run_dist/run_build, etc. do, I instead
> opted for transparent naming and explicit function calls, IOW so that when you
> look at run_rpmbuild you immediately know we run meson setup followed by a
> project build, etc., but then if you call run_dist alone in a local interactive
> environment then run_dist would also call meson_setup because it can't go
> without it, so hence the redundancy in all the functions.
> If you still feel like it's undesirable even in ^this case, I will drop the
> calls.

In practice I don't think users need to worry about what each
functions runs, because your implementation has ensured that
every single cmd is "self contained" and will run any dependancies
is has. Given that I think it is overkill to have the transitive
expansion of every dependancy, just the immediate dependency is
sufficient.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|