{"thread":{"id":"21447","subject":"[PATCH 1/2] configure: add function to directly set Makefile variables","startedAt":"2009-10-31T20:41:33Z","lastAt":"2009-11-04T20:16:58Z","messageCount":15,"participants":["Ben Walton","Jonathan Nieder","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"126534","messageId":"1257021695-21260-1-git-send-email-bwalton@artsci.utoronto.ca","threadId":"21447","inReplyTo":null,"subject":"[PATCH 0/2] Set Makefile variables from configure","fromName":"Ben Walton","fromEmail":"bwalton@artsci.utoronto.ca","sentAt":"2009-10-31T20:41:33Z","receivedAt":"2009-10-31T20:41:33Z","isPatch":true,"sender":{"key":"bdwalton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/396061?v=4"},"body":"These patches add support for setting the newly created DEFAULT_EDITOR\nand DEFAULT_PAGER from the configure script.  I also tacked in\nETC_GITCONFIG, since I can't currently toggle this without setting a\ncommand line value when building, but have need to alter it.\n\nThe function added is generic, and will allow for setting new\nvariables as needed in the future.  No validation is done on the\nvalues.  It is less specific than the *_PATH setting functions that\nalready exist.\n\nBen Walton (2):\n  configure: add function to directly set Makefile variables\n  configure: allow user to set gitconfig, pager and editor\n\n configure.ac |   24 ++++++++++++++++++++++++\n 1 files changed, 24 insertions(+), 0 deletions(-)\n"},{"id":"126533","messageId":"1257021695-21260-2-git-send-email-bwalton@artsci.utoronto.ca","threadId":"21447","inReplyTo":"1257021695-21260-1-git-send-email-bwalton@artsci.utoronto.ca","subject":"[PATCH 1/2] configure: add function to directly set Makefile variables","fromName":"Ben Walton","fromEmail":"bwalton@artsci.utoronto.ca","sentAt":"2009-10-31T20:41:34Z","receivedAt":"2009-10-31T20:41:34Z","isPatch":true,"sender":{"key":"bdwalton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/396061?v=4"},"body":"Add function GIT_WITH_MAKE_VAR to provide an easy way to allow user\ninput to directly specify values for variables in the Makefile.\n\nAn example use is:\nGIT_WITH_MAKE_VAR(gitconfig, ETC_GITCONFIG)\n\nThis would allow the user to add --with-gitconfig=/etc/mysiteconf\nto their ./configure command line to add\nETC_GITCONFIG=/etc/mysiteconf to the config.mak.autogen file.\n\nSigned-off-by: Ben Walton <bwalton@artsci.utoronto.ca>\n---\n configure.ac |   14 ++++++++++++++\n 1 files changed, 14 insertions(+), 0 deletions(-)\n\ndiff --git a/configure.ac b/configure.ac\nindex 84b6cf4..2829dbb 100644\n--- a/configure.ac\n+++ b/configure.ac\n@@ -68,6 +68,20 @@ else \\\n \tGIT_CONF_APPEND_LINE(${PACKAGE}DIR=$withval); \\\n fi \\\n ])# GIT_PARSE_WITH\n+dnl\n+dnl GIT_WITH_MAKE_VAR(withname, VAR)\n+dnl ---------------------\n+dnl Set VAR to the value specied by --with-$withname if --with-$withname\n+dnl is specified.  This is a direct way to allow setting variables in the\n+dnl Makefile.\n+AC_DEFUN([GIT_WITH_MAKE_VAR],\n+[AC_ARG_WITH([$1],\n+ [AS_HELP_STRING([--with-$1=VALUE],\n+\t\t [provide value for $2])],\n+ if test -n \"$withval\"; then \\\n+    AC_MSG_NOTICE([Setting $2 to $withval]); \\\n+    GIT_CONF_APPEND_LINE($2=$withval); \\\n+ fi)])# GIT_WITH_MAKE_VAR\n \n dnl\n dnl GIT_CHECK_FUNC(FUNCTION, IFTRUE, IFFALSE)\n-- \n1.6.5\n"},{"id":"126535","messageId":"1257021695-21260-3-git-send-email-bwalton@artsci.utoronto.ca","threadId":"21447","inReplyTo":"1257021695-21260-2-git-send-email-bwalton@artsci.utoronto.ca","subject":"[PATCH 2/2] configure: allow user to set gitconfig, pager and editor","fromName":"Ben Walton","fromEmail":"bwalton@artsci.utoronto.ca","sentAt":"2009-10-31T20:41:35Z","receivedAt":"2009-10-31T20:41:35Z","isPatch":true,"sender":{"key":"bdwalton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/396061?v=4"},"body":"Use the new GIT_WITH_MAKE_VAR function to allow user specified\nvalues for ETC_GITCONFIG, DEFAULT_PAGER and DEFAULT_EDITOR.\n\nSigned-off-by: Ben Walton <bwalton@artsci.utoronto.ca>\n---\n configure.ac |   10 ++++++++++\n 1 files changed, 10 insertions(+), 0 deletions(-)\n\ndiff --git a/configure.ac b/configure.ac\nindex 2829dbb..d50d492 100644\n--- a/configure.ac\n+++ b/configure.ac\n@@ -241,6 +241,16 @@ GIT_PARSE_WITH(iconv))\n # change being considered an inode change from the update-index perspective.\n \n #\n+# Allow user to set ETC_GITCONFIG variable\n+GIT_WITH_MAKE_VAR(gitconfig, ETC_GITCONFIG)\n+#\n+# Allow user to set the default pager\n+GIT_WITH_MAKE_VAR(pager, DEFAULT_PAGER)\n+#\n+# Allow user to set the default editor\n+GIT_WITH_MAKE_VAR(editor, DEFAULT_EDITOR)\n+\n+#\n # Define SHELL_PATH to provide path to shell.\n GIT_ARG_SET_PATH(shell)\n #\n-- \n1.6.5\n"},{"id":"126660","messageId":"1257269082-sup-5056@ntdws12.chass.utoronto.ca","threadId":"21447","inReplyTo":"1257021695-21260-1-git-send-email-bwalton@artsci.utoronto.ca","subject":"Re: [PATCH 0/2] Set Makefile variables from configure","fromName":"Ben Walton","fromEmail":"bwalton@artsci.utoronto.ca","sentAt":"2009-11-03T17:25:48Z","receivedAt":"2009-11-03T17:25:48Z","isPatch":true,"sender":{"key":"bdwalton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/396061?v=4"},"body":"Excerpts from Ben Walton's message of Sat Oct 31 16:41:33 -0400 2009:\n\nPing?  Is this useful or should I drop it?\n\nThanks\n-Ben\n\n> These patches add support for setting the newly created DEFAULT_EDITOR\n> and DEFAULT_PAGER from the configure script.  I also tacked in\n> ETC_GITCONFIG, since I can't currently toggle this without setting a\n> command line value when building, but have need to alter it.\n> \n> The function added is generic, and will allow for setting new\n> variables as needed in the future.  No validation is done on the\n> values.  It is less specific than the *_PATH setting functions that\n> already exist.\n> \n> Ben Walton (2):\n>   configure: add function to directly set Makefile variables\n>   configure: allow user to set gitconfig, pager and editor\n> \n>  configure.ac |   24 ++++++++++++++++++++++++\n>  1 files changed, 24 insertions(+), 0 deletions(-)\n-- \nBen Walton\nSystems Programmer - CHASS\nUniversity of Toronto\nC:416.407.5610 | W:416.978.4302\n\nGPG Key Id: 8E89F6D2; Key Server: pgp.mit.edu\nContact me to arrange for a CAcert assurance meeting.\n"},{"id":"126680","messageId":"20091103215408.GA13720@progeny.tock","threadId":"21447","inReplyTo":"1257269082-sup-5056@ntdws12.chass.utoronto.ca","subject":"Re: [PATCH 0/2] Set Makefile variables from configure","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2009-11-03T21:54:08Z","receivedAt":"2009-11-03T21:54:08Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Ben Walton wrote:\n> Excerpts from Ben Walton's message of Sat Oct 31 16:41:33 -0400 2009:\n \n>> These patches add support for setting the newly created DEFAULT_EDITOR\n>> and DEFAULT_PAGER from the configure script.  I also tacked in\n>> ETC_GITCONFIG, since I can't currently toggle this without setting a\n>> command line value when building, but have need to alter it.\n[...]\n> Ping?  Is this useful or should I drop it?\n\nI am not sure these patches alone would make the configuration much\neasier.  The --help text means one has to check the Makefile to learn\nwhat these options do, anyway:\n\n--with-editor=VALUE\tprovide value for DEFAULT_EDITOR\n--with-pager=VALUE\tprovide value for DEFAULT_PAGER\n\nBut then, why not just set the value directly in the Makefile, 'make'\ncommand line, or config.mak?\n\nMaybe something like this could help.  The help text is not carefully\nwritten; it’s just a placeholder to show the idea.\n\n configure.ac |   56 ++++++++++++++++++++++++++++++++++++++++----------------\n 1 files changed, 40 insertions(+), 16 deletions(-)\n\ndiff --git a/configure.ac b/configure.ac\nindex f8db034..aeef08c 100644\n--- a/configure.ac\n+++ b/configure.ac\n@@ -68,20 +68,25 @@ else \\\n \tGIT_CONF_APPEND_LINE(${PACKAGE}DIR=$withval); \\\n fi \\\n ])# GIT_PARSE_WITH\n+\n dnl\n-dnl GIT_WITH_MAKE_VAR(withname, VAR)\n-dnl ---------------------\n-dnl Set VAR to the value specied by --with-$withname if --with-$withname\n-dnl is specified.  This is a direct way to allow setting variables in the\n-dnl Makefile.\n-AC_DEFUN([GIT_WITH_MAKE_VAR],\n-[AC_ARG_WITH([$1],\n- [AS_HELP_STRING([--with-$1=VALUE],\n-\t\t [provide value for $2])],\n- if test -n \"$withval\"; then \\\n-    AC_MSG_NOTICE([Setting $2 to $withval]); \\\n-    GIT_CONF_APPEND_LINE($2=$withval); \\\n- fi)])# GIT_WITH_MAKE_VAR\n+dnl GIT_PARSE_VALUE(WITHNAME, VAR)\n+dnl ------------------------------\n+dnl For use in AC_ARG_WITH action-if-found, for tweakable values that\n+dnl are not actually enabling or disabling anything.\n+dnl  * Set VAR=ARG for --with-WITHNAME=ARG\n+dnl  * Complain for --without-WITHNAME or --with-WITHNAME without ARG\n+AC_DEFUN([GIT_PARSE_VALUE], [\n+if test \"$withval\" = \"no\"; then\n+\tAC_MSG_ERROR([$1 cannot be disabled])\n+fi\n+if test \"$withval\" = \"yes\"; then\n+\tAC_MSG_WARN([You should provide a value for --with-$1])\n+else\n+\tAC_MSG_NOTICE([Setting $2 to $withval])\n+\tGIT_CONF_APPEND_LINE($2=$withval)\n+fi\n+])\n \n dnl\n dnl GIT_CHECK_FUNC(FUNCTION, IFTRUE, IFFALSE)\n@@ -242,13 +247,32 @@ GIT_PARSE_WITH(iconv))\n \n #\n # Allow user to set ETC_GITCONFIG variable\n-GIT_WITH_MAKE_VAR(gitconfig, ETC_GITCONFIG)\n+AC_ARG_WITH(gitconfig,\n+  [AS_HELP_STRING([--with-gitconfig=PATH],\n+                  [Use PATH instead of /etc/gitconfig.]\n+                  [Interpreted by git as a path relative to the computed]\n+                  [prefix at run-time.])],\n+  [GIT_PARSE_VALUE(gitconfig, ETC_GITCONFIG)])\n+\n #\n # Allow user to set the default pager\n-GIT_WITH_MAKE_VAR(pager, DEFAULT_PAGER)\n+AC_ARG_WITH(pager,\n+  [AS_HELP_STRING([--with-pager=CMD],\n+                  [Set fall-back pager to CMD instead of less.]\n+                  [The fall-back pager will be used by commands such]\n+                  [as git log if the user does not specify some]\n+                  [preferred pager.])],\n+  [GIT_PARSE_VALUE(pager, DEFAULT_PAGER)])\n+\n #\n # Allow user to set the default editor\n-GIT_WITH_MAKE_VAR(editor, DEFAULT_EDITOR)\n+AC_ARG_WITH(editor,\n+  [AS_HELP_STRING([--with-editor=CMD],\n+                  [Set fall-back text editor to CMD instead of vi.]\n+                  [The fall-back editor will be used by commands such]\n+                  [as git commit if the user does not specify some]\n+                  [preferred editor.])],\n+  [GIT_PARSE_VALUE(editor, DEFAULT_EDITOR)])\n \n #\n # Define SHELL_PATH to provide path to shell.\n-- \n1.6.5.2\n"},{"id":"126682","messageId":"20091103222123.GA17097@progeny.tock","threadId":"21447","inReplyTo":"1257021695-21260-1-git-send-email-bwalton@artsci.utoronto.ca","subject":"Re: [PATCH 0/2] Set Makefile variables from configure","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2009-11-03T22:21:23Z","receivedAt":"2009-11-03T22:21:23Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Ben Walton wrote:\n> These patches add support for setting the newly created DEFAULT_EDITOR\n> and DEFAULT_PAGER from the configure script.  I also tacked in\n> ETC_GITCONFIG, since I can't currently toggle this without setting a\n> command line value when building, but have need to alter it.\n\nWould the --sysconfdir option work for you here?  Setting --sysconfdir\ncurrently does nothing, so the question is kind of moot without some\nchange like this (untested):\n\nHook up more of ./configure’s installation directory options to\nactually do something.  Unfortunately, this defeats the RUNTIME_PREFIX\nfacility unless all the relevant paths are explicitly set.  Probably\nthe defaults set in Makefile should not be overridden unless\nexplicitly requested.\n\nThis patch also changes the default location for HTML documentation\nafter running ./configure from /usr/local/share/git-doc to\n/usr/local/share/git.  In my opinion, the fix to this would also be to\nmake the configure script not override defaults from Makefile.\n\n config.mak.in |   14 +++++++++-----\n 1 files changed, 9 insertions(+), 5 deletions(-)\n\ndiff --git a/config.mak.in b/config.mak.in\nindex 67b12f7..430134a 100644\n--- a/config.mak.in\n+++ b/config.mak.in\n@@ -11,17 +11,21 @@ TAR = @TAR@\n TCLTK_PATH = @TCLTK_PATH@\n \n prefix = @prefix@\n-exec_prefix = @exec_prefix@\n bindir = @bindir@\n+mandir = @mandir@\n+infodir = @infodir@\n gitexecdir = @libexecdir@/git-core\n-datarootdir = @datarootdir@\n+sharedir = @datadir@\n template_dir = @datadir@/git-core/templates\n+htmldir = @htmldir@\n+sysconfdir = @sysconfdir@\n \n-mandir=@mandir@\n-\n+exec_prefix = @exec_prefix@\n+datarootdir = @datarootdir@\n+PACKAGE_TARNAME = @PACKAGE_TARNAME@\n+docdir = @docdir@\n srcdir = @srcdir@\n VPATH = @srcdir@\n-\n export exec_prefix mandir\n export srcdir VPATH\n \n-- \n1.6.5.2\n"},{"id":"126784","messageId":"1257357960-12763-1-git-send-email-bwalton@artsci.utoronto.ca","threadId":"21447","inReplyTo":"20091103222123.GA17097@progeny.tock","subject":"[PATCH 0/2] Set Makefile variables from configure","fromName":"Ben Walton","fromEmail":"bwalton@artsci.utoronto.ca","sentAt":"2009-11-04T18:05:58Z","receivedAt":"2009-11-04T18:05:58Z","isPatch":true,"sender":{"key":"bdwalton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/396061?v=4"},"body":"Ok, a second attempt, taking into account Jonathan's feedback and\nproposed updates.  The revised macro allows for specified help text.\n\nI opted to use a more 'dry' approach to the macro instead of repeating\nthe boilerplate AC_WITH_ARG bits when calling this feature.\n\nAdditionally, I only WARN when 'yes' or 'no' is passed.  Bailing on\nerrors in most cases is reasonable, but I don't think it's globally\nappropriate.\n\nAs to why I want this...It's cleaner, in my opinion, to do all build\nconfiguration through a single mechanism than to do most with\n./configure and the rest with variables passed to make.  In other\nwords, this is purely a style issue.\n\nI like the extension of config.mak.in to support more of the autoconf\nvariables as well and would like to see it applied.\n\nBen Walton (2):\n  configure: add macro to set arbitrary make variables\n  configure: add settings for gitconfig, editor and pager\n\n configure.ac |   43 +++++++++++++++++++++++++++++++++++++++++++\n 1 files changed, 43 insertions(+), 0 deletions(-)\n"},{"id":"126785","messageId":"1257357960-12763-2-git-send-email-bwalton@artsci.utoronto.ca","threadId":"21447","inReplyTo":"1257357960-12763-1-git-send-email-bwalton@artsci.utoronto.ca","subject":"[PATCH 1/2] configure: add macro to set arbitrary make variables","fromName":"Ben Walton","fromEmail":"bwalton@artsci.utoronto.ca","sentAt":"2009-11-04T18:05:59Z","receivedAt":"2009-11-04T18:05:59Z","isPatch":true,"sender":{"key":"bdwalton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/396061?v=4"},"body":"Add macro GIT_PARSE_WITH_SET_MAKE_VAR to configure.ac to allow --with\nstyle options that set values for variables used during the make\nprocess.\n\nArguments are the $name part of --with-$name, the name of\nthe variable to set in the Makefile (config.mak.autogen) and\nthe help text for the option.\n\nSigned-off-by: Ben Walton <bwalton@artsci.utoronto.ca>\n---\n configure.ac |   20 ++++++++++++++++++++\n 1 files changed, 20 insertions(+), 0 deletions(-)\n\ndiff --git a/configure.ac b/configure.ac\nindex 84b6cf4..a305d0b 100644\n--- a/configure.ac\n+++ b/configure.ac\n@@ -68,6 +68,26 @@ else \\\n \tGIT_CONF_APPEND_LINE(${PACKAGE}DIR=$withval); \\\n fi \\\n ])# GIT_PARSE_WITH\n+#\n+# GIT_PARSE_WITH_SET_MAKE_VAR(WITHNAME, VAR, HELP_TEXT)\n+# ---------------------\n+# Set VAR to the value specied by --with-WITHNAME.\n+# No verification of arguments is performed, but warnings are issued\n+# if either 'yes' or 'no' is specified.\n+# HELP_TEXT is presented when --help is called.\n+# This is a direct way to allow setting variables in the Makefile.\n+AC_DEFUN([GIT_PARSE_WITH_SET_MAKE_VAR],\n+[AC_ARG_WITH([$1],\n+ [AS_HELP_STRING([--with-$1=VALUE], $3)],\n+ if test -n \"$withval\"; then \\\n+  if test \"$withval\" = \"yes\" -o \"$withval\" = \"no\"; then \\\n+    AC_MSG_WARN([You likely do not want either 'yes' or 'no' as]\n+\t\t     [a value for $1 ($2).  Maybe you do...?]); \\\n+  fi; \\\n+  \\\n+  AC_MSG_NOTICE([Setting $2 to $withval]); \\\n+  GIT_CONF_APPEND_LINE($2=$withval); \\\n+ fi)])# GIT_PARSE_WITH_SET_MAKE_VAR\n \n dnl\n dnl GIT_CHECK_FUNC(FUNCTION, IFTRUE, IFFALSE)\n-- \n1.6.5.1\n"},{"id":"126786","messageId":"1257357960-12763-3-git-send-email-bwalton@artsci.utoronto.ca","threadId":"21447","inReplyTo":"1257357960-12763-2-git-send-email-bwalton@artsci.utoronto.ca","subject":"[PATCH 2/2] configure: add settings for gitconfig, editor and pager","fromName":"Ben Walton","fromEmail":"bwalton@artsci.utoronto.ca","sentAt":"2009-11-04T18:06:00Z","receivedAt":"2009-11-04T18:06:00Z","isPatch":true,"sender":{"key":"bdwalton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/396061?v=4"},"body":"Use the new GIT_PARSE_WITH_SET_MAKE_VAR macro to allow configuration\nsettings for ETC_GITCONFIG, DEFAULT_PAGER and DEFAULT_EDITOR.\n\nSigned-off-by: Ben Walton <bwalton@artsci.utoronto.ca>\n---\n configure.ac |   23 +++++++++++++++++++++++\n 1 files changed, 23 insertions(+), 0 deletions(-)\n\ndiff --git a/configure.ac b/configure.ac\nindex a305d0b..78345eb 100644\n--- a/configure.ac\n+++ b/configure.ac\n@@ -247,6 +247,29 @@ GIT_PARSE_WITH(iconv))\n # change being considered an inode change from the update-index perspective.\n \n #\n+# Allow user to set ETC_GITCONFIG variable\n+GIT_PARSE_WITH_SET_MAKE_VAR(gitconfig, ETC_GITCONFIG,\n+\t\t\tUse VALUE instead of /etc/gitconfig as the\n+\t\t\tglobal git configuration file.\n+\t\t\tIf VALUE is not fully qualified it will be interpretted\n+\t\t\tas a path relative to the computed prefix at runtime.)\n+\n+#\n+# Allow user to set the default pager\n+GIT_PARSE_WITH_SET_MAKE_VAR(pager, DEFAULT_PAGER,\n+\t\t\tUse VALUE as the fall-back pager instead of 'less'.\n+\t\t\tThis is used by things like 'git log' when the user\n+\t\t\tdoes not specify a pager to use through alternate\n+\t\t\tmethods. eg: /usr/bin/pager)\n+#\n+# Allow user to set the default editor\n+GIT_PARSE_WITH_SET_MAKE_VAR(editor, DEFAULT_EDITOR,\n+\t\t\tUse VALUE as the fall-back editor instead of 'vi'.\n+\t\t\tThis is used by things like 'git commit' when the user\n+\t\t\tdoes not specify a preferred editor through other\n+\t\t\tmethods. eg: /usr/bin/editor)\n+\n+#\n # Define SHELL_PATH to provide path to shell.\n GIT_ARG_SET_PATH(shell)\n #\n-- \n1.6.5.1\n"},{"id":"126787","messageId":"1257357992-sup-8444@ntdws12.chass.utoronto.ca","threadId":"21447","inReplyTo":"20091103222123.GA17097@progeny.tock","subject":"Re: [PATCH 0/2] Set Makefile variables from configure","fromName":"Ben Walton","fromEmail":"bwalton@artsci.utoronto.ca","sentAt":"2009-11-04T18:07:17Z","receivedAt":"2009-11-04T18:07:17Z","isPatch":true,"sender":{"key":"bdwalton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/396061?v=4"},"body":"Excerpts from Jonathan Nieder's message of Tue Nov 03 17:21:23 -0500 2009:\n\n> Would the --sysconfdir option work for you here?  Setting --sysconfdir\n> currently does nothing, so the question is kind of moot without some\n> change like this (untested):\n\nI think this is a generally useful patch.\n\nThanks\n-Ben\n-- \nBen Walton\nSystems Programmer - CHASS\nUniversity of Toronto\nC:416.407.5610 | W:416.978.4302\n\nGPG Key Id: 8E89F6D2; Key Server: pgp.mit.edu\nContact me to arrange for a CAcert assurance meeting.\n"},{"id":"126798","messageId":"7v7hu6gjzz.fsf@alter.siamese.dyndns.org","threadId":"21447","inReplyTo":"1257357960-12763-1-git-send-email-bwalton@artsci.utoronto.ca","subject":"Re: [PATCH 0/2] Set Makefile variables from configure","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-11-04T19:36:32Z","receivedAt":"2009-11-04T19:36:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ben Walton <bwalton@artsci.utoronto.ca> writes:\n\n> Ok, a second attempt, taking into account Jonathan's feedback and\n> proposed updates.  The revised macro allows for specified help text.\n>\n> I opted to use a more 'dry' approach to the macro instead of repeating\n> the boilerplate AC_WITH_ARG bits when calling this feature.\n>\n> Additionally, I only WARN when 'yes' or 'no' is passed.  Bailing on\n> errors in most cases is reasonable, but I don't think it's globally\n> appropriate.\n>\n> As to why I want this...It's cleaner, in my opinion, to do all build\n> configuration through a single mechanism than to do most with\n> ./configure and the rest with variables passed to make.  In other\n> words, this is purely a style issue.\n\nSorry, I wasn't following the discussion primarily because I am totally\nuninterested in autoconf myself (it's purely a style issue and using a\nsingle mechanism of echoing into config.mak is just as clean), so allow me\nto ask a stupid question that might have been already answered while the\ninitial round was discussed.\n\nI am a bit puzzled about the \"warning\" logic.  Is this because you expect\nvariables we typically give YesPlease/NoThanks as their values will not be\nhandled with this new PARSE_WITH_SET_MAKE_VAR() macro?\n"},{"id":"126799","messageId":"1257363937-sup-5123@ntdws12.chass.utoronto.ca","threadId":"21447","inReplyTo":"7v7hu6gjzz.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 0/2] Set Makefile variables from configure","fromName":"Ben Walton","fromEmail":"bwalton@artsci.utoronto.ca","sentAt":"2009-11-04T19:47:45Z","receivedAt":"2009-11-04T19:47:45Z","isPatch":true,"sender":{"key":"bdwalton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/396061?v=4"},"body":"Excerpts from Junio C Hamano's message of Wed Nov 04 14:36:32 -0500 2009:\n\n> I am a bit puzzled about the \"warning\" logic.  Is this because you expect\n> variables we typically give YesPlease/NoThanks as their values will not be\n> handled with this new PARSE_WITH_SET_MAKE_VAR() macro?\n\nNo, this is because it's perfectly acceptable, in my opinion for a\nuser to say:\n\n--with-pager=no\nor\n--with-editor=yes\n\nNeither of those are smart things to do, but they're not necessarily\nwrong, either.  I'm open to bailing on error for these, but I thought\nleaving these as unvalidated, but with a warning, was more\nflexible...if say a user wanted to have a pager called 'no' or\nsomething.\n\nFor the variables that are accepting YesPlease/NoThanks, I think it's\nmore suitable to use the standard autoconf header/function/library\ndetection as it stands now.  This macro is more for 'oddball'\nvariables like DEFAULT_PAGER, DEFAULT_EDITOR, etc that aren't\nnecessarily easily detectable.  In some cases, even if it were\ndetectable, the detection might not be right.\n\nThanks\n-Ben\n-- \nBen Walton\nSystems Programmer - CHASS\nUniversity of Toronto\nC:416.407.5610 | W:416.978.4302\n\nGPG Key Id: 8E89F6D2; Key Server: pgp.mit.edu\nContact me to arrange for a CAcert assurance meeting.\n"},{"id":"126815","messageId":"7vy6mmf4hi.fsf@alter.siamese.dyndns.org","threadId":"21447","inReplyTo":"1257363937-sup-5123@ntdws12.chass.utoronto.ca","subject":"Re: [PATCH 0/2] Set Makefile variables from configure","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-11-04T19:56:57Z","receivedAt":"2009-11-04T19:56:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ben Walton <bwalton@artsci.utoronto.ca> writes:\n\n> Excerpts from Junio C Hamano's message of Wed Nov 04 14:36:32 -0500 2009:\n>\n>> I am a bit puzzled about the \"warning\" logic.  Is this because you expect\n>> variables we typically give YesPlease/NoThanks as their values will not be\n>> handled with this new PARSE_WITH_SET_MAKE_VAR() macro?\n>\n> No, this is because it's perfectly acceptable, in my opinion for a\n> user to say:\n>\n> --with-pager=no\n> or\n> --with-editor=yes\n>\n> Neither of those are smart things to do, but they're not necessarily\n> wrong, either.  I'm open to bailing on error for these, but I thought\n> leaving these as unvalidated, but with a warning, was more\n> flexible...if say a user wanted to have a pager called 'no' or\n> something.\n\nWhat puzzled me was not \"why is it not an error but just a warning?\", which\nis what you just explained, but \"why should we even care what value is\nbeing given to begin with?\".\n\nI am _not_ saying \"I think it is more correct not to check the value at\nall\".  I just wanted to understand in what situation it may be benefitial\nto give this warning in the first place.\n\n> For the variables that are accepting YesPlease/NoThanks, I think it's\n> more suitable to use the standard autoconf header/function/library\n> detection as it stands now.  This macro is more for 'oddball'\n> variables like DEFAULT_PAGER, DEFAULT_EDITOR, etc that aren't\n> necessarily easily detectable.  In some cases, even if it were\n> detectable, the detection might not be right.\n\nI am guessing from this description of 'oddball variables' that your\nanswer to my question is yes.  That is, configure.ac writers are strongly\ndiscouraged from using this new macro for variables that would usually get\nYesPlease/NoThanks kind of values.\n\nAnd then it makes sense to warn 'yes/no', as it is unlikely that the user\nwants to set name (or path) of programs we allow to be tweaked to 'yes' or\n'no'.\n"},{"id":"126818","messageId":"1257364915-sup-9761@ntdws12.chass.utoronto.ca","threadId":"21447","inReplyTo":"7vy6mmf4hi.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 0/2] Set Makefile variables from configure","fromName":"Ben Walton","fromEmail":"bwalton@artsci.utoronto.ca","sentAt":"2009-11-04T20:16:40Z","receivedAt":"2009-11-04T20:16:40Z","isPatch":true,"sender":{"key":"bdwalton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/396061?v=4"},"body":"Excerpts from Junio C Hamano's message of Wed Nov 04 14:56:57 -0500 2009:\n\n> What puzzled me was not \"why is it not an error but just a warning?\", which\n> is what you just explained, but \"why should we even care what value is\n> being given to begin with?\".\n\nI'm sorry.  I misunderstood then.  I think that in most cases, setting\neither 'yes' or 'no' is wrong, but I wanted to leave the door open for\nit for the 1%.  I thought that a warning message might be noticed and\ncause someone to rethink.\n\n> I am guessing from this description of 'oddball variables' that your\n> answer to my question is yes.  That is, configure.ac writers are\n> strongly discouraged from using this new macro for variables that\n> would usually get YesPlease/NoThanks kind of values.\n\nYes, that's correct.  This new macro is for variables that are not a\nsimple toggle.  It is also distinct from the mechanism used to specify\na path for perl, tcl/tk, etc, in that for those, we do want to error\nout on no.\n\nThanks\n-Ben\n\n-- \nBen Walton\nSystems Programmer - CHASS\nUniversity of Toronto\nC:416.407.5610 | W:416.978.4302\n\nGPG Key Id: 8E89F6D2; Key Server: pgp.mit.edu\nContact me to arrange for a CAcert assurance meeting.\n"},{"id":"126817","messageId":"20091104201657.GA30073@progeny.tock","threadId":"21447","inReplyTo":"1257363937-sup-5123@ntdws12.chass.utoronto.ca","subject":"Re: [PATCH 0/2] Set Makefile variables from configure","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2009-11-04T20:16:58Z","receivedAt":"2009-11-04T20:16:58Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Ben Walton wrote:\n> Excerpts from Junio C Hamano's message of Wed Nov 04 14:36:32 -0500 2009:\n> \n>> I am a bit puzzled about the \"warning\" logic.  Is this because you expect\n>> variables we typically give YesPlease/NoThanks as their values will not be\n>> handled with this new PARSE_WITH_SET_MAKE_VAR() macro?\n> \n> No, this is because it's perfectly acceptable, in my opinion for a\n> user to say:\n> \n> --with-pager=no\n> or\n> --with-editor=yes\n\nMore to the point, that’s what autoconf gives for \"--without-pager\" or\nplain \"--with-editor\".  It seems strange to silently use PAGER=no or\nEDITOR=yes in that case.\n\nMaybe the options should just be --pager=foo and --editor=bar, which\nwould be less misleading and avoid this problem.  But this is not my\nitch (I find it cleaner to use the Makefile directly), so I have no\nstrong opinion.  Unfortunately, my autoconf-fu is too weak for a demo.\n\nJonathan\n"}]}