From bdb3e651f5ef8ecc020016e5603f3b3d003fb26c Mon Sep 17 00:00:00 2001 From: Alex Myers Date: Thu, 17 Apr 2025 16:41:55 -0500 Subject: [PATCH 1/6] reckless: fix installation from local directories with subpaths This could previously copy the parent directory of a plugin into the installed reckless directory, which was unnecessary. --- tests/test_reckless.py | 1 - tools/reckless | 7 +++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/tests/test_reckless.py b/tests/test_reckless.py index 3615b71a5518..4a1e59a93ef7 100644 --- a/tests/test_reckless.py +++ b/tests/test_reckless.py @@ -231,7 +231,6 @@ def test_poetry_install(node_factory): @unittest.skipIf(VALGRIND, "virtual environment triggers memleak detection") -@unittest.skip("Broken") def test_local_dir_install(node_factory): """Test search and install from local directory source.""" n = get_reckless_node(node_factory) diff --git a/tools/reckless b/tools/reckless index 7b692d391e0d..13535aa3dd77 100755 --- a/tools/reckless +++ b/tools/reckless @@ -1175,10 +1175,13 @@ def _install_plugin(src: InstInfo) -> Union[InstInfo, None]: log.debug(f'{clone_path} already exists - deleting') shutil.rmtree(clone_path) if src.srctype == Source.DIRECTORY: + full_source_path = Path(src.source_loc) + if src.subdir: + full_source_path /= src.subdir log.debug(("copying local directory contents from" - f" {src.source_loc}")) + f" {full_source_path}")) create_dir(clone_path) - shutil.copytree(src.source_loc, plugin_path) + shutil.copytree(full_source_path, plugin_path) elif src.srctype in [Source.LOCAL_REPO, Source.GITHUB_REPO, Source.OTHER_URL, Source.GIT_LOCAL_CLONE]: # clone git repository to /tmp/reckless-... From 5ced3247873de69f9a5433c81ec33205565deaf5 Mon Sep 17 00:00:00 2001 From: Alex Myers Date: Thu, 17 Apr 2025 17:13:49 -0500 Subject: [PATCH 2/6] reckless: accept a full local path as source+name This allows installing a local plugin directly without having to modify reckless sources. Changelog-changed: Reckless can be passed a local file or directory for installation. --- tools/reckless | 89 ++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 65 insertions(+), 24 deletions(-) diff --git a/tools/reckless b/tools/reckless index 13535aa3dd77..1422516b469b 100755 --- a/tools/reckless +++ b/tools/reckless @@ -1300,6 +1300,30 @@ def _install_plugin(src: InstInfo) -> Union[InstInfo, None]: return staged_src +def location_from_name(plugin_name: str) -> (str, str): + """Maybe the location was passed in place of the plugin name. Check + if this looks like a filepath or URL and return that as well as the + plugin name.""" + if not Path(plugin_name).exists(): + # No path included, return the name only. + return (None, plugin_name) + + # Directory containing the plugin? The plugin name should match the dir. + if os.path.isdir(plugin_name): + return (Path(plugin_name).parent, Path(plugin_name).name) + + # Possibly the entrypoint itself was passed? + elif os.path.isfile(plugin_name): + if Path(plugin_name).with_suffix('').name != Path(plugin_name).parent.name or \ + not Path(plugin_name).parent.parent.exists(): + # If the directory is not named for the plugin, we can't infer what + # should be done. + # FIXME: return InstInfo with entrypoint rather than source str. + return (None, plugin_name) + # We have to make inferences as to the naming here. + return (Path(plugin_name).parent.parent, Path(plugin_name).with_suffix('').name) + + def install(plugin_name: str) -> Union[str, None]: """Downloads plugin from source repos, installs and activates plugin. Returns the location of the installed plugin or "None" in the case of @@ -1312,33 +1336,48 @@ def install(plugin_name: str) -> Union[str, None]: else: name = plugin_name commit = None - log.debug(f"Searching for {name}") - if search(name): - global LAST_FOUND - src = LAST_FOUND - src.commit = commit - log.debug(f'Retrieving {src.name} from {src.source_loc}') - try: - installed = _install_plugin(src) - except FileExistsError as err: - log.error(f'File exists: {err.filename}') - return None - LAST_FOUND = None - if not installed: - log.warning(f'{plugin_name}: installation aborted') + # Is the install request specifying a path to the plugin? + direct_location, name = location_from_name(name) + if direct_location: + logging.debug(f"install of {name} requested from {direct_location}") + src = InstInfo(name, direct_location, None) + if not src.get_inst_details(): + src = None + # Treating a local git repo as a directory allows testing + # uncommitted changes. + if src and src.srctype == Source.LOCAL_REPO: + src.srctype = Source.DIRECTORY + if not direct_location or not src: + log.debug(f"direct_location {direct_location}, src: {src}") + log.debug(f"Searching for {name}") + if search(name): + global LAST_FOUND + src = LAST_FOUND + src.commit = commit + log.debug(f'Retrieving {src.name} from {src.source_loc}') + else: return None - # Match case of the containing directory - for dirname in os.listdir(RECKLESS_CONFIG.reckless_dir): - if dirname.lower() == installed.name.lower(): - inst_path = Path(RECKLESS_CONFIG.reckless_dir) - inst_path = inst_path / dirname / installed.entry - RECKLESS_CONFIG.enable_plugin(inst_path) - enable(installed.name) - return f"{installed.source_loc}" - log.error(('dynamic activation failed: ' - f'{installed.name} not found in reckless directory')) + try: + installed = _install_plugin(src) + except FileExistsError as err: + log.error(f'File exists: {err.filename}') return None + LAST_FOUND = None + if not installed: + log.warning(f'{plugin_name}: installation aborted') + return None + + # Match case of the containing directory + for dirname in os.listdir(RECKLESS_CONFIG.reckless_dir): + if dirname.lower() == installed.name.lower(): + inst_path = Path(RECKLESS_CONFIG.reckless_dir) + inst_path = inst_path / dirname / installed.entry + RECKLESS_CONFIG.enable_plugin(inst_path) + enable(installed.name) + return f"{installed.source_loc}" + log.error(('dynamic activation failed: ' + f'{installed.name} not found in reckless directory')) return None @@ -1388,6 +1427,8 @@ def search(plugin_name: str) -> Union[InstInfo, None]: if srctype in [Source.DIRECTORY, Source.LOCAL_REPO, Source.GITHUB_REPO, Source.OTHER_URL]: found = _source_search(plugin_name, source) + if found: + log.debug(f"{found}, {found.srctype}") if not found: continue log.info(f"found {found.name} in source: {found.source_loc}") From 2e204101b6be33c06c3ccc7318af4fe951d2a656 Mon Sep 17 00:00:00 2001 From: Alex Myers Date: Tue, 22 Apr 2025 15:28:58 -0500 Subject: [PATCH 3/6] reckless: handle a direct source in the form of a git repo url --- tools/reckless | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tools/reckless b/tools/reckless index 1422516b469b..7d13e2b0e18e 100755 --- a/tools/reckless +++ b/tools/reckless @@ -1305,6 +1305,12 @@ def location_from_name(plugin_name: str) -> (str, str): if this looks like a filepath or URL and return that as well as the plugin name.""" if not Path(plugin_name).exists(): + try: + parsed = urlparse(plugin_name) + if parsed.scheme in ['http', 'https']: + return (plugin_name, Path(plugin_name).with_suffix('').name) + except ValueError: + pass # No path included, return the name only. return (None, plugin_name) @@ -1338,6 +1344,7 @@ def install(plugin_name: str) -> Union[str, None]: commit = None # Is the install request specifying a path to the plugin? direct_location, name = location_from_name(name) + src = None if direct_location: logging.debug(f"install of {name} requested from {direct_location}") src = InstInfo(name, direct_location, None) @@ -1348,7 +1355,6 @@ def install(plugin_name: str) -> Union[str, None]: if src and src.srctype == Source.LOCAL_REPO: src.srctype = Source.DIRECTORY if not direct_location or not src: - log.debug(f"direct_location {direct_location}, src: {src}") log.debug(f"Searching for {name}") if search(name): global LAST_FOUND From 3bf30d051df92f865962df80883b5dc3305f6aa3 Mon Sep 17 00:00:00 2001 From: Alex Myers Date: Sun, 4 May 2025 16:51:13 -0500 Subject: [PATCH 4/6] reckless: store absolute paths in metadata --- tools/reckless | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tools/reckless b/tools/reckless index 7d13e2b0e18e..3bc0984d6908 100755 --- a/tools/reckless +++ b/tools/reckless @@ -1099,12 +1099,16 @@ def add_installation_metadata(installed: InstInfo, updating the plugin.""" install_dir = Path(installed.source_loc) assert install_dir.is_dir() + if urlparse(original_request.source_loc).scheme in ['http', 'https']: + abs_source_path = original_request.source_loc + else: + abs_source_path = Path(original_request.source_loc).resolve() 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' + f'{abs_source_path}\n' 'requested commit\n' f'{original_request.commit}\n' 'installed commit\n' From 7309dc83b9f5792a84a4ac2a5c00546798830d49 Mon Sep 17 00:00:00 2001 From: Alex Myers Date: Tue, 13 May 2025 12:06:10 -0500 Subject: [PATCH 5/6] pytest: test reckless direct install --- tests/test_reckless.py | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/tests/test_reckless.py b/tests/test_reckless.py index 4a1e59a93ef7..e0244fa10659 100644 --- a/tests/test_reckless.py +++ b/tests/test_reckless.py @@ -235,9 +235,8 @@ def test_local_dir_install(node_factory): """Test search and install from local directory source.""" n = get_reckless_node(node_factory) n.start() - r = reckless([f"--network={NETWORK}", "-v", "source", "add", - os.path.join(n.lightning_dir, '..', 'lightningd', 'testplugpass')], - dir=n.lightning_dir) + source_dir = str(Path(n.lightning_dir / '..' / 'lightningd' / 'testplugpass').resolve()) + r = reckless([f"--network={NETWORK}", "-v", "source", "add", source_dir], dir=n.lightning_dir) assert r.returncode == 0 r = reckless([f"--network={NETWORK}", "-v", "install", "testplugpass"], dir=n.lightning_dir) assert r.returncode == 0 @@ -246,6 +245,15 @@ def test_local_dir_install(node_factory): print(plugin_path) assert os.path.exists(plugin_path) + # Retry with a direct install passing the full path to the local source + r = reckless(['uninstall', 'testplugpass', '-v'], dir=n.lightning_dir) + assert not os.path.exists(plugin_path) + r = reckless(['source', 'remove', source_dir], dir=n.lightning_dir) + assert 'plugin source removed' in r.stdout + r = reckless(['install', '-v', source_dir], dir=n.lightning_dir) + assert 'testplugpass enabled' in r.stdout + assert os.path.exists(plugin_path) + @unittest.skipIf(VALGRIND, "virtual environment triggers memleak detection") def test_disable_enable(node_factory): From 4a061a45f78ddf3e57d1bb530284ea703c3dd9e0 Mon Sep 17 00:00:00 2001 From: Alex Myers Date: Tue, 13 May 2025 12:42:08 -0500 Subject: [PATCH 6/6] doc: update reckless documentation for direct install --- doc/lightning-reckless.1.md | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/doc/lightning-reckless.1.md b/doc/lightning-reckless.1.md index f645ff232f32..c9eaabcaad95 100644 --- a/doc/lightning-reckless.1.md +++ b/doc/lightning-reckless.1.md @@ -16,6 +16,11 @@ lightningd config file. Reckless does all of these by invoking: **reckless** **install**[@*commit/tag*] *plugin\_name* +Alternatively, the source path or URL to the plugin repository can be +passed directly in place of the *plugin\_name*. In either case, the +containing directory or repository should be named for the plugin, don't +pass the plugin's executable/entrypoint directly. + reckless will exit early in the event that: - the plugin is not found in any available source repositories @@ -81,7 +86,7 @@ Available option flags: NOTES ----- -Reckless currently supports python and javascript plugins. +Reckless currently supports python, rust, 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