From 411147988b730dabf8b9e761a5426e12d648f008 Mon Sep 17 00:00:00 2001 From: Debarshi Ray Date: Sat, 13 Nov 2021 14:16:32 +0100 Subject: [PATCH] cmd, test/system: Make the behaviour of 'toolbox' conditional Commit 6c86cabbe5da6e542b changed the command line interface to behave a lot similar to that of github.com/coreos/toolbox, which makes things easier for those switching over from it. Make it conditional so that only those OS distributors who truly need it may enable it, and restore the previous behaviour as the default. The tests were updated to test the default behaviour that the vast majority of users would be seeing. Ideally, the test suite would be run twice with the migration path turned off and on. However, that would require a more intrusive surgery of the test suite and likely make it slower. It might not be worth the hassle because of the small number of users who should be using the migration path. Note that the copyright and license notices really must use C++-style // line comments, because build constraints can only be preceded by blank lines and other line comments. C-style /* */ block comments can't precede the build constraints. This reverts commit ca899c8a561f357ae32c6ba6813520fd8b682abb and parts of commit 3aeb7cf288319e35eb9c5e26ea18d97452462c1e. [1] go help buildconstraint https://pkg.go.dev/cmd/go#hdr-Build_constraints https://github.com/containers/toolbox/pull/951 --- src/cmd/root.go | 62 +----------------------- src/cmd/rootDefault.go | 43 +++++++++++++++++ src/cmd/rootMigrationPath.go | 93 ++++++++++++++++++++++++++++++++++++ src/meson.build | 2 + test/system/002-help.bats | 11 +++++ test/system/100-root.bats | 27 ----------- 6 files changed, 150 insertions(+), 88 deletions(-) create mode 100644 src/cmd/rootDefault.go create mode 100644 src/cmd/rootMigrationPath.go delete mode 100644 test/system/100-root.bats diff --git a/src/cmd/root.go b/src/cmd/root.go index 9c6e615..8f990f4 100644 --- a/src/cmd/root.go +++ b/src/cmd/root.go @@ -181,67 +181,7 @@ func rootHelp(cmd *cobra.Command, args []string) { } func rootRun(cmd *cobra.Command, args []string) error { - if len(args) != 0 { - panic("unexpected argument: commands known or unknown shouldn't reach here") - } - - if utils.IsInsideContainer() { - if !utils.IsInsideToolboxContainer() { - return errors.New("this is not a toolbox container") - } - - if _, err := utils.ForwardToHost(); err != nil { - return err - } - - return nil - } - - image, release, err := utils.ResolveImageName("", "", "") - if err != nil { - return err - } - - container, err := utils.ResolveContainerName("", image, release) - if err != nil { - return err - } - - userShell := os.Getenv("SHELL") - if userShell == "" { - return errors.New("failed to get the current user's default shell") - } - - command := []string{userShell, "-l"} - - hostID, err := utils.GetHostID() - if err != nil { - return fmt.Errorf("failed to get the host ID: %w", err) - } - - hostVariantID, err := utils.GetHostVariantID() - if err != nil { - return errors.New("failed to get the host VARIANT_ID") - } - - var emitEscapeSequence bool - - if hostID == "fedora" && (hostVariantID == "silverblue" || hostVariantID == "workstation") { - emitEscapeSequence = true - } - - if err := runCommand(container, - true, - image, - release, - command, - emitEscapeSequence, - true, - false); err != nil { - return err - } - - return nil + return rootRunImpl(cmd, args) } func rootUsage(cmd *cobra.Command) error { diff --git a/src/cmd/rootDefault.go b/src/cmd/rootDefault.go new file mode 100644 index 0000000..3b88266 --- /dev/null +++ b/src/cmd/rootDefault.go @@ -0,0 +1,43 @@ +// +// Copyright © 2021 Red Hat Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +//go:build !migration_path_for_coreos_toolbox +// +build !migration_path_for_coreos_toolbox + +package cmd + +import ( + "errors" + "fmt" + "strings" + + "github.com/spf13/cobra" +) + +func rootRunImpl(cmd *cobra.Command, args []string) error { + var builder strings.Builder + fmt.Fprintf(&builder, "missing command\n") + fmt.Fprintf(&builder, "\n") + + usage := getUsageForCommonCommands() + fmt.Fprintf(&builder, "%s", usage) + + fmt.Fprintf(&builder, "\n") + fmt.Fprintf(&builder, "Run '%s --help' for usage.", executableBase) + + errMsg := builder.String() + return errors.New(errMsg) +} diff --git a/src/cmd/rootMigrationPath.go b/src/cmd/rootMigrationPath.go new file mode 100644 index 0000000..6c28480 --- /dev/null +++ b/src/cmd/rootMigrationPath.go @@ -0,0 +1,93 @@ +// +// Copyright © 2021 Red Hat Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +//go:build migration_path_for_coreos_toolbox +// +build migration_path_for_coreos_toolbox + +package cmd + +import ( + "errors" + "fmt" + "os" + + "github.com/containers/toolbox/pkg/utils" + "github.com/spf13/cobra" +) + +func rootRunImpl(cmd *cobra.Command, args []string) error { + if len(args) != 0 { + panic("unexpected argument: commands known or unknown shouldn't reach here") + } + + if utils.IsInsideContainer() { + if !utils.IsInsideToolboxContainer() { + return errors.New("this is not a toolbox container") + } + + if _, err := utils.ForwardToHost(); err != nil { + return err + } + + return nil + } + + image, release, err := utils.ResolveImageName("", "", "") + if err != nil { + return err + } + + container, err := utils.ResolveContainerName("", image, release) + if err != nil { + return err + } + + userShell := os.Getenv("SHELL") + if userShell == "" { + return errors.New("failed to get the current user's default shell") + } + + command := []string{userShell, "-l"} + + hostID, err := utils.GetHostID() + if err != nil { + return fmt.Errorf("failed to get the host ID: %w", err) + } + + hostVariantID, err := utils.GetHostVariantID() + if err != nil { + return errors.New("failed to get the host VARIANT_ID") + } + + var emitEscapeSequence bool + + if hostID == "fedora" && (hostVariantID == "silverblue" || hostVariantID == "workstation") { + emitEscapeSequence = true + } + + if err := runCommand(container, + true, + image, + release, + command, + emitEscapeSequence, + true, + false); err != nil { + return err + } + + return nil +} diff --git a/src/meson.build b/src/meson.build index 410923c..d810fcb 100644 --- a/src/meson.build +++ b/src/meson.build @@ -11,6 +11,8 @@ sources = files( 'cmd/rm.go', 'cmd/rmi.go', 'cmd/root.go', + 'cmd/rootDefault.go', + 'cmd/rootMigrationPath.go', 'cmd/run.go', 'cmd/utils.go', 'pkg/podman/podman.go', diff --git a/test/system/002-help.bats b/test/system/002-help.bats index 525d444..689f95e 100644 --- a/test/system/002-help.bats +++ b/test/system/002-help.bats @@ -8,6 +8,17 @@ setup() { _setup_environment } +@test "help: Try to run toolbox with no command" { + run $TOOLBOX + + assert_failure + assert_line --index 0 "Error: missing command" + assert_line --index 1 "create Create a new toolbox container" + assert_line --index 2 "enter Enter an existing toolbox container" + assert_line --index 3 "list List all existing toolbox containers and images" + assert_line --index 4 "Run 'toolbox --help' for usage." +} + @test "help: Run command 'help'" { if ! command -v man 2>/dev/null; then skip "Test works only if man is in PATH" diff --git a/test/system/100-root.bats b/test/system/100-root.bats deleted file mode 100644 index 32d8790..0000000 --- a/test/system/100-root.bats +++ /dev/null @@ -1,27 +0,0 @@ -#!/usr/bin/env bats - -load 'libs/bats-support/load' -load 'libs/bats-assert/load' -load 'libs/helpers' - -setup() { - _setup_environment - cleanup_containers -} - -teardown() { - cleanup_containers -} - -@test "root: Try to enter the default container with no containers created" { - run $TOOLBOX <<< "n" - - assert_success - assert_line --index 0 "No toolbox containers found. Create now? [y/N] A container can be created later with the 'create' command." - assert_line --index 1 "Run 'toolbox --help' for usage." -} - -# TODO: Write the test -@test "root: Enter the default container when 1 non-default container is present" { - skip "Testing of entering toolboxes is not implemented" -}