Re: [PATCH] Display configuration details in --help

Doug Evans
2013-04-09T16:39:46Z

Eli Zaretskii writes:
> > Date: Thu, 21 Mar 2013 23:16:50 +0200
> > From: Eli Zaretskii <eliz@gnu.org>
> > Cc: brobecker@adacore.com, palves@redhat.com, dje@google.com, gdb-patches@sourceware.org
> >
> > > From: Tom Tromey <tromey@redhat.com>
> > > Cc: Eli Zaretskii <eliz@gnu.org>, palves@redhat.com, dje@google.com,
> > > gdb-patches@sourceware.org
> > > Date: Thu, 21 Mar 2013 14:58:03 -0600
> > >
> > > For the new information, "show configuration" might also be nice to have.
> > > It would not be much more code and would let people see the info for the
> > > gdb they are currently using.
> >
> > OK, will do.
>
> Respinning, here's v2 of that patch, which accommodates all the
> comments till now. It also includes documentation; please someone
> review that, as I'm not objective in this case ;-).
>
> OK to commit?

Hi. A few comments inline.

> 2013-04-09 Eli Zaretskii <eliz@gnu.org>
>
> * top.c (print_gdb_configuration): New function, displays the
> details about GDB configure-time parameters.
> (print_gdb_version): Mention "show configuration".
>
> * cli/cli-cmds.c (show_configuration): New function.
> (_initialize_cli_cmds): Add the "show configuration" command.
>
> * main.c (captured_main) <print_configuration>: New static var.
> <long_options>: Use it.
> If --configuration was given, call print_gdb_configuration.
>
>
> --- gdb/cli/cli-cmds.c~0 2013-03-12 19:39:44.000000000 +0200
> +++ gdb/cli/cli-cmds.c 2013-04-09 08:02:46.088591300 +0300
> @@ -314,6 +314,12 @@ show_version (char *args, int from_tty)
> printf_filtered ("\n");
> }
>
> +static void
> +show_configuration (char *args, int from_tty)
> +{
> + print_gdb_configuration (gdb_stdout);
> +}
> +
> /* Handle the quit command. */
>
> void
> @@ -1756,6 +1762,9 @@ the previous command number shown."),
> add_cmd ("version", no_set_class, show_version,
> _("Show what version of GDB this is."), &showlist);
>
> + add_cmd ("configuration", no_set_class, show_configuration,
> + _("Show how GDB was configured at build time."), &showlist);
> +
> /* If target is open when baud changes, it doesn't take effect until
> the next open (I think, not sure). */
> add_setshow_zinteger_cmd ("remotebaud", no_class, &baud_rate, _("\
>
>
>
> --- gdb/top.c~0 2013-01-25 16:17:10.000000000 +0200
> +++ gdb/top.c 2013-04-09 08:02:27.508872200 +0300
> @@ -1146,15 +1146,101 @@ and \"show warranty\" for details.\n");
> {
> fprintf_filtered (stream, "%s", host_name);
> }
> - fprintf_filtered (stream, "\".");
> + fprintf_filtered (stream, "\".\n\
> +Type \"show configuration\" for configuration details.");
>
> if (REPORT_BUGS_TO[0])
> {
> - fprintf_filtered (stream,
> + fprintf_filtered (stream,
> _("\nFor bug reporting instructions, please see:\n"));
> fprintf_filtered (stream, "%s.", REPORT_BUGS_TO);
> }
> }

I have a bit of a phobia of adding more lines to gdb's initial output.
It's too long already IMO. [I realize there's -q.]
I'm not objecting per se. Just wondering how critical this is.

