Revert "cmd, pkg/utils: Split distro and release parsing and ..."

The idea of splitting ResolveContainerAndImageNames into two public functions [1] didn't turn out to be so useful [2]. Splitting things even further might make it worse. A better approach might be to (re-)unify the code further. This is the first step towards that. This reverts the following commits: * 5c8ad7a * 02f45fd * 8b6418d ... but retains the test cases that were not tied to the changes in those commits. [1] Commit fd75608 containers#828 containers#838 [2] containers#977 containers#937
debarshiray · Jul 31, 2022 · 5f675ed · 5f675ed
1 parent f58a904
commit 5f675ed
Show file tree

Hide file tree

Showing 9 changed files with 34 additions and 284 deletions.
diff --git a/src/cmd/create.go b/src/cmd/create.go
@@ -159,23 +159,17 @@ func create(cmd *cobra.Command, args []string) error {
 		}
 	}
 
-	distro, err := utils.ResolveDistro(createFlags.distro)
-	if err != nil {
-		err := createErrorInvalidDistro()
-		return err
-	}
-
-	release := createFlags.release
-	if release != "" {
+	var release string
+	if createFlags.release != "" {
 		var err error
-		release, err = utils.ParseRelease(distro, release)
+		release, err = utils.ParseRelease(createFlags.distro, createFlags.release)
 		if err != nil {
-			err := createErrorInvalidRelease(distro)
+			err := createErrorInvalidRelease()
 			return err
 		}
 	}
 
-	image, release, err := utils.ResolveImageName(distro, createFlags.image, release)
+	image, release, err := utils.ResolveImageName(createFlags.distro, createFlags.image, release)
 	if err != nil {
 		return err
 	}

diff --git a/src/cmd/enter.go b/src/cmd/enter.go
@@ -113,25 +113,19 @@ func enter(cmd *cobra.Command, args []string) error {
 		}
 	}
 
-	distro, err := utils.ResolveDistro(enterFlags.distro)
-	if err != nil {
-		err := createErrorInvalidDistro()
-		return err
-	}
-
-	release := enterFlags.release
-	if release != "" {
+	var release string
+	if enterFlags.release != "" {
 		defaultContainer = false
 
 		var err error
-		release, err = utils.ParseRelease(distro, release)
+		release, err = utils.ParseRelease(enterFlags.distro, enterFlags.release)
 		if err != nil {
-			err := createErrorInvalidRelease(distro)
+			err := createErrorInvalidRelease()
 			return err
 		}
 	}
 
-	image, release, err := utils.ResolveImageName(distro, "", release)
+	image, release, err := utils.ResolveImageName(enterFlags.distro, "", release)
 	if err != nil {
 		return err
 	}

diff --git a/src/cmd/run.go b/src/cmd/run.go
@@ -112,20 +112,14 @@ func run(cmd *cobra.Command, args []string) error {
 		}
 	}
 
