Merge remote-tracking branch 'origin/topic/robin/plugin-version-check'

I added another small change - since we are inlining Configure(), we can
just set bro_plugin directly to BRO_PLUGIN_BRO_VERSION in
src/plugin/Plugin.h, instead of depending on the plugin to do it. This
also means we do not need to change init-plugin in bro-aux at this
moment.

BIT-1828 #closed

* origin/topic/robin/plugin-version-check:
  Adding plugin API number into versioned function name, and removing old runtime API version check.
  Extend plugin infrastructure to catch Bro version mismatches at link time.

CHANGES

@@ -1,4 +1,34 @@
 
+2.5-181 | 2017-07-25 16:02:41 -0700
+
+  * Extend plugin infrastructure to catch Bro version mismatches at link
+    time.
+
+    The version number used for the function name is slightly normalized
+    to skip any git revision postfixes (i.e., "2.5-xxx" is always treated
+    as "2.5-git") so that one doesn't need to recompile all plugins after
+    every master commit. That seems good enough, usually people run into
+    this when upgrading to a new release. The Plugin API version is also
+    part of the version number.
+
+    If one loads an old plugin into a new Bro, the error message looks
+    like this:
+
+        $ bro -NN Demo::Foo
+        fatal error in /home/robin/bro/master/scripts/base/init-bare.bro, line 1:
+        cannot load plugin library /home/robin/tmp/p/build//lib/Demo-Foo.linux-x86_64.so:
+        /home/robin/tmp/p/build//lib/Demo-Foo.linux-x86_64.so: undefined symbol: bro_version_2_5_git_debug
+
+    (Robin Sommer)
+
+  * Several fixes and improvements for software version parsing.
+
+     - Addresses Philip Romero's question from the Bro mailing list.
+     - Adds Microsoft Edge as a detected browser.
+     - We are now unescaping encoded characters in software names. (Seth Hall)
+
+  * Remove another reference to now removed bro-plugins. (Johanna Amann)
+
 2.5-175 | 2017-07-07 14:35:11 -0700
 
   * Removing aux/plugins. Most of the plugins are now Bro packages.

CMakeLists.txt

@@ -40,12 +40,26 @@ file(WRITE ${CMAKE_CURRENT_BINARY_DIR}/bro-path-dev.csh
      "setenv PATH \"${CMAKE_CURRENT_BINARY_DIR}/src\":$PATH\n")
 
 file(STRINGS "${CMAKE_CURRENT_SOURCE_DIR}/VERSION" VERSION LIMIT_COUNT 1)
+execute_process(COMMAND grep "^#define  *BRO_PLUGIN_API_VERSION"
+                INPUT_FILE ${CMAKE_CURRENT_SOURCE_DIR}/src/plugin/Plugin.h
+                OUTPUT_VARIABLE API_VERSION
+                OUTPUT_STRIP_TRAILING_WHITESPACE)
+string(REGEX REPLACE "^#define.*VERSION *" "" API_VERSION "${API_VERSION}")
+
 string(REPLACE "." " " version_numbers ${VERSION})
 separate_arguments(version_numbers)
 list(GET version_numbers 0 VERSION_MAJOR)
 list(GET version_numbers 1 VERSION_MINOR)
 set(VERSION_MAJ_MIN "${VERSION_MAJOR}.${VERSION_MINOR}")
 
+set(VERSION_C_IDENT "${VERSION}_plugin_${API_VERSION}")
+string(REGEX REPLACE "-[0-9]*$" "_git" VERSION_C_IDENT "${VERSION_C_IDENT}")
+string(REGEX REPLACE "[^a-zA-Z0-9_\$]" "_" VERSION_C_IDENT "${VERSION_C_IDENT}")
+
+if(${ENABLE_DEBUG})
+    set(VERSION_C_IDENT "${VERSION_C_IDENT}_debug")
+endif()
+
 ########################################################################
 ## Dependency Configuration
 

VERSION

@@ -1 +1 @@
-2.5-175
+2.5-181

bro-config.h.in

@@ -229,3 +229,14 @@
 #ifndef BRO_PLUGIN_INTERNAL_BUILD
 #define BRO_PLUGIN_INTERNAL_BUILD @BRO_PLUGIN_INTERNAL_BUILD@
 #endif
+
+/* A C function that has the Bro version encoded into its name. */
+#define BRO_VERSION_FUNCTION bro_version_@VERSION_C_IDENT@
+#ifdef __cplusplus
+extern "C" {
+#endif
+extern const char* BRO_VERSION_FUNCTION();
+#ifdef __cplusplus
+}
+#endif
+

src/plugin/Manager.cc