> +/* Print the details of GDB build-time configuration. */
> +void
> +print_gdb_configuration (struct ui_file *stream)
> +{
> + fprintf_filtered (stream, _("\
> +This GDB was configured as follows:\n\
> + configure --host=%s --target=%s\n\
> +"), host_name, target_name);
> + fprintf_filtered (stream, _("\
> + --with-auto-load-dir=%s\n\
> + --with-auto-load-safe-path=%s\n\
> +"), AUTO_LOAD_DIR, AUTO_LOAD_SAFE_PATH);
> +#if HAVE_LIBEXPAT
> + fprintf_filtered (stream, _("\
> + --with-expat\n\
> +"));
> +#else
> + fprintf_filtered (stream, _("\
> + --without-expat\n\
> +"));
> +#endif

If we've already discussed this, please ignore, but ... :-)
I'd prefer one line per fprintf instead of three.
[I realize there are some cases where one line won't do,
and this style makes them all consistent. I still prefer
one line for the common case.]
If others don't object to it, then it's not that important.

Also, is there something driving the choice of indenting 13 spaces in?
How about 2 or 4?
For consistency with print_gdb_help I'd go with 2.

> + if (GDB_DATADIR[0])
> + fprintf_filtered (stream, _("\
> + --with-gdb-datadir=%s%s\n\
> +"), GDB_DATADIR, GDB_DATADIR_RELOCATABLE ? " (relocatable)" : "");
> +#ifdef ICONV_BIN
> + fprintf_filtered (stream, _("\
> + --with-iconv-bin=%s%s\n\
> +"), ICONV_BIN, ICONV_BIN_RELOCATABLE ? " (relocatable)" : "");
> +#endif
> + if (JIT_READER_DIR[0])
> + fprintf_filtered (stream, _("\
> + --with-jit-reader-dir=%s%s\n\
> +"), JIT_READER_DIR, JIT_READER_DIR_RELOCATABLE ? " (relocatable)" : "");
> +#if HAVE_LIBUNWIND_IA64_H
> + fprintf_filtered (stream, _("\
> + --with-libunwind-ia64\n\
> +"));
> +#else
> + fprintf_filtered (stream, _("\
> + --without-libunwind-ia64\n\
> +"));
> +#endif
> +#if HAVE_LIBLZMA
> + fprintf_filtered (stream, _("\
> + --with-lzma\n\
> +"));
> +#else
> + fprintf_filtered (stream, _("\
> + --without-lzma\n\
> +"));
> +#endif
> +#ifdef WITH_PYTHON_PATH
> + fprintf_filtered (stream, _("\
> + --with-python=%s%s\n\
> +"), WITH_PYTHON_PATH, PYTHON_PATH_RELOCATABLE ? " (relocatable)" : "");
> +#endif
> +#ifdef RELOC_SRCDIR
> + fprintf_filtered (stream, _("\
> + --with-relocated-sources=%s\n\
> +"), RELOC_SRCDIR);
> +#endif
> + if (DEBUGDIR[0])
> + fprintf_filtered (stream, _("\
> + --with-separate-debug-dir=%s%s\n\
> +"), DEBUGDIR, DEBUGDIR_RELOCATABLE ? " (relocatable)" : "");
> + if (TARGET_SYSTEM_ROOT[0])
> + fprintf_filtered (stream, _("\
> + --with-sysroot=%s%s\n\
> +"), TARGET_SYSTEM_ROOT, TARGET_SYSTEM_ROOT_RELOCATABLE ? " (relocatable)" : "");
> + if (SYSTEM_GDBINIT[0])
> + fprintf_filtered (stream, _("\
> + --with-system-gdbinit=%s%s\n\
> +"), SYSTEM_GDBINIT, SYSTEM_GDBINIT_RELOCATABLE ? " (relocatable)" : "");
> +#if HAVE_ZLIB_H
> + fprintf_filtered (stream, _("\
> + --with-zlib\n\
> +"));
> +#else
> + fprintf_filtered (stream, _("\
> + --without-zlib\n\
> +"));
> +#endif
> +}
> \f
>
> /* The current top level prompt, settable with "set prompt", and/or
>
>
> --- gdb/main.c~4 2013-04-09 08:40:03.404632700 +0300
> +++ gdb/main.c 2013-04-09 08:08:00.461806500 +0300
> @@ -322,6 +322,7 @@ captured_main (void *data)
> initializer. */
> static int print_help;
> static int print_version;
> + static int print_configuration;
>
> /* Pointers to all arguments of --command option. */
> VEC (cmdarg_s) *cmdarg_vec = NULL;
> @@ -484,6 +485,7 @@ captured_main (void *data)
> {"command", required_argument, 0, 'x'},
> {"eval-command", required_argument, 0, 'X'},
> {"version", no_argument, &print_version, 1},
> + {"configuration", no_argument, &print_configuration, 1},
> {"x", required_argument, 0, 'x'},
> {"ex", required_argument, 0, 'X'},
> {"init-command", required_argument, 0, OPT_IX},
> @@ -727,8 +729,9 @@ captured_main (void *data)
> }
> }
>
> - /* If --help or --version, disable window interface. */
> - if (print_help || print_version)
> + /* If --help or --version or --configuration, disable window
> + interface. */
> + if (print_help || print_version || print_configuration)
> {
> use_windows = 0;
> }
> @@ -819,6 +822,14 @@ captured_main (void *data)
> exit (0);
> }
>
> + if (print_configuration)
> + {
> + print_gdb_configuration (gdb_stdout);
> + wrap_here ("");
> + printf_filtered ("\n");
> + exit (0);
> + }
> +
> /* FIXME: cagney/2003-02-03: The big hack (part 1 of 2) that lets
> GDB retain the old MI1 interpreter startup behavior. Output the
> copyright message before the interpreter is installed. That way
> @@ -1130,6 +1141,7 @@ Options:\n\n\
> #endif
> fputs_unfiltered (_("\
> --version Print version information and then exit.\n\
> + --configuration Print details about GDB configuration and then exit.\n\
> -w Use a window interface.\n\
> --write Set writing into executable and core files.\n\
> --xdb XDB compatibility mode.\n\

The options here are (mostly) sorted alphabetically.