It doesn't make sense to show the image download prompt when the
standard input or output stream is redirected to something other than a
terminal device.
During such non-interactive use, there's no way for the user to see the
prompt and the size of the image and then make a decision based on them.
The decision has to be made differently and earlier. The user will
either never download or always download or will use 'skopeo inspect'
to decide for themself.
Secondly, when the input and output are not connected to a terminal, the
terminal escape sequences and the terminal-specific ioctl(2) requests
used to show the prompt won't work anyway.
Some changes by Debarshi Ray.
https://github.com/containers/toolbox/pull/1428
Prompt is a new terminal emulation application [1] designed for a
container-oriented desktop that implements the OSC 777 escape sequence
to track and preserve the user's current Toolbx container [2]. Hence,
Fedora's fork of GNOME Terminal is no longer the only one to offer this.
The implementation in Prompt is already better because it has a
user-visible setting to disable this integration with Toolbx, in case
the user doesn't want it. Therefore, it's time to let users of all host
operating systems enjoy this feature.
This reverts commits a3e8d8d12b and
3a96feba47.
[1] https://gitlab.gnome.org/chergert/prompt
[2] https://gitlab.freedesktop.org/terminal-wg/specifications/-/issues/17https://github.com/containers/toolbox/issues/218
Without a sufficient buffer size the discard function does not read
fast/efficiently enough causing multiple lines indicating "passed and
discarded input" to show up.
I used an already defined constant[0] for the buffer size to prevent
the use of a yet-another magical constant.
[0] https://pkg.go.dev/bytes#pkg-constantshttps://github.com/containers/toolbox/pull/1427
On some Toolbx images with systemd-resolved(8), like the fedora-toolbox
images for Fedora 39 onwards, /etc/resolv.conf can end up being a
symbolic link inside the container that expects the host operating
system to also use systemd-resolved(8):
$ ls -l /etc/resolv.conf
lrwxrwxrwx. 1 root root 39 Nov 28 08:50 /etc/resolv.conf ->
../run/systemd/resolve/stub-resolv.conf
This happens because systemd-resolved(8) already makes /etc/resolv.conf
a symbolic link inside the image, and, hence, the container's entry
point doesn't change it to point at the host's copy of the file at
/run/host/etc/resolv.conf. Instead, it's left pointing at the host's
copy of the files maintained by systemd-resolved(8) under
/run/systemd/resolve, which happen to be also available inside the
container [1].
If the host OS doesn't use systemd-resolved(8), like Red Hat Enterprise
Linux 9, then this leads to a dangling symbolic link and breaks DNS
queries.
Note that the presence of systemd-resolved(8) in the recent
fedora-toolbox images is a regression caused by the ToolbxReleaseBlocker
Change [2] for Fedora 39 where the image was rewritten in terms of
fedora-kickstarts and pungi-fedora instead of a Container/Dockerfile.
By mistake, systemd crept in as an RPM needed by the image [3], which
in turn pulled in the systemd-resolved RPM as a weak dependency [4].
Hopefully, that will get fixed. However, it's also not practical to
keep track of all the Toolbx images out there in the wild, so it's
wise to make toolbox(1) more resilient to such things.
This will have the downside of overwriting some custom user-made
modifications to the container's /etc/resolv.conf. While that's
unfortunate, it's more important to have Toolbx images produce working
containers on a wide range of host OSes. It will be better to come up
with a more explicit way to support custom user-made modifications to
the container's configuration. Perhaps with a persistent stamp file.
[1] Commit af602c7d22https://github.com/containers/toolbox/commit/af602c7d227617d2https://github.com/containers/toolbox/pull/707
[2] https://fedoraproject.org/wiki/Changes/ToolbxReleaseBlocker
[3] fedora-kickstarts commit 48e2c3b5598de32f
https://pagure.io/fedora-kickstarts/c/48e2c3b5598de32f
[4] fedora-kickstarts commit 49306cb6eada8777
https://pagure.io/fedora-kickstarts/c/49306cb6eada8777https://github.com/containers/toolbox/issues/1410
It takes 'skopeo inspect' a few seconds to fetch the image size from the
remote registry, and while that happens the user can't interact with the
image download prompt:
$ toolbox create
Image required to create toolbox container.
<wait for a few seconds>
Download registry.fedoraproject.org/fedora-toolbox:39 (359.8MB)? [y/N]:
This feels awkward because it's not clear to the user what's going on
during those few seconds. Moreover, while knowing the image size can be
convenient at times, for example when disk space and network bandwidth
are limited, it's not always important.
It will be better if 'skopeo inspect' ran in the background, while
waiting for the user to respond to the image download prompt, and once
the image size has been fetched, the image download prompt can be
updated to include it.
So, initially:
$ toolbox create
Image required to create toolbox container.
Download registry.fedoraproject.org/fedora-toolbox:39 ( ... MB)? [y/N]:
... and then once the size is available:
$ toolbox create
Image required to create toolbox container.
Download registry.fedoraproject.org/fedora-toolbox:39 (359.8MB)? [y/N]:
If skopeo(1) is missing or too old, then the prompt can continue without
the size, as it did before:
$ toolbox create
Image required to create toolbox container.
Download registry.fedoraproject.org/fedora-toolbox:39 [y/N]:
The placeholder for the missing image size (ie., ' ... MB') was chosen
to have seven characters, so that it matches the most common sizes. The
human-readable representation of the image size is capped at four valid
numbers [1]. Unless it's a perfect round number like 1KB or 1.2MB, it
will likely use all four numbers and the decimal point, which is five
characters. Then two more for the unit, because it's very unlikely that
there will be an image that's less than 1KB in size and will be shown in
bytes with a B. That makes it seven characters in total.
Updating the image download prompt with the results of 'skopeo inspect'
is vulnerable to races. At the same time as the terminal's cursor is
being moved to the beginning of the current line to overwrite the
earlier prompt with the new one, the user can keep typing and keep
moving the cursor forward. This competition over the cursor can lead to
awkward outcomes.
For example, the prompt can overwrite the characters typed in by the
user, leaving characters in the terminal's input buffer waiting for the
user to hit ENTER, even though they are not visible on the screen.
Another example is that hitting BACKSPACE can end up deleting parts of
the prompt, instead of stopping at the edge.
This is solved by putting the terminal device into non-canonical mode
input and disabling the echoing of input characters, while the prompt is
being updated. This prevents input from moving the terminal's cursor
forward, and from accumulating in the terminal's input buffer even if
it might not be visible. Any input during this interim period is
discarded and replaced by '...', and a fresh new prompt is shown in the
following line.
In practice, this race shouldn't be too common. It can only happen if
the user is typing right when the prompt is being updated, which is
unlikely because it's only supposed to be a short 'yes' or 'no' input.
The use of the context.Cause and context.WithCancelCause functions [2]
requires Go >= 1.20. Bumping the Go version in src/go.mod then requires
a 'go mod tidy'. Otherwise, it leads to:
$ meson compile -C builddir --verbose
...
/home/rishi/devel/containers/git/toolbox/src/go-build-wrapper
/home/rishi/devel/containers/git/toolbox/src
/home/rishi/devel/containers/git/toolbox/builddir src/toolbox
0.0.99.4 cc /lib64/ld-linux-x86-64.so.2 false
go: updates to go.mod needed; to update it:
go mod tidy
ninja: build stopped: subcommand failed.
[1] https://pkg.go.dev/github.com/docker/go-units#HumanSize
[2] https://pkg.go.dev/contexthttps://github.com/containers/toolbox/issues/752https://github.com/containers/toolbox/issues/1263
A subsequent commit will use this to ensure that the user can still
interact with the image download prompt while 'skopeo inspect' fetches
the image size from the remote registry. Initially, the prompt will be
shown without the image size. If the user responds before the size is
fetched, then the pending 'skopeo inspect' will be cancelled.
https://github.com/containers/toolbox/issues/752https://github.com/containers/toolbox/issues/1263
A subsequent commit will use this to ensure that the user can still
interact with the image download prompt while 'skopeo inspect' fetches
the image size from the remote registry. Initially, the prompt will be
shown without the image size. If the user responds before the size is
fetched, then the pending 'skopeo inspect' will be cancelled.
https://github.com/containers/toolbox/issues/752https://github.com/containers/toolbox/issues/1263
A subsequent commit will use this to ensure that the user can still
interact with the image download prompt while 'skopeo inspect' fetches
the image size from the remote registry. To do this, at some point, the
terminal device will be put into non-canonical mode input and the
echoing of input characters will be disabled to retain full control of
the cursor position.
https://github.com/containers/toolbox/issues/752https://github.com/containers/toolbox/issues/1263
A subsequent commit will use this to ensure that the user can still
interact with the image download prompt while 'skopeo inspect' fetches
the image size from the remote registry.
To do this, at some point, the terminal device will be put into
non-canonical mode input and the echoing of input characters will be
disabled to retain full control of the cursor position. Unfortunately,
this will require access to the full termios(3) struct that isn't given
by golang.org/x/term, and, hence, the code needs to be written using the
underlying termios(3) API.
This future code will have enough overlap with the IsTerminal API from
golang.org/x/term that it doesn't make sense to use a separate module
(ie., golang.org/x/term) for it.
https://github.com/containers/toolbox/issues/752https://github.com/containers/toolbox/issues/1263
A subsequent commit will use this to ensure that the user can still
interact with the image download prompt while 'skopeo inspect' fetches
the image size from the remote registry.
Initially, the prompt will be shown without the image size. Once the
size has been fetched, the older prompt will be cancelled and a new one
will be shown that includes the size. While the prompt is getting
updated, the terminal device will be put into non-canonical mode input
and the echoing of input characters will be disabled to retain full
control of the cursor position. Once the new prompt is in place, the
previous state of the terminal will be restored. However, anything that
was typed in the interim will be discarded to avoid surprising the user
with invisible input.
Even though this code is only expected to be used to read from the
standard input stream when it's connected to a terminal device, the use
of poll(2) here was tested with FIFOs or named pipes and regular files
as well, in case they might be necessary in future.
https://github.com/containers/toolbox/issues/752https://github.com/containers/toolbox/issues/1263
A subsequent commit will use this to ensure that the user can still
interact with the image download prompt while 'skopeo inspect' fetches
the image size from the remote registry. Initially, the prompt will be
shown without the image size. Once the size has been fetched, the older
prompt will be cancelled and a new one will be shown that includes the
size.
Even though this code is only expected to be used to read from the
standard input stream when it's connected to a terminal device, the use
of poll(2) here was tested with FIFOs or named pipes and regular files
as well, in case they might be necessary in future.
An eventfd(2) file descriptor expects a 8-byte or 64-bit integer value
to be given to write(2) to increase its counter by that amount [1]. In
C, it could be phrased as:
uint64_t one = 1;
write (eventfd, &one, sizeof (one));
However, Go's wrapper for write(2) expects a sequence of bytes (ie.,
[]byte), and not an arbitrary memory address [2]. Therefore, the
'encoding/binary' package [3] is used to encode the integer into a byte
sequence as a varint.
Even though a varint-encoded 64-bit integer takes a maximum of 10
bytes, as defined by binary.MaxVarintLen64, 1 byte is enough to encode
the number 1 as an unsigned 64-bit integer [4]. That's enough to fit
into a byte sequence of length 8 to satisfy what an eventfd(2) file
descriptor expects. Ultimately, it doesn't matter exactly what value
the receiving end assigns to the number given to write(2), as long as
it's not zero.
[1] https://man7.org/linux/man-pages/man2/eventfd.2.html
[2] https://pkg.go.dev/golang.org/x/sys/unix#Write
[3] https://pkg.go.dev/encoding/binary
[4] https://protobuf.dev/programming-guides/encoding/https://github.com/containers/toolbox/issues/752https://github.com/containers/toolbox/issues/1263
Currently, inotify(7) is used to keep /etc/timezone inside the Toolbx
container synchronized with the host operating system's /etc/localtime.
However, /etc/timezone is only there for compatibility with Java. The
vast majority of non-ancient code bases use /etc/localtime, which does
not need inotify(7) to stay synchronized.
Therefore, it's not worth preventing the container from starting when
the operating system is suffering from a shortage of resources needed
for inotify(7). Especially because this shortage can be caused by a bug
in another program that's consuming too many inotify(7) instances and
watches.
https://github.com/containers/toolbox/issues/1329
Any image or container that has APT or systemd may have /etc/kernel.
eg., the arch-toolbox and ubuntu-toolbox images.
https://github.com/containers/toolbox/pull/1409
Signed-off-by: Penn Bauman <me@pennbauman.com>
Christian Hergert requested this. He is working on improving the
integration of Toolbx with the terminal emulation stack in GNOME and
Fedora, and he is using Fedora Linux Asahi Remix for his work.
https://github.com/containers/toolbox/pull/1413
The arguments for the D-Bus method are accepted separately by gdbus(1)
without any options. Therefore, they shouldn't be indented by another
additional level.
https://github.com/containers/toolbox/pull/1412
There's no need to run these cat(1) and gdbus(1) invocations through a
shell (ie., 'sh -c'), because there's no shell expansion that needs to
be performed.
These are unlike cases where shell expansion does need to be performed.
eg., 'readlink /proc/$$/ns/user', where the $$ needs to be expanded.
Fallout from 58134f8497 and
a0514cba12https://github.com/containers/toolbox/pull/1412
GNU roff 1.23 stopped remapping unescaped Hyphen-Minus (ie., - or 0x2D)
characters in the input to Hyphen-Minus in the output. Instead, it
follows the specified behaviour of converting unescaped Hyphen-Minus
characters in the input to Hyphen (ie., ‐ or 0x2010) in the output. To
get Hyphen-Minus characters in the output, one needs to escape the
Hyphen-Minus with a backslash (ie., \-) in the input [1].
Therefore, the command line options documented in the manuals are no
longer prefixed with the Hyphen-Minus character that's needed to
ctually use them. This breaks copying and pasting from the manuals and
searching within them.
Unfortunately, escaping the Hyphen-Minus characters in Markdown doesn't
have the intended effect of having Hyphen-Minus in the generated manual
pages [2]. Therefore, this is worked around by having the tests check
for both Hyphen-Minus and Hyphen.
Note that some operating system distributions, like Debian, have
reverted this change from GNU roff, but others haven't. So, unless it
can be guaranteed that the manuals will always have Hyphen-Minus
regardless of which GNU roff version or variant is being used, the tests
need to check for both.
[1] https://lwn.net/Articles/947941/https://lists.gnu.org/archive/html/info-gnu/2023-07/msg00001.htmlhttps://git.savannah.gnu.org/cgit/groff.git/tree/PROBLEMS?h=1.23.0#n82
[2] https://github.com/cpuguy83/go-md2man/issues/101https://github.com/containers/toolbox/pull/1398
This should finally ensure that the fedora-toolbox image doesn't have
any package that had its content, such as documentation or translations,
stripped out by the fedora base image.
Until now, missing-docs had a hand-maintained list of packages that had
their content stripped out by the fedora base image. These packages are
reinstalled when building the fedora-toolbox image to restore the lost
content. Unfortunately, this list was incomplete because it was only
updated when someone noticed that something is missing.
Now, the list is generated with:
$ rpm --all --query --state --queryformat "PACKAGE: %{NAME}\n"
... to ensure that it's always complete.
The existing built-in test to ensure that the desired files are actually
present in the final image was extended to cover some of those that were
absent. A new built-in test, based on the above rpm(1) command, was
added as a fallback to ensure that the final image doesn't have any
package with missing content.
Only the images for currently maintained Fedoras (ie., 37, 38 and 39)
were updated.
As suggested by Brian Campbell.
https://github.com/containers/toolbox/issues/603
The shadow-utils package has always been part of the fedora base image.
It's explicitly listed in extra-packages as a safeguard against losing
useradd(8) and usermod(8) by mistake because they are needed by the
entry point of a Toolbx container [1]. Hence, the need to restore the
shadow-utils documentation that was stripped out in the base image.
Only the images for currently maintained Fedoras (ie., 37, 38 and 39)
were updated.
[1] Commit c6772f0f11https://github.com/containers/toolbox/commit/c6772f0f112e8004https://github.com/containers/toolbox/pull/1394
Ansible's built-in 'package' module doesn't show any details when
installing the RPMs. All that can be seen is:
TASK [Install RPM packages]
fedora-rawhide | changed
Therefore, there's no way to know what version of the packages got
installed.
In this case, not knowing the go-md2man(1) version being used by the CI
makes it difficult to know why the tests are failing on Fedora Rawhide
and Fedora 39 with:
not ok 3 help: Command 'help' in 177ms
# (from function `assert_line' in file
test/system/libs/bats-assert/src/assert.bash, line 479,
# in test file test/system/002-help.bats, line 48)
# `assert_line --index 0 --partial "toolbox(1)"' failed
# /usr/bin/man
#
# -- line does not contain substring --
# index : 0
# substring : toolbox(1)
# line : troff:<standard input>:33: warning: cannot select font
'C'
# --
#
It could be either because the CI is still using an older version of
go-md2man(1) [1,2], or that there's some other problem.
[1] Fedora golang-github-cpuguy83-md2man commit 117806d50e401c19
https://src.fedoraproject.org/rpms/golang-github-cpuguy83-md2man/c/117806d50e401c19https://src.fedoraproject.org/rpms/golang-github-cpuguy83-md2man/pull-request/3
[2] go-md2man commit d85280db9b54b574
https://github.com/cpuguy83/go-md2man/commit/d85280db9b54b574https://github.com/cpuguy83/go-md2man/issues/99https://github.com/containers/toolbox/pull/1386
Currently, some of the names of the tests were too long, and had
inconsistent and verbose wording. This made it difficult to look at
them and get a gist of all the scenarios being tested. The names are
like headings. They shouldn't be too long, should capture the primary
objective of the test and be consistent in their wording.
Note that the term 'usage screen' was particularly confusing. Prior to
commit 3dc106e10a, 'usage screen' in the names of the tests also
referred to the very brief listing of the commands and options that's
shown by 'toolbox help' and 'toolbox --help' in the absence of man(1).
In the context of this change, the term referred to the brief two line
error message that's shown when an unknown command or flag is used. So,
it will be good to not use it anymore.
https://github.com/containers/toolbox/pull/1386
Commit 5e63e9ec9b added a 'help' command to show the toolbox(1)
manual or a manual page for a specific command, and made the --help flag
identical to it. Therefore it's misleading to say that the --help flag
should show the usage screen. The usage screen is a brief listing of
the commands and options, which isn't the same thing as the more
detailed manuals.
Later, after this test was written, commit 40fc1689a3 added a
fallback for host operating systems without man(1), like Fedora CoreOS,
that would show a very brief usage screen with only the most common
commands.
To make it more confusing, the test was checking for a string that's
common to both the toolbox(1) manual and the fallback brief usage screen
that might be shown by 'toolbox --help'. This meant that it was neither
able to distinguish between the code paths nor ensure that they were
working as intended.
This was resolved by adapting the existing 'toolbox --help' test to
strictly ensure that it's showing the toolbox(1) manual when man(1) is
present, and by adding a new test to strictly ensure that it's showing
the fallback brief usage screen when man(1) is absent.
Until Bats 1.10.0, 'run --keep-empty-lines' had a bug where it counted
the trailing newline on the last line as a separate line [1]. However,
Bats 1.10.0 is only available in Fedora >= 39 and is absent from Fedoras
37 and 38.
Fallout from b27795a03e
[1] Bats commit 6648e2143bffb933
https://github.com/bats-core/bats-core/commit/6648e2143bffb933https://github.com/bats-core/bats-core/issues/708https://github.com/containers/toolbox/pull/1386
Until now, only the packages that are present in the fedora base image,
and had their documentation stripped out, were being tested for the
availability of documentation. There were no tests for the extra
packages that get added to the base image to form the fedora-toolbox
image.
The util-linux and xz packages were picked as examples for these new
tests. The xz package is a particularly good example because it has
translations for its manuals. It can help test that the fedora-toolbox
image is localized just like Fedora Silverblue and Workstation.
Only the images for currently maintained Fedoras (ie., 37, 38 and 39)
were updated.
https://github.com/containers/toolbox/pull/1384
Myxa
Debarshi Ray
Ondřej Míchal
dependabot[bot]
Ievgen Popovych
Penn Bauman
Jens Petersen
Nieves Montero