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.

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

Install rust globally #331

Merged
merged 36 commits into from
Oct 2, 2023
Merged
Show file tree
Hide file tree
Changes from 26 commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
a5b0c84
install rust globally
popenta Sep 13, 2023
7250ffa
try fix env
popenta Sep 13, 2023
69fba5e
code cleanup
popenta Sep 13, 2023
0bb5667
update version
popenta Sep 15, 2023
5295378
remove code related to old rust env
popenta Sep 15, 2023
2845d07
install rust for tests
popenta Sep 15, 2023
6906afa
remove explicit rust installation for workflow
popenta Sep 18, 2023
cad2cbd
Merge branch 'main' into globally-install-rust
popenta Sep 18, 2023
47c897e
add pytest flag for showing logs
popenta Sep 18, 2023
7ef887f
set flag for pytest logs
popenta Sep 18, 2023
6c22bac
remove pytest for testing purposes
popenta Sep 21, 2023
e9542fe
debugging
popenta Sep 21, 2023
67defe7
Merge branch 'main' into globally-install-rust
popenta Sep 21, 2023
45e9698
remove python 3.8 from GH action
popenta Sep 21, 2023
362cb9b
install sc-meta on GH action
popenta Sep 21, 2023
edb1c59
set rust toolchain to nightly
popenta Sep 21, 2023
0c9aaa9
remove default sc-meta installation when installing rust
popenta Sep 21, 2023
d7681dd
fix typo
popenta Sep 21, 2023
87df9d4
add new pytest marker
popenta Sep 22, 2023
84ce195
install sc-meta in GH workflow
popenta Sep 22, 2023
b3c6a0c
remove unnecessary prints
popenta Sep 22, 2023
3cbff95
bump version
popenta Sep 22, 2023
c597e34
started refactoring
popenta Sep 22, 2023
485bfaf
Install sc-meta when installing rust
popenta Sep 25, 2023
d5999e9
skip some tests on windows
popenta Sep 25, 2023
837b794
skip test on windows
popenta Sep 25, 2023
39f1b77
install wasm-opt and twiggy using cargo
popenta Sep 26, 2023
177d263
fix resolution
popenta Sep 27, 2023
648cb4c
Merge pull request #340 from multiversx/cargo-install-wasm-opt
popenta Sep 27, 2023
340e5e1
remove clang installation
popenta Sep 27, 2023
fc8609a
fix localnet and unit tests
popenta Sep 27, 2023
8eea743
fix workflow
popenta Sep 27, 2023
ad585ea
run GH actions using newer python version & wasm-opt fixes
popenta Sep 28, 2023
851bd30
fix test assert
popenta Sep 28, 2023
d2af8f5
fixes after review
popenta Sep 28, 2023
d8bb7a4
add link to mxpy cookbook
popenta Sep 28, 2023
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
76 changes: 38 additions & 38 deletions .github/workflows/build-windows.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,13 @@ name: build

