[Asterisk-code-review] menuselect: Don't erroneously recompile modules. (asterisk[18])

[N A]

N A has uploaded this change for review. ( https://gerrit.asterisk.org/c/asterisk/+/18471 )


Change subject: menuselect: Don't erroneously recompile modules.
......................................................................

menuselect: Don't erroneously recompile modules.

A bug in menuselect can cause modules that are disabled
by default to be recompiled every time a recompilation
occurs. This occurs for module categories that are NOT
positive output, as for these categories, the modules
contained in the makeopts file indicate modules which
should NOT be selected. The existing procedure of iterating
through these modules to mark modules as present is thus
insufficient. This has led to modules with a default_enabled
tag of "no" to get deleted and recompiled every time, even
when they haven't changed.

To fix this, we now modify the mark as present behavior
for module categories that are not positive output. For
these, we start by iterating through the module tree
and marking all modules as present, then go back and
mark anything contained in the makeopts file as not
present. This ensures that makeopt selections are actually
used properly, regardless of whether a module category
uses positive output or not.

ASTERISK-29728 #close

Change-Id: Idf2974c4ed8d0ba3738a92f08a6082b234277b95
---
M menuselect/menuselect.c
1 file changed, 59 insertions(+), 11 deletions(-)



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

diff --git a/menuselect/menuselect.c b/menuselect/menuselect.c
index 3153ce0..54283ed 100644
--- a/menuselect/menuselect.c
+++ b/menuselect/menuselect.c
@@ -1130,8 +1130,7 @@
 	return res;
 }
 
-/*! \brief Given the string representation of a member and category, mark it as present in a given input file */
-static void mark_as_present(const char *member, const char *category)
+static void mark_as_present_helper(const char *member, const char *category, int present)
 {
 	struct category *cat;
 	struct member *mem;
@@ -1142,31 +1141,44 @@
 		negate = 1;
 	}
 
-	print_debug("Marking %s of %s as present\n", member, category);
+	print_debug("Marking %s of %s as %s\n", member, category, present ? "present" : "not present");
 
 	AST_LIST_TRAVERSE(&categories, cat, list) {
-		if (strcmp(category, cat->name))
+		if (strcmp(category, cat->name)) {
 			continue;
+		}
 		AST_LIST_TRAVERSE(&cat->members, mem, list) {
 			if (mem->is_separator) {
 				continue;
 			}
 
 			if (!strcmp(member, mem->name)) {
-				mem->was_enabled = mem->enabled = (negate ? !cat->positive_output : cat->positive_output);
+				if (present) {
+					mem->was_enabled = mem->enabled = (negate ? !cat->positive_output : cat->positive_output);
+				} else {
+					mem->was_enabled = mem->enabled = 0;
+				}
 				print_debug("Just set %s enabled to %d\n", mem->name, mem->enabled);
 				break;
 			}
 		}
-		if (!mem)
+		if (!mem) {
 			fprintf(stderr, "member '%s' in category '%s' not found, ignoring.\n", member, category);
+		}
 		break;
 	}
 
-	if (!cat)
+	if (!cat) {
 		fprintf(stderr, "category '%s' not found! Can't mark '%s' as disabled.\n", category, member);
+	}
 }
 
+/*! \brief Given the string representation of a member and category, mark it as present in a given input file */
+#define mark_as_present(member, category) mark_as_present_helper(member, category, 1)
+
+/*! \brief Given the string representation of a member and category, mark it as not present in a given input file */
+#define mark_as_not_present(member, category) mark_as_present_helper(member, category, 0)
+
 unsigned int enable_member(struct member *mem)
 {
 	struct reference *dep;
@@ -1380,6 +1392,9 @@
 	}
 
 	while (fgets(buf, PARSE_BUF_SIZE, f)) {
+		struct category *cat;
+		struct member *mem;
+
 		lineno++;
 
 		if (strlen_zero(buf))
@@ -1414,11 +1429,44 @@
 			continue;
 		}
 
-		while ((member = strsep(&parse, " \n"))) {
-			member = skip_blanks(member);
-			if (strlen_zero(member))
+		AST_LIST_TRAVERSE(&categories, cat, list) {
+			if (strcmp(category, cat->name)) {
 				continue;
-			mark_as_present(member, category);
+			}
+			if (!cat->positive_output) {
+				print_debug("Category %s is NOT positive output\n", cat->name);
+				/* if NOT positive_output, then if listed in makeopts, it's disabled! */
+				/* this means that what's listed in menuselect.makeopts is a list of modules
+				 * that are NOT selected, so we can't use that to mark things as present.
+				 * In fact, we need to mark everything as present, UNLESS it's listed
+				* in menuselect.makeopts */
+				AST_LIST_TRAVERSE(&cat->members, mem, list) {
+					if (mem->is_separator) {
+						continue;
+					}
+					mem->was_enabled = 1;
+					print_debug("Just set %s enabled to %d\n", mem->name, mem->enabled);
+				}	
+				/* okay, now go ahead, and mark anything listed in makeopts as NOT present */
+				while ((member = strsep(&parse, " \n"))) {
+					member = skip_blanks(member);
+					if (strlen_zero(member)) {
+						continue;
+					}
+					mark_as_not_present(member, category);
+				}
+			} else {
+				print_debug("Category %s is positive output\n", cat->name);
+				/* if present, it was enabled (e.g. MENUSELECT_CFLAGS, MENUSELECT_UTILS, MENUSELECT_MOH, etc. */
+				while ((member = strsep(&parse, " \n"))) {
+					member = skip_blanks(member);
+					if (strlen_zero(member)) {
+						continue;
+					}
+					mark_as_present(member, category);
+				}
+			}
+			break;
 		}
 	}
 

-- 
To view, visit https://gerrit.asterisk.org/c/asterisk/+/18471
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: 18
Gerrit-Change-Id: Idf2974c4ed8d0ba3738a92f08a6082b234277b95
Gerrit-Change-Number: 18471
Gerrit-PatchSet: 1
Gerrit-Owner: N A <mail at interlinked.x10host.com>
Gerrit-MessageType: newchange
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20220427/60d16020/attachment.html>