On Fri, Aug 25, 2023 at 07:55:08PM +0200, Erik Skultety wrote:
> Technically a v2 of:
> https://listman.redhat.com/archives/libvir-list/2023-February/237552.html
> 
> However, the approach here is slightly different and what that series said
> about migration to lcitool container executions as a replacement for
> ci/Makefile is actually done here. One of the core problems of the above
> pointed out in review was that more Shell logic was introduced including CLI
> parsing, conditional executions, etc. which we fought hard to get rid of in 
> the
> past. I reworked the Shell functions quite a bit and dropped whatever extra
> Shell logic the original series added.
> Obviously we can't get rid of Shell completely because of .gitlab-ci.yml and 
> so
> I merely extracted the recipes into functions which are then sourced as
> ci/build.sh and executed. Now, that on its own would hide the actual commands
> being run in the GitLab job log, so before any command is actually executed, 
> it
> is formatted with a color sequence so we don't miss that information as that
> would be a regression to the status quo.
> 
> Lastly, this series then takes the effort inside the ci/build.sh script and
> basically mirrors whatever GitLab would do to run a job inside a local
> container which is executed by lcitool (yes, we already have that capability).
> 
> Please give this a try and I'm already looking forward to comments as I'd like
> to expand this effort to local VM executions running the TCK integration 
> tests,
> so this series is quite important in that regard.

Overall I'm fine with what's proposed here.

Two general thoughts

 * ci/Makefile appears pretty much redundant - ci/helper can provide
   the same level of functionality AFAICT, and it'd be nice to kill
   an outstanding usage of 'make' given our goal to standardize on
   meson + python

 * ci/helper looks almost entirely independent of libvirt, aside
   from the list of 'choices' for the --job arg, and the --namespace
   arg default value, it would work with any virt project we have if
   the project created its own ci/build.sh file

   Can we fold all its logic into lcitool, and just have that as
   the entrypoint ? In ci/manifest.yml we can get the project
   namespace, and we could possibly just extra the commands by
   crudely regex matching 'ci/build.sh' content against the
   pattern '^run_.*\(\)$ {'


The removal of ci/Makefil feels like it could be done in this series,
but its fine if the ci/helper suggestion is left as separate future
work.


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

Reply via email to