-	distro, err := utils.ResolveDistro(runFlags.distro)
-	if err != nil {
-		err := createErrorInvalidDistro()
-		return err
-	}
-
-	release := runFlags.release
-	if release != "" {
+	var release string
+	if runFlags.release != "" {
 		defaultContainer = false
 
 		var err error
-		release, err = utils.ParseRelease(distro, release)
+		release, err = utils.ParseRelease(runFlags.distro, runFlags.release)
 		if err != nil {
-			err := createErrorInvalidRelease(distro)
+			err := createErrorInvalidRelease()
 			return err
 		}
 	}
@@ -141,7 +135,7 @@ func run(cmd *cobra.Command, args []string) error {
 
 	command := args
 
-	image, release, err := utils.ResolveImageName(distro, "", release)
+	image, release, err := utils.ResolveImageName(runFlags.distro, "", release)
 	if err != nil {
 		return err
 	}

diff --git a/src/cmd/utils.go b/src/cmd/utils.go
@@ -23,8 +23,6 @@ import (
 	"os/exec"
 	"strings"
 	"syscall"
-
-	"github.com/containers/toolbox/pkg/utils"
 )
 
 // askForConfirmation prints prompt to stdout and waits for response from the
@@ -71,20 +69,9 @@ func createErrorContainerNotFound(container string) error {
 	return errors.New(errMsg)
 }
 
-func createErrorInvalidDistro() error {
-	var builder strings.Builder
-	fmt.Fprintf(&builder, "invalid argument for '--distro'\n")
-	fmt.Fprintf(&builder, "Supported values are: %s\n", strings.Join(utils.GetSupportedDistros(), " "))
-	fmt.Fprintf(&builder, "Run '%s --help' for usage.", executableBase)
-
-	errMsg := builder.String()
-	return errors.New(errMsg)
-}
-
-func createErrorInvalidRelease(distro string) error {
+func createErrorInvalidRelease() error {
 	var builder strings.Builder
 	fmt.Fprintf(&builder, "invalid argument for '--release'\n")
-	fmt.Fprintf(&builder, "Supported values for distribution %s are in format: %s\n", distro, utils.GetReleaseFormat(distro))
 	fmt.Fprintf(&builder, "Run '%s --help' for usage.", executableBase)
 
 	errMsg := builder.String()

diff --git a/src/pkg/utils/utils.go b/src/pkg/utils/utils.go
@@ -44,7 +44,6 @@ type Distro struct {
 	ContainerNamePrefix    string
 	ImageBasename          string
 	ParseRelease           ParseReleaseFunc
-	ReleaseFormat          string
 	Registry               string
 	Repository             string
 	RepositoryNeedsRelease bool
@@ -61,10 +60,6 @@ const (
 	ContainerNameRegexp = "[a-zA-Z0-9][a-zA-Z0-9_.-]*"
 )
 
-var (
-	ErrUnsupportedDistro = errors.New("linux distribution is not supported")
-)
-
 var (
 	containerNamePrefixDefault = "fedora-toolbox"
 
@@ -103,7 +98,6 @@ var (
 			"fedora-toolbox",
 			"fedora-toolbox",
 			parseReleaseFedora,
-			"<release>/f<release>",
 			"registry.fedoraproject.org",
 			"",
 			false,
@@ -112,7 +106,6 @@ var (
 			"rhel-toolbox",
 			"toolbox",
 			parseReleaseRHEL,
-			"<major.minor>",
 			"registry.access.redhat.com",
 			"ubi8",
 			false,
@@ -249,8 +242,8 @@ func GetContainerNamePrefixForImage(image string) (string, error) {
 }
 
 func getDefaultImageForDistro(distro, release string) string {
-	if !IsDistroSupported(distro) {
-		distro = distroDefault
+	if _, supportedDistro := supportedDistros[distro]; !supportedDistro {
+		distro = "fedora"
 	}
 
 	distroObj, supportedDistro := supportedDistros[distro]
@@ -414,20 +407,6 @@ func GetMountOptions(target string) (string, error) {
 	return mountOptions, nil
 }
 
-// GetReleaseFormat returns the format string signifying supported release
-// version formats.
-//
-// distro should be value found under ID in os-release.
-//
-// If distro is unsupported an empty string is returned.
-func GetReleaseFormat(distro string) string {
-	if !IsDistroSupported(distro) {
-		return ""
-	}
-
-	return supportedDistros[distro].ReleaseFormat
-}
-
 func GetRuntimeDirectory(targetUser *user.User) (string, error) {
 	gid, err := strconv.Atoi(targetUser.Gid)
 	if err != nil {
@@ -484,14 +463,6 @@ func HumanDuration(duration int64) string {
 	return units.HumanDuration(time.Since(time.Unix(duration, 0))) + " ago"
 }
 
-// IsDistroSupported signifies if a distribution has a toolbx image for it.
-//
-// distro should be value found under ID in os-release.
-func IsDistroSupported(distro string) bool {
-	_, ok := supportedDistros[distro]
-	return ok
-}
-
 // ImageReferenceCanBeID checks if 'image' might be the ID of an image
 func ImageReferenceCanBeID(image string) bool {
 	matched, err := regexp.MatchString("^[a-f0-9]{6,64}$", image)
@@ -636,18 +607,16 @@ func ShortID(id string) string {
 	return id
 }
 
-// ParseRelease assesses if the requested version of a distribution is in
-// the correct format.
-//
-// If distro is an empty string, a default value (value under the
-// 'general.distro' key in a config file or 'fedora') is assumed.
 func ParseRelease(distro, release string) (string, error) {
 	if distro == "" {
-		distro, _ = ResolveDistro(distro)
+		distro = distroDefault
+		if viper.IsSet("general.distro") {
+			distro = viper.GetString("general.distro")
+		}
 	}
 
-	if !IsDistroSupported(distro) {
-		distro = distroDefault
+	if _, supportedDistro := supportedDistros[distro]; !supportedDistro {
+		distro = "fedora"
 	}
 
 	distroObj, supportedDistro := supportedDistros[distro]
@@ -752,33 +721,6 @@ func ResolveContainerName(container, image, release string) (string, error) {
 	return container, nil
 }
 
-// ResolveDistro assess if the requested distribution is supported and provides
-// a default value if none is requested.
-//
-// If distro is empty, and the "general.distro" key in a config file is set,
-// the value is read from the config file. If the key is not set, the default
-// value ('fedora') is used instead.
-func ResolveDistro(distro string) (string, error) {
-	logrus.Debug("Resolving distribution")
-	logrus.Debugf("Distribution: %s", distro)
-
-	if distro == "" {
-		distro = distroDefault
-		if viper.IsSet("general.distro") {
-			distro = viper.GetString("general.distro")
-		}
-	}
-
-	if !IsDistroSupported(distro) {
-		return "", ErrUnsupportedDistro
-	}
-
-	logrus.Debug("Resolved distribution")
-	logrus.Debugf("Distribution: %s", distro)
-
-	return distro, nil
-}
-
 // ResolveImageName standardizes the name of an image.
 //
 // If no image name is specified then the base image will reflect the platform of the host (even the version).
@@ -793,7 +735,10 @@ func ResolveImageName(distroCLI, imageCLI, releaseCLI string) (string, string, e
 	distro, image, release := distroCLI, imageCLI, releaseCLI
 
 	if distroCLI == "" {
-		distro, _ = ResolveDistro(distroCLI)
+		distro = distroDefault
+		if viper.IsSet("general.distro") {
+			distro = viper.GetString("general.distro")
+		}
 	}
 
 	if distro != distroDefault && releaseCLI == "" && !viper.IsSet("general.release") {