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

[Kevin Harwell]

Kevin Harwell has submitted this change. ( https://gerrit.asterisk.org/c/asterisk/+/18472 )

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(-)

Approvals:
  Joshua Colp: Looks good to me, but someone else must approve
  Kevin Harwell: Looks good to me, approved; Approved for Submit



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/+/18472
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: 19
Gerrit-Change-Id: Idf2974c4ed8d0ba3738a92f08a6082b234277b95
Gerrit-Change-Number: 18472
Gerrit-PatchSet: 2
Gerrit-Owner: N A <mail at interlinked.x10host.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: Joshua Colp <jcolp at sangoma.com>
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20220428/61943282/attachment-0001.html>