search

containers / toolbox

Tool for interactive command line environments on Linux
https://containertoolbx.org/
Apache License 2.0
2.38k stars 208 forks source link

cmd/create: Require valid terminal when prompting to pull an image #1428

Closed HarryMichal closed 6 months ago

HarryMichal commented 6 months ago

To limit the possible misbehaviour of Toolbx when ued in scripting, require the presence of valid terminal on stdin when an image needs to be pulled to create a new toolbx.

softwarefactory-project-zuul[bot] commented 6 months ago

Build failed. https://softwarefactory-project.io/zuul/t/local/buildset/b1fea5a5e20e4743a1d04426dd0a3e92

:heavy_check_mark: unit-test SUCCESS in 7m 08s :heavy_check_mark: unit-test-migration-path-for-coreos-toolbox SUCCESS in 3m 22s :heavy_check_mark: unit-test-restricted SUCCESS in 5m 55s :x: system-test-fedora-rawhide FAILURE in 28m 50s :x: system-test-fedora-39 FAILURE in 28m 35s :x: system-test-fedora-38 FAILURE in 30m 58s

softwarefactory-project-zuul[bot] commented 6 months ago

Build succeeded. https://softwarefactory-project.io/zuul/t/local/buildset/caa3c7b1627c4a4ca22a801d74078a93

:heavy_check_mark: unit-test SUCCESS in 8m 47s :heavy_check_mark: unit-test-migration-path-for-coreos-toolbox SUCCESS in 3m 39s :heavy_check_mark: unit-test-restricted SUCCESS in 6m 07s :heavy_check_mark: system-test-fedora-rawhide SUCCESS in 31m 56s :heavy_check_mark: system-test-fedora-39 SUCCESS in 30m 59s :heavy_check_mark: system-test-fedora-38 SUCCESS in 29m 18s

debarshiray commented 6 months ago

This test failure:

[...]

... seems to say that even when toolbox create was run non-interactively with the --assumeyes option, it still gave the error about requiring user interaction, instead of saying that it failed to pull the image.

Once --assumeyes has been given, it shouldn't worry about the standard input stream not being connected to a terminal device.

I took the liberty to fix this to get the tests to pass, and addressed the above nits.

The fact that this tripped up the tests suggests that we should add some new tests for this new error as well. :)

softwarefactory-project-zuul[bot] commented 6 months ago

Build succeeded. https://softwarefactory-project.io/zuul/t/local/buildset/3877c7fe79164a8aaf9a3b661707ad26

:heavy_check_mark: unit-test SUCCESS in 7m 13s :heavy_check_mark: unit-test-migration-path-for-coreos-toolbox SUCCESS in 3m 06s :heavy_check_mark: unit-test-restricted SUCCESS in 6m 20s :heavy_check_mark: system-test-fedora-rawhide SUCCESS in 46m 59s :heavy_check_mark: system-test-fedora-39 SUCCESS in 41m 45s :heavy_check_mark: system-test-fedora-38 SUCCESS in 37m 09s

debarshiray commented 6 months ago

The fact that this tripped up the tests suggests that we should add some new tests for this new error as well. :)

Added some tests and merged. Thanks, @HarryMichal !