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

[Daniel P. Berrangé]

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 at 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 :|