The errors should be propagated up the call chain either verbatim or by
wrapping them with all relevant context when necessary (as long as they
don't violate the API boundaries).
The errors should be logged only when there's a break in the upward
propagation, either because they need to be reformatted before being
shown to the user or because they would expose implementation details
that aren't part of the API contract. Not logging the errors in such
cases might make it difficult to debug problems later on.
https://github.com/containers/toolbox/pull/1202
Currently, the titles of the manuals are rendered with a pair of empty
parentheses and no section title:
toolbox(1)() toolbox(1)()
NAME
toolbox - Tool for containerized command line environments...
However, they should be:
toolbox(1) General Commands Manual toolbox(1)
NAME
toolbox - Tool for containerized command line environments...
This is because the troff generated by go-md2man from Markdown has a
faulty invocation of the .TH macro [1]:
.nh
.TH toolbox(1)
.SH NAME
.PP
toolbox - Tool for containerized command line environments on Linux
It should be:
.nh
.TH toolbox 1
.SH NAME
.PP
toolbox - Tool for containerized command line environments on Linux
Original patch from Andrew Denton for Podman [2].
[1] https://www.gnu.org/software/groff/manual/groff.html
[2] Podman commit 63c779a857b55b00
https://github.com/containers/podman/pull/15621https://github.com/containers/toolbox/pull/1210
Otherwise https://www.shellcheck.net/ would complain:
Line 2479:
shift
^---^ SC2317 (info): Command appears to be unreachable. Check usage
(or ignore if invoked indirectly).
See: https://www.shellcheck.net/wiki/SC2317
Fedora Rawhide now has ShellCheck-0.9.0, which flags these new problems,
while so far it only had ShellCheck-0.8.0.
ShellCheck is correct that this is unreachable code. However, given the
lack of built-in command line parsing facilities in POSIX shell, this
code pattern has so far turned out to be quite handy. It's flexible
enough to be able to handle different combinations of commands and
options, and is easy to read. Trying to 'fix' the code will likely
cause more problems than it will solve.
Moreover, the POSIX shell implementation has been replaced by the Go
implementation quite a long time ago. It's no longer maintained and has
been kept only for historical reasons. Therefore, it's not worth
spending any significant amount of time on it.
https://github.com/containers/toolbox/pull/1211
The name of a node in a nodeset is meant to be a human-readable name. A
name with an obscure prefix like 'ci-node-' makes it look more profound
than it really is.
https://github.com/containers/toolbox/pull/1206
The 'unit tests' are no longer just unit tests. They also run a bunch
of static analysis tools like ShellCheck, codespell, gofmt and 'go vet'.
Since newer versions of these tools are generally better at catching
problems in the codebase, it will be better to run the 'unit tests' on
Fedora Rawhide with the latest versions than older stable Fedoras.
The timeout for the 'unit tests' need to be increased because Fedora
Rawhide is slower than stable Fedoras. Currently, the timeout for the
'unit tests' running on Fedora 36 is 10 minutes. Increasing it to 20
minutes when running on Fedora Rawhide wasn't enough, so maybe 30 will
be sufficient.
Note that this is only feasible because the Fedora Rawhide builds are
now more robust against stale DNF caches [1]. Otherwise, it wouldn't
have been wise to use Fedora Rawhide to test anything which isn't also
being tested elsewhere, because the Fedora Rawhide builds might have
stayed broken for extended periods of time due to reasons completely
unrelated to Toolbx.
[1] Commit 995c6d175ehttps://github.com/containers/toolbox/pull/1201https://github.com/containers/toolbox/pull/1206
This will be used by the subsequent commit to have a separate set of
dependencies for CentOS Stream 9 builds. eg., unlike Fedora, CentOS
Stream 9 doesn't have the ShellCheck, bats and fish RPMs.
https://github.com/containers/toolbox/pull/1171
Currently, the standard error and output streams of the child commands
invoked by 'meson test' are redirected to a separate log file. When the
tests fail, it's difficult, or maybe even impossible, to access this
file from the Zuul CI, and all that can be seen is something like:
1/7 shellcheck src/go-build-wrapper OK 0.04s
2/7 shellcheck profile.d/toolbox.sh FAIL 0.06s exit status 1
>>> MALLOC_PERTURB_=241 /usr/bin/shellcheck
--shell=sh
/home/zuul-worker/src/github.com/containers/toolbox/builddir/../profile.d/toolbox.sh
3/7 go fmt FAIL 0.05s exit status 1
>>> MALLOC_PERTURB_=209 /usr/bin/python3
/home/zuul-worker/src/github.com/containers/toolbox/src/meson_go_fmt.py
/home/zuul-worker/src/github.com/containers/toolbox/src
4/7 codespell FAIL 0.31s exit status 65
>>> MALLOC_PERTURB_=180 /usr/bin/codespell
--check-filenames
--check-hidden
--context 3
--exclude-file /home/zuul-worker/src/github.com/containers/toolbox/.codespellexcludefile
--skip /home/zuul-worker/src/github.com/containers/toolbox/builddir
--skip /home/zuul-worker/src/github.com/containers/toolbox/.git
--skip /home/zuul-worker/src/github.com/containers/toolbox/test/system/libs/bats-assert
--skip /home/zuul-worker/src/github.com/containers/toolbox/test/system/libs/bats-support
/home/zuul-worker/src/github.com/containers/toolbox
5/7 shellcheck toolbox (deprecated) FAIL 1.09s exit status 1
>>> MALLOC_PERTURB_=233 /usr/bin/shellcheck
/home/zuul-worker/src/github.com/containers/toolbox/builddir/../toolbox
6/7 go test OK 1.89s
7/7 go vet OK 17.60s
This doesn't have enough information to understand what caused the tests
to fail on non-interactive CI environments.
Not redirecting the standard error and output streams of the child
commands invoked by 'meson test' will readily reveal more details about
the test failures and remove the need to find the log file created by
Meson.
https://github.com/containers/toolbox/pull/1171
Otherwise codespell would complain:
: @test "create: Try to create a container with invalid custom name...
> run $TOOLBOX -y create "ßpeci@l.Nam€"
:
./test/system/101-create.bats:57: Nam ==> Name
CentOS Stream 9 has codespell-2.2.1, while so far the 'unit tests' were
being run on Fedora 36, which only has codespell-2.1.0.
This is a step towards testing on CentOS Stream 9.
https://github.com/containers/toolbox/pull/1200
CentOS Stream 9 has codespell-2.2.1, while so far the 'unit tests' were
being run on Fedora 36, which only has codespell-2.1.0.
This is a step towards testing on CentOS Stream 9.
Fallout from ecd1ced719https://github.com/containers/toolbox/pull/1200
Otherwise codespell would complain:
: {"/tmp", "/run/host/tmp", "rslave"},
> {"/var/lib/flatpak", "/run/host/var/lib/flatpak", "ro"},
: {"/var/lib/libvirt", "/run/host/var/lib/libvirt", ""},
./src/cmd/initContainer.go:61: ro ==> to, row, rob, rod, roe, rot
CentOS Stream 9 has codespell-2.2.1, while so far the 'unit tests' were
being run on Fedora 36, which only has codespell-2.1.0.
This is a step towards testing on CentOS Stream 9.
https://github.com/containers/toolbox/pull/1200
Otherwise https://www.shellcheck.net/ would complain:
Line 86:
term_just_first_character="${TERM%$term_without_first_character}"
^-- SC2295 (info): Expansions inside
${..} need to be quoted
separately, otherwise they match
as patterns.
See: https://www.shellcheck.net/wiki/SC2295
CentOS Stream 9 has ShellCheck-0.8.0, while so far the 'unit tests' were
being run on Fedora 36, which only has ShellCheck-0.7.2.
This is a step towards testing on CentOS Stream 9.
https://github.com/containers/toolbox/pull/1200
CentOS Stream 9 has golang-1.19.2, while so far the 'unit tests' were
being run on Fedora 36, which only has golang-1.18.8.
This is a step towards testing on CentOS Stream 9.
https://github.com/containers/toolbox/pull/1199
CentOS Stream 9 has codespell-2.2.1, while so far the 'unit tests' were
being run on Fedora 36, which only has codespell-2.1.0.
This is a step towards testing on CentOS Stream 9.
Fallout from 708fa593e2https://github.com/containers/toolbox/pull/1199
Different versions of ShellCheck and codespell may treat the same code
base differently. eg., these tools are currently being used on Fedora
36 as part of the 'unit tests', but CentOS Stream 9 has newer versions
that are stricter and catch several new problems.
Knowing the versions of the tools used in the tests helps to understand
these differences, and is a step towards testing on CentOS Stream 9.
https://github.com/containers/toolbox/pull/1199
Note that 'run --keep-empty-lines' counts the trailing newline on the
last line as a separate line.
Until Bats 1.7.0, 'run --keep-empty-lines' had a bug where even when a
command produced no output, it would report a line count of one [1] due
to a stray line feed character. This needs to be conditionalized, since
Fedora 35 has Bats 1.5.0.
[1] https://github.com/bats-core/bats-core/issues/573https://github.com/containers/toolbox/issues/1043
Currently, if an image was copied with:
$ skopeo copy \
containers-storage:registry.fedoraproject.org/fedora-toolbox:36 \
containers-storage:localhost/fedora-toolbox:36
... or:
$ podman tag \
registry.fedoraproject.org/fedora-toolbox:36 \
localhost/fedora-toolbox:36
... then it would show up twice in 'list' with the same name, and in the
wrong order.
Either as:
$ toolbox list --images
IMAGE ID IMAGE NAME CREATED
2110dbbc33d2 localhost/fedora-toolbox:36 1 day...
e085805ade4a registry.access.redhat.com/ubi8/toolbox:latest 1 day...
2110dbbc33d2 localhost/fedora-toolbox:36 1 day...
70cbe2ce60ca registry.fedoraproject.org/fedora-toolbox:34 1 day...
... or as:
$ toolbox list --images
IMAGE ID IMAGE NAME CREATED
2110dbbc33d2 registry.fedoraproject.org/fedora-toolbox:36 1 day...
e085805ade4a registry.access.redhat.com/ubi8/toolbox:latest 1 day...
2110dbbc33d2 registry.fedoraproject.org/fedora-toolbox:36 1 day...
70cbe2ce60ca registry.fedoraproject.org/fedora-toolbox:34 1 day...
The correct output should be similar to 'podman images', and be sorted
in ascending order of the names:
$ toolbox list --images
IMAGE ID IMAGE NAME CREATED
2110dbbc33d2 localhost/fedora-toolbox:36 1 day...
e085805ade4a registry.access.redhat.com/ubi8/toolbox:latest 1 day...
70cbe2ce60ca registry.fedoraproject.org/fedora-toolbox:34 1 day...
2110dbbc33d2 registry.fedoraproject.org/fedora-toolbox:36 1 day...
The problem is that, in these situations, 'podman images --format json'
returns separate identical JSON collections for each copy of the image,
and all of those copies have multiple names:
[
{
"Id": "2110dbbc33d2",
...
"Names": [
"localhost/fedora-toolbox:36",
"registry.fedoraproject.org/fedora-toolbox:36"
],
...
},
{
"Id": "e085805ade4a",
...
"Names": [
"registry.access.redhat.com/ubi8/toolbox:latest"
],
...
},
{
"Id": "2110dbbc33d2",
...
"Names": [
"localhost/fedora-toolbox:36",
"registry.fedoraproject.org/fedora-toolbox:36"
],
...
}
{
"Id": "70cbe2ce60ca",
...
"Names": [
"registry.fedoraproject.org/fedora-toolbox:34"
],
...
},
]
The image objects need to be flattened to have only one unique name per
copy, but with the same ID, and then sorted to ensure the right order.
Note that the ordering was already broken since commit 2369da5d31,
which started using 'podman images --sort repository'. Podman can sort
by either the image's repository or tag, but not by the unified name,
which is what Toolbx needs. Therefore, even without copied images,
Toolbx really does need to sort the images itself.
Prior to commit 2369da5d31, the ordering was correct, but copied
images would only show up once.
Fallout from 2369da5d31
This reverts parts of commit 67e210378e.
https://github.com/containers/toolbox/issues/1043
With the recent expansion of the test suite, it's necessary to increase
the timeout for Fedora Rawhide nodes to prevent the CI from timing out.
https://github.com/containers/toolbox/pull/1195
If an image was copied with:
$ skopeo copy \
containers-storage:registry.fedoraproject.org/fedora-toolbox:36 \
containers-storage:localhost/fedora-toolbox:36
... or:
$ podman tag \
registry.fedoraproject.org/fedora-toolbox:36 \
localhost/fedora-toolbox:36
... then the image ID is only showed once in 'podman images --quiet',
not twice.
A subsequent commit will use this to write tests to ensure that copied
images are correctly handled.
https://github.com/containers/toolbox/issues/1043
Note that 'run --keep-empty-lines' counts the trailing newline on the
last line as a separate line.
Until Bats 1.7.0, 'run --keep-empty-lines' had a bug where even when a
command produced no output, it would report a line count of one [1] due
to a stray line feed character. This needs to be conditionalized, since
Fedora 35 has Bats 1.5.0.
[1] https://github.com/bats-core/bats-core/issues/573https://github.com/containers/toolbox/pull/1192
Note that 'run --keep-empty-lines' counts the trailing newline on the
last line as a separate line.
Until Bats 1.7.0, 'run --keep-empty-lines' had a bug where even when a
command produced no output, it would report a line count of one [1] due
to a stray line feed character. This needs to be conditionalized, since
Fedora 35 has Bats 1.5.0.
[1] https://github.com/bats-core/bats-core/issues/573https://github.com/containers/toolbox/pull/1192
A subsequent commit will test the order in which images with and without
names are listed. It's logical for that test to come after the one
about the basic support for images without names.
https://github.com/containers/toolbox/pull/1192
Skopeo was already listed, so it didn't make sense to leave out the
others. It's useful to give the user a heads-up to make it obvious what
the requirements are.
https://github.com/containers/toolbox/pull/1194
This was making it difficult to read the Bats assertions on test
failures, by polluting it with unexpected and irrelevant output from
'podman images'. For example [1]:
not ok 39 list: Images with and without names in 12332ms
# (from function `assert' in file test/system/libs/bats-assert/src/assert.bash, line 46,
# in test file test/system/102-list.bats, line 126)
# `assert [ ${#stderr_lines[@]} -eq 0 ]' failed
# REPOSITORY TAG IMAGE ID CREATED SIZE
# registry.fedoraproject.org/fedora-toolbox 35 862705390e8b 4 weeks ago 332 MB
# REPOSITORY TAG IMAGE ID CREATED SIZE
# registry.fedoraproject.org/fedora-toolbox 35 862705390e8b 4 weeks ago 332 MB
# registry.fedoraproject.org/fedora-toolbox 34 70cbe2ce60ca 7 months ago 354 MB
#
# -- assertion failed --
# expression : [ 1 -eq 0 ]
# --
#
Fallout from 7973181136
[1] https://github.com/containers/toolbox/pull/1192https://github.com/containers/toolbox/pull/1193
This builds on top of commit 0465d78fd9034ce9.
The toolboxImage type has been renamed to Image and moved into the
podman package.
There is nothing Toolbx specific about the type - it represents any
image returned by 'podman images'. The images are only later filtered
for Toolbx images.
Secondly, having the Image type inside the podman package makes it
possible to encapsulate the unmarshalling of the JSON within the package
without exposing the raw JSON to outside consumers. This is desirable
because the unmarshalling involves tracking changes in the JSON output
by different Podman versions, and it's better to limit such details to
the podman package.
https://github.com/containers/toolbox/pull/1190
It's better to avoid single letter variables in general, because they
are so hard to grep for.
This will make the subsequent commit easier to read.
https://github.com/containers/toolbox/pull/1190
