-
Notifications
You must be signed in to change notification settings - Fork 219
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Don't block user interaction while fetching the image size #1415
Don't block user interaction while fetching the image size #1415
Conversation
Build succeeded. ✔️ unit-test SUCCESS in 9m 33s |
52b9c19
to
902e945
Compare
This makes it easier to spot the beginning of functions with long lists of parameters and return values. containers#1415
Build succeeded. ✔️ unit-test SUCCESS in 8m 38s |
902e945
to
aa48a0c
Compare
We need a newer Go (at least 1.20, to be precise) on Ubuntu 22.04:
|
aa48a0c
to
556b8f0
Compare
Build failed. ❌ unit-test FAILURE in 6m 30s |
194044a
to
6af107a
Compare
Build succeeded. ✔️ unit-test SUCCESS in 7m 14s |
6af107a
to
4de06b3
Compare
Build failed. ✔️ unit-test SUCCESS in 11m 33s |
recheck |
Build succeeded. ✔️ unit-test SUCCESS in 8m 52s |
Build succeeded. ✔️ unit-test SUCCESS in 8m 25s |
This makes it easier to spot the beginning of functions with long lists of parameters and return values. containers#1415
9da3b7f
to
cfa65cd
Compare
Build succeeded. ✔️ unit-test SUCCESS in 7m 20s |
Build succeeded. ✔️ unit-test SUCCESS in 8m 29s |
This makes it easier to spot the beginning of functions with long lists of parameters and return values. containers#1415
Build succeeded. ✔️ unit-test SUCCESS in 9m 27s |
Build succeeded. ✔️ unit-test SUCCESS in 8m 25s |
99963a8
to
ddcd28c
Compare
Build succeeded. ✔️ unit-test SUCCESS in 8m 51s |
Build succeeded. ✔️ unit-test SUCCESS in 9m 26s |
Build succeeded. ✔️ unit-test SUCCESS in 9m 07s |
Build succeeded. ✔️ unit-test SUCCESS in 8m 55s |
d0968a8
to
c154123
Compare
Other than removing and installing
ie., image sizes of different lengths, |
Build succeeded. ✔️ unit-test SUCCESS in 7m 02s |
This makes it easier to spot the beginning of functions with long lists of parameters and return values. containers#1415
c154123
to
7d50dcf
Compare
Build succeeded. ✔️ unit-test SUCCESS in 6m 59s |
7d50dcf
to
540947f
Compare
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/ containers#752 containers#1263
This is meant to avoid duplicating the code that shows the prompt, and reads and parses the user's input. containers#752 containers#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. containers#752 containers#1263
Build succeeded. ✔️ unit-test SUCCESS in 6m 55s |
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. containers#752 containers#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. containers#752 containers#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. containers#752 containers#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. containers#752 containers#1263
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/context containers#752 containers#1263
540947f
to
5784754
Compare
Build succeeded. ✔️ unit-test SUCCESS in 7m 07s |
It takes
skopeo inspect
a few seconds to fetch the image size from theremote registry, and while that happens the user can't interact with the
image download prompt:
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, whilewaiting for the user to respond to the image download prompt, and once
the image size has been fetched, the prompt can be updated to include
it.
So, initially:
... and then once the size is available:
If skopeo(1) is missing or too old, then the prompt can continue without
the size, as it did before:
#752
#1263