Hi,
this is patch is heavily based on David's original patch here:
https://gcc.gnu.org/ml/gcc-patches/2019-12/msg01409.html
and addresses Jakub's review comments here:
https://gcc.gnu.org/ml/gcc-patches/2019-12/msg01412.html
So I hope you don't mind, when I pick up this patch since there
was not much activity recently on this issue, so I assumed you
would appreciate some help.
I will try to improve the patch a bit, and hope you are gonna like
it. I agree that this feature is fine, and should be enabled by
default, and just if it is positively clear that it won't work,
disabled in the auto mode.
Also as requested by Jakub this tries to be more compatible to
GCC_COLORS define, and adds the capability to switch between ST
and BEL string termination and also have a way to disable urls
even with -fdiagnostics-urls=always (like GCC_COLORS= disables colors).
In addition to that I propose to use GCC_URLS and if that
is not defined use TERM_URLS to control that feature for
all applications willing to support that.
Since I have seen much garbage from the URLs in the xfce4-terminal 0.6.3
admittedly an old Ubuntu installation, but still in LTS status,
and no attempt from the xfc4-terminal to _ever_ implement URLs, I would
like to detect the xfce4-terminal, and use that in auto mode
to switch off the URLs feature, since in best case the URLs will
just be ignored, but can and will often create garbage on the screen.
Likewise on MinGW, since the windows 10 cmd prompt does at best
ignore the URLs, but windows terminal and windows 7 cmd prompt
print garbage when URLs are output.
I have one question though, regarding the autoconf version that
we use, it is autoconf-2.69, right, at least that is what I read in
gcc/doc/install.texi?
I don't understand why the generated configure has additional
changes than what is changed in configure.ac. Should I commit the
generated version as is, or use a different autoconf version?
Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
Is it OK for trunk?
Thanks
Benrd.
From fae99e05a356363ff719b5504e6250abe1c16aea Mon Sep 17 00:00:00 2001
From: Bernd Edlinger <bernd.edlin...@hotmail.de>
Date: Wed, 29 Jan 2020 15:31:10 +0100
Subject: [PATCH] PR 87488: Add --with-diagnostics-urls configuration option
2020-01-30 David Malcolm <dmalc...@redhat.com>
Bernd Edlinger <bernd.edlin...@hotmail.de>
PR 87488
* config.in (DIAGNOSTICS_URLS_DEFAULT): New define.
* configure.ac (--with-diagnostics-urls): New configuration
option, based on --with-diagnostics-color.
(DIAGNOSTICS_URLS_DEFAULT): New define.
* config.h: Regenerate.
* configure: Regenerate.
* diagnostic.c (diagnostic_urls_init): Handle -1 for
DIAGNOSTICS_URLS_DEFAULT from configure-time
--with-diagnostics-urls=auto-if-env by querying for a GCC_URLS
and TERM_URLS environment variable.
* diagnostic-url.h (diagnostic_urls_use_st): New variable.
* diagnostic-color.c (diagnostic_urls_use_st): Declare.
(parse_gcc_urls, auto_enable_urls): New helper functions.
(diagnostic_urls_enabled_p): Adjust.
* pretty-print.c (pp_begin_url, pp_end_url, test_urls): Use
diagnostic_urls_use_st.
* doc/install.texi (--with-diagnostics-urls): Document new
configuration option.
* doc/invoke.texi (-fdiagnostics-urls): Add GCC_URLS vindex
reference. Update description of defaults based on the above.
---
gcc/config.in | 6 ++++
gcc/configure | 87 +++++++++++++++++++++++++++++++++++++++++++++-----
gcc/configure.ac | 28 ++++++++++++++++
gcc/diagnostic-color.c | 57 +++++++++++++++++++++++++++++++--
gcc/diagnostic-url.h | 1 +
gcc/diagnostic.c | 17 +++++++++-
gcc/doc/install.texi | 9 ++++++
gcc/doc/invoke.texi | 15 +++++++--
gcc/pretty-print.c | 11 +++++--
9 files changed, 215 insertions(+), 16 deletions(-)
diff --git a/gcc/config.in b/gcc/config.in
index ec5c46a..b30fb20 100644
--- a/gcc/config.in
+++ b/gcc/config.in
@@ -76,6 +76,12 @@
#endif
+/* The default for -fdiagnostics-urls option */
+#ifndef USED_FOR_TARGET
+#undef DIAGNOSTICS_URLS_DEFAULT
+#endif
+
+
/* Define 0/1 if static analyzer feature is enabled. */
#ifndef USED_FOR_TARGET
#undef ENABLE_ANALYZER
diff --git a/gcc/configure b/gcc/configure
index 490fe6a..68340df 100755
--- a/gcc/configure
+++ b/gcc/configure
@@ -974,6 +974,7 @@ with_zstd_include
with_zstd_lib
enable_rpath
with_libiconv_prefix
+with_libiconv_type
enable_sjlj_exceptions
with_gcc_major_version_only
enable_secureplt
@@ -1014,6 +1015,7 @@ enable_host_shared
enable_libquadmath_support
with_linker_hash_style
with_diagnostics_color
+with_diagnostics_urls
enable_default_pie
'
ac_precious_vars='build_alias
@@ -1811,6 +1813,7 @@ Optional Packages:
--with-gnu-ld assume the C compiler uses GNU ld default=no
--with-libiconv-prefix[=DIR] search for libiconv in DIR/include and DIR/lib
--without-libiconv-prefix don't search for libiconv in includedir and libdir
+ --with-libiconv-type=TYPE type of library to search for (auto/static/shared)
--with-gcc-major-version-only
use only GCC major number in filesystem paths
--with-pic try to use only PIC/non-PIC objects [default=use
@@ -1834,6 +1837,11 @@ Optional Packages:
auto-if-env stands for -fdiagnostics-color=auto if
GCC_COLOR environment variable is present and
-fdiagnostics-color=never otherwise
+ --with-diagnostics-urls={never,auto,auto-if-env,always}
+ specify the default of -fdiagnostics-urls option
+ auto-if-env stands for -fdiagnostics-urls=auto if
+ GCC_URLS or TERM_URLS environment variable is
+ present and -fdiagnostics-urls=never otherwise
Some influential environment variables:
CC C compiler command
@@ -10730,6 +10738,16 @@ if test "${with_libiconv_prefix+set}" = set; then :
fi
+
+# Check whether --with-libiconv-type was given.
+if test "${with_libiconv_type+set}" = set; then :
+ withval=$with_libiconv_type; with_libiconv_type=$withval
+else
+ with_libiconv_type=auto
+fi
+
+ lib_type=`eval echo \$with_libiconv_type`
+
LIBICONV=
LTLIBICONV=
INCICONV=
@@ -10767,13 +10785,13 @@ fi
found_so=
found_a=
if test $use_additional = yes; then
- if test -n "$shlibext" && test -f "$additional_libdir/lib$name.$shlibext"; then
+ if test -n "$shlibext" && test -f "$additional_libdir/lib$name.$shlibext" && test x$lib_type != xstatic; then
found_dir="$additional_libdir"
found_so="$additional_libdir/lib$name.$shlibext"
if test -f "$additional_libdir/lib$name.la"; then
found_la="$additional_libdir/lib$name.la"
fi
- else
+ elif test x$lib_type != xshared; then
if test -f "$additional_libdir/lib$name.$libext"; then
found_dir="$additional_libdir"
found_a="$additional_libdir/lib$name.$libext"
@@ -10797,13 +10815,13 @@ fi
case "$x" in
-L*)
dir=`echo "X$x" | sed -e 's/^X-L//'`
- if test -n "$shlibext" && test -f "$dir/lib$name.$shlibext"; then
+ if test -n "$shlibext" && test -f "$dir/lib$name.$shlibext" && test x$lib_type != xstatic; then
found_dir="$dir"
found_so="$dir/lib$name.$shlibext"
if test -f "$dir/lib$name.la"; then
found_la="$dir/lib$name.la"
fi
- else
+ elif test x$lib_type != xshared; then
if test -f "$dir/lib$name.$libext"; then
found_dir="$dir"
found_a="$dir/lib$name.$libext"
@@ -11031,8 +11049,13 @@ fi
done
fi
else
- LIBICONV="${LIBICONV}${LIBICONV:+ }-l$name"
- LTLIBICONV="${LTLIBICONV}${LTLIBICONV:+ }-l$name"
+ if x$lib_type = xauto || x$lib_type = xshared; then
+ LIBICONV="${LIBICONV}${LIBICONV:+ }-l$name"
+ LTLIBICONV="${LTLIBICONV}${LTLIBICONV:+ }-l$name"
+ else
+ LIBICONV="${LIBICONV}${LIBICONV:+ }-l:lib$name.$libext"
+ LTLIBICONV="${LTLIBICONV}${LTLIBICONV:+ }-l:lib$name.$libext"
+ fi
fi
fi
fi
@@ -18957,7 +18980,7 @@ else
lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
lt_status=$lt_dlunknown
cat > conftest.$ac_ext <<_LT_EOF
-#line 18960 "configure"
+#line 18983 "configure"
#include "confdefs.h"
#if HAVE_DLFCN_H
@@ -19063,7 +19086,7 @@ else
lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
lt_status=$lt_dlunknown
cat > conftest.$ac_ext <<_LT_EOF
-#line 19066 "configure"
+#line 19089 "configure"
#include "confdefs.h"
#if HAVE_DLFCN_H
@@ -29800,6 +29823,23 @@ $as_echo "#define TARGET_LIBC_PROVIDES_HWCAP_IN_TCB 1" >>confdefs.h
fi
+# Check if the target LIBC handles PT_GNU_STACK.
+gcc_cv_libc_gnustack=unknown
+case "$target" in
+ mips*-*-linux*)
+
+if test $glibc_version_major -gt 2 \
+ || ( test $glibc_version_major -eq 2 && test $glibc_version_minor -ge 31 ); then :
+ gcc_cv_libc_gnustack=yes
+fi
+ ;;
+esac
+if test x$gcc_cv_libc_gnustack = xyes; then
+
+$as_echo "#define TARGET_LIBC_GNUSTACK 1" >>confdefs.h
+
+fi
+
{ $as_echo "$as_me:${as_lineno-$LINENO}: checking dl_iterate_phdr in target C library" >&5
$as_echo_n "checking dl_iterate_phdr in target C library... " >&6; }
gcc_cv_target_dl_iterate_phdr=unknown
@@ -30504,6 +30544,37 @@ cat >>confdefs.h <<_ACEOF
_ACEOF
+# Specify what should be the default of -fdiagnostics-urls option.
+
+# Check whether --with-diagnostics-urls was given.
+if test "${with_diagnostics_urls+set}" = set; then :
+ withval=$with_diagnostics_urls; case x"$withval" in
+ xnever)
+ DIAGNOSTICS_URLS_DEFAULT=DIAGNOSTICS_URL_NO
+ ;;
+ xauto)
+ DIAGNOSTICS_URLS_DEFAULT=DIAGNOSTICS_URL_AUTO
+ ;;
+ xauto-if-env)
+ DIAGNOSTICS_URLS_DEFAULT=-1
+ ;;
+ xalways)
+ DIAGNOSTICS_URLS_DEFAULT=DIAGNOSTICS_URL_YES
+ ;;
+ *)
+ as_fn_error $? "$withval is an invalid option to --with-diagnostics-urls" "$LINENO" 5
+ ;;
+ esac
+else
+ DIAGNOSTICS_URLS_DEFAULT=DIAGNOSTICS_URL_AUTO
+fi
+
+
+cat >>confdefs.h <<_ACEOF
+#define DIAGNOSTICS_URLS_DEFAULT $DIAGNOSTICS_URLS_DEFAULT
+_ACEOF
+
+
# Generate gcc-driver-name.h containing GCC_DRIVER_NAME for the benefit
# of jit/jit-playback.c.
gcc_driver_version=`eval "${get_gcc_base_ver} $srcdir/BASE-VER"`
diff --git a/gcc/configure.ac b/gcc/configure.ac
index a7521ee..25952a0 100644
--- a/gcc/configure.ac
+++ b/gcc/configure.ac
@@ -6730,6 +6730,34 @@ AC_ARG_WITH([diagnostics-color],
AC_DEFINE_UNQUOTED(DIAGNOSTICS_COLOR_DEFAULT, $DIAGNOSTICS_COLOR_DEFAULT,
[The default for -fdiagnostics-color option])
+# Specify what should be the default of -fdiagnostics-urls option.
+AC_ARG_WITH([diagnostics-urls],
+[AC_HELP_STRING([--with-diagnostics-urls={never,auto,auto-if-env,always}],
+ [specify the default of -fdiagnostics-urls option
+ auto-if-env stands for -fdiagnostics-urls=auto if
+ GCC_URLS or TERM_URLS environment variable is present and
+ -fdiagnostics-urls=never otherwise])],
+[case x"$withval" in
+ xnever)
+ DIAGNOSTICS_URLS_DEFAULT=DIAGNOSTICS_URL_NO
+ ;;
+ xauto)
+ DIAGNOSTICS_URLS_DEFAULT=DIAGNOSTICS_URL_AUTO
+ ;;
+ xauto-if-env)
+ DIAGNOSTICS_URLS_DEFAULT=-1
+ ;;
+ xalways)
+ DIAGNOSTICS_URLS_DEFAULT=DIAGNOSTICS_URL_YES
+ ;;
+ *)
+ AC_MSG_ERROR([$withval is an invalid option to --with-diagnostics-urls])
+ ;;
+ esac],
+[DIAGNOSTICS_URLS_DEFAULT=DIAGNOSTICS_URL_AUTO])
+AC_DEFINE_UNQUOTED(DIAGNOSTICS_URLS_DEFAULT, $DIAGNOSTICS_URLS_DEFAULT,
+ [The default for -fdiagnostics-urls option])
+
# Generate gcc-driver-name.h containing GCC_DRIVER_NAME for the benefit
# of jit/jit-playback.c.
gcc_driver_version=`eval "${get_gcc_base_ver} $srcdir/BASE-VER"`
diff --git a/gcc/diagnostic-color.c b/gcc/diagnostic-color.c
index d554795..ad84d1f 100644
--- a/gcc/diagnostic-color.c
+++ b/gcc/diagnostic-color.c
@@ -239,6 +239,56 @@ colorize_init (diagnostic_color_rule_t rule)
}
}
+bool diagnostic_urls_use_st = false;
+
+/* Return true if we should use urls when in always mode, false otherwise.
+ Additionally initialize diagnostic_urls_use_st to true if ST escapes
+ should be used and false for BEL escapes. */
+
+static bool
+parse_gcc_urls()
+{
+ const char *p;
+
+ p = getenv ("GCC_URLS"); /* Plural! */
+ if (p == NULL)
+ p = getenv ("TERM_URLS");
+
+ if (p == NULL)
+ return true;
+ if (*p == '\0')
+ return false;
+
+ if (!strcmp (p, "no"))
+ return false;
+ if (!strcmp (p, "st"))
+ diagnostic_urls_use_st = true;
+ else if (!strcmp (p, "bel"))
+ diagnostic_urls_use_st = false;
+ return true;
+}
+
+/* Return true if we should use urls when in auto mode, false otherwise. */
+
+static bool
+auto_enable_urls ()
+{
+#ifdef __MINGW32__
+ return false;
+#else
+ const char *p;
+
+ if (!should_colorize ())
+ return false;
+ p = getenv ("COLORTERM");
+ if (p == NULL)
+ return true;
+ if (!strcmp (p, "xfce4-terminal"))
+ return false;
+ return true;
+#endif
+}
+
/* Determine if URLs should be enabled, based on RULE.
This reuses the logic for colorization. */
@@ -250,9 +300,12 @@ diagnostic_urls_enabled_p (diagnostic_url_rule_t rule)
case DIAGNOSTICS_URL_NO:
return false;
case DIAGNOSTICS_URL_YES:
- return true;
+ return parse_gcc_urls ();
case DIAGNOSTICS_URL_AUTO:
- return should_colorize ();
+ if (auto_enable_urls ())
+ return parse_gcc_urls ();
+ else
+ return false;
default:
gcc_unreachable ();
}
diff --git a/gcc/diagnostic-url.h b/gcc/diagnostic-url.h
index 6be0569..06b7784 100644
--- a/gcc/diagnostic-url.h
+++ b/gcc/diagnostic-url.h
@@ -32,5 +32,6 @@ typedef enum
} diagnostic_url_rule_t;
extern bool diagnostic_urls_enabled_p (diagnostic_url_rule_t);
+extern bool diagnostic_urls_use_st;
#endif /* ! GCC_DIAGNOSTIC_URL_H */
diff --git a/gcc/diagnostic.c b/gcc/diagnostic.c
index 3386f07..55e323f 100644
--- a/gcc/diagnostic.c
+++ b/gcc/diagnostic.c
@@ -260,8 +260,23 @@ diagnostic_color_init (diagnostic_context *context, int value /*= -1 */)
void
diagnostic_urls_init (diagnostic_context *context, int value /*= -1 */)
{
+ /* value == -1 is the default value. */
if (value < 0)
- value = DIAGNOSTICS_COLOR_DEFAULT;
+ {
+ /* If DIAGNOSTICS_URLS_DEFAULT is -1, default to
+ -fdiagnostics-urls=auto if GCC_URLS or TERM_URLS is in the
+ environment, otherwise default to -fdiagnostics-urls=never,
+ for other values default to that
+ -fdiagnostics-urls={never,auto,always}. */
+ if (DIAGNOSTICS_URLS_DEFAULT == -1)
+ {
+ if (!getenv ("GCC_URLS") && !getenv ("TERM_URLS"))
+ return;
+ value = DIAGNOSTICS_URL_AUTO;
+ }
+ else
+ value = DIAGNOSTICS_URLS_DEFAULT;
+ }
context->printer->show_urls
= diagnostic_urls_enabled_p ((diagnostic_url_rule_t) value);
diff --git a/gcc/doc/install.texi b/gcc/doc/install.texi
index 6ffafac..f3f63be 100644
--- a/gcc/doc/install.texi
+++ b/gcc/doc/install.texi
@@ -2100,6 +2100,15 @@ where @samp{auto} is the default. @samp{auto-if-env} means that
is present and non-empty in the environment, and
@option{-fdiagnostics-color=never} otherwise.
+@item --with-diagnostics-urls=@var{choice}
+Tells GCC to use @var{choice} as the default for @option{-fdiagnostics-urls=}
+option (if not used explicitly on the command line). @var{choice}
+can be one of @samp{never}, @samp{auto}, @samp{always}, and @samp{auto-if-env}
+where @samp{auto} is the default. @samp{auto-if-env} means that
+@option{-fdiagnostics-urls=auto} will be the default if @env{GCC_URLS}
+or @env{TERM_URLS} is present and non-empty in the environment, and
+@option{-fdiagnostics-urls=never} otherwise.
+
@item --enable-lto
@itemx --disable-lto
Enable support for link-time optimization (LTO). This is enabled by
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 2cd8d7e..4ebb411 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -4032,14 +4032,25 @@ arguments in the C++ frontend.
@item -fdiagnostics-urls[=@var{WHEN}]
@opindex fdiagnostics-urls
@cindex urls
+@vindex GCC_URLS @r{environment variable}
Use escape sequences to embed URLs in diagnostics. For example, when
@option{-fdiagnostics-show-option} emits text showing the command-line
option controlling a diagnostic, embed a URL for documentation of that
option.
@var{WHEN} is @samp{never}, @samp{always}, or @samp{auto}.
-The default is @samp{auto}, which means to use URL escape sequences only
-when the standard error is a terminal.
+@samp{auto} means to use URL escape sequences only when the standard error
+is a terminal, and @env{TERM} is not set to "dumb".
+
+The default depends on how the compiler has been configured,
+it can be any of the above @var{WHEN} options or also @samp{never}
+if neither @env{GCC_URLS} nor @env{TERM_URLS} environment variable is
+present in the environment, and @samp{auto} otherwise.
+If @env{GCC_URLS} set to empty or "no" do not embed URLs in diagnostics.
+If set to "st", URLs use ST escape sequences.
+If set to "bel", the default, URLs use BEL escape sequences.
+Any other non-empty value enables the feature.
+If @env{GCC_URLS} is not set, use @env{TERM_URLS} as a fallback.
@item -fno-diagnostics-show-option
@opindex fno-diagnostics-show-option
diff --git a/gcc/pretty-print.c b/gcc/pretty-print.c
index 817c105..02649a5 100644
--- a/gcc/pretty-print.c
+++ b/gcc/pretty-print.c
@@ -25,6 +25,7 @@ along with GCC; see the file COPYING3. If not see
#include "pretty-print.h"
#include "diagnostic-color.h"
#include "diagnostic-event-id.h"
+#include "diagnostic-url.h"
#include "selftest.h"
#if HAVE_ICONV
@@ -2172,7 +2173,8 @@ void
pp_begin_url (pretty_printer *pp, const char *url)
{
if (pp->show_urls)
- pp_printf (pp, "\33]8;;%s\a", url);
+ pp_printf (pp, "\33]8;;%s%s", url,
+ diagnostic_urls_use_st ? "\33\\" : "\a");
}
/* If URL-printing is enabled, write a "close URL" escape sequence to PP. */
@@ -2181,7 +2183,8 @@ void
pp_end_url (pretty_printer *pp)
{
if (pp->show_urls)
- pp_string (pp, "\33]8;;\a");
+ pp_string (pp, diagnostic_urls_use_st ? "\33]8;;\33\\"
+ : "\33]8;;\a");
}
#if CHECKING_P
@@ -2504,7 +2507,9 @@ test_urls ()
pp_begin_url (&pp, "http://example.com";);
pp_string (&pp, "This is a link");
pp_end_url (&pp);
- ASSERT_STREQ ("\33]8;;http://example.com\aThis is a link\33]8;;\a",
+ ASSERT_STREQ (diagnostic_urls_use_st
+ ? "\33]8;;http://example.com\33\\This is a link\33]8;;\33\\"
+ : "\33]8;;http://example.com\aThis is a link\33]8;;\a",
pp_formatted_text (&pp));
}
}
--
1.9.1
