[asterisk-dev] Change in asterisk[master]: git migration: Remove support for file versions

Matt Jordan (Code Review) asteriskteam at digium.com
Sun Apr 12 13:12:53 CDT 2015


Matt Jordan has uploaded a new change for review.

  https://gerrit.asterisk.org/59

Change subject: git migration: Remove support for file versions
......................................................................

git migration: Remove support for file versions

Git does not support the ability to replace a token with a version
string during check-in. While it does have support for replacing a
token on clone, this is somewhat sub-optimal: the token is replaced
with the object hash, which is not particularly easy for human
consumption. What's more, in practice, the source file version was often
not terribly useful. Generally, when triaging bugs, the overall version
of Asterisk is far more useful than an individual SVN version of a file.
As a result, this patch removes Asterisk's support for showing source file
versions.

Specifically, it does the following:
* main/asterisk:
  - Refactor the file_version structure to reflect that it no longer
    tracks a version field.
  - Remove the "core show file version" CLI command. Without the
    file version, it is no longer useful.

* main/manager: Remove value from the Version key of the ModuleCheck
  Action. The actual key itself has not been removed, as doing so would
  absolutely constitute a backwards incompatible change. However, since
  the file version is no longer tracked, there is no need to attempt
  to include it in the Version key.

* UPGRADE: Add notes for:
  - Modification to the ModuleCheck AMI Action
  - Removal of the "core show file version" CLI command

Change-Id: Ib82ec4e23d9f1d445e94430884ef60134fd0bfa7
---
M UPGRADE.txt
M include/asterisk.h
M main/asterisk.c
M main/manager.c
4 files changed, 51 insertions(+), 132 deletions(-)


  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/59/59/1

diff --git a/UPGRADE.txt b/UPGRADE.txt
index 8b848cc..0ae82c1 100644
--- a/UPGRADE.txt
+++ b/UPGRADE.txt
@@ -33,6 +33,15 @@
    should have no adverse effects for those upgrading; this note merely
    serves as an indication that a new module exists.
 
+AMI:
+ - The 'ModuleCheck' Action's Version key will no longer show the module
+   version. The value will always be blank.
+
+CLI:
+ - The 'core show file version' command has been removed. When Asterisk
+   moved to Git, the source control version support was removed. As a
+   result, the CLi command was no longer useful and was removed as well.
+
 From 13.2.0 to 13.3.0:
 
 chan_dahdi:
diff --git a/include/asterisk.h b/include/asterisk.h
index 9a0264e..ff8715a 100644
--- a/include/asterisk.h
+++ b/include/asterisk.h
@@ -159,6 +159,8 @@
  * \param version the version string (typically a SVN revision keyword string)
  * \return nothing
  *
+ * \note As for 13.4.0, the \c version parameter is ignored.
+ *
  * This function should not be called directly, but instead the
  * ASTERISK_FILE_VERSION macro should be used to register a file with the core.
  */
@@ -175,12 +177,28 @@
  */
 void ast_unregister_file_version(const char *file);
 
-/*! \brief Find version for given module name
+/*!
+ * \brief Find version for given module name
  * \param file Module name (i.e. chan_sip.so)
- * \return version string or NULL if the module is not found
+ *
+ * \note As of 13.4.0, the file version is no longer tracked. As such,
+ * this function is a NoOp and will always return NULL.
+ *
+ * \retval NULL
  */
 const char *ast_file_version_find(const char *file);
 