@@ -243,10 +243,6 @@ bool Manager::ActivateDynamicPluginInternal(const std::string& name, bool ok_if_
 
 			plugins_by_path.insert(std::make_pair(normalize_path(dir), current_plugin));
 
-			if ( current_plugin->APIVersion() != BRO_PLUGIN_API_VERSION )
-				reporter->FatalError("plugin's API version does not match Bro (expected %d, got %d in %s)",
-						     BRO_PLUGIN_API_VERSION, current_plugin->APIVersion(), path);
-
 			// We execute the pre-script initialization here; this in
 			// fact could be *during* script initialization if we got
 			// triggered via @load-plugin.

src/plugin/Plugin.cc

@@ -242,11 +242,6 @@ VersionNumber Plugin::Version() const
 	return config.version;
 	}
 
-int Plugin::APIVersion() const
-	{
-	return config.api_version;
-	}
-
 bool Plugin::DynamicPlugin() const
 	{
 	return dynamic;

src/plugin/Plugin.h

@@ -13,10 +13,11 @@
 #include "iosource/Component.h"
 #include "logging/WriterBackend.h"
 
-// We allow to override this externally for testing purposes.
+// Increase this when making incompatible changes to the plugin API. Note
-#ifndef BRO_PLUGIN_API_VERSION
+// that the constant is never used in C code. It's picked up on by CMake.
-#define BRO_PLUGIN_API_VERSION 5
+#define BRO_PLUGIN_API_VERSION 6
-#endif
+
+#define BRO_PLUGIN_BRO_VERSION BRO_VERSION_FUNCTION
 
 class ODesc;
 class Func;
@@ -93,16 +94,21 @@ public:
 	// strong hint.). The attribute seems generally available.
 	inline Configuration() __attribute__((always_inline));
 
+        /**
+         * One can assign BRO_PLUGIN_BRO_VERSION to this to catch
+         * version mismatches at link(!) time.
+         */
+	const char* (*bro_version)();
+
 private:
 	friend class Plugin;
-	int api_version;	// Current BRO_PLUGIN_API_VERSION. Automatically set.
 };
 
 inline Configuration::Configuration()
 		{
 		name = "";
 		description = "";
-		api_version = BRO_PLUGIN_API_VERSION;
+		bro_version = BRO_PLUGIN_BRO_VERSION;
 		}
 
 /**
@@ -442,15 +448,6 @@ public:
 	 **/
 	const std::string& PluginPath() const;
 
-	/**
-	 * Returns the internal version of the Bro API that this plugin
-	 * relies on. Only plugins that match Bro's current API version can
-	 * be used. For statically compiled plugins this is automatically the
-	 * case, but dynamically loaded plugins may cause a mismatch if they
-	 * were compiled for a different Bro version.
-	 */
-	int APIVersion() const;
-
 	/**
 	 * Returns a list of all components the plugin provides.
 	 */

src/version.c.in

@@ -1 +1,13 @@
+
+#include "bro-config.h"
+
 char version[] = "@VERSION@";
+
+// A C function that has the current version built into its name.
+// One can link a shared library against this to ensure that it won't
+// load if the version of the main Bro binary differs compared to
+// what the library was compiled against.
+const char* BRO_VERSION_FUNCTION()
+{
+    return "@VERSION_C_IDENT@";
+}

testing/btest/Baseline/plugins.api-version-mismatch/output

@@ -1 +0,0 @@
-fatal error in /home/robin/bro/plugins/scripts/base/init-bare.bro, line 1: plugin's API version does not match Bro (expected 2, got 42 in /home/robin/bro/plugins/testing/btest/.tmp/plugins.api-version-mismatch/build//lib/XXX)

testing/btest/plugins/api-version-mismatch.sh

@@ -1,8 +0,0 @@
-# @TEST-EXEC: ${DIST}/aux/bro-aux/plugin-support/init-plugin -u . Demo Foo
-# @TEST-EXEC: bash %INPUT
-# @TEST-EXEC: ./configure --bro-dist=${DIST} && make
-# @TEST-EXEC-FAIL: BRO_PLUGIN_PATH=`pwd` bro -NN Demo::Foo >tmp 2>&1
-# @TEST-EXEC: cat tmp | sed 's/Demo-Foo[-a-zA-Z0-9_.]*/XXX/' >>output
-# @TEST-EXEC: TEST_DIFF_CANONIFIER=$SCRIPTS/diff-remove-abspath btest-diff output
-
-( echo '#define BRO_PLUGIN_API_VERSION 42'; cat src/Plugin.cc; ) >src/Plugin.cc.tmp && mv src/Plugin.cc.tmp src/Plugin.cc