plugins: detect and fixup old relative paths.

Note that we move adding the plugin to the plugins list to the end, otherwise
the hook from logging can examine the (uninitialized) plugin.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

CHANGELOG.md

@@ -32,6 +32,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 Note: You should always set `allow-deprecated-apis=false` to test for
 changes.
 
+- plugins: using startup-relative paths for `plugin` and `plugin-dir`: they're now relative to `lightning-dir`.
+
 ### Removed
 
 ### Fixed

lightningd/lightningd.c

@@ -214,6 +214,11 @@ static struct lightningd *new_lightningd(const tal_t *ctx)
 	 */
 	jsonrpc_setup(ld);
 
+	/*~ We changed when we start plugins, messing up relative paths.
+	 * This saves our original dirs so we can fixup and warn for the
+	 * moment (0.7.2). */
+	ld->original_directory = path_cwd(ld);
+
 	/*~ We run a number of plugins (subprocesses that we talk JSON-RPC with)
 	 *alongside this process. This allows us to have an easy way for users
 	 *to add their own tools without having to modify the c-lightning source

lightningd/lightningd.h

@@ -223,6 +223,9 @@ struct lightningd {
 	char *tor_service_password;
 	bool pure_tor_setup;
 
+	/* Original directory for deprecated plugin-relative-to-cwd */
+	const char *original_directory;
+
 	struct plugins *plugins;
 };
 

lightningd/plugin.c

@@ -63,9 +63,26 @@ void plugin_register(struct plugins *plugins, const char* path TAKES)
 	}
 
 	p = tal(plugins, struct plugin);
-	list_add_tail(&plugins->plugins, &p->list);
 	p->plugins = plugins;
 	p->cmd = tal_strdup(p, path);
+
+	/* Fix up old-style relative paths */
+	if (deprecated_apis
+	    && !path_is_abs(p->cmd)
+	    && access(p->cmd, X_OK) != 0) {
+		char *oldpath = path_join(tmpctx,
+					  plugins->ld->original_directory,
+					  p->cmd);
+		if (access(oldpath, X_OK) == 0) {
+			log_unusual(plugins->log, "DEPRECATED WARNING:"
+				    " plugin is now relative to"
+				    " lightning-dir, please change to"
+				    " plugin=%s",
+				    oldpath);
+			tal_free(p->cmd);
+			p->cmd = tal_steal(p, oldpath);
+		}
+	}
 	p->plugin_state = UNCONFIGURED;
 	p->js_arr = tal_arr(p, struct json_stream *, 0);
 	p->used = 0;
@@ -76,6 +93,8 @@ void plugin_register(struct plugins *plugins, const char* path TAKES)
 			 path_basename(tmpctx, p->cmd));
 	p->methods = tal_arr(p, const char *, 0);
 	list_head_init(&p->plugin_opts);
+
+	list_add_tail(&plugins->plugins, &p->list);
 	tal_add_destructor(p, destroy_plugin);
 }
 
@@ -877,11 +896,25 @@ char *add_plugin_dir(struct plugins *plugins, const char *dir, bool nonexist_ok)
 	struct dirent *di;
 	DIR *d = opendir(dir);
 	if (!d) {
-		if (nonexist_ok && errno == ENOENT)
+		if (deprecated_apis && !path_is_abs(dir)) {
+			dir = path_join(tmpctx,
+					plugins->ld->original_directory, dir);
+			d = opendir(dir);
+			if (d) {
+				log_unusual(plugins->log, "DEPRECATED WARNING:"
+					    " plugin-dir is now relative to"
+					    " lightning-dir, please change to"
+					    " plugin-dir=%s",
+					    dir);
+			}
+		}
+		if (!d) {
+			if (!nonexist_ok && errno == ENOENT)
 				return NULL;
 			return tal_fmt(NULL, "Failed to open plugin-dir %s: %s",
 				       dir, strerror(errno));
 		}
+	}
 
 	while ((di = readdir(d)) != NULL) {
 		const char *fullpath;

tests/test_plugin.py

@@ -651,3 +651,22 @@ def test_forward_event_notification(node_factory, bitcoind, executor):
     assert l2.rpc.call('recordcheck', {'payment_hash': payment_hash14, 'status': 'failed', 'dbforward': stats['forwards'][1]})
     assert l2.rpc.call('recordcheck', {'payment_hash': payment_hash15, 'status': 'offered', 'dbforward': stats['forwards'][2]})
     assert l2.rpc.call('recordcheck', {'payment_hash': payment_hash15, 'status': 'local_failed', 'dbforward': stats['forwards'][2]})
+
+
+def test_plugin_deprecated_relpath(node_factory):
+    """Test that we can use old-style relative plugin paths with deprecated-apis"""
+    l1 = node_factory.get_node(options={'plugin-dir': 'contrib/plugins',
+                                        'plugin': 'tests/plugins/millisatoshis.py',
+                                        'allow-deprecated-apis': True})
+
+    plugins = l1.rpc.plugin_list()['plugins']
+    assert ('helloworld.py', True) in [(os.path.basename(p['name']), p['active']) for p in plugins]
+    assert ('millisatoshis.py', True) in [(os.path.basename(p['name']), p['active']) for p in plugins]
+
+    assert l1.daemon.is_in_log('DEPRECATED WARNING.*plugin-dir={}'
+                               .format(os.path.join(os.getcwd(),
+                                                    'contrib/plugins')))
+
+    assert l1.daemon.is_in_log('DEPRECATED WARNING.*plugin={}'
+                               .format(os.path.join(os.getcwd(),
+                                                    'tests/plugins/millisatoshis.py')))