+/*!
+ * \brief Complete a source file name
+ * \param partial The partial name of the file to look up.
+ * \param n The n-th match to return.
+ *
+ * \retval NULL if there is no match for partial at the n-th position
+ * \retval Matching source file name
+ *
+ * \note A matching source file is allocataed on the heap, and must be
+ * free'd by the caller.
+ */
 char *ast_complete_source_filename(const char *partial, int n);
 
 /*!
diff --git a/main/asterisk.c b/main/asterisk.c
index aa9d1f6..aee6403 100644
--- a/main/asterisk.c
+++ b/main/asterisk.c
@@ -475,84 +475,67 @@
 } sig_flags;
 
 #if !defined(LOW_MEMORY)
-struct file_version {
-	AST_RWLIST_ENTRY(file_version) list;
+struct registered_file {
+	AST_RWLIST_ENTRY(registered_file) list;
 	const char *file;
-	char *version;
 };
 
-static AST_RWLIST_HEAD_STATIC(file_versions, file_version);
+static AST_RWLIST_HEAD_STATIC(registered_files, registered_file);
 
 void ast_register_file_version(const char *file, const char *version)
 {
-	struct file_version *new;
-	char *work;
-	size_t version_length;
+	struct registered_file *reg;
 
-	work = ast_strdupa(version);
-	work = ast_strip(ast_strip_quoted(work, "$", "$"));
-	version_length = strlen(work) + 1;
-
-	if (!(new = ast_calloc(1, sizeof(*new) + version_length)))
+	reg = ast_calloc(1, sizeof(*reg));
+	if (!reg) {
 		return;
+	}
 
-	new->file = file;
-	new->version = (char *) new + sizeof(*new);
-	memcpy(new->version, work, version_length);
-	AST_RWLIST_WRLOCK(&file_versions);
-	AST_RWLIST_INSERT_HEAD(&file_versions, new, list);
-	AST_RWLIST_UNLOCK(&file_versions);
+	reg->file = file;
+	AST_RWLIST_WRLOCK(&registered_files);
+	AST_RWLIST_INSERT_HEAD(&registered_files, reg, list);
+	AST_RWLIST_UNLOCK(&registered_files);
 }
 
 void ast_unregister_file_version(const char *file)
 {
-	struct file_version *find;
+	struct registered_file *find;
 
-	AST_RWLIST_WRLOCK(&file_versions);
-	AST_RWLIST_TRAVERSE_SAFE_BEGIN(&file_versions, find, list) {
+	AST_RWLIST_WRLOCK(&registered_files);
+	AST_RWLIST_TRAVERSE_SAFE_BEGIN(&registered_files, find, list) {
 		if (!strcasecmp(find->file, file)) {
 			AST_RWLIST_REMOVE_CURRENT(list);
 			break;
 		}
 	}
 	AST_RWLIST_TRAVERSE_SAFE_END;
-	AST_RWLIST_UNLOCK(&file_versions);
+	AST_RWLIST_UNLOCK(&registered_files);
 
-	if (find)
+	if (find) {
 		ast_free(find);
+	}
 }
 
 char *ast_complete_source_filename(const char *partial, int n)
 {
-	struct file_version *find;
+	struct registered_file *find;
 	size_t len = strlen(partial);
 	int count = 0;
 	char *res = NULL;
 
-	AST_RWLIST_RDLOCK(&file_versions);
-	AST_RWLIST_TRAVERSE(&file_versions, find, list) {
+	AST_RWLIST_RDLOCK(&registered_files);
+	AST_RWLIST_TRAVERSE(&registered_files, find, list) {
 		if (!strncasecmp(find->file, partial, len) && ++count > n) {
 			res = ast_strdup(find->file);
 			break;
 		}
 	}
-	AST_RWLIST_UNLOCK(&file_versions);
+	AST_RWLIST_UNLOCK(&registered_files);
 	return res;
 }
 
-/*! \brief Find version for given module name */
 const char *ast_file_version_find(const char *file)
 {
-	struct file_version *iterator;
-
-	AST_RWLIST_WRLOCK(&file_versions);
-	AST_RWLIST_TRAVERSE(&file_versions, iterator, list) {
-		if (!strcasecmp(iterator->file, file))
-			break;
-	}
-	AST_RWLIST_UNLOCK(&file_versions);
-	if (iterator)
-		return iterator->version;
 	return NULL;
 }
 
@@ -1030,88 +1013,6 @@
 	return CLI_SUCCESS;
 }
 #undef DEFINE_PROFILE_MIN_MAX_VALUES
