Respond to comments

cmd/juju/application/deploy.go

@@ -367,7 +367,7 @@ when refreshing the application in the future.
 A local charm may be deployed by giving the path to its directory:
 
   juju deploy /path/to/charm
-  juju deploy /path/to/charm ---base ubuntu@22.04
+  juju deploy /path/to/charm --base ubuntu@22.04
 
 You will need to be explicit if there is an ambiguity between a local and a
 remote charm:

cmd/juju/application/deployer/charm_test.go

@@ -79,7 +79,7 @@ func (s *charmSuite) TestRepositoryCharmDeployDryRunCompatibility(c *gc.C) {
 
 	dCharm := s.newDeployCharm()
 	dCharm.dryRun = true
-	dCharm.validateCharmBaseWithName = func(_ corebase.Base, _ string, _ string) error {
+	dCharm.validateCharmBaseWithName = func(corebase.Base, string, string) error {
 		return nil
 	}
 	repoCharm := &repositoryCharm{

cmd/juju/application/deployer/deployer.go

@@ -226,40 +226,39 @@ func (d *factory) localBundleDeployer() (DeployerKind, error) {
 }
 
 func (d *factory) localCharmDeployer(getter ModelConfigGetter) (DeployerKind, error) {
-	// Determine series
+	// Determine base
 	charmOrBundle := d.charmOrBundle
 	if isLocalSchema(charmOrBundle) {
 		charmOrBundle = charmOrBundle[6:]
 	}
-	base, imageStream, seriesErr := d.determineBaseForLocalCharm(charmOrBundle, getter)
-	if seriesErr != nil {
-		return nil, errors.Trace(seriesErr)
+	base, imageStream, baseErr := d.determineBaseForLocalCharm(charmOrBundle, getter)
+	if baseErr != nil {
+		return nil, errors.Trace(baseErr)
 	}
 
 	// Charm may have been supplied via a path reference.
 	ch, curl, err := corecharm.NewCharmAtPathForceBase(charmOrBundle, base, d.force)
 	// We check for several types of known error which indicate
 	// that the supplied reference was indeed a path but there was
 	// an issue reading the charm located there.
-	if corecharm.IsMissingSeriesError(err) {
+	if corecharm.IsMissingBaseError(err) {
 		return nil, err
-	} else if corecharm.IsUnsupportedSeriesError(err) {
+	} else if corecharm.IsUnsupportedBaseError(err) {
 		return nil, errors.Trace(err)
 	} else if errors.Cause(err) == zip.ErrFormat {
 		return nil, errors.Errorf("invalid charm or bundle provided at %q", charmOrBundle)
 	} else if errors.Is(err, errors.NotFound) {
 		return nil, errors.Wrap(err, errors.NotFoundf("charm or bundle at %q", charmOrBundle))
-	} else if err != nil && err != os.ErrNotExist {
+	} else if errors.Is(err, os.ErrNotExist) {
+		logger.Debugf("cannot interpret as local charm: %v", err)
+		return nil, nil
+	} else if err != nil {
 		// If we get a "not exists" error then we attempt to interpret
 		// the supplied charm reference as a URL elsewhere, otherwise
 		// we return the error.
 		return nil, errors.Annotatef(err, "attempting to deploy %q", charmOrBundle)
-	} else if err != nil {
-		logger.Debugf("cannot interpret as local charm: %v", err)
-		return nil, nil
-	} else {
-		return &localCharmDeployerKind{base, imageStream, ch, curl}, nil
 	}
+	return &localCharmDeployerKind{base, imageStream, ch, curl}, nil
 }
 
 func (d *factory) localPreDeployedCharmDeployer() (DeployerKind, error) {
@@ -277,61 +276,51 @@ func (d *factory) localPreDeployedCharmDeployer() (DeployerKind, error) {
 }
 
 func (d *factory) determineBaseForLocalCharm(charmOrBundle string, getter ModelConfigGetter) (corebase.Base, string, error) {
-	// TODO (cderici): check the validity of the comments below
-	// NOTE: Here we select the series using the algorithm defined by
-	// `baseSelector.charmBase`. This serves to override the algorithm found
-	// in `charmrepo.NewCharmAtPath` which is outdated (but must still be
-	// called since the code is coupled with path interpretation logic which
-	// cannot easily be factored out).
-
 	// NOTE: Reading the charm here is only meant to aid in inferring the
-	// correct series, if this fails we fall back to the argument series. If
-	// reading the charm fails here it will also fail below (the charm is read
-	// again below) where it is handled properly. This is just an expedient to
-	// get the correct series. A proper refactoring of the charmrepo package is
-	// needed for a more elegant fix.
+	// correct base. If this fails, we simply return with default values
+	// and trust the caller to handle this failure properly (the charm is
+	// read again later).
+	// TODO: A proper refactoring is required for a proper fix
 	var (
 		imageStream  string
 		selectedBase corebase.Base
 	)
 	ch, err := d.charmReader.ReadCharm(charmOrBundle)
-	if err == nil {
-		modelCfg, err := getModelConfig(getter)
-		if err != nil {
-			return corebase.Base{}, "", errors.Trace(err)
-		}
+	if err != nil {
+		return corebase.Base{}, "", nil
+	}
 
-		imageStream = modelCfg.ImageStream()
-		workloadBases, err := SupportedJujuBases(d.clock.Now(), d.base, imageStream)
-		if err != nil {
-			return corebase.Base{}, "", errors.Trace(err)
-		}
+	modelCfg, err := getModelConfig(getter)
+	if err != nil {
+		return corebase.Base{}, "", errors.Trace(err)
+	}
 
-		supportedBases, err := corecharm.ComputedBases(ch)
-		if err != nil {
-			return corebase.Base{}, "", errors.Trace(err)
-		}
-		if len(supportedBases) == 0 {
-			logger.Warningf("%s does not declare supported bases", ch.Meta().Name)
-		}
+	imageStream = modelCfg.ImageStream()
+	workloadBases, err := SupportedJujuBases(d.clock.Now(), d.base, imageStream)
+	if err != nil {
+		return corebase.Base{}, "", errors.Trace(err)
+	}
 
-		baseSelector, err := corecharm.ConfigureBaseSelector(corecharm.SelectorConfig{
-			Config:              modelCfg,
-			Force:               d.force,
-			Logger:              logger,
-			RequestedBase:       d.base,
-			SupportedCharmBases: supportedBases,
-			WorkloadBases:       workloadBases,
-			UsingImageID:        d.constraints.HasImageID() || d.modelConstraints.HasImageID(),
-		})
-		if err != nil {
-			return corebase.Base{}, "", errors.Trace(err)
-		}
+	supportedBases, err := corecharm.ComputedBases(ch)
+	if err != nil {
+		return corebase.Base{}, "", errors.Trace(err)
+	}
+	baseSelector, err := corecharm.ConfigureBaseSelector(corecharm.SelectorConfig{
+		Config:              modelCfg,
+		Force:               d.force,
+		Logger:              logger,
+		RequestedBase:       d.base,
+		SupportedCharmBases: supportedBases,
+		WorkloadBases:       workloadBases,
+		UsingImageID:        d.constraints.HasImageID() || d.modelConstraints.HasImageID(),
+	})
+	if err != nil {
+		return corebase.Base{}, "", errors.Trace(err)
+	}
 
-		selectedBase, err = baseSelector.CharmBase()
-		if err = charmValidationError(ch.Meta().Name, errors.Trace(err)); err != nil {
-			return corebase.Base{}, "", errors.Trace(err)
-		}
+	selectedBase, err = baseSelector.CharmBase()
+	if err = charmValidationError(ch.Meta().Name, errors.Trace(err)); err != nil {
+		return corebase.Base{}, "", errors.Trace(err)
 	}
 	return selectedBase, imageStream, nil
 }
@@ -569,7 +558,7 @@ func (dk *localPreDeployerKind) CreateDeployer(d factory) (Deployer, error) {
 }
 
 func (dk *localCharmDeployerKind) CreateDeployer(d factory) (Deployer, error) {
-	// Avoid deploying charm if the charm series is not correct for the
+	// Avoid deploying charm if the charm base is not correct for the
 	// available image streams.
 	var err error
 	if err = d.validateCharmBaseWithName(dk.base, dk.curl.Name, dk.imageStream); err != nil {