On Mon, Apr 26, 2021 at 11:59:19AM +0200, Erik Skultety wrote:
On Fri, Apr 23, 2021 at 05:03:04PM +0200, Andrea Bolognani wrote:
Later on, when we change the actions that operate on container images to accept an lcitool-style --cross-arch argument instead of expecting the name of the image to have the cross-building architecture baked in, this will allow users to quickly copy-and-paste the necessary information.
...you can quickly copy-and-paste the image name even now and I would argue that it's even quicker than after this series:
./helper list-images ./helper <action> <copy-paste image name> vs
./helper list-images ./helper <action> <copy-paste image name> --cross-arch <copy-paste cross arch>
Right now you wouldn't even need the verification code introduced in patch 12 since you have to paste the image verbatim which IMO for this utility helper that only serves a very specific use case of a single repo is "good enough".
That assumes the target name only comes from copying and pasting from the output of the list-images action... Right now, if you pass some invalid / obsolete value you get $ ./ci/helper build ubuntu-1604 make: Entering directory '/home/abologna/src/upstream/libvirt/ci' make -C /home/abologna/src/upstream/libvirt/ci ci-run-command@ubuntu-1604 CI_COMMAND="/home/abologna/build" make[1]: Entering directory '/home/abologna/src/upstream/libvirt/ci' Checking if podman is available...yes Cloning /home/abologna/src/upstream/libvirt to /home/abologna/src/upstream/libvirt/ci/scratch/src Cloning /home/abologna/src/upstream/libvirt/src/keycodemapdb to /home/abologna/src/upstream/libvirt/ci/scratch/src/src/keycodemapdb podman run \ --rm --interactive --tty --user "1000":"1000" --workdir "/home/abologna" --env CI_CONT_SRCDIR="/home/abologna/libvirt" --env CI_MESON_ARGS="" --env CI_NINJA_ARGS="" --uidmap 0:1:1000 --uidmap 1000:0:1 --uidmap 1001:1001:64536 --gidmap 0:1:1000 --gidmap 1000:0:1 --gidmap 1001:1001:64536 --volume /home/abologna/src/upstream/libvirt/ci/scratch/group:/etc/group:ro,z --volume /home/abologna/src/upstream/libvirt/ci/scratch/passwd:/etc/passwd:ro,z --volume /home/abologna/src/upstream/libvirt/ci/scratch/home:/home/abologna:z --volume /home/abologna/src/upstream/libvirt/ci/scratch/build:/home/abologna/build:z --volume /home/abologna/src/upstream/libvirt/ci/scratch/src:/home/abologna/libvirt:z --ulimit nofile=1024:1024 --cap-add=SYS_PTRACE \ registry.gitlab.com/libvirt/libvirt/ci-ubuntu-1604:latest \ /home/abologna/build Trying to pull registry.gitlab.com/libvirt/libvirt/ci-ubuntu-1604:latest... manifest unknown: manifest unknown Error: Error initializing source docker://registry.gitlab.com/libvirt/libvirt/ci-ubuntu-1604:latest: Error reading manifest latest in registry.gitlab.com/libvirt/libvirt/ci-ubuntu-1604: manifest unknown: manifest unknown make[1]: *** [Makefile:199: ci-run-command@ubuntu-1604] Error 125 make[1]: Leaving directory '/home/abologna/src/upstream/libvirt/ci' make: *** [Makefile:209: ci-build@ubuntu-1604] Error 2 make: Leaving directory '/home/abologna/src/upstream/libvirt/ci' error: 'make' failed With my patches, the output is a much more reasonable $ ./ci/helper build ubuntu-1604 Unknown target 'ubuntu-1604' I agree that having to input the target OS and the target architecture separately is slightly more copy-and-paste work, but really those are two semantically separate bits of information and it's just wrong for the user to provide a single argument that encodes both. It was an acceptable trade-off when we were using make, but now that we have an actual command-line parser at our disposal we should take advantage of it.
I guess the idea is to eventually integrate this helper somehow to lcitool, so I'm wondering what is it we're trying to solve here. As for the output itself, if we want to change it, then I'd be probably more inclined towards something like virt-builder --list. There's massive information redundancy in there - no need to argue - but because the output is formatted like a simple table, tools like grep,sort,uniq and cut make the customization of the output for the average linux user an absolute no-brainer and they always get information they were looking for easily with almost no added effort.
Changing the output to be similar to that of 'virt-builder --list' sounds good to me.