-
-/*! \brief CLI command to list module versions */
-static char *handle_show_version_files(struct ast_cli_entry *e, int cmd, struct ast_cli_args *a)
-{
-#define FORMAT "%-25.25s %-40.40s\n"
-	struct file_version *iterator;
-	regex_t regexbuf;
-	int havepattern = 0;
-	int havename = 0;
-	int count_files = 0;
-	char *ret = NULL;
-	int matchlen, which = 0;
-	struct file_version *find;
-
-	switch (cmd) {
-	case CLI_INIT:
-		e->command = "core show file version [like]";
-		e->usage =
-			"Usage: core show file version [like <pattern>]\n"
-			"       Lists the revision numbers of the files used to build this copy of Asterisk.\n"
-			"       Optional regular expression pattern is used to filter the file list.\n";
-		return NULL;
-	case CLI_GENERATE:
-		matchlen = strlen(a->word);
-		if (a->pos != 3)
-			return NULL;
-		AST_RWLIST_RDLOCK(&file_versions);
-		AST_RWLIST_TRAVERSE(&file_versions, find, list) {
-			if (!strncasecmp(a->word, find->file, matchlen) && ++which > a->n) {
-				ret = ast_strdup(find->file);
-				break;
-			}
-		}
-		AST_RWLIST_UNLOCK(&file_versions);
-		return ret;
-	}
-
-
-	switch (a->argc) {
-	case 6:
-		if (!strcasecmp(a->argv[4], "like")) {
-			if (regcomp(&regexbuf, a->argv[5], REG_EXTENDED | REG_NOSUB))
-				return CLI_SHOWUSAGE;
-			havepattern = 1;
-		} else
-			return CLI_SHOWUSAGE;
-		break;
-	case 5:
-		havename = 1;
-		break;
-	case 4:
-		break;
-	default:
-		return CLI_SHOWUSAGE;
-	}
-
-	ast_cli(a->fd, FORMAT, "File", "Revision");
-	ast_cli(a->fd, FORMAT, "----", "--------");
-	AST_RWLIST_RDLOCK(&file_versions);
-	AST_RWLIST_TRAVERSE(&file_versions, iterator, list) {
-		if (havename && strcasecmp(iterator->file, a->argv[4]))
-			continue;
-
-		if (havepattern && regexec(&regexbuf, iterator->file, 0, NULL, 0))
-			continue;
-
-		ast_cli(a->fd, FORMAT, iterator->file, iterator->version);
-		count_files++;
-		if (havename)
-			break;
-	}
-	AST_RWLIST_UNLOCK(&file_versions);
-	if (!havename) {
-		ast_cli(a->fd, "%d files listed.\n", count_files);
-	}
-
-	if (havepattern)
-		regfree(&regexbuf);
-
-	return CLI_SUCCESS;
-#undef FORMAT
-}
 
 #endif /* ! LOW_MEMORY */
 
@@ -2702,7 +2603,6 @@
 	AST_CLI_DEFINE(handle_version, "Display version info"),
 	AST_CLI_DEFINE(handle_bang, "Execute a shell command"),
 #if !defined(LOW_MEMORY)
-	AST_CLI_DEFINE(handle_show_version_files, "List versions of files used to build Asterisk"),
 	AST_CLI_DEFINE(handle_show_threads, "Show running threads"),
 #if defined(HAVE_SYSINFO) || defined(HAVE_SYSCTL)
 	AST_CLI_DEFINE(handle_show_sysinfo, "Show System Information"),
diff --git a/main/manager.c b/main/manager.c
index be780e7..edfdfd8 100644
--- a/main/manager.c
+++ b/main/manager.c
@@ -5961,9 +5961,6 @@
 	const char *module = astman_get_header(m, "Module");
 	const char *id = astman_get_header(m, "ActionID");
 	char idText[256];
-#if !defined(LOW_MEMORY)
-	const char *version;
-#endif
 	char filename[PATH_MAX];
 	char *cut;
 
@@ -5980,11 +5977,6 @@
 		astman_send_error(s, m, "Module not loaded");
 		return 0;
 	}
-	snprintf(cut, (sizeof(filename) - strlen(filename)) - 1, ".c");
-	ast_debug(1, "**** ModuleCheck .c file %s\n", filename);
-#if !defined(LOW_MEMORY)
-	version = ast_file_version_find(filename);
-#endif
 
 	if (!ast_strlen_zero(id)) {
 		snprintf(idText, sizeof(idText), "ActionID: %s\r\n", id);
@@ -5993,7 +5985,7 @@
 	}
 	astman_append(s, "Response: Success\r\n%s", idText);
 #if !defined(LOW_MEMORY)
-	astman_append(s, "Version: %s\r\n\r\n", version ? version : "");
+	astman_append(s, "Version: %s\r\n\r\n", "");
 #endif
 	return 0;
 }

-- 
To view, visit https://gerrit.asterisk.org/59
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib82ec4e23d9f1d445e94430884ef60134fd0bfa7
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Matt Jordan <mjordan at digium.com>



More information about the asterisk-dev mailing list