[RFC] --with-iconv-path

[RFC] --with-iconv-path

[Doug Evans]

Hi.

We have one environment where the iconv binary comes from a non-standard
location.  This patch lets one tell gdb where to find it.

Before I write docs/NEWS, can anyone think of any problems with this approach?

One alternative that I don't like is requiring the user to set $PATH
to find the right iconv - that is an internal implementation detail
the user shouldn't have to deal with.

[Ideally one could query some iconv_*() function at runtime: It knows where
its going to find its .so files, and iconv is typically at a fixed path
relative to that.  But I couldn't find such an interface.
Another alternative would be to have an iconv_*() function that returned
a list of all the charsets: that's all gdb uses the iconv program for.
And I realize some environments have iconvlist.
Alas one also has to work with the glibcs that are out there.]

TIA

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

	* configure.ac: New configure option --with-iconv-path.
	* configure: Regenerate.
	* config.in: Regenerate.
	* charset.c (find_charset_names): Use ICONV_PATH if defined.

Index: charset.c
===================================================================
RCS file: /cvs/src/src/gdb/charset.c,v
retrieving revision 1.44
diff -u -p -r1.44 charset.c
--- charset.c	21 Apr 2011 14:26:38 -0000	1.44
+++ charset.c	6 May 2011 00:14:37 -0000
@@ -799,6 +799,7 @@ find_charset_names (void)
   char *args[3];
   int err, status;
   int fail = 1;