on:
pull_request:
branches: [ main, feat/* ]
branches: [main, feat/*]
Copy link
Contributor

Choose a reason for hiding this comment

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

There are some unrelated whitespace changes in this file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll blame it on the formatter.

workflow_dispatch:

jobs:
build:
name: Build and Test mxpy for ${{ matrix.os }}, python ${{ matrix.python-version }}

runs-on: ${{ matrix.os }}

strategy:
Expand All @@ -20,39 +20,39 @@ jobs:
python-version: [3.8]

steps:
- uses: actions/checkout@v2
- name: Set up Python ${{ matrix.python-version }}
uses: actions/setup-python@v2
with:
python-version: ${{ matrix.python-version }}
- name: Install dependencies
shell: bash
run: |
python3 -m pip install --upgrade pip
pip3 install -r requirements.txt
pip3 install pytest
- name: Set github_api_token
shell: bash
run: |
mkdir ~/multiversx-sdk
export PYTHONPATH=.
python3 -m multiversx_sdk_cli.cli config new test
python3 -m multiversx_sdk_cli.cli config set github_api_token ${{ secrets.GITHUB_TOKEN }}
- name: Setup test dependencies
shell: bash
run: |
python3 -m multiversx_sdk_cli.cli deps install testwallets
- name: Run unit tests
shell: bash
run: |
export PYTHONPATH=.
pytest -m "not skip_on_windows" .
- name: Run CLI tests
shell: bash
run: |
export PROXY=https://testnet-gateway.multiversx.com
export CHAIN_ID=T
cd ./multiversx_sdk_cli/tests
source ./test_cli_tx.sh && testAll || return 1
source ./test_cli_dns.sh && testOffline || return 1
source ./test_cli_validators.sh && testAll || return 1
- uses: actions/checkout@v2
- name: Set up Python ${{ matrix.python-version }}
uses: actions/setup-python@v2
with:
python-version: ${{ matrix.python-version }}
- name: Install dependencies
shell: bash
run: |
python3 -m pip install --upgrade pip
pip3 install -r requirements.txt
pip3 install pytest
- name: Set github_api_token
shell: bash
run: |
mkdir ~/multiversx-sdk
export PYTHONPATH=.
python3 -m multiversx_sdk_cli.cli config new test
python3 -m multiversx_sdk_cli.cli config set github_api_token ${{ secrets.GITHUB_TOKEN }}
- name: Setup test dependencies
shell: bash
run: |
python3 -m multiversx_sdk_cli.cli deps install testwallets
- name: Run unit tests
shell: bash
run: |
export PYTHONPATH=.
pytest -m "not skip_on_windows" .
- name: Run CLI tests
shell: bash
run: |
export PROXY=https://testnet-gateway.multiversx.com
export CHAIN_ID=T
cd ./multiversx_sdk_cli/tests
source ./test_cli_tx.sh && testAll || return 1
source ./test_cli_dns.sh && testOffline || return 1
source ./test_cli_validators.sh && testAll || return 1
77 changes: 39 additions & 38 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,53 +5,54 @@ name: build

on:
pull_request:
branches: [ main, feat/* ]
branches: [main, feat/*]
workflow_dispatch:

jobs:
build:
name: Build and Test mxpy for ${{ matrix.os }}, python ${{ matrix.python-version }}

runs-on: ${{ matrix.os }}

strategy:
matrix:
os: [ubuntu-latest, macos-latest]
python-version: [3.8, 3.11]
python-version: [3.11]

steps:
- uses: actions/checkout@v2
- name: Set up Python ${{ matrix.python-version }}
uses: actions/setup-python@v2
with:
python-version: ${{ matrix.python-version }}
- name: Install dependencies
run: |
python3 -m pip install --upgrade pip
pip3 install -r requirements.txt
pip3 install pytest
- name: Install libtinfo5
if: ${{ matrix.os != 'macos-latest' }}
run: |
sudo apt update
sudo apt install -y libtinfo5
- name: Set github_api_token
run: |
mkdir ~/multiversx-sdk
export PYTHONPATH=.
python3 -m multiversx_sdk_cli.cli config new test
python3 -m multiversx_sdk_cli.cli config set github_api_token ${{ secrets.GITHUB_TOKEN }}
- name: Setup test dependencies
run: |
python3 -m multiversx_sdk_cli.cli deps install testwallets
python3 -m multiversx_sdk_cli.cli deps install wasm-opt
- name: Run unit tests
run: |
export PYTHONPATH=.
pytest .
- name: Run CLI tests
run: |
python3 -m multiversx_sdk_cli.cli config set dependencies.vmtools.tag v1.4.60
cd ./multiversx_sdk_cli/tests
source ./test_cli_contracts.sh && testAll || return 1
source ./test_cli_dns.sh && testOffline || return 1
- uses: actions/checkout@v2
- name: Set up Python ${{ matrix.python-version }}
uses: actions/setup-python@v2
with:
python-version: ${{ matrix.python-version }}
- name: Install dependencies
run: |
python3 -m pip install --upgrade pip
pip3 install -r requirements.txt
pip3 install pytest
- name: Install libtinfo5
if: ${{ matrix.os != 'macos-latest' }}
run: |
sudo apt update
sudo apt install -y libtinfo5
- name: Set github_api_token
run: |
mkdir ~/multiversx-sdk
export PYTHONPATH=.
python3 -m multiversx_sdk_cli.cli config new test
python3 -m multiversx_sdk_cli.cli config set github_api_token ${{ secrets.GITHUB_TOKEN }}
- name: Setup test dependencies
run: |
python3 -m multiversx_sdk_cli.cli deps install testwallets
python3 -m multiversx_sdk_cli.cli deps install wasm-opt
python3 -m multiversx_sdk_cli.cli deps install rust --overwrite
- name: Run unit tests
run: |
export PYTHONPATH=.
pytest .
- name: Run CLI tests
run: |
python3 -m multiversx_sdk_cli.cli config set dependencies.vmtools.tag v1.4.60
cd ./multiversx_sdk_cli/tests
source ./test_cli_contracts.sh && testAll || return 1
Copy link
Contributor

Choose a reason for hiding this comment

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

How come that we don't need to install clang / llvm, as well? No C smart contracts in our tests?

If so, we should add some tests for C contracts in the next PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we only use rust contracts in our tests

source ./test_cli_dns.sh && testOffline || return 1
1 change: 0 additions & 1 deletion multiversx_sdk_cli/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,6 @@ def get_defaults() -> Dict[str, Any]:
# ide.elrond.com will be removed, TBD if clang will still be downloaded
"dependencies.llvm.urlTemplate.linux": "https://ide.elrond.com/vendor-llvm/{TAG}/linux-amd64.tar.gz?t=19feb",
"dependencies.llvm.urlTemplate.osx": "https://ide.elrond.com/vendor-llvm/{TAG}/darwin-amd64.tar.gz?t=19feb",
"dependencies.rust.resolution": "SDK",
"dependencies.rust.tag": "nightly-2023-05-26",
"dependencies.golang.resolution": "SDK",
"dependencies.golang.tag": "go1.20.7",
Expand Down
115 changes: 45 additions & 70 deletions multiversx_sdk_cli/dependencies/modules.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
myprocess, utils, workstation)
from multiversx_sdk_cli.dependencies.resolution import (
DependencyResolution, get_dependency_resolution)
from multiversx_sdk_cli.ux import show_warning

logger = logging.getLogger("modules")

Expand All @@ -34,7 +35,6 @@
logger.info("Already exists. Skip install.")
return

self._guard_cannot_install_on_host()
self.uninstall(tag)
self._do_install(tag)

Expand Down Expand Up @@ -66,10 +66,6 @@
def get_resolution(self) -> DependencyResolution:
return get_dependency_resolution(self.key)

def _guard_cannot_install_on_host(self):
if self.get_resolution() == DependencyResolution.Host:
raise errors.KnownError(f"Installation of {self.key} on the host machine is not supported. Perhaps set 'dependencies.{self.key}.resolution' to 'SDK' in config?")


class StandaloneModule(DependencyModule):
def __init__(self,
Expand Down Expand Up @@ -256,6 +252,29 @@


class Rust(DependencyModule):
def is_installed(self, tag: str) -> bool:
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should remove tag from is_installed, as well. Can be done in next PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've thought about it, but the other modules need the parameter. I think. We can further look into this.

which_rustc = shutil.which("rustc")
which_cargo = shutil.which("cargo")
logger.info(f"which rustc: {which_rustc}")
logger.info(f"which cargo: {which_cargo}")

return which_rustc is not None and which_cargo is not None

def install(self, tag: str, overwrite: bool) -> None:
Copy link
Contributor

Choose a reason for hiding this comment

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

Indeed, here we completely override the function DependencyModule.install().

# Fallback to default tag if not provided
tag = tag or config.get_dependency_tag(self.key)

logger.info(f"install: key={self.key}, tag={tag}, overwrite={overwrite}")

if overwrite:
logger.info("Overwriting the current rust version...")
elif self._is_installed_and_set_to_nightly():
return

self._do_install(tag)
self._install_sc_meta()
self._post_install(tag)

def _do_install(self, tag: str) -> None:
installer_url = self._get_installer_url()
installer_path = self._get_installer_path()
Expand All @@ -269,12 +288,14 @@
toolchain = "nightly"

args = [str(installer_path), "--verbose", "--default-toolchain", toolchain, "--profile",
"minimal", "--target", "wasm32-unknown-unknown", "--no-modify-path", "-y"]
output = myprocess.run_process(args, env=self.get_env_for_install())
"minimal", "--target", "wasm32-unknown-unknown", "-y"]
Copy link
Contributor

Choose a reason for hiding this comment

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

🚀


myprocess.run_process(args)

if output:
sc_meta_args = ["cargo", "install", "multiversx-sc-meta"]
myprocess.run_process(sc_meta_args, env=self.get_env_for_install())
def _install_sc_meta(self):
logger.info("Installing multiversx-sc-meta")
args = ["cargo", "install", "multiversx-sc-meta"]
myprocess.run_process(args)

def _get_installer_url(self) -> str:
if workstation.is_windows():
Expand All @@ -293,73 +314,27 @@
if os.path.isdir(directory):
shutil.rmtree(directory)

def is_installed(self, tag: str) -> bool:
resolution = self.get_resolution()
def _is_installed_and_set_to_nightly(self) -> bool:
Copy link
Contributor

Choose a reason for hiding this comment

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

This function seems to be a clone of is_installed(). Do we still require it?

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. Removed it.

# the method parameter is not used in this specific module
is_rust_installed = self.is_installed("")

if resolution == DependencyResolution.Host:
which_rustc = shutil.which("rustc")
which_cargo = shutil.which("cargo")
logger.info(f"which rustc: {which_rustc}")
logger.info(f"which cargo: {which_cargo}")

return which_rustc is not None and which_cargo is not None
if resolution == DependencyResolution.SDK:
try:
env = self._get_env_for_is_installed_in_sdk()
myprocess.run_process(["rustc", "--version"], env=env)
return True
except Exception:
return False
if not is_rust_installed:
return is_rust_installed
Copy link
Contributor

Choose a reason for hiding this comment

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

Can say return False (more explicit). Then, continue without else (less indentation).

else:
self._recommend_default_rust_version()
Copy link
Contributor

Choose a reason for hiding this comment

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

Actually, let's move this warning in the if-else that starts on line 269. This way, _is_installed_and_set_to_nightly is a bit more "pure" (does not take any decision, does not produce any effect as a consequence of its own internal checks).

Copy link
Contributor

Choose a reason for hiding this comment

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

Additionally, you can directly inline the implementation of _recommend_default_rust_version inside install - also because the warning message mentions the "overwrite" flag, and the overwrite check is performed there.

Can also stay as it is.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved it inside the install() method.

return True

raise errors.BadDependencyResolution(self.key, resolution)
def _recommend_default_rust_version(self):
module = dependencies.get_module_by_key("rust")
default_tag: str = config.get_dependency_tag(module.key)

Check warning on line 329 in multiversx_sdk_cli/dependencies/modules.py

View workflow job for this annotation

GitHub Actions / runner / mypy

[mypy] reported by reviewdog 🐶 By default the bodies of untyped functions are not checked, consider using --check-untyped-defs [annotation-unchecked] Raw Output: /home/runner/work/mx-sdk-py-cli/mx-sdk-py-cli/multiversx_sdk_cli/dependencies/modules.py:329:9: note: By default the bodies of untyped functions are not checked, consider using --check-untyped-defs [annotation-unchecked]
show_warning(f"We recommend using rust {default_tag}. If you'd like to overwrite your current version please run `mxpy deps install rust --overwrite`.")

def get_directory(self, tag: str) -> Path:
tools_folder = workstation.get_tools_folder()
return tools_folder / "vendor-rust"

def get_env(self):
directory = self.get_directory("")
resolution = self.get_resolution()

if resolution == DependencyResolution.Host:
return {
"PATH": os.environ.get("PATH", ""),
"RUSTUP_HOME": os.environ.get("RUSTUP_HOME", ""),
"CARGO_HOME": os.environ.get("CARGO_HOME", "")
}
if resolution == DependencyResolution.SDK:
return {
# At this moment, cc (build-essential) is sometimes required by the meta crate (e.g. for reports)
"PATH": f"{path.join(directory, 'bin')}:{os.environ['PATH']}",
"RUSTUP_HOME": str(directory),
"CARGO_HOME": str(directory)
}

raise errors.BadDependencyResolution(self.key, resolution)

def _get_env_for_is_installed_in_sdk(self) -> Dict[str, str]:
directory = self.get_directory("")

return {
"PATH": str(directory / "bin"),
"RUSTUP_HOME": str(directory),
"CARGO_HOME": str(directory)
}

def get_env_for_install(self):
directory = self.get_directory("")

env = {
# For installation, wget (or curl) and cc (build-essential) are also required.
"PATH": f"{path.join(directory, 'bin')}:{os.environ['PATH']}",
"RUSTUP_HOME": str(directory),
"CARGO_HOME": str(directory)
}

if workstation.is_windows():
env["RUSTUP_USE_HYPER"] = "1"

return env
def get_env(self) -> Dict[str, str]:
return dict(os.environ)
Copy link
Contributor

Choose a reason for hiding this comment

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

Correct.


def get_latest_release(self) -> str:
raise errors.UnsupportedConfigurationValue("Rust tag must either be explicit, empty or 'nightly'")
Expand Down
10 changes: 10 additions & 0 deletions multiversx_sdk_cli/projects/project_rust.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import logging
import shutil
import subprocess
from pathlib import Path
from typing import Any, Dict, List, Set, cast
Expand Down Expand Up @@ -53,7 +54,14 @@ def prepare_build_wasm_args(self, args: List[str]):
self.get_output_folder()
])

def check_if_sc_meta_is_installed(self):
which_sc_meta = shutil.which("sc-meta")
Copy link
Contributor

Choose a reason for hiding this comment

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

How about checking if the rust dependency is installed (thus, all parts of the rust "module")?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

check_if_sc_meta_is_installed can actually be deleted. Before the code reaches this part, the ensure_dependencies_installed() is called. That calls the install() method of the Rust module. If not all the dependencies are installed, it should install them.


if which_sc_meta is None:
raise errors.KnownError("'sc-meta' is not installed. Run 'cargo install multiversx-sc-meta' then try again.")
Copy link
Contributor

Choose a reason for hiding this comment

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

👍


def run_meta(self):
self.check_if_sc_meta_is_installed()
env = self.get_env()

with_wasm_opt = not self.options.get("no-wasm-opt")
Expand Down Expand Up @@ -123,6 +131,8 @@ def get_env(self):
return dependencies.get_module_by_key("rust").get_env()

def build_wasm_with_debug_symbols(self, build_options: Dict[str, Any]):
self.check_if_sc_meta_is_installed()

cwd = self.path
env = self.get_env()
target_dir: str = build_options.get("target-dir", "")
Expand Down
2 changes: 1 addition & 1 deletion multiversx_sdk_cli/tests/shared.sh
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ PROXY="${PROXY:-http://localhost:7950}"
CHAIN_ID="${CHAIN_ID:-localnet}"
TestUser=./testdata/testUser.pem
TestUser2=./testdata/testUser2.pem
RUST_VERSION="nightly-2023-04-24"
RUST_VERSION="nightly-2023-05-26"

cleanSandbox() {
rm -rf ${SANDBOX}
Expand Down
Loading
Loading