From 4b2db4e04ef3d35ae3edf60838558f75d8a62676 Mon Sep 17 00:00:00 2001 From: Alex Myers Date: Mon, 13 Nov 2023 15:58:54 -0600 Subject: [PATCH 1/8] reckless: properly remove entry from reckless sources Fixes a bug introduced in 61631384203fda2a3ef04407d88b53c1af589dde which avoided gratuitous rewrites of the lightningd config --- tests/test_reckless.py | 32 +++++++++++++++++++++++++++++++- tools/reckless | 6 +++--- 2 files changed, 34 insertions(+), 4 deletions(-) diff --git a/tests/test_reckless.py b/tests/test_reckless.py index ac4f0db19ee5..498fae982c8c 100644 --- a/tests/test_reckless.py +++ b/tests/test_reckless.py @@ -101,7 +101,8 @@ def get_reckless_node(node_factory): def check_stderr(stderr): def output_okay(out): - for warning in ['[notice]', 'WARNING:', 'npm WARN', 'npm notice']: + for warning in ['[notice]', 'WARNING:', 'npm WARN', + 'npm notice', 'DEPRECATION:']: if out.startswith(warning): return True return False @@ -135,6 +136,35 @@ def test_sources(node_factory): n = get_reckless_node(node_factory) r = reckless(["source", "-h"], dir=n.lightning_dir) assert r.returncode == 0 + r = reckless(["source", "list"], dir=n.lightning_dir) + print(r.stdout) + assert r.returncode == 0 + print(n.lightning_dir) + reckless_dir = Path(n.lightning_dir) / 'reckless' + print(dir(reckless_dir)) + assert (reckless_dir / '.sources').exists() + print(os.listdir(reckless_dir)) + print(reckless_dir / '.sources') + r = reckless([f"--network={NETWORK}", "-v", "source", "add", + "tests/data/recklessrepo/lightningd/testplugfail"], + dir=n.lightning_dir) + r = reckless([f"--network={NETWORK}", "-v", "source", "add", + "tests/data/recklessrepo/lightningd/testplugpass"], + dir=n.lightning_dir) + with open(reckless_dir / '.sources') as sources: + contents = [c.strip() for c in sources.readlines()] + print('contents:', contents) + assert 'https://github.com/lightningd/plugins' in contents + assert "tests/data/recklessrepo/lightningd/testplugfail" in contents + assert "tests/data/recklessrepo/lightningd/testplugpass" in contents + r = reckless([f"--network={NETWORK}", "-v", "source", "remove", + "tests/data/recklessrepo/lightningd/testplugfail"], + dir=n.lightning_dir) + with open(reckless_dir / '.sources') as sources: + contents = [c.strip() for c in sources.readlines()] + print('contents:', contents) + assert "tests/data/recklessrepo/lightningd/testplugfail" not in contents + assert "tests/data/recklessrepo/lightningd/testplugpass" in contents def test_search(node_factory): diff --git a/tools/reckless b/tools/reckless index 531527da59b7..df283cabd594 100755 --- a/tools/reckless +++ b/tools/reckless @@ -474,10 +474,10 @@ class Config(): # The white space is getting excessive. remove_these_lines.append(n) continue - if not addline: + if not addline and not write_required: return # No write necessary if addline is already in config. - if not write_required: + if addline and not write_required: for line in original: if line.strip() == addline.strip(): return @@ -490,7 +490,7 @@ class Config(): conf_write.write(f'\n{l.strip()}') else: conf_write.write(l.strip()) - if addline.strip() == l.strip(): + if addline and addline.strip() == l.strip(): # addline is idempotent line_exists = True if not line_exists: From 4d43a22e52212f959a40e0734212aec3b25e5960 Mon Sep 17 00:00:00 2001 From: Alex Myers Date: Fri, 26 Jan 2024 09:58:17 -0600 Subject: [PATCH 2/8] reckless: add staging directory and symlink This creates a separate staging directory from the one used to clone the source. A symlink is then added to the plugin's entrypoint which is now in the source directory. --- tools/reckless | 46 +++++++++++++++++++++++++++++++++++++--------- 1 file changed, 37 insertions(+), 9 deletions(-) diff --git a/tools/reckless b/tools/reckless index df283cabd594..f22bc810e566 100755 --- a/tools/reckless +++ b/tools/reckless @@ -648,6 +648,12 @@ def _git_clone(src: InstInfo, dest: Union[PosixPath, str]) -> bool: return True +def get_temp_reckless_dir() -> PosixPath: + random_dir = 'reckless-{}'.format(str(hash(os.times()))[-9:]) + new_path = Path(tempfile.gettempdir()) / random_dir + return new_path + + def _install_plugin(src: InstInfo) -> Union[InstInfo, None]: """make sure the repo exists and clone it.""" logging.debug(f'Install requested from {src}.') @@ -656,8 +662,15 @@ def _install_plugin(src: InstInfo) -> Union[InstInfo, None]: sys.exit(2) # Use a unique directory for each cloned repo. - clone_path = 'reckless-{}'.format(str(hash(os.times()))[-9:]) - clone_path = Path(tempfile.gettempdir()) / clone_path + tmp_path = get_temp_reckless_dir() + if not create_dir(tmp_path): + logging.debug(f'failed to create {tmp_path}') + return None + clone_path = tmp_path / 'clone' + if not create_dir(tmp_path): + logging.debug(f'failed to create {clone_path}') + return None + # we rename the original repo here. plugin_path = clone_path / src.name inst_path = Path(RECKLESS_CONFIG.reckless_dir) / src.name if Path(clone_path).exists(): @@ -709,15 +722,30 @@ def _install_plugin(src: InstInfo) -> Union[InstInfo, None]: if not cloned_src.entry: # The plugin entrypoint may not be discernable prior to cloning. # Need to search the newly cloned directory, not the original - cloned_src.src_loc = plugin_path + cloned_src.source_loc = plugin_path + + # Relocate plugin to a staging directory prior to testing + staging_path = tmp_path / src.name / 'source' + shutil.copytree(str(plugin_path), staging_path) + staged_src = cloned_src + # Because the source files are copied to a 'source' directory, the + # get_inst_details function no longer works. (dir must match plugin name) + # Set these manually instead. + staged_src.source_loc = str(staging_path.parent) + staged_src.srctype = Source.DIRECTORY + staged_src.subdir = None + # Create symlink in staging tree to redirect to the plugins entrypoint + Path(staging_path.parent / cloned_src.entry).\ + symlink_to(staging_path / cloned_src.entry) + # try it out if INSTALLER.dependency_call: for call in INSTALLER.dependency_call: logging.debug(f"Install: invoking '{' '.join(call)}'") if logging.root.level < logging.WARNING: - pip = Popen(call, cwd=plugin_path, text=True) + pip = Popen(call, cwd=staging_path, text=True) else: - pip = Popen(call, cwd=plugin_path, stdout=PIPE, stderr=PIPE, + pip = Popen(call, cwd=staging_path, stdout=PIPE, stderr=PIPE, text=True) pip.wait() # FIXME: handle output of multiple calls @@ -731,8 +759,8 @@ def _install_plugin(src: InstInfo) -> Union[InstInfo, None]: return None test_log = [] try: - test = run([Path(plugin_path).joinpath(cloned_src.entry)], - cwd=str(plugin_path), stdout=PIPE, stderr=PIPE, + test = run([Path(staging_path).joinpath(cloned_src.entry)], + cwd=str(staging_path), stdout=PIPE, stderr=PIPE, text=True, timeout=3) for line in test.stderr: test_log.append(line.strip('\n')) @@ -748,10 +776,10 @@ def _install_plugin(src: InstInfo) -> Union[InstInfo, None]: return None # Find this cute little plugin a forever home - shutil.copytree(str(plugin_path), inst_path) + shutil.copytree(str(staging_path), inst_path) print(f'plugin installed: {inst_path}') remove_dir(clone_path) - return cloned_src + return staged_src def install(plugin_name: str): From 6bd6867ff2f75fef8e53e6597e96a02fed2e5296 Mon Sep 17 00:00:00 2001 From: Alex Myers Date: Fri, 26 Jan 2024 10:08:46 -0600 Subject: [PATCH 3/8] reckless: add installation metadata The metadata includes an original retrieval source, timestamp, and commit hash. This will allow a future update command to more easily evaluate the status of the existing installation. --- tools/reckless | 33 ++++++++++++++++++++++++++++----- 1 file changed, 28 insertions(+), 5 deletions(-) diff --git a/tools/reckless b/tools/reckless index f22bc810e566..625a99478ff7 100755 --- a/tools/reckless +++ b/tools/reckless @@ -1,19 +1,21 @@ #!/usr/bin/env python3 -from subprocess import Popen, PIPE, TimeoutExpired, run import sys +import argparse +import copy +import datetime +from enum import Enum import json +import logging import os -import argparse from pathlib import Path, PosixPath import shutil +from subprocess import Popen, PIPE, TimeoutExpired, run import tempfile +import time from typing import Union from urllib.parse import urlparse from urllib.request import urlopen -import logging -import copy -from enum import Enum logging.basicConfig( @@ -654,6 +656,26 @@ def get_temp_reckless_dir() -> PosixPath: return new_path +def add_installation_metadata(installed: InstInfo, + original_request: InstInfo): + """Document the install request and installation details for use when + updating the plugin.""" + install_dir = Path(installed.source_loc) + assert install_dir.is_dir() + data = ('installation date\n' + f'{datetime.date.today().isoformat()}\n' + 'installation time\n' + f'{int(time.time())}\n' + 'original source\n' + f'{original_request.source_loc}\n' + 'requested commit\n' + f'{original_request.commit}\n' + 'installed commit\n' + f'{installed.commit}\n') + with open(install_dir / '.metadata', 'w') as metadata: + metadata.write(data) + + def _install_plugin(src: InstInfo) -> Union[InstInfo, None]: """make sure the repo exists and clone it.""" logging.debug(f'Install requested from {src}.') @@ -777,6 +799,7 @@ def _install_plugin(src: InstInfo) -> Union[InstInfo, None]: # Find this cute little plugin a forever home shutil.copytree(str(staging_path), inst_path) + add_installation_metadata(staged_src, src) print(f'plugin installed: {inst_path}') remove_dir(clone_path) return staged_src From 06695475817fabab976ba3047c463676b9744130 Mon Sep 17 00:00:00 2001 From: Alex Myers Date: Fri, 26 Jan 2024 10:53:02 -0600 Subject: [PATCH 4/8] reckless: add option to check out a specific commit ... using the syntax reckless install @ --- tools/reckless | 68 +++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 54 insertions(+), 14 deletions(-) diff --git a/tools/reckless b/tools/reckless index 625a99478ff7..a4b881e438f0 100755 --- a/tools/reckless +++ b/tools/reckless @@ -676,6 +676,45 @@ def add_installation_metadata(installed: InstInfo, metadata.write(data) +def _checkout_commit(orig_src: InstInfo, + cloned_src: InstInfo, + cloned_path: PosixPath): + # Check out and verify commit/tag if source was a repository + if orig_src.srctype in [Source.LOCAL_REPO, Source.GITHUB_REPO, + Source.OTHER_URL]: + if orig_src.commit: + logging.debug(f"Checking out {orig_src.commit}") + checkout = Popen(['git', 'checkout', orig_src.commit], + cwd=str(cloned_path), + stdout=PIPE, stderr=PIPE) + checkout.wait() + if checkout.returncode != 0: + print('failed to checkout referenced ' + f'commit {orig_src.commit}') + return None + else: + logging.debug("using latest commit of default branch") + + # Log the commit we actually used (for installation metadata) + git = run(['git', 'rev-parse', 'HEAD'], cwd=str(cloned_path), + stdout=PIPE, stderr=PIPE, text=True, check=False, timeout=60) + if git.returncode == 0: + head_commit = git.stdout.splitlines()[0] + logging.debug(f'checked out HEAD: {head_commit}') + cloned_src.commit = head_commit + else: + logging.debug(f'unable to collect commit: {git.stderr}') + else: + if orig_src.commit: + logging.warning("unable to checkout commit/tag on non-repository " + "source") + return cloned_path + + if cloned_src.subdir is not None: + return Path(cloned_src.source_loc) / cloned_src.subdir + return cloned_path + + def _install_plugin(src: InstInfo) -> Union[InstInfo, None]: """make sure the repo exists and clone it.""" logging.debug(f'Install requested from {src}.') @@ -716,16 +755,11 @@ def _install_plugin(src: InstInfo) -> Union[InstInfo, None]: if not cloned_src: logging.debug('failed to find plugin after cloning repo.') return None - if cloned_src.subdir is not None: - plugin_path = Path(cloned_src.source_loc) / cloned_src.subdir - if cloned_src.commit: - logging.debug(f"Checking out commit {cloned_src.commit}") - checkout = Popen(['git', 'checkout', cloned_src.commit], - cwd=str(plugin_path), stdout=PIPE, stderr=PIPE) - checkout.wait() - if checkout.returncode != 0: - print(f'failed to checkout referenced commit {cloned_src.commit}') - return None + + # If a specific commit or tag was requested, check it out now. + plugin_path = _checkout_commit(src, cloned_src, plugin_path) + if not plugin_path: + return None # Find a suitable installer INSTALLER = None @@ -808,12 +842,18 @@ def _install_plugin(src: InstInfo) -> Union[InstInfo, None]: def install(plugin_name: str): """downloads plugin from source repos, installs and activates plugin""" assert isinstance(plugin_name, str) - logging.debug(f"Searching for {plugin_name}") - src = search(plugin_name) - # print('src:', src) + # Specify a tag or commit to checkout by adding @ to plugin name + if '@' in plugin_name: + logging.debug("testing for a commit/tag in plugin name") + name, commit = plugin_name.split('@', 1) + else: + name = plugin_name + commit = None + logging.debug(f"Searching for {name}") + src = search(name) if src: + src.commit = commit logging.debug(f'Retrieving {src.name} from {src.source_loc}') - # if not _install_plugin(src): installed = _install_plugin(src) if not installed: print('installation aborted') From c70c4da8266fbd5d13863c303c0465b78b9bf2ba Mon Sep 17 00:00:00 2001 From: Alex Myers Date: Fri, 2 Feb 2024 16:43:11 -0600 Subject: [PATCH 5/8] reckless: create virtual environments for python plugins This uses pip to install to a venv when the dependencies are specified with requirements.txt and poetry when it's present on the system and the plugin has a pyproject.toml. The directory structure will be: reckless/ source/ (original plugin code here) plugin_entrypoint (original entrypoint) plugin_name (symlink or wrapper to activate venv) .metadata (installation information) .venv/ (python virtual environment) The wrapper matches the naming of the original plugin entrypoint. The shebang is modified to use the virtual environment's python binary, then the original plugin is imported as a module and executed as though it was run directly. Changelog-Changed: reckless installs python plugins in virtual environments --- tests/test_reckless.py | 6 +- tools/reckless | 187 ++++++++++++++++++++++++++++++++++------- 2 files changed, 160 insertions(+), 33 deletions(-) diff --git a/tests/test_reckless.py b/tests/test_reckless.py index 498fae982c8c..ee1d38f02403 100644 --- a/tests/test_reckless.py +++ b/tests/test_reckless.py @@ -2,10 +2,12 @@ import subprocess from pathlib import PosixPath, Path import socket +from pyln.testing.utils import VALGRIND import pytest import os import shutil import time +import unittest @pytest.fixture(autouse=True) @@ -102,7 +104,7 @@ def get_reckless_node(node_factory): def check_stderr(stderr): def output_okay(out): for warning in ['[notice]', 'WARNING:', 'npm WARN', - 'npm notice', 'DEPRECATION:']: + 'npm notice', 'DEPRECATION:', 'Creating virtualenv']: if out.startswith(warning): return True return False @@ -189,6 +191,7 @@ def test_install(node_factory): assert os.path.exists(plugin_path) +@unittest.skipIf(VALGRIND, "virtual environment triggers memleak detection") def test_local_dir_install(node_factory): """Test search and install from local directory source.""" n = get_reckless_node(node_factory) @@ -205,6 +208,7 @@ def test_local_dir_install(node_factory): assert os.path.exists(plugin_path) +@unittest.skipIf(VALGRIND, "virtual environment triggers memleak detection") def test_disable_enable(node_factory): """test search, git clone, and installation to folder.""" n = get_reckless_node(node_factory) diff --git a/tools/reckless b/tools/reckless index a4b881e438f0..bbe95adb9156 100755 --- a/tools/reckless +++ b/tools/reckless @@ -13,9 +13,11 @@ import shutil from subprocess import Popen, PIPE, TimeoutExpired, run import tempfile import time +import types from typing import Union from urllib.parse import urlparse from urllib.request import urlopen +import venv logging.basicConfig( @@ -590,16 +592,129 @@ class InferInstall(): raise Exception(f'plugin entrypoint not found in {self.dir}') -python3pip = Installer('python3pip', 'text/x-python', exe='python3', - manager='pip', entry='{name}.py') -python3pip.add_entrypoint('{name}') -python3pip.add_entrypoint('__init__.py') -python3pip.add_dependency_file('requirements.txt') -python3pip.add_dependency_call(['pip', 'install', '-r', 'requirements.txt']) +class InstallationFailure(Exception): + "raised when pip fails to complete dependency installation" + + +def create_python3_venv(staged_plugin: InstInfo) -> InstInfo: + "Create a virtual environment, install dependencies and test plugin." + env_path = Path('.venv') + env_path_full = Path(staged_plugin.source_loc) / env_path + plugin_path = Path(staged_plugin.source_loc) / 'source' + + # subdir should always be None at this point + if staged_plugin.subdir: + logging.warning("cloned plugin contains subdirectory") + plugin_path = plugin_path / staged_plugin.subdir + + if shutil.which('poetry') and staged_plugin.deps == 'pyproject.toml': + logging.debug('configuring a python virtual environment (poetry) in ' + f'{env_path_full}') + # The virtual environment should be located with the plugin. + # This installs it to .venv instead of in the global location. + mod_poetry_env = os.environ + mod_poetry_env['POETRY_VIRTUALENVS_IN_PROJECT'] = 'true' + # This ensures poetry installs to a new venv even though one may + # already be active (i.e., under CI) + if 'VIRTUAL_ENV' in mod_poetry_env: + del mod_poetry_env['VIRTUAL_ENV'] + # to avoid relocating and breaking the venv, symlink pyroject.toml + # to the location of poetry's .venv dir + (Path(staged_plugin.source_loc) / 'pyproject.toml') \ + .symlink_to(plugin_path / 'pyproject.toml') + (Path(staged_plugin.source_loc) / 'poetry.lock') \ + .symlink_to(plugin_path / 'poetry.lock') + + # Avoid redirecting stdout in order to stream progress. + # Timeout excluded as armv7 grpcio build/install can take 1hr. + pip = run(['poetry', 'install', '--no-root'], check=False, + cwd=staged_plugin.source_loc, env=mod_poetry_env) + + (Path(staged_plugin.source_loc) / 'pyproject.toml').unlink() + (Path(staged_plugin.source_loc) / 'poetry.lock').unlink() + + else: + builder = venv.EnvBuilder(with_pip=True) + builder.create(env_path_full) + logging.debug('configuring a python virtual environment (pip) in ' + f'{env_path_full}') + logging.debug(f'virtual environment created in {env_path_full}.') + if staged_plugin.deps == 'pyproject.toml': + pip = run(['bin/pip', 'install', str(plugin_path)], + check=False, cwd=plugin_path) + elif staged_plugin.deps == 'requirements.txt': + pip = run([str(env_path_full / 'bin/pip'), 'install', '-r', + str(plugin_path / 'requirements.txt')], + check=False, cwd=plugin_path) + else: + logging.debug("no python dependency file") + if pip and pip.returncode != 0: + logging.debug("install to virtual environment failed") + print('error encountered installing dependencies') + raise InstallationFailure + + staged_plugin.venv = env_path + print('dependencies installed successfully') + return staged_plugin + + +def create_wrapper(plugin: InstInfo): + '''The wrapper will activate the virtual environment for this plugin and + then run the plugin from within the same process.''' + assert hasattr(plugin, 'venv') + venv_full_path = Path(plugin.source_loc) / plugin.venv + with open(Path(plugin.source_loc) / plugin.entry, 'w') as wrapper: + wrapper.write((f"#!{venv_full_path}/bin/python\n" + "import sys\n" + "import runpy\n\n" + f"if '{plugin.source_loc}/source' not in sys.path:\n" + f" sys.path.append('{plugin.source_loc}/source')\n" + f"if '{plugin.source_loc}' in sys.path:\n" + f" sys.path.remove('{plugin.source_loc}')\n" + f"runpy.run_module(\"{plugin.name}\", " + "{}, \"__main__\")")) + wrapper_file = Path(plugin.source_loc) / plugin.entry + wrapper_file.chmod(0o755) + + +def install_to_python_virtual_environment(cloned_plugin: InstInfo): + '''Called during install in place of a subprocess.run list''' + # Delete symlink so that a venv wrapper can take it's place + (Path(cloned_plugin.source_loc) / cloned_plugin.entry).unlink() + # The original entrypoint is imported as a python module - ensure + # it has a .py extension. The wrapper can keep the original naming. + entry = Path(cloned_plugin.source_loc) / 'source' / cloned_plugin.entry + entry.rename(entry.with_suffix('.py')) + create_python3_venv(cloned_plugin) + if not hasattr(cloned_plugin, 'venv'): + raise InstallationFailure + logging.debug('virtual environment for cloned plugin: ' + f'{cloned_plugin.venv}') + create_wrapper(cloned_plugin) + return cloned_plugin + + +python3venv = Installer('python3venv', 'text/x-python', exe='python3', + manager='pip', entry='{name}.py') +python3venv.add_entrypoint('{name}') +python3venv.add_entrypoint('__init__.py') +python3venv.add_dependency_file('requirements.txt') +python3venv.dependency_call = install_to_python_virtual_environment + +poetryvenv = Installer('poetryvenv', 'text/x-python', exe='python3', + manager='poetry', entry='{name}.py') +poetryvenv.add_entrypoint('{name}') +poetryvenv.add_entrypoint('__init__.py') +poetryvenv.add_dependency_file('pyproject.toml') +poetryvenv.dependency_call = install_to_python_virtual_environment + +pyprojectViaPip = Installer('pyprojectViaPip', 'text/x-python', exe='python3', + manager='pip', entry='{name}.py') +pyprojectViaPip.add_entrypoint('{name}') +pyprojectViaPip.add_entrypoint('__init__.py') +pyprojectViaPip.add_dependency_file('pyproject.toml') +pyprojectViaPip.dependency_call = install_to_python_virtual_environment -python3pip3 = python3pip.copy() -python3pip3.manager = 'pip3' -python3pip3.dependency_call = [['pip3', 'install', '-r', 'requirements.txt']] # Nodejs plugin installer nodejs = Installer('nodejs', 'application/javascript', exe='node', @@ -608,7 +723,7 @@ nodejs.add_entrypoint('{name}') nodejs.add_dependency_call(['npm', 'install', '--omit=dev']) nodejs.add_dependency_file('package.json') -INSTALLERS = {python3pip, python3pip3, nodejs} +INSTALLERS = [python3venv, poetryvenv, pyprojectViaPip, nodejs] def help_alias(targets: list): @@ -781,7 +896,7 @@ def _install_plugin(src: InstInfo) -> Union[InstInfo, None]: cloned_src.source_loc = plugin_path # Relocate plugin to a staging directory prior to testing - staging_path = tmp_path / src.name / 'source' + staging_path = inst_path / 'source' shutil.copytree(str(plugin_path), staging_path) staged_src = cloned_src # Because the source files are copied to a 'source' directory, the @@ -796,30 +911,38 @@ def _install_plugin(src: InstInfo) -> Union[InstInfo, None]: # try it out if INSTALLER.dependency_call: - for call in INSTALLER.dependency_call: - logging.debug(f"Install: invoking '{' '.join(call)}'") - if logging.root.level < logging.WARNING: - pip = Popen(call, cwd=staging_path, text=True) - else: - pip = Popen(call, cwd=staging_path, stdout=PIPE, stderr=PIPE, - text=True) - pip.wait() - # FIXME: handle output of multiple calls - - if pip.returncode == 0: - print('dependencies installed successfully') + if isinstance(INSTALLER.dependency_call, types.FunctionType): + try: + staged_src = INSTALLER.dependency_call(staged_src) + except InstallationFailure: + return None else: - print('error encountered installing dependencies') - if pip.stdout: - logging.debug(pip.stdout.read()) - return None + for call in INSTALLER.dependency_call: + logging.debug(f"Install: invoking '{' '.join(call)}'") + if logging.root.level < logging.WARNING: + pip = Popen(call, cwd=staging_path, text=True) + else: + pip = Popen(call, cwd=staging_path, stdout=PIPE, + stderr=PIPE, text=True) + pip.wait() + # FIXME: handle output of multiple calls + + if pip.returncode == 0: + print('dependencies installed successfully') + else: + print('error encountered installing dependencies') + if pip.stdout: + logging.debug(pip.stdout.read()) + remove_dir(clone_path) + remove_dir(inst_path) + return None test_log = [] try: - test = run([Path(staging_path).joinpath(cloned_src.entry)], + test = run([Path(staged_src.source_loc).joinpath(staged_src.entry)], cwd=str(staging_path), stdout=PIPE, stderr=PIPE, text=True, timeout=3) - for line in test.stderr: - test_log.append(line.strip('\n')) + for line in test.stderr.splitlines(): + test_log.append(line) returncode = test.returncode except TimeoutExpired: # If the plugin is still running, it's assumed to be okay. @@ -829,10 +952,10 @@ def _install_plugin(src: InstInfo) -> Union[InstInfo, None]: for line in test_log: logging.debug(f' {line}') print('plugin testing failed') + remove_dir(clone_path) + remove_dir(inst_path) return None - # Find this cute little plugin a forever home - shutil.copytree(str(staging_path), inst_path) add_installation_metadata(staged_src, src) print(f'plugin installed: {inst_path}') remove_dir(clone_path) From 0f335f1fd8611b58f93a7213f33b97d5429b6066 Mon Sep 17 00:00:00 2001 From: Alex Myers Date: Wed, 31 Jan 2024 13:04:01 -0600 Subject: [PATCH 6/8] reckless: update timeouts --- tools/reckless | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tools/reckless b/tools/reckless index bbe95adb9156..76f2e1bc1733 100755 --- a/tools/reckless +++ b/tools/reckless @@ -234,7 +234,7 @@ class Source(Enum): # returns 0 if git repository proc = run(['git', '-C', source, 'rev-parse'], cwd=os.path.realpath(source), stdout=PIPE, - stderr=PIPE, text=True, timeout=3) + stderr=PIPE, text=True, timeout=5) if proc.returncode == 0: return cls(2) return cls(1) @@ -940,7 +940,7 @@ def _install_plugin(src: InstInfo) -> Union[InstInfo, None]: try: test = run([Path(staged_src.source_loc).joinpath(staged_src.entry)], cwd=str(staging_path), stdout=PIPE, stderr=PIPE, - text=True, timeout=3) + text=True, timeout=10) for line in test.stderr.splitlines(): test_log.append(line) returncode = test.returncode @@ -1141,7 +1141,7 @@ def load_config(reckless_dir: Union[str, None] = None, net_conf = None # Does the lightning-cli already reference an explicit config? try: - active_config = lightning_cli('listconfigs', timeout=3)['configs'] + active_config = lightning_cli('listconfigs', timeout=10)['configs'] if 'conf' in active_config: net_conf = LightningBitcoinConfig(path=active_config['conf'] ['value_str']) From b0cde626cabd9a4b32eba45134bbf2ab585432a4 Mon Sep 17 00:00:00 2001 From: Alex Myers Date: Fri, 26 Jan 2024 12:32:47 -0600 Subject: [PATCH 7/8] pytest: add tests for pip and poetry installed virtual envs Also test checkout of a git tag on installation. --- .../lightningd/testplugpass/requirements.txt | 2 + .../lightningd/testplugpass/testplugpass.py | 8 +++ .../lightningd/testplugpyproj/pyproject.toml | 17 +++++ .../testplugpyproj/testplugpyproj.py | 17 +++++ .../rkls_api_lightningd_plugins.json | 9 +++ tests/test_reckless.py | 66 ++++++++++++++++++- 6 files changed, 118 insertions(+), 1 deletion(-) create mode 100644 tests/data/recklessrepo/lightningd/testplugpyproj/pyproject.toml create mode 100755 tests/data/recklessrepo/lightningd/testplugpyproj/testplugpyproj.py diff --git a/tests/data/recklessrepo/lightningd/testplugpass/requirements.txt b/tests/data/recklessrepo/lightningd/testplugpass/requirements.txt index e69de29bb2d1..7b19e677138d 100644 --- a/tests/data/recklessrepo/lightningd/testplugpass/requirements.txt +++ b/tests/data/recklessrepo/lightningd/testplugpass/requirements.txt @@ -0,0 +1,2 @@ +pyln-client + diff --git a/tests/data/recklessrepo/lightningd/testplugpass/testplugpass.py b/tests/data/recklessrepo/lightningd/testplugpass/testplugpass.py index 444043531dab..435f979f23a0 100755 --- a/tests/data/recklessrepo/lightningd/testplugpass/testplugpass.py +++ b/tests/data/recklessrepo/lightningd/testplugpass/testplugpass.py @@ -3,6 +3,8 @@ plugin = Plugin() +__version__ = 'v1' + @plugin.init() def init(options, configuration, plugin, **kwargs): @@ -14,4 +16,10 @@ def testmethod(plugin): return ("I live.") +@plugin.method("gettestplugversion") +def gettestplugversion(plugin): + "to test commit/tag checkout" + return __version__ + + plugin.run() diff --git a/tests/data/recklessrepo/lightningd/testplugpyproj/pyproject.toml b/tests/data/recklessrepo/lightningd/testplugpyproj/pyproject.toml new file mode 100644 index 000000000000..738c943c0878 --- /dev/null +++ b/tests/data/recklessrepo/lightningd/testplugpyproj/pyproject.toml @@ -0,0 +1,17 @@ +[project] +dependencies = ["pyln-client"] + +[tool.poetry] +name = "testplugpyproj" +version = "0.1.0" +description = "testing poetry installation of python plugins" +authors = ["Alex Myers "] + +[tool.poetry.dependencies] +# Build dependencies belong here +python = "^3.8" +pyln-client = "^23.11" + +[build-system] +requires = ["poetry-core>=1.0.0"] +build-backend = "poetry.core.masonry.api" diff --git a/tests/data/recklessrepo/lightningd/testplugpyproj/testplugpyproj.py b/tests/data/recklessrepo/lightningd/testplugpyproj/testplugpyproj.py new file mode 100755 index 000000000000..444043531dab --- /dev/null +++ b/tests/data/recklessrepo/lightningd/testplugpyproj/testplugpyproj.py @@ -0,0 +1,17 @@ +#!/usr/bin/env python3 +from pyln.client import Plugin + +plugin = Plugin() + + +@plugin.init() +def init(options, configuration, plugin, **kwargs): + plugin.log("testplug initialized") + + +@plugin.method("testmethod") +def testmethod(plugin): + return ("I live.") + + +plugin.run() diff --git a/tests/data/recklessrepo/rkls_api_lightningd_plugins.json b/tests/data/recklessrepo/rkls_api_lightningd_plugins.json index 5c30a0232f36..3a0484f4f1ee 100644 --- a/tests/data/recklessrepo/rkls_api_lightningd_plugins.json +++ b/tests/data/recklessrepo/rkls_api_lightningd_plugins.json @@ -16,5 +16,14 @@ "git_url": "https://api.github.com/repos/lightningd/plugins/git/trees/testplugfail", "download_url": null, "type": "dir" + }, + { + "name": "testplugpyproj", + "path": "testplugpyproj", + "url": "https://api.github.com/repos/lightningd/plugins/contents/webhook?ref=master", + "html_url": "https://github.com/lightningd/plugins/tree/master/testplugpyproj", + "git_url": "https://api.github.com/repos/lightningd/plugins/git/trees/testplugpyproj", + "download_url": null, + "type": "dir" } ] diff --git a/tests/test_reckless.py b/tests/test_reckless.py index ee1d38f02403..bca922e013f7 100644 --- a/tests/test_reckless.py +++ b/tests/test_reckless.py @@ -61,8 +61,15 @@ def canned_github_server(directory): repo_initialization = (f'cp -r {plugins_path}/* .;' 'git add --all;' 'git commit -m "initial commit - autogenerated by test_reckless.py";') + tag_and_update = ('git tag v1;' + "sed -i 's/v1/v2/g' testplugpass/testplugpass.py;" + 'git add testplugpass/testplugpass.py;' + 'git commit -m "update to v2";' + 'git tag v2;') subprocess.check_output([repo_initialization], env=my_env, shell=True, cwd=repo_dir) + subprocess.check_output([tag_and_update], env=my_env, + shell=True, cwd=repo_dir) del my_env['HOME'] del my_env['GIT_DIR'] del my_env['GIT_WORK_TREE'] @@ -191,6 +198,25 @@ def test_install(node_factory): assert os.path.exists(plugin_path) +@unittest.skipIf(VALGRIND, "virtual environment triggers memleak detection") +def test_poetry_install(node_factory): + """test search, git clone, and installation to folder.""" + n = get_reckless_node(node_factory) + r = reckless([f"--network={NETWORK}", "-v", "install", "testplugpyproj"], dir=n.lightning_dir) + assert r.returncode == 0 + assert 'dependencies installed successfully' in r.stdout + assert 'plugin installed:' in r.stdout + assert 'testplugpyproj enabled' in r.stdout + check_stderr(r.stderr) + plugin_path = Path(n.lightning_dir) / 'reckless/testplugpyproj' + print(plugin_path) + assert os.path.exists(plugin_path) + n.start() + print(n.rpc.testmethod()) + assert n.daemon.is_in_log(r'plugin-manager: started\([0-9].*\) /tmp/ltests-[a-z0-9_].*/test_poetry_install_1/lightning-1/reckless/testplugpyproj/testplugpyproj.py') + assert n.rpc.testmethod() == 'I live.' + + @unittest.skipIf(VALGRIND, "virtual environment triggers memleak detection") def test_local_dir_install(node_factory): """Test search and install from local directory source.""" @@ -236,4 +262,42 @@ def test_disable_enable(node_factory): 'active': True, 'dynamic': True} time.sleep(1) print(n.rpc.plugin_list()['plugins']) - assert(test_plugin in n.rpc.plugin_list()['plugins']) + assert test_plugin in n.rpc.plugin_list()['plugins'] + + +@unittest.skipIf(VALGRIND, "virtual environment triggers memleak detection") +def test_tag_install(node_factory): + "install a plugin from a specific commit hash or tag" + node = get_reckless_node(node_factory) + node.start() + r = reckless([f"--network={NETWORK}", "-v", "install", "testPlugPass"], + dir=node.lightning_dir) + assert r.returncode == 0 + metadata = node.lightning_dir / "reckless/testplugpass/.metadata" + with open(metadata, "r") as md: + header = '' + for line in md.readlines(): + line = line.strip() + if header == 'requested commit': + assert line == 'None' + header = line + # should install v2 (latest) without specifying + version = node.rpc.gettestplugversion() + assert version == 'v2' + r = reckless([f"--network={NETWORK}", "-v", "uninstall", "testplugpass"], + dir=node.lightning_dir) + r = reckless([f"--network={NETWORK}", "-v", "install", "testplugpass@v1"], + dir=node.lightning_dir) + assert r.returncode == 0 + # v1 should now be checked out. + version = node.rpc.gettestplugversion() + assert version == 'v1' + installed_path = Path(node.lightning_dir) / 'reckless/testplugpass' + assert installed_path.is_dir() + with open(metadata, "r") as md: + header = '' + for line in md.readlines(): + line = line.strip() + if header == 'requested commit': + assert line == 'v1' + header = line From 1251743c1893532b94ccf6718a2e34d78c6a9891 Mon Sep 17 00:00:00 2001 From: Alex Myers Date: Fri, 2 Feb 2024 11:12:13 -0600 Subject: [PATCH 8/8] doc: update reckless documentation --- doc/reckless.7.md | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/doc/reckless.7.md b/doc/reckless.7.md index 14e5112d13fb..98d84e931ef8 100644 --- a/doc/reckless.7.md +++ b/doc/reckless.7.md @@ -14,12 +14,12 @@ installation involves: finding the source plugin, copying, installing dependencies, testing, activating, and updating the lightningd config file. Reckless does all of these by invoking: -**reckless** **install** *plugin\_name* +**reckless** **install**[@*commit/tag*] *plugin\_name* reckless will exit early in the event that: - the plugin is not found in any available source repositories -- dependencies are not sucessfully installed +- dependencies are not successfully installed - the plugin fails to execute Reckless-installed plugins reside in the 'reckless' subdirectory @@ -74,11 +74,14 @@ Available option flags: **-v**, **--verbose** request additional debug output +**--network**=*network* + specify bitcoin, regtest, liquid, liquid-regtest, litecoin, signet, + or testnet networks. (default: bitcoin) + NOTES ----- -Reckless currently supports python plugins only. Additional language -support will be provided in future releases. +Reckless currently supports python and javascript plugins. Running the first time will prompt the user that their lightningd's bitcoin config will be appended (or created) to inherit the reckless @@ -97,6 +100,10 @@ the option flag **reckless -d=** may be used to relocate the reckless directory from its default. Consider creating a permanent alias in this case. +Python plugins are installed to their own virtual environments. The +environment is activated by a wrapper (named the same as the plugin) +which then imports and executes the actual plugin entrypoint. + For Plugin Developers: To make your plugin compatible with reckless install: