-
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 all 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"] |
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.