| Summary: | ASTERISK-13271: menuselect sets defaults too late | ||||
| Reporter: | John Nemeth (jnemeth) | Labels: | |||
| Date Opened: | 2008-12-25 05:17:38.000-0600 | Date Closed: | 2015-04-15 21:27:12 | ||
| Priority: | Minor | Regression? | No | ||
| Status: | Closed/Complete | Components: | Utilities/General | ||
| Versions: | Frequency of Occurrence | ||||
| Related Issues: |
| ||||
| Environment: | Attachments: | ( 0) makeopts-fix.patch | |||
| Description: | The main Makefile contains the following target (which is called at the start of the all target):
{noformat} menuselect.makeopts: menuselect/menuselect menuselect-tree makeopts menuselect/menuselect --check-deps menuselect.makeopts $(GLOBAL_MAKEOPTS) $(USER_MAKEOPTS) {noformat} The purpose of this target is to allow the user to have {{/etc/asterisk.makeopts}} and/or {{~/.asterisk.makeopts}} to have your options set for you (i.e. to easily set the same options when upgrading as documented in "Build Process (module selection)" section of UPGRADE-1.4.txt; however, it doesn't work. The bug is in {{menuselect/menuselect.c}}. Its {{main()}} function contains the following: {code} ... 1414 for (x = 1; x < argc; x++) { 1415 if (!strcmp(argv[x], "--check-deps")) 1416 check_deps = 1; 1417 else { 1418 res = parse_existing_config(argv[x]); 1419 if (!res && !strcasecmp(argv[x], OUTPUT_MAKEOPTS_DEFAULT)) 1420 existing_config = 1; 1421 res = 0; 1422 } 1423 } ... 1430 if (!existing_config) 1431 process_defaults(); 1432 else if (check_deps) 1433 res = sanity_check(); ... {code} The loop starting at 1414 processes the arguments to menuselect. Starting at 1418 it processes things that are files. At 1419 if {{menuselect.makeopts}} is seen and successfully processed then it sets the {{existing_config}} flag. The first time that the {{menuselect.makeopts}} target of the main Makefile is called, the {{menuselect.makeopts}} file does not exist. However, if {{/etc/asterisk.makeopts}} or {{~/.asterisk.makeopts}} does, then it will be processed. If you use a negative member to flip an option from its default (i.e. {{MENUSELECT_PBX=-pbx_gtkconsole}}) this will happen. However, because the {{existing_config}} flag hasn't been set then down in line 1431 all options will be set to their default negating the effect of the options that you set. {{menuselect}} will then write out a {{menuselect.makeopts}} containing all the default options. If you then do "make menuselect.makeopts" a second time, it would work because menuselect.makeopts exists so the existing_config flag gets set. *ADDITIONAL INFORMATION* I work around this problem by doing "make menuselect.makeopts" after running configure. This way when "make all" runs the {{menuselect.makeopts}} target, the {{menuselect.makeopts}} file exists so it works properly. Additionally, it would be nice if you could set a positive option (i.e. {{MENUSELECT_PBX=+pbx_gtkconsole}}) to always enable an option regardless of its current setting (assuming dependencies have been met). | ||||
| Comments: | By: Digium Subversion (svnbot) 2009-01-30 17:36:37.000-0600 Repository: menuselect Revision: 465 U branches/1.0/menuselect.c U branches/1.0/menuselect.h ------------------------------------------------------------------------ r465 | jpeeler | 2009-01-30 17:36:37 -0600 (Fri, 30 Jan 2009) | 6 lines (closes issue ASTERISK-13271) Reported by: jnemeth Add the string "asterisk.makeopts" to search for valid makeopts configuration files in the arguments provided to menuselect. Previously, the existing_config flag was not getting set causing the defaults to load when a menuselect.makeopts file was not present, even though a valid /etc/asterisk.makeopts or ~/.asterisk.makeopts file existed. Technically the use of strstr is going to allow any valid file with the string "asterisk.makeopts" in it to work. I believe this is best though since it avoids any hardcoded paths and the encouraged method of invoking menuselect is through use of the menuselect makefile targets. ------------------------------------------------------------------------ http://svn.digium.com/view/menuselect?view=rev&revision=465 By: Digium Subversion (svnbot) 2009-01-30 17:51:30.000-0600 Repository: menuselect Revision: 466 _U trunk/ U trunk/menuselect.c U trunk/menuselect.h ------------------------------------------------------------------------ r466 | jpeeler | 2009-01-30 17:51:30 -0600 (Fri, 30 Jan 2009) | 12 lines Merged revisions 465 via svnmerge from https://origsvn.digium.com/svn/menuselect/branches/1.0 ........ r465 | jpeeler | 2009-01-30 17:37:29 -0600 (Fri, 30 Jan 2009) | 6 lines (closes issue ASTERISK-13271) Reported by: jnemeth Add the string "asterisk.makeopts" to search for valid makeopts configuration files in the arguments provided to menuselect. Previously, the existing_config flag was not getting set causing the defaults to load when a menuselect.makeopts file was not present, even though a valid /etc/asterisk.makeopts or ~/.asterisk.makeopts file existed. Technically the use of strstr is going to allow any valid file with the string "asterisk.makeopts" in it to work. I believe this is best though since it avoids any hardcoded paths and the encouraged method of invoking menuselect is through use of the menuselect makefile targets. ........ ------------------------------------------------------------------------ http://svn.digium.com/view/menuselect?view=rev&revision=466 By: Digium Subversion (svnbot) 2009-02-02 11:40:20.000-0600 Repository: menuselect Revision: 467 U branches/1.0/menuselect.c U branches/1.0/menuselect.h ------------------------------------------------------------------------ r467 | jpeeler | 2009-02-02 11:40:20 -0600 (Mon, 02 Feb 2009) | 11 lines Fix incorrect assumption about saved menuselect options being project specific in r465. I did not realize that DAHDI also supports saved options, so checking specifically for an asterisk.makeopts file doesn't fix the issue entirely. This commit removes any filename checking for the existing_config flag to be set. Any valid file passed in will now work. (related to issue ASTERISK-13271) ------------------------------------------------------------------------ http://svn.digium.com/view/menuselect?view=rev&revision=467 By: Digium Subversion (svnbot) 2009-02-02 11:45:24.000-0600 Repository: menuselect Revision: 468 _U trunk/ U trunk/menuselect.c U trunk/menuselect.h ------------------------------------------------------------------------ r468 | jpeeler | 2009-02-02 11:45:24 -0600 (Mon, 02 Feb 2009) | 17 lines Merged revisions 467 via svnmerge from https://origsvn.digium.com/svn/menuselect/branches/1.0 ........ r467 | jpeeler | 2009-02-02 11:41:22 -0600 (Mon, 02 Feb 2009) | 11 lines Fix incorrect assumption about saved menuselect options being project specific in r465. I did not realize that DAHDI also supports saved options, so checking specifically for an asterisk.makeopts file doesn't fix the issue entirely. This commit removes any filename checking for the existing_config flag to be set. Any valid file passed in will now work. (related to issue ASTERISK-13271) ........ ------------------------------------------------------------------------ http://svn.digium.com/view/menuselect?view=rev&revision=468 By: Digium Subversion (svnbot) 2009-02-12 10:17:32.000-0600 Repository: menuselect Revision: 472 U branches/1.0/menuselect.c ------------------------------------------------------------------------ r472 | russell | 2009-02-12 09:58:51 -0600 (Thu, 12 Feb 2009) | 10 lines Revert rev 468. This caused a regression. This change assumed that the makeopts files in /etc/or ~/ were full makeopts files. For example, if you had a makeopts file in ~/ that only had one line to ensure that you always built with DEBUG_THREADS enabled, it would no longer work, because the menuselect sanity check would freak out. (issue ASTERISK-13271) ------------------------------------------------------------------------ ------------------------------------------------------------------------ http://svn.digium.com/view/menuselect?view=rev&revision=472 By: Digium Subversion (svnbot) 2009-02-12 10:17:45.000-0600 Repository: menuselect Revision: 471 U trunk/menuselect.c ------------------------------------------------------------------------ r471 | russell | 2009-02-11 09:11:17 -0600 (Wed, 11 Feb 2009) | 9 lines Revert rev 468. This caused a regression. This change assumed that the makeopts files in /etc/or ~/ were full makeopts files. For example, if you had a makeopts file in ~/ that only had one line to ensure that you always built with DEBUG_THREADS enabled, it would no longer work, because the menuselect sanity check would freak out. (issue ASTERISK-13271) ------------------------------------------------------------------------ http://svn.digium.com/view/menuselect?view=rev&revision=471 By: Digium Subversion (svnbot) 2009-02-12 10:32:32.000-0600 Repository: menuselect Revision: 473 U branches/1.0/menuselect.c U branches/1.0/menuselect.h ------------------------------------------------------------------------ r473 | russell | 2009-02-12 10:32:32 -0600 (Thu, 12 Feb 2009) | 5 lines Fix an issue related to processing default values after reading existing config. (closes issue ASTERISK-13271) Reported by: jnemeth ------------------------------------------------------------------------ http://svn.digium.com/view/menuselect?view=rev&revision=473 By: Digium Subversion (svnbot) 2009-02-12 10:34:10.000-0600 Repository: menuselect Revision: 474 _U trunk/ U trunk/menuselect.c U trunk/menuselect.h ------------------------------------------------------------------------ r474 | russell | 2009-02-12 10:34:10 -0600 (Thu, 12 Feb 2009) | 12 lines Merged revisions 473 via svnmerge from https://origsvn.digium.com/svn/menuselect/branches/1.0 ........ r473 | russell | 2009-02-12 10:32:32 -0600 (Thu, 12 Feb 2009) | 5 lines Fix an issue related to processing default values after reading existing config. (closes issue ASTERISK-13271) Reported by: jnemeth ........ ------------------------------------------------------------------------ http://svn.digium.com/view/menuselect?view=rev&revision=474 By: Digium Subversion (svnbot) 2009-02-12 13:23:35.000-0600 Repository: menuselect Revision: 477 U branches/1.0/menuselect.c U branches/1.0/menuselect.h U branches/1.0/menuselect_curses.c ------------------------------------------------------------------------ r477 | russell | 2009-02-12 13:23:34 -0600 (Thu, 12 Feb 2009) | 4 lines Revert back to revision 464, as I am still having problems with the changes. (related to issue ASTERISK-13271) ------------------------------------------------------------------------ http://svn.digium.com/view/menuselect?view=rev&revision=477 By: Digium Subversion (svnbot) 2009-02-12 13:24:14.000-0600 Repository: menuselect Revision: 478 _U trunk/ U trunk/menuselect.c U trunk/menuselect.h U trunk/menuselect_curses.c ------------------------------------------------------------------------ r478 | russell | 2009-02-12 13:24:14 -0600 (Thu, 12 Feb 2009) | 11 lines Merged revisions 477 via svnmerge from https://origsvn.digium.com/svn/menuselect/branches/1.0 ........ r477 | russell | 2009-02-12 13:23:34 -0600 (Thu, 12 Feb 2009) | 4 lines Revert back to revision 464, as I am still having problems with the changes. (related to issue ASTERISK-13271) ........ ------------------------------------------------------------------------ http://svn.digium.com/view/menuselect?view=rev&revision=478 By: John Nemeth (jnemeth) 2010-05-06 16:34:01 The person that worked on this issue found that their attempted solutions didn't work and reverted all of them. Thus, this issue was never actually resolved. By: Corey Farrell (coreyfarrell) 2015-03-27 21:57:02.605-0500 I looked into this ticket, and I was able to verify that {{~/.asterisk.makeopts}} was ignored by {{make menuselect}}. I believe my patch resolves this issue, it would be nice if someone could take a look. I have not tested with {{/etc/asterisk.makeopts}}, but this patch should cover that. menuselect.makeopts is created if not present from: # Copy of file in $HOME if it exists # Copy of file in /etc if it exists # Generated defaults by menuselect So one thought, {{~/.asterisk.makeopts}} and {{/etc/asterisk.makeopts}} do not work in any current release. For this reason maybe we should only fix this in trunk. Someone could have one of these files on their system left over from ages ago, it would be a problem for this to suddenly start working. | ||||