Cleanup dev-platform package installer

platformio/commands/platform.py

@@ -20,6 +20,8 @@ import click
 
 from platformio.cache import cleanup_content_cache
 from platformio.commands.boards import print_boards
+from platformio.exception import UserSideException
+from platformio.package.exception import UnknownPackageError
 from platformio.package.manager.platform import PlatformPackageManager
 from platformio.package.meta import PackageItem, PackageSpec
 from platformio.package.version import get_original_version
@@ -179,7 +181,7 @@ def platform_show(platform, json_output):  # pylint: disable=too-many-branches
     is_flag=True,
     help="Reinstall/redownload dev/platform and its packages if exist",
 )
-def platform_install(  # pylint: disable=too-many-arguments
+def platform_install(  # pylint: disable=too-many-arguments,too-many-locals
     platforms,
     with_package,
     without_package,
@@ -188,37 +190,50 @@ def platform_install(  # pylint: disable=too-many-arguments
     silent,
     force,
 ):
-    return _platform_install(
+    def _find_pkg_names(p, candidates):
-        platforms,
+        result = []
-        with_package,
+        for candidate in candidates:
-        without_package,
+            found = False
-        skip_default_package,
+            # lookup by package types
-        with_all_packages,
+            for _name, _opts in p.packages.items():
-        silent,
+                if _opts.get("type") == candidate:
-        force,
+                    result.append(_name)
-    )
+                    found = True
+            if (
+                p.frameworks
+                and candidate.startswith("framework-")
+                and candidate[10:] in p.frameworks
+            ):
+                result.append(p.frameworks[candidate[10:]]["package"])
+                found = True
+            if not found:
+                result.append(candidate)
+        return result
 
-
-def _platform_install(  # pylint: disable=too-many-arguments
-    platforms,
-    with_package=None,
-    without_package=None,
-    skip_default_package=False,
-    with_all_packages=False,
-    silent=False,
-    force=False,
-):
     pm = PlatformPackageManager()
     pm.set_log_level(logging.WARN if silent else logging.DEBUG)
     for platform in platforms:
-        pkg = pm.install(
+        if with_package or without_package or with_all_packages:
-            spec=platform,
+            pkg = pm.install(platform, skip_dependencies=True)
-            with_packages=with_package or [],
+            p = PlatformFactory.new(pkg)
-            without_packages=without_package or [],
+            if with_all_packages:
-            skip_default_package=skip_default_package,
+                with_package = list(p.packages)
-            with_all_packages=with_all_packages,
+            with_package = set(_find_pkg_names(p, with_package or []))
-            force=force,
+            without_package = set(_find_pkg_names(p, without_package or []))
-        )
+            upkgs = with_package | without_package
+            ppkgs = set(p.packages)
+            if not upkgs.issubset(ppkgs):
+                raise UnknownPackageError(", ".join(upkgs - ppkgs))
+            for name, options in p.packages.items():
+                if name in without_package:
+                    continue
+                if name in with_package or not (
+                    skip_default_package or options.get("optional", False)
+                ):
+                    p.pm.install(p.get_package_spec(name), force=force)
+        else:
+            pkg = pm.install(platform, skip_dependencies=skip_default_package)
+
         if pkg and not silent:
             click.secho(
                 "The platform '%s' has been successfully installed!\n"

platformio/package/commands/install.py

@@ -70,7 +70,7 @@ def install_global_dependencies(options):
     for spec in options.get("platforms"):
         pm.install(
             spec,
-            skip_default_package=options.get("skip_dependencies"),
+            skip_dependencies=options.get("skip_dependencies"),
             force=options.get("force"),
         )
     for spec in options.get("tools"):
@@ -145,7 +145,7 @@ def _install_project_env_platform(project_env, options):
         spec,
         project_env=project_env,
         project_targets=options.get("project_targets"),
-        skip_default_package=options.get("skip_dependencies"),
+        skip_dependencies=options.get("skip_dependencies"),
         force=options.get("force"),
     )
     return not already_up_to_date
@@ -163,7 +163,7 @@ def _install_project_env_custom_platforms(project_env, options):
             spec,
             project_env=project_env,
             project_targets=options.get("project_targets"),
-            skip_default_package=options.get("skip_dependencies"),
+            skip_dependencies=options.get("skip_dependencies"),
             force=options.get("force"),
         )
     return not already_up_to_date

platformio/package/commands/uninstall.py

@@ -134,7 +134,7 @@ def _uninstall_project_env_platform(project_env, options):
         pm.set_log_level(logging.WARN)
     spec = config.get(f"env:{project_env}", "platform")
     if not spec:
-        return False
+        return None
     already_up_to_date = True
     if not pm.get_package(spec):
         return None

platformio/package/manager/platform.py

@@ -38,13 +38,10 @@ class PlatformPackageManager(BasePackageManager):  # pylint: disable=too-many-an
     def manifest_names(self):
         return PackageType.get_manifest_map()[PackageType.PLATFORM]
 
-    def install(  # pylint: disable=arguments-differ, too-many-arguments
+    def install(  # pylint: disable=arguments-differ,too-many-arguments
         self,
         spec,
-        with_packages=None,
+        skip_dependencies=False,
-        without_packages=None,
-        skip_default_package=False,
-        with_all_packages=False,
         force=False,
         project_env=None,
         project_targets=None,
@@ -55,26 +52,16 @@ class PlatformPackageManager(BasePackageManager):  # pylint: disable=too-many-an
         )
         try:
             p = PlatformFactory.new(pkg)
+            # set logging level for underlying tool manager
+            p.pm.set_log_level(self.log.getEffectiveLevel())
             p.ensure_engine_compatible()
         except IncompatiblePlatform as e:
             super(PlatformPackageManager, self).uninstall(pkg, skip_dependencies=True)
             raise e
-
-        # set logging level for underlying tool manager
-        p.pm.set_log_level(self.log.getEffectiveLevel())
-
         if project_env:
             p.configure_project_packages(project_env, project_targets)
-
+        if not skip_dependencies:
-        if with_all_packages:
+            p.install_required_packages(force=force)
-            with_packages = list(p.packages)
-
-        p.install_packages(
-            with_packages,
-            without_packages,
-            skip_default_package,
-            force=force,
-        )
         if not already_installed:
             p.on_installed()
         return pkg

platformio/platform/_packages.py

@@ -12,7 +12,6 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-from platformio.package.exception import UnknownPackageError
 from platformio.package.meta import PackageSpec
 
 
@@ -65,67 +64,19 @@ class PlatformPackagesMixin(object):
             result.append(item)
         return result
 
-    def install_packages(  # pylint: disable=too-many-arguments
+    def install_required_packages(self, force=False):
-        self,
-        with_packages=None,
-        without_packages=None,
-        skip_default_package=False,
-        force=False,
-    ):
-        with_packages = set(self._find_pkg_names(with_packages or []))
-        without_packages = set(self._find_pkg_names(without_packages or []))
-
-        upkgs = with_packages | without_packages
-        ppkgs = set(self.packages)
-        if not upkgs.issubset(ppkgs):
-            raise UnknownPackageError(", ".join(upkgs - ppkgs))
-
         for name, options in self.packages.items():
-            if name in without_packages:
+            if options.get("optional"):
                 continue
-            if name in with_packages or not (
-                skip_default_package or options.get("optional", False)
-            ):
             self.pm.install(self.get_package_spec(name), force=force)
 
-        return True
-
-    def _find_pkg_names(self, candidates):
-        result = []
-        for candidate in candidates:
-            found = False
-
-            # lookup by package types
-            for _name, _opts in self.packages.items():
-                if _opts.get("type") == candidate:
-                    result.append(_name)
-                    found = True
-
-            if (
-                self.frameworks
-                and candidate.startswith("framework-")
-                and candidate[10:] in self.frameworks
-            ):
-                result.append(self.frameworks[candidate[10:]]["package"])
-                found = True
-
-            if not found:
-                result.append(candidate)
-
-        return result
-
     def uninstall_packages(self):
         for pkg in self.get_installed_packages():
             self.pm.uninstall(pkg)
 
-    def update_packages(self, only_check=False):
+    def update_packages(self):
         for pkg in self.get_installed_packages():
-            self.pm.update(
+            self.pm.update(pkg, to_spec=self.get_package_spec(pkg.metadata.name))
-                pkg,
-                to_spec=self.get_package_spec(pkg.metadata.name),
-                only_check=only_check,
-                show_incompatible=False,
-            )
 
     def are_outdated_packages(self):
         for pkg in self.get_installed_packages():

tests/commands/pkg/test_install.py

@@ -56,13 +56,13 @@ def test_global_packages(
         [
             "--global",
             "-l",
-            "milesburton/DallasTemperature@^3.9.1",
+            "https://github.com/milesburton/Arduino-Temperature-Control-Library.git#3.9.0",
             "--skip-dependencies",
         ],
     )
     validate_cliresult(result)
     assert pkgs_to_specs(LibraryPackageManager().get_installed()) == [
-        PackageSpec("DallasTemperature@3.9.1")
+        PackageSpec("DallasTemperature@3.9.0+sha.964939d")
     ]
     # with dependencies
     result = clirunner.invoke(
@@ -70,7 +70,7 @@ def test_global_packages(
         [
             "--global",
             "-l",
-            "milesburton/DallasTemperature@^3.9.1",
+            "https://github.com/milesburton/Arduino-Temperature-Control-Library.git#3.9.0",
             "-l",
             "bblanchon/ArduinoJson@^6.19.2",
         ],
@@ -78,7 +78,7 @@ def test_global_packages(
     validate_cliresult(result)
     assert pkgs_to_specs(LibraryPackageManager().get_installed()) == [
         PackageSpec("ArduinoJson@6.19.3"),
-        PackageSpec("DallasTemperature@3.9.1"),
+        PackageSpec("DallasTemperature@3.9.0+sha.964939d"),
         PackageSpec("OneWire@2.3.6"),
     ]
     # custom storage

tests/package/test_manager.py

@@ -455,7 +455,7 @@ def test_update_with_metadata(isolated_pio_core, tmpdir_factory):
     assert outdated.latest > semantic_version.Version("1.10.0")
 
     pkg = lm.install("ArduinoJson @ 5.10.1")
-    # tesy latest
+    # test latest
     outdated = lm.outdated(pkg)
     assert str(outdated.current) == "5.10.1"
     assert outdated.wanted is None