Re: [doc RFA]: New option --with-iconv-bin

[Doug Evans <dje@google.com>]

[-- Attachment #1: Type: text/plain, Size: 1198 bytes --]

[Sorry for the resend.]

On Mon, May 9, 2011 at 12:08 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>> Date: Mon,  9 May 2011 11:19:47 -0700 (PDT)
>> From: dje@google.com (Doug Evans)
>>
>>  *** Changes since GDB 7.3
>>
>> +* New configure option --with-iconv-bin to specify where to find the
>> +  iconv program.
>
> Should we tell here what `iconv' is used for?  Users might not know
> whether they should bother about this option.

Sure.  Though there is the gdb.texinfo entry for elaboration.

>> +If the GNU C Library you are using is installed in a non-standard place,
>> +you will need to tell @value{GDBN} where to find the @code{iconv} program.
>> +This is done with @option{--with-iconv-bin} which specifies the directory
>> +that contains the @code{iconv} program.
>
> I'm confused: what does `iconv's location have to do with the place
> where glibc is installed?
>
> Why not just say "if your @command{iconv} program is installed in a
> non-standard place, you will need to tell @value{GDBN} where to find
> it."?

The iconv program comes from glibc, but you're right, it doesn't
*necessarily* come from there.

How about this?

free -> xfree fixed as well

[-- Attachment #2: gdb-110509-with-iconv-bin-2.patch.txt --]
[-- Type: text/plain, Size: 8556 bytes --]

2011-05-09  Doug Evans  <dje@google.com>

	* NEWS: Mention --with-iconv-bin.
	* configure.ac: New option --with-iconv-bin.
	* configure: Regenerate.
	* config.in: Regenerate.
	* defs.h (relocate_gdb_directory): Declare.
	* main.c (relocate_gdb_directory): Renamed from relocate_directory
	and exported.  All callers updated.
	* charset.c (find_charset_names): Use --with-iconv-bin if specified.

	doc/
	gdb.texinfo (Requirements): Fix typo.  Mention --with-iconv-bin.

Index: NEWS
===================================================================
RCS file: /cvs/src/src/gdb/NEWS,v
retrieving revision 1.437
diff -u -p -d -u -r1.437 NEWS
--- NEWS	6 May 2011 18:46:31 -0000	1.437
+++ NEWS	9 May 2011 19:49:47 -0000
@@ -3,6 +3,12 @@
 
 *** Changes since GDB 7.3
 
+* New configure option --with-iconv-bin.
+  When using the internationalization support like the one in the GNU C
+  library, GDB will invoke the "iconv" program to get a list of supported
+  character sets.  If this program lives in a non-standard location, one can
+  use this option to specify where to find it.
+
 * When natively debugging programs on PowerPC BookE processors running
   a Linux kernel version 2.6.34 or later, GDB supports masked hardware
   watchpoints, which specify a mask in addition to an address to watch.
Index: charset.c
===================================================================
RCS file: /cvs/src/src/gdb/charset.c,v
retrieving revision 1.44
diff -u -p -d -u -r1.44 charset.c
--- charset.c	21 Apr 2011 14:26:38 -0000	1.44
+++ charset.c	9 May 2011 19:49:47 -0000
@@ -799,7 +799,9 @@ find_charset_names (void)
   char *args[3];
   int err, status;
   int fail = 1;
+  int flags;
   struct gdb_environ *iconv_env;
+  char *iconv_program;
 
   /* Older iconvs, e.g. 2.2.2, don't omit the intro text if stdout is
      not a tty.  We need to recognize it and ignore it.  This text is
@@ -811,12 +813,26 @@ find_charset_names (void)
 
   child = pex_init (PEX_USE_PIPES, "iconv", NULL);
 
-  args[0] = "iconv";
+#ifdef ICONV_BIN
+  {
+    char *iconv_dir = relocate_gdb_directory (ICONV_BIN,
+					      ICONV_BIN_RELOCATABLE);
+    iconv_program = concat (iconv_dir, SLASH_STRING, "iconv", NULL);
+    xfree (iconv_dir);
+  }
+#else
+  iconv_program = xstrdup ("iconv");
+#endif
+  args[0] = iconv_program;
   args[1] = "-l";
   args[2] = NULL;
+  flags = PEX_STDERR_TO_STDOUT;
+#ifndef ICONV_BIN
+  flags |= PEX_SEARCH;
+#endif
   /* Note that we simply ignore errors here.  */
-  if (!pex_run_in_environment (child, PEX_SEARCH | PEX_STDERR_TO_STDOUT,
-			       "iconv", args, environ_vector (iconv_env),
+  if (!pex_run_in_environment (child, flags,
+			       args[0], args, environ_vector (iconv_env),
 			       NULL, NULL, &err))
     {
       FILE *in = pex_read_output (child, 0);
@@ -888,6 +904,7 @@ find_charset_names (void)
 
     }
 
+  xfree (iconv_program);
   pex_free (child);
   free_environ (iconv_env);
 
Index: configure.ac
===================================================================
RCS file: /cvs/src/src/gdb/configure.ac,v
retrieving revision 1.144
diff -u -p -d -u -r1.144 configure.ac
--- configure.ac	17 Mar 2011 13:19:10 -0000	1.144
+++ configure.ac	9 May 2011 19:49:47 -0000
@@ -433,6 +433,29 @@ AC_SEARCH_LIBS(dlgetmodinfo, [dl xpdl])
 
 AM_ICONV
 
+# GDB may fork/exec the iconv program to get the list of supported character
+# sets.  Allow the user to specify where to find it.
+# There are several factors affecting the choice of option name:
+# - There is already --with-libiconv-prefix but we can't use it, it specifies
+#   the build-time location of libiconv files.
+# - The program we need to find is iconv, which comes with glibc.  The user
+#   doesn't necessarily have libiconv installed.  Therefore naming this
+#   --with-libiconv-foo feels wrong.
+# - We want the path to be relocatable, but GDB_AC_DEFINE_RELOCATABLE is
+#   defined to work on directories not files (though it really doesn't know
+#   the difference).
+# - Calling this --with-iconv-prefix is perceived to cause too much confusion
+#   with --with-libiconv-prefix.
+# Putting these together is why the option name is --with-iconv-bin.
+
+AC_ARG_WITH(iconv-bin,
+AS_HELP_STRING([--with-iconv-bin=PATH], [specify where to find the iconv program]),
+[iconv_bin="${withval}"
+ AC_DEFINE_UNQUOTED([ICONV_BIN], ["${iconv_bin}"],
+                    [Path of directory of iconv program.])
+ GDB_AC_DEFINE_RELOCATABLE(ICONV_BIN, iconv, ${iconv_bin})
+])
+
 # On alpha-osf, it appears that libtermcap and libcurses are not compatible.
 # There is a very specific comment in /usr/include/curses.h explaining that
 # termcap routines built into libcurses must not be used.
Index: defs.h
===================================================================
RCS file: /cvs/src/src/gdb/defs.h,v
retrieving revision 1.291
diff -u -p -d -u -r1.291 defs.h
--- defs.h	25 Apr 2011 18:28:52 -0000	1.291
+++ defs.h	9 May 2011 19:49:47 -0000
@@ -282,6 +282,12 @@ struct breakpoint;
 struct frame_info;
 struct gdbarch;
 
+/* From main.c.  */
+
+/* This really belong in utils.c (path-utils.c?), but it references some
+   globals that are currently only available to main.c.  */
+extern char *relocate_gdb_directory (const char *initial, int flag);
+
 /* From utils.c */
 
 extern void initialize_utils (void);
Index: main.c
===================================================================
RCS file: /cvs/src/src/gdb/main.c,v
retrieving revision 1.93
diff -u -p -d -u -r1.93 main.c
--- main.c	7 Mar 2011 18:34:31 -0000	1.93
+++ main.c	9 May 2011 19:49:47 -0000
@@ -104,6 +104,7 @@ extern char *external_editor_command;
    file or directory.  FLAG is true if the value is relocatable, false
    otherwise.  Returns a newly allocated string; this may return NULL
    under the same conditions as make_relative_prefix.  */
+
 static char *
 relocate_path (const char *progname, const char *initial, int flag)
 {
@@ -117,12 +118,13 @@ relocate_path (const char *progname, con
    the result is a directory, it is used; otherwise, INITIAL is used.
    The chosen directory is then canonicalized using lrealpath.  This
    function always returns a newly-allocated string.  */
-static char *
-relocate_directory (const char *progname, const char *initial, int flag)
+
+char *
+relocate_gdb_directory (const char *initial, int flag)
 {
   char *dir;
 
-  dir = relocate_path (progname, initial, flag);
+  dir = relocate_path (gdb_program_name, initial, flag);
   if (dir)
     {
       struct stat s;
@@ -342,22 +344,21 @@ captured_main (void *data)
   current_directory = gdb_dirbuf;
 
   /* Set the sysroot path.  */
-  gdb_sysroot = relocate_directory (argv[0], TARGET_SYSTEM_ROOT,
-				    TARGET_SYSTEM_ROOT_RELOCATABLE);
+  gdb_sysroot = relocate_gdb_directory (TARGET_SYSTEM_ROOT,
+					TARGET_SYSTEM_ROOT_RELOCATABLE);
 
-  debug_file_directory = relocate_directory (argv[0], DEBUGDIR,
-					     DEBUGDIR_RELOCATABLE);
+  debug_file_directory = relocate_gdb_directory (DEBUGDIR,
+						 DEBUGDIR_RELOCATABLE);
 
-  gdb_datadir = relocate_directory (argv[0], GDB_DATADIR,
-				    GDB_DATADIR_RELOCATABLE);
+  gdb_datadir = relocate_gdb_directory (GDB_DATADIR,
+					GDB_DATADIR_RELOCATABLE);
 
 #ifdef WITH_PYTHON_PATH
   {
     /* For later use in helping Python find itself.  */
     char *tmp = concat (WITH_PYTHON_PATH, SLASH_STRING, "lib", NULL);
 
-    python_libdir = relocate_directory (argv[0], tmp,
-					PYTHON_PATH_RELOCATABLE);
+    python_libdir = relocate_gdb_directory (tmp, PYTHON_PATH_RELOCATABLE);
     xfree (tmp);
   }
 #endif
Index: doc/gdb.texinfo
===================================================================
RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v
retrieving revision 1.830
diff -u -p -d -u -r1.830 gdb.texinfo
--- doc/gdb.texinfo	6 May 2011 18:46:33 -0000	1.830
+++ doc/gdb.texinfo	9 May 2011 19:49:47 -0000
@@ -31214,7 +31214,12 @@ Sets}) require a functioning @code{iconv
 on a GNU system, then this is provided by the GNU C Library.  Some
 other systems also provide a working @code{iconv}.
 
-On systems with @code{iconv}, you can install GNU Libiconv.  If you
+If @value{GDBN} is using the @code{iconv} program and it is installed in
+a non-standard place, you will need to tell @value{GDBN} where to find it.
+This is done with @option{--with-iconv-bin} which specifies the
+directory that contains the @code{iconv} program.
+
+On systems without @code{iconv}, you can install GNU Libiconv.  If you
 have previously installed Libiconv, you can use the
 @option{--with-libiconv-prefix} option to configure.