-
Notifications
You must be signed in to change notification settings - Fork 36
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix "deps check all" command #388
Changes from 4 commits
d42df24
3306bc1
fca22c9
45375a9
57cace9
11f0f0d
1ece80a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,8 +1,9 @@ | ||
import logging | ||
from typing import Any | ||
from typing import Any, List, Tuple | ||
|
||
from multiversx_sdk_cli import cli_shared, config, dependencies, errors | ||
from multiversx_sdk_cli.dependencies.install import get_deps_dict | ||
from multiversx_sdk_cli.dependencies.modules import DependencyModule | ||
|
||
logger = logging.getLogger("cli.deps") | ||
|
||
|
@@ -34,16 +35,38 @@ def install(args: Any): | |
|
||
def check(args: Any): | ||
name: str = args.name | ||
module = dependencies.get_module_by_key(name) | ||
tag_to_check: str = config.get_dependency_tag(module.key) | ||
|
||
if name == "all": | ||
Comment on lines
+38
to
+39
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. tested and it seems to work now 👍 |
||
all_dependencies = dependencies.get_all_deps() | ||
missing_dependencies: List[Tuple[str, str]] = [] | ||
|
||
for dependency in all_dependencies: | ||
tag_to_check: str = config.get_dependency_tag(dependency.key) | ||
is_installed = check_module_is_installed(dependency, tag_to_check) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This does not handle There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed |
||
|
||
if not is_installed: | ||
missing_dependencies.append((dependency.key, tag_to_check)) | ||
|
||
if len(missing_dependencies): | ||
raise errors.DependenciesMissing(missing_dependencies) | ||
return | ||
else: | ||
module = dependencies.get_module_by_key(name) | ||
tag_to_check: str = config.get_dependency_tag(module.key) | ||
|
||
is_installed = check_module_is_installed(module, tag_to_check) | ||
if is_installed and name != "rust": | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For Rust, we don't display this log info? Or without this There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Without the |
||
logger.info(f"[{module.key} {tag_to_check}] is installed.") | ||
return | ||
elif not is_installed: | ||
raise errors.DependencyMissing(module.key, tag_to_check) | ||
|
||
|
||
def check_module_is_installed(module: DependencyModule, tag_to_check: str) -> bool: | ||
resolution: str = config.get_dependency_resolution(module.key) | ||
resolution = resolution if resolution else "HOST" | ||
|
||
logger.info(f"Checking dependency: module = {module.key}, tag = {tag_to_check}, resolution = {resolution}") | ||
|
||
installed = module.is_installed(tag_to_check) | ||
if installed: | ||
logger.info(f"[{name} {tag_to_check}] is installed.") | ||
return | ||
|
||
raise errors.DependencyMissing(name, tag_to_check) | ||
return installed |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,6 @@ | ||
from multiversx_sdk_cli.dependencies.install import install_module, get_module_directory, get_module_by_key, get_golang | ||
from multiversx_sdk_cli.dependencies.install import (get_all_deps, get_golang, | ||
get_module_by_key, | ||
get_module_directory, | ||
install_module) | ||
|
||
__all__ = ["install_module", "get_module_directory", "get_module_by_key", "get_golang"] | ||
__all__ = ["install_module", "get_module_directory", "get_module_by_key", "get_golang", "get_all_deps"] |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -26,9 +26,6 @@ def install(self, overwrite: bool) -> None: | |
# We install the default tag | ||
tag = config.get_dependency_tag(self.key) | ||
|
||
if tag == 'latest': | ||
tag = self.get_latest_release() | ||
|
||
logger.info(f"install: key={self.key}, tag={tag}, overwrite={overwrite}") | ||
|
||
if self._should_skip(tag, overwrite): | ||
|
@@ -60,9 +57,6 @@ def is_installed(self, tag: str) -> bool: | |
def get_env(self) -> Dict[str, str]: | ||
raise NotImplementedError() | ||
|
||
def get_latest_release(self) -> str: | ||
raise NotImplementedError() | ||
|
||
def get_resolution(self) -> DependencyResolution: | ||
return get_dependency_resolution(self.key) | ||
|
||
|
@@ -135,14 +129,6 @@ def _get_download_url(self, tag: str) -> str: | |
url = url.replace("{TAG}", tag) | ||
return url | ||
|
||
def get_latest_release(self) -> str: | ||
if self.repo_name is None or self.organisation is None: | ||
raise ValueError(f'{self.key}: repo_name or organisation not specified') | ||
|
||
org_repo = f'{self.organisation}/{self.repo_name}' | ||
tag = utils.query_latest_release_tag(org_repo) | ||
return tag | ||
|
||
def _get_archive_path(self, tag: str) -> Path: | ||
tools_folder = Path(workstation.get_tools_folder()) | ||
archive = tools_folder / f"{self.key}.{tag}.{self.archive_type}" | ||
|
@@ -252,9 +238,6 @@ def get_env(self) -> Dict[str, str]: | |
def get_gopath(self) -> Path: | ||
return self.get_parent_directory() / "GOPATH" | ||
|
||
def get_latest_release(self) -> str: | ||
raise errors.UnsupportedConfigurationValue("Golang tag must always be explicit, not latest") | ||
|
||
|
||
class Rust(DependencyModule): | ||
def is_installed(self, tag: str) -> bool: | ||
|
@@ -270,7 +253,24 @@ def is_installed(self, tag: str) -> bool: | |
logger.info(f"which twiggy: {which_twiggy}") | ||
|
||
dependencies = [which_rustc, which_cargo, which_sc_meta, which_wasm_opt, which_twiggy] | ||
return all(dependency is not None for dependency in dependencies) | ||
installed = all(dependency is not None for dependency in dependencies) | ||
|
||
if installed: | ||
actual_version_installed = self._get_actual_installed_version() | ||
|
||
if tag in actual_version_installed: | ||
logger.info(f"[{self.key} {tag}] is installed.") | ||
elif "Command 'rustup' not found" in actual_version_installed: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The error message might look differently on MacOS. E.g.
Thus, we can check for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Indeed. Now the |
||
show_warning("You have installed Rust without using `rustup`.") | ||
else: | ||
show_warning(f"The Rust version you have installed does not match the recommended version.\nInstalled [{actual_version_installed}], expected [{tag}].") | ||
|
||
return installed | ||
|
||
def _get_actual_installed_version(self) -> str: | ||
args = ["rustup", "default"] | ||
output = myprocess.run_process(args, dump_to_stdout=False) | ||
return output.strip() | ||
|
||
def install(self, overwrite: bool) -> None: | ||
self._check_install_env(apply_correction=overwrite) | ||
|
@@ -338,7 +338,7 @@ def _install_sc_meta(self): | |
tag = config.get_dependency_tag("sc-meta") | ||
args = ["cargo", "install", "multiversx-sc-meta", "--locked"] | ||
|
||
if tag != "latest": | ||
if tag: | ||
args.extend(["--version", tag]) | ||
|
||
myprocess.run_process(args) | ||
|
@@ -354,7 +354,7 @@ def _install_twiggy(self): | |
tag = config.get_dependency_tag("twiggy") | ||
args = ["cargo", "install", "twiggy"] | ||
|
||
if tag != "latest": | ||
if tag: | ||
args.extend(["--version", tag]) | ||
|
||
myprocess.run_process(args) | ||
|
@@ -383,9 +383,6 @@ def get_directory(self, tag: str) -> Path: | |
def get_env(self) -> Dict[str, str]: | ||
return dict(os.environ) | ||
|
||
def get_latest_release(self) -> str: | ||
raise errors.UnsupportedConfigurationValue("Rust tag must either be explicit, empty or 'nightly'") | ||
|
||
|
||
class TestWalletsModule(StandaloneModule): | ||
def __init__(self, key: str): | ||
|
This file was deleted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After re-thinking about this, maybe we can drop
check all
andinstall all
? On the long run, maybe it's better (less issues, less corner cases) if we do.Let's double check with @ovidiuolteanu and @ccorcoveanu. I feel
deps install all
is quite exotic.