If systemd-tmpfiles(8) couldn't be spawned, then the attempted command
is already included in the traceback:
Traceback (most recent call last):
File "toolbox/meson_post_install.py", line 26, in <module>
subprocess.run(['systemd-tmpfiles', '--create'], check=True)
File "/usr/lib64/python3.10/subprocess.py", line 524, in run
raise CalledProcessError(retcode, process.args,
subprocess.CalledProcessError: Command '['systemd-tmpfiles',
'--create']' returned non-zero exit status 73.
https://github.com/containers/toolbox/pull/1122
In short, it's a lot of effort to cover all possible exceptions that can
be thrown, and things work reasonably well even without handling them.
Since this is just part of the build, there's no point in complicating
things for aesthetic reasons.
More details below.
First, not every runtime error leads to a subprocess.CalledProcessError.
It's only thrown if the spawned process returns with a non-zero exit
code. There can be other problems. eg., if the gofmt file isn't
executable then a PermissionError is thrown that's currently not
handled, and the wrapper Python script returns with a non-zero exit
code:
Traceback (most recent call last):
File "toolbox/src/meson_go_fmt.py", line 28, in <module>
gofmt = subprocess.run(['gofmt', '-d', source_dir],
capture_output=True, check=True)
File "/usr/lib64/python3.10/subprocess.py", line 501, in run
with Popen(*popenargs, **kwargs) as process:
File "/usr/lib64/python3.10/subprocess.py", line 969, in __init__
self._execute_child(args, executable, preexec_fn, close_fds,
File "/usr/lib64/python3.10/subprocess.py", line 1845, in
_execute_child
raise child_exception_type(errno_num, err_msg, err_filename)
PermissionError: [Errno 13] Permission denied: 'gofmt'
Second, when a subprocess.CalledProcessError is thrown, the wrapper
Python script will still return with a non-zero exit code with an
understandable error message, even if the exception isn't handled. eg.,
if 'meson install' is called without the adequate permissions, then
systemd-tmpfiles(8) will return with a non-zero exit code, which shows
up as:
--- stdout ---
Calling systemd-tmpfiles --create ...
--- stderr ---
Failed to open directory 'cryptsetup': Permission denied
Failed to open directory 'certs': Permission denied
Failed to create directory or subvolume "/var/spool/cups/tmp":
Permission denied
...
...
...
Traceback (most recent call last):
File "toolbox/meson_post_install.py", line 26, in <module>
subprocess.run(['systemd-tmpfiles', '--create'], check=True)
File "/usr/lib64/python3.10/subprocess.py", line 524, in run
raise CalledProcessError(retcode, process.args,
subprocess.CalledProcessError: Command '['systemd-tmpfiles',
'--create']' returned non-zero exit status 73.
Similarly, if there problems generating the shell completions:
--- stderr ---
Error: unknown command "__completion" for "toolbox"
Run 'toolbox --help' for usage.
exit status 1
Traceback (most recent call last):
File "toolbox/completion/generate_completions.py", line 35, in
<module>
output = subprocess.run(['go', 'run', '.', '__completion',
completion_type], check=True)
File "/usr/lib64/python3.10/subprocess.py", line 524, in run
raise CalledProcessError(retcode, process.args,
subprocess.CalledProcessError: Command '['go', 'run', '.',
'__completion', 'bash']' returned non-zero exit status 1.
https://github.com/containers/toolbox/pull/1122
Cobra provides a default command 'completion' that is always visible.
The reverted change caused an additional command 'completion' to show up
in the list because the then called command '__completion' didn't
override the default one. This became apparent due to d69ce6794b
dynamically generating completion arguments for the 'help' command.
This reverts commit 4469774fb1.
https://github.com/containers/toolbox/pull/1055https://github.com/containers/toolbox/pull/1121
By default, the value of the 'build_by_default' argument is determined
by the value of the 'install' argument, which was set to 'true' once the
Go implementation was considered stable enough for end users.
Fallout from 0b3c66434ehttps://github.com/containers/toolbox/pull/1116
When describing the --authfile option, the word 'private' is used to
refer to images needing authentication. Using the same word shortens
the text so that the word 'custom' can be used in the same way as in the
other examples.
https://github.com/containers/toolbox/pull/1107
It turns out that Viper's custom error implementations use non-pointer
receivers, whereas often people assume pointer receivers. This can
cause confusion when trying to use errors.As(...) with those errors [1].
Secondly, Viper may or may not throw ConfigFileNotFoundError depending
on its build tags.
[1] https://github.com/spf13/viper/issues/1139https://github.com/containers/toolbox/pull/1105
Currently, the container name and release are only validated if they
were specified as command line options. Neither the value of release
in the configuration file nor the container name generated from an
image are validated.
There's also a lot of repeated code in the command front-ends to
validate the container name and release. This opens the door for
mistakes. Any adjustment to the code must be repeated elsewhere, and
there are subtle interactions and overlaps between the validation code
and the code to resolve container and image names.
It's worth noting that the container and image name resolution happens
for both the command line and configuration file options, and generates
the container name from the image when necessary.
Therefore, validating everything while resolving cleans up the command
front-ends and increases the coverage of the validation.
This introduces the use of sentinel error values and custom error
implementations to identify the different errors that can occur while
resolving the container and images, so that they can be appropriately
shown to the user.
https://github.com/containers/toolbox/pull/1101
Currently, if an invalid or unsupported string is specified as the
distro on the command line or in the configuration file, then it would
silently fallback to Fedora. This shouldn't happen.
It should only fallback to Fedora when no distro was specified and
there's no supported Toolbox image matching the host operating system.
If a distro was explicitly specified then it should either be supported
or it should error out.
The test cases were resurrected from commit 8b6418d8aa.
https://github.com/containers/toolbox/issues/937https://github.com/containers/toolbox/pull/1080
The terms 'default' and 'fallback' are used to mean very specific
things in this context.
The 'default' values are those that are used when the 'create', 'enter'
and 'run' commands were used without any option. These values are
picked to match the host operating system.
However, if there's no supported Toolbox image matching the host
operating system, and no options were provided to the 'create', 'enter'
and 'run' commands, then the 'fallback' values are used as a last
resort.
Consistently using this terminology leads to a clear mental model and
makes the code easier to read.
This rough arrangement of the code was already being used for
'release', and has now been been extended to 'container name prefix'
and 'distro'. The suffix for the 'fallback' values was simplified to
'Fallback', instead of 'DefaultFallback'.
https://github.com/containers/toolbox/issues/937https://github.com/containers/toolbox/pull/1080
Figuring out the container name prefix for a given image only needs to
happen as part of resolving the final Toolbox container name from the
given command line and configuration options.
Fallout from c990fb43cahttps://github.com/containers/toolbox/pull/1098
Otherwise, Meson complains:
completion/meson.build:4: WARNING: Project targeting '>= 0.58.0' but
tried to use feature deprecated since '0.56.0':
dependency.get_pkgconfig_variable. use
dependency.get_variable(pkgconfig : ...) instead
Fallout from bafbbe81c9https://github.com/containers/toolbox/pull/1096
The -Dmigration_path_for_coreos_toolbox option enables a different code
path that's currently not tested by the CI at all. In fact, since it's
a build-time option, the corresponding code path is not even built by
the CI.
To properly support the -Dmigration_path_for_coreos_toolbox option, it
needs to be covered by the CI. This is a step in that direction by
running the unit tests on it.
https://github.com/containers/toolbox/pull/1095
A subsequent commit will introduce builds performed with the
-Dmigration_path_for_coreos_toolbox option to the CI. It will be good
to avoid duplicating the build and installation steps for builds with
and without the -Dmigration_path_for_coreos_toolbox option.
https://github.com/containers/toolbox/pull/1095
A subsequent commit will introduce builds performed with the
-Dmigration_path_for_coreos_toolbox option to the CI. It will be good
to avoid duplicating the installation of RPM packages, Git submodule
handling, and the listing of various debug and version information for
builds with and without -Dmigration_path_for_coreos_toolbox option.
https://github.com/containers/toolbox/pull/1095
This will provide a path forward to those who stumble across the POSIX
shell implementation and don't know how to use the Go implementation.
https://github.com/containers/toolbox/pull/1094
The subsequent commit will touch the POSIX shell implementation, and
hence ShellCheck needs to be run on it.
As long as the POSIX shell implementation is part of the Git repository,
ShellCheck needs to keep running on it, unless it causes some serious
problems. The ShellCheck test is very fast, and the reassurance and
mental peace that it provides is invaluable.
This reverts commit 8c1d441916.
https://github.com/containers/toolbox/pull/1094
This isn't causing any problems at the moment. However, the test can
break if the order in which the command line arguments are validated
changes. eg., if the presence of a command is checked before the
release, then the error message will be different.
Fallout from 8b6418d8aahttps://github.com/containers/toolbox/pull/1091
From now on, Debarshi Ray <debarshir@gnome.org> will show up as
Debarshi Ray <rishi@fedoraproject.org>.
Toolbox isn't quite a GNOME project (it doesn't use elements from the
GNOME platform, like GLib), even though it's part of the same
ecosystem and many Toolbox contributors are also GNOME contributors.
Toolbox was conceived to improve the developer experience on Fedora
Silverblue, and expanded over time to cover other use-cases (eg.,
troubleshooting the operating system) and Fedora editions (eg., CoreOS
and Workstation). Even though there's a growing number of users on
other distributions, they are not the primary reason for Toolbox to
exist.
Toolbox heavily depends on Podman, and as a result is more aligned with
the Containers organization on GitHub than anything else, which is
driven, to a large degree, by Fedora contributors.
Hence, my desire to use my Fedora identity.
https://github.com/containers/toolbox/pull/1083
When a command is executed with toolbox run and it returns a non-zero
exit code, it is just ignored if that exit code is not handled. This
prevents users to identify errors when executing commands in toolbox.
With this fix, the exit codes of the invoked command are propagated
and returned by 'toolbox run'. This includes even exit codes returned
by Podman on error.
https://github.com/containers/toolbox/pull/1013
Co-authored-by: Ondřej Míchal <harrymichal@seznam.cz>
Without stderr being attached stderr output of the invoked command goes
into stdout.
Behaviour before:
; output="$(toolbox run /etc)"
Error: failed to invoke command /etc in container <name-of-container>
; echo -e "$output"
/bin/sh: line 1: /etc: Is a directory
/bin/sh: line 1: exec: /etc: cannot execute: Is a directory
Behaviour after:
; output="$(toolbox run /etc)"
/bin/sh: line 1: /etc: Is a directory
/bin/sh: line 1: exec: /etc: cannot execute: Is a directory
Error: failed to invoke command /etc in container <name-of-container>
; echo -e "$output"
https://github.com/containers/toolbox/pull/1013
Passing '--tty' to 'podman exec' unconditionally causes Podman to
allocate a pseudo-TTY for the command execution. This causes problems
with piping (values not being piped in and values being piped out with
carriage return at the end of a line). The solution is to track the
presence of a terminal on stdin/stdout and based on its presence use the
'--tty' flag.
Original behaviour:
; echo foo | toolbox run less
; toolbox echo foo | od -c
0000000 f o o \r \n
0000005
New behaviour:
; echo foo | toolbox run less
foo
; toolbox echo foo | od -c
0000000 f o o \n
0000004
As seen in the 'Piping in' example, the value gets only printed into
stdout. Not ideal from the point of view of using 'less' (or similar
tools) but still a move forward.
Based on a discussion in Podman's bugtracker[0].
Fixes https://github.com/containers/toolbox/issues/157
Fixes https://github.com/containers/toolbox/issues/848
[0] https://github.com/containers/podman/issues/9718https://github.com/containers/toolbox/pull/1013
