Skip to content
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.

Sign up for GitHub

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

Merged
merged 7 commits into from
Jan 18, 2024
Merged

Fix "deps check all" command #388

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
d42df24
fix deps check all command
popenta Jan 5, 2024
3306bc1
fix deps check all and remove latest tags
popenta Jan 5, 2024
fca22c9
fix check of Rust version
popenta Jan 8, 2024
45375a9
refactoring and fixes
popenta Jan 9, 2024
57cace9
change if statement to check if rustup exists
popenta Jan 9, 2024
11f0f0d
source cargo after installing rust
popenta Jan 17, 2024
1ece80a
add cargo in env
popenta Jan 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 3 additions & 29 deletions multiversx_sdk_cli/cli_deps.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
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
from multiversx_sdk_cli.myprocess import run_process

logger = logging.getLogger("cli.deps")

Expand Down Expand Up @@ -51,35 +50,16 @@ def check(args: Any):
if len(missing_dependencies):
raise errors.DependenciesMissing(missing_dependencies)
return
if name == "rust":
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:
actual_rust = _get_actual_installed_rust_version()

if tag_to_check in actual_rust:
logger.info(f"[{module.key} {tag_to_check}] is installed.")
return
if "command not found" in actual_rust:
logger.warning("You have installed Rust without using `rustup`.")
return
else:
logger.warning(f"The Rust version you have installed does not match the recommended version.\nInstalled [{actual_rust}], expected [{tag_to_check}].")
return

raise errors.DependencyMissing(module.key, tag_to_check)
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:
if is_installed and name != "rust":
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For Rust, we don't display this log info? Or without this if the message will be duplicated by the one from modules.py? I think better to have it duplicated on the CLI than to have this custom if here (e.g. in the future we will forget its reason).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without the if statement the message will be duplicated by the one in modules.py. Why I added it it's because if you have a different Rust version other than the recommended one, you'll see a warning that the versions don't match but the log will show that the correct version is installed since it takes the tag from the config.
A problem that could occur with the other dependencies as well, now that I think about it, but is less common.

logger.info(f"[{module.key} {tag_to_check}] is installed.")
return

raise errors.DependencyMissing(module.key, tag_to_check)
elif not is_installed:
raise errors.DependencyMissing(module.key, tag_to_check)


def check_module_is_installed(module: DependencyModule, tag_to_check: str) -> bool:
Expand All @@ -90,9 +70,3 @@ def check_module_is_installed(module: DependencyModule, tag_to_check: str) -> bo

installed = module.is_installed(tag_to_check)
return installed


def _get_actual_installed_rust_version() -> str:
args = ["rustup", "default"]
output = run_process(args, dump_to_stdout=False)
return output.rstrip("\n")
23 changes: 20 additions & 3 deletions multiversx_sdk_cli/dependencies/modules.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -253,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:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error message might look differently on MacOS. E.g.

multiversx@papple ~ % rustupfoobar
zsh: command not found: rustupfoobar
multiversx@papple ~ %

Thus, we can check for "not found" in actual_version_installed perhaps?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed. Now the elif statement is as above.

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)
Expand Down Expand Up @@ -321,7 +338,7 @@ def _install_sc_meta(self):
tag = config.get_dependency_tag("sc-meta")
args = ["cargo", "install", "multiversx-sc-meta", "--locked"]

if tag != "":
if tag:
args.extend(["--version", tag])

myprocess.run_process(args)
Expand All @@ -337,7 +354,7 @@ def _install_twiggy(self):
tag = config.get_dependency_tag("twiggy")
args = ["cargo", "install", "twiggy"]

if tag != "":
if tag:
args.extend(["--version", tag])

myprocess.run_process(args)
Expand Down
Loading