+  int flags;
   struct gdb_environ *iconv_env;
 
   /* Older iconvs, e.g. 2.2.2, don't omit the intro text if stdout is
@@ -811,12 +812,20 @@ find_charset_names (void)
 
   child = pex_init (PEX_USE_PIPES, "iconv", NULL);
 
+#ifdef ICONV_PATH
+  args[0] = ICONV_PATH;
+#else
   args[0] = "iconv";
+#endif
   args[1] = "-l";
   args[2] = NULL;
+  flags = PEX_STDERR_TO_STDOUT;
+#ifndef ICONV_PATH
+  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);
Index: configure.ac
===================================================================
RCS file: /cvs/src/src/gdb/configure.ac,v
retrieving revision 1.144
diff -u -p -r1.144 configure.ac
--- configure.ac	17 Mar 2011 13:19:10 -0000	1.144
+++ configure.ac	6 May 2011 00:14:37 -0000
@@ -433,6 +433,16 @@ AC_SEARCH_LIBS(dlgetmodinfo, [dl xpdl])
 
 AM_ICONV
 
+# GDB may invoke iconv to get the list of supported character sets.
+# Allow the user to specify where to find it.
+
+AC_ARG_WITH(iconv-path,
+AS_HELP_STRING([--with-iconv-path=PATH], [specify where to find the iconv program]),
+[iconv_path="${withval}"
+ AC_DEFINE_UNQUOTED([ICONV_PATH], ["${iconv_path}"],
+                    [Path of iconv program.])
+])
+
 # 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.

Re: [RFC] --with-iconv-path

[Joel Brobecker]

> We have one environment where the iconv binary comes from a
> non-standard location.  This patch lets one tell gdb where to find it.

Does it mean that the libiconv libraries are at one location, and
that the binary is at a different location? Looking at your patch,
it looks like you might have a situation where the name of the iconv
binary itself is different...

> 2011-05-05  Doug Evans  <dje@google.com>
> 
> 	* configure.ac: New configure option --with-iconv-path.
> 	* configure: Regenerate.
> 	* config.in: Regenerate.
> 	* charset.c (find_charset_names): Use ICONV_PATH if defined.

"path" had me confused, and made me think that you were going
to pass the name of the directory where iconv is located. But
the patch seems to be contradicting that.

If we don't need to specify the name of the iconv directory,
then perhaps we could just focus on the directory name itself.
We could even make that directory name relatocatable, the same
way we make some of the paths relocatable as well.

Normally, we have sets of switches that look like this:

        --with-xxx=PATH
        --with-xxx-lib=PATH
        --with-xxx-include=PATH
        etc

We already have --with-libiconv-prefix.  So, how about naming it
--with-libiconv-bin ?

If we think that it would be convenient to specify the iconv exe
as well, how about --with-libiconv-exe or --with-libiconv-program?

-- 
Joel

Re: [RFC] --with-iconv-path

[Doug Evans]

On Fri, May 6, 2011 at 8:12 AM, Joel Brobecker <brobecker@adacore.com> wrote:
>> We have one environment where the iconv binary comes from a
>> non-standard location.  This patch lets one tell gdb where to find it.
>
> Does it mean that the libiconv libraries are at one location, and
> that the binary is at a different location? Looking at your patch,
> it looks like you might have a situation where the name of the iconv
> binary itself is different...

There is no libiconv library, there's just iconv_{open,close,} in
glibc and the iconv binary.

>> 2011-05-05  Doug Evans  <dje@google.com>
>>
>>       * configure.ac: New configure option --with-iconv-path.
>>       * configure: Regenerate.
>>       * config.in: Regenerate.
>>       * charset.c (find_charset_names): Use ICONV_PATH if defined.
>
> "path" had me confused, and made me think that you were going
> to pass the name of the directory where iconv is located. But
> the patch seems to be contradicting that.
>
> If we don't need to specify the name of the iconv directory,
> then perhaps we could just focus on the directory name itself.
> We could even make that directory name relatocatable, the same
> way we make some of the paths relocatable as well.
>
> Normally, we have sets of switches that look like this:
>
>        --with-xxx=PATH
>        --with-xxx-lib=PATH
>        --with-xxx-include=PATH
>        etc
>
> We already have --with-libiconv-prefix.  So, how about naming it
> --with-libiconv-bin ?
>
> If we think that it would be convenient to specify the iconv exe
> as well, how about --with-libiconv-exe or --with-libiconv-program?

--with-libiconv-{bin,exe} "works for me"

For reference sake, the binary lives in the expected location given
--with-libiconv-prefix (i.e., bin/iconv), but how do I effect the
necessary change without breaking anything.

If folks are happy with adding $WITH_LIBICONV_PREFIX/bin to the front
of the search path instead (so no new configure option), I can do
that.
Seems reasonable, but I guess it *could* break someone's installation somewhere.
OTOH, if people are passing --with-libiconv-prefix they may have
iconvlist() in which case the iconv binary isn't used, so we're safe.

Re: [RFC] --with-iconv-path

[Joseph S. Myers]

On Fri, 6 May 2011, Doug Evans wrote:

> For reference sake, the binary lives in the expected location given
> --with-libiconv-prefix (i.e., bin/iconv), but how do I effect the
> necessary change without breaking anything.
> 
> If folks are happy with adding $WITH_LIBICONV_PREFIX/bin to the front
> of the search path instead (so no new configure option), I can do
> that.
> Seems reasonable, but I guess it *could* break someone's installation somewhere.
> OTOH, if people are passing --with-libiconv-prefix they may have
> iconvlist() in which case the iconv binary isn't used, so we're safe.

--with-libiconv-prefix gives a build-system path which may not be a 
directory present on the host system when the gdb binary is used, so I 
don't think it's suitable for this purpose (which should specify a host 
system path - which I suppose should be relocated if under $exec_prefix).

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [RFC] --with-iconv-path

[Tom Tromey]

>>>>> "Doug" == Doug Evans <dje@google.com> writes:

Doug> We have one environment where the iconv binary comes from a
Doug> non-standard location.  This patch lets one tell gdb where to find
Doug> it.

Doug> Before I write docs/NEWS, can anyone think of any problems with
Doug> this approach?

It seems reasonable to me.

I don't have an opinion on the option name.

Doug> [Ideally one could query some iconv_*() function at runtime: It
Doug> knows where its going to find its .so files, and iconv is
Doug> typically at a fixed path relative to that.  But I couldn't find
Doug> such an interface.

This is:

    http://sourceware.org/bugzilla/show_bug.cgi?id=12234

Tom