Add a configure option to disable system header canonicalizations

[simonb, 2012-09-05 13:56:42]

Add a configure option to disable system header canonicalizations.

Libcpp may canonicalize system header paths with lrealpath() for diagnostics,
dependency output, and similar.  If gcc is held in a symlink farm the
canonicalized paths may be meaningless to users, and will also conflict with
build frameworks that (for example) disallow absolute paths to header files.

This change adds --[en/dis]able-canonical-system-headers, allowing configure
to select whether or not to implement r186991.  See also PR c++/52974.

Tested for regressions with bootstrap builds of C and C++, both with and
without configure --disable-canonical-system-headers.

Okay for trunk?

libcpp/ChangeLog.google-integration
2012-09-05  Simon Baldwin  <simonb@google.com>

	* files.c (maybe_shorter_path): Suppress function definition if
	ENABLE_CANONICAL_SYSTEM_HEADERS is not defined.
	* (find_file_in_dir): Call maybe_shorter_path() only if
	ENABLE_CANONICAL_SYSTEM_HEADERS is defined.
	* configure.ac: Add new --enable-canonical-system-headers.
	* configure: Regenerate.
	* config.in: Regenerate.

gcc/ChangeLog.google-integration
2012-09-05  Simon Baldwin  <simonb@google.com>

	* doc/install.texi: Document --enable-canonical-system-headers.


Index: gcc/doc/install.texi
===================================================================
--- gcc/doc/install.texi	(revision 190968)
+++ gcc/doc/install.texi	(working copy)
@@ -1710,6 +1710,14 @@ link time when @option{-fuse-linker-plug
 This linker should have plugin support such as gold starting with
 version 2.20 or GNU ld starting with version 2.21.
 See @option{-fuse-linker-plugin} for details.
+
+@item --enable-canonical-system-headers
+@itemx --disable-canonical-system-headers
+Enable system header path canonicalization for @file{libcpp}.  This can
+produce shorter header file paths in diagnostics and dependency output
+files, but these changed header paths may conflict with some compilation
+environments.  Enabled by default, and may be disabled using
+@option{--disable-canonical-system-headers}.
 @end table
 
 @subheading Cross-Compiler-Specific Options
Index: libcpp/configure
===================================================================
--- libcpp/configure	(revision 190968)
+++ libcpp/configure	(working copy)
@@ -700,6 +700,7 @@ enable_rpath
 with_libiconv_prefix
 enable_maintainer_mode
 enable_checking
+enable_canonical_system_headers
 '
       ac_precious_vars='build_alias
 host_alias
@@ -1333,6 +1334,8 @@ Optional Features:
   --disable-rpath         do not hardcode runtime library paths
   --enable-maintainer-mode enable rules only needed by maintainers
   --enable-checking      enable expensive run-time checks
+  --enable-canonical-system-headers
+                          enable or disable system headers canonicalization
 
 Optional Packages:
   --with-PACKAGE[=ARG]    use PACKAGE [ARG=yes]
@@ -7094,6 +7097,19 @@ $as_echo "#define ENABLE_CHECKING 1" >>c
 
 fi
 
+# Check whether --enable-canonical-system-headers was given.
+if test "${enable_canonical_system_headers+set}" = set; then :
+  enableval=$enable_canonical_system_headers;
+else
+  enable_canonical_system_headers=yes
+fi
+
+if test $enable_canonical_system_headers != no; then
+
+$as_echo "#define ENABLE_CANONICAL_SYSTEM_HEADERS 1" >>confdefs.h
+
+fi
+
 
 case $target in
 	alpha*-*-* | \
Index: libcpp/files.c
===================================================================
--- libcpp/files.c	(revision 190968)
+++ libcpp/files.c	(working copy)
@@ -345,6 +345,7 @@ pch_open_file (cpp_reader *pfile, _cpp_f
    shorter, otherwise return NULL.  This function does NOT free the
    memory pointed by FILE.  */
 
+#ifdef ENABLE_CANONICAL_SYSTEM_HEADERS
 static char *
 maybe_shorter_path (const char * file)
 {
@@ -359,6 +360,7 @@ maybe_shorter_path (const char * file)
       return NULL;
     }
 }
+#endif
 
 /* Try to open the path FILE->name appended to FILE->dir.  This is
    where remap and PCH intercept the file lookup process.  Return true
@@ -384,6 +386,7 @@ find_file_in_dir (cpp_reader *pfile, _cp
       char *copy;
       void **pp;
 
+#ifdef ENABLE_CANONICAL_SYSTEM_HEADERS
       /* We try to canonicalize system headers.  */
       if (file->dir->sysp)
 	{
@@ -396,6 +399,7 @@ find_file_in_dir (cpp_reader *pfile, _cp
 	      path = canonical_path;
 	    }
 	}
+#endif
 
       hv = htab_hash_string (path);
       if (htab_find_with_hash (pfile->nonexistent_file_hash, path, hv) !=
NULL)
Index: libcpp/configure.ac
===================================================================
--- libcpp/configure.ac	(revision 190968)
+++ libcpp/configure.ac	(working copy)
@@ -132,6 +132,16 @@ if test $enable_checking != no ; then
 [Define if you want more run-time sanity checks.])
 fi
 
+AC_ARG_ENABLE(canonical-system-headers,
+[  --enable-canonical-system-headers
+                          enable or disable system headers canonicalization],
+[],
+enable_canonical_system_headers=yes)
+if test $enable_canonical_system_headers != no; then
+  AC_DEFINE(ENABLE_CANONICAL_SYSTEM_HEADERS,
+            1, [Define to enable system headers canonicalization.])
+fi
+
 m4_changequote(,)
 case $target in
 	alpha*-*-* | \
Index: libcpp/config.in
===================================================================
--- libcpp/config.in	(revision 190968)
+++ libcpp/config.in	(working copy)
@@ -11,6 +11,9 @@
 /* Define to 1 if using `alloca.c'. */
 #undef C_ALLOCA
 
+/* Define to enable system headers canonicalization. */
+#undef ENABLE_CANONICAL_SYSTEM_HEADERS
+
 /* Define if you want more run-time sanity checks. */
 #undef ENABLE_CHECKING
 

--
This patch is available for review at http://codereview.appspot.com/6495088

[iant2, 2012-09-05 14:03:38]

On Wed, Sep 5, 2012 at 6:56 AM, Simon Baldwin <simonb@google.com> wrote:
> Add a configure option to disable system header canonicalizations.

Why should this be a configure option rather than a command-line option?

Ian



> Libcpp may canonicalize system header paths with lrealpath() for diagnostics,
> dependency output, and similar.  If gcc is held in a symlink farm the
> canonicalized paths may be meaningless to users, and will also conflict with
> build frameworks that (for example) disallow absolute paths to header files.
>
> This change adds --[en/dis]able-canonical-system-headers, allowing configure
> to select whether or not to implement r186991.  See also PR c++/52974.
>
> Tested for regressions with bootstrap builds of C and C++, both with and
> without configure --disable-canonical-system-headers.
>
> Okay for trunk?
>
> libcpp/ChangeLog.google-integration
> 2012-09-05  Simon Baldwin  <simonb@google.com>
>
>         * files.c (maybe_shorter_path): Suppress function definition if
>         ENABLE_CANONICAL_SYSTEM_HEADERS is not defined.
>         * (find_file_in_dir): Call maybe_shorter_path() only if
>         ENABLE_CANONICAL_SYSTEM_HEADERS is defined.
>         * configure.ac: Add new --enable-canonical-system-headers.
>         * configure: Regenerate.
>         * config.in: Regenerate.
>
> gcc/ChangeLog.google-integration
> 2012-09-05  Simon Baldwin  <simonb@google.com>
>
>         * doc/install.texi: Document --enable-canonical-system-headers.
>
>
> Index: gcc/doc/install.texi
> ===================================================================
> --- gcc/doc/install.texi        (revision 190968)
> +++ gcc/doc/install.texi        (working copy)
> @@ -1710,6 +1710,14 @@ link time when @option{-fuse-linker-plug
>  This linker should have plugin support such as gold starting with
>  version 2.20 or GNU ld starting with version 2.21.
>  See @option{-fuse-linker-plugin} for details.
> +
> +@item --enable-canonical-system-headers
> +@itemx --disable-canonical-system-headers
> +Enable system header path canonicalization for @file{libcpp}.  This can
> +produce shorter header file paths in diagnostics and dependency output
> +files, but these changed header paths may conflict with some compilation
> +environments.  Enabled by default, and may be disabled using
> +@option{--disable-canonical-system-headers}.
>  @end table
>
>  @subheading Cross-Compiler-Specific Options
> Index: libcpp/configure
> ===================================================================
> --- libcpp/configure    (revision 190968)
> +++ libcpp/configure    (working copy)
> @@ -700,6 +700,7 @@ enable_rpath
>  with_libiconv_prefix
>  enable_maintainer_mode
>  enable_checking
> +enable_canonical_system_headers
>  '
>        ac_precious_vars='build_alias
>  host_alias
> @@ -1333,6 +1334,8 @@ Optional Features:
>    --disable-rpath         do not hardcode runtime library paths
>    --enable-maintainer-mode enable rules only needed by maintainers
>    --enable-checking      enable expensive run-time checks
> +  --enable-canonical-system-headers
> +                          enable or disable system headers canonicalization
>
>  Optional Packages:
>    --with-PACKAGE[=ARG]    use PACKAGE [ARG=yes]
> @@ -7094,6 +7097,19 @@ $as_echo "#define ENABLE_CHECKING 1" >>c
>
>  fi
>
> +# Check whether --enable-canonical-system-headers was given.
> +if test "${enable_canonical_system_headers+set}" = set; then :
> +  enableval=$enable_canonical_system_headers;
> +else
> +  enable_canonical_system_headers=yes
> +fi
> +
> +if test $enable_canonical_system_headers != no; then
> +
> +$as_echo "#define ENABLE_CANONICAL_SYSTEM_HEADERS 1" >>confdefs.h
> +
> +fi
> +
>
>  case $target in
>         alpha*-*-* | \
> Index: libcpp/files.c
> ===================================================================
> --- libcpp/files.c      (revision 190968)
> +++ libcpp/files.c      (working copy)
> @@ -345,6 +345,7 @@ pch_open_file (cpp_reader *pfile, _cpp_f
>     shorter, otherwise return NULL.  This function does NOT free the
>     memory pointed by FILE.  */
>
> +#ifdef ENABLE_CANONICAL_SYSTEM_HEADERS
>  static char *
>  maybe_shorter_path (const char * file)
>  {
> @@ -359,6 +360,7 @@ maybe_shorter_path (const char * file)
>        return NULL;
>      }
>  }
> +#endif
>
>  /* Try to open the path FILE->name appended to FILE->dir.  This is
>     where remap and PCH intercept the file lookup process.  Return true
> @@ -384,6 +386,7 @@ find_file_in_dir (cpp_reader *pfile, _cp
>        char *copy;
>        void **pp;
>
> +#ifdef ENABLE_CANONICAL_SYSTEM_HEADERS
>        /* We try to canonicalize system headers.  */
>        if (file->dir->sysp)
>         {
> @@ -396,6 +399,7 @@ find_file_in_dir (cpp_reader *pfile, _cp
>               path = canonical_path;
>             }
>         }
> +#endif
>
>        hv = htab_hash_string (path);
>        if (htab_find_with_hash (pfile->nonexistent_file_hash, path, hv) !=
NULL)
> Index: libcpp/configure.ac
> ===================================================================
> --- libcpp/configure.ac (revision 190968)
> +++ libcpp/configure.ac (working copy)
> @@ -132,6 +132,16 @@ if test $enable_checking != no ; then
>  [Define if you want more run-time sanity checks.])
>  fi
>
> +AC_ARG_ENABLE(canonical-system-headers,
> +[  --enable-canonical-system-headers
> +                          enable or disable system headers
canonicalization],
> +[],
> +enable_canonical_system_headers=yes)
> +if test $enable_canonical_system_headers != no; then
> +  AC_DEFINE(ENABLE_CANONICAL_SYSTEM_HEADERS,
> +            1, [Define to enable system headers canonicalization.])
> +fi
> +
>  m4_changequote(,)
>  case $target in
>         alpha*-*-* | \
> Index: libcpp/config.in
> ===================================================================
> --- libcpp/config.in    (revision 190968)
> +++ libcpp/config.in    (working copy)
> @@ -11,6 +11,9 @@
>  /* Define to 1 if using `alloca.c'. */
>  #undef C_ALLOCA
>
> +/* Define to enable system headers canonicalization. */
> +#undef ENABLE_CANONICAL_SYSTEM_HEADERS
> +
>  /* Define if you want more run-time sanity checks. */
>  #undef ENABLE_CHECKING
>
>
> --
> This patch is available for review at http://codereview.appspot.com/6495088

[simonb, 2012-09-05 14:29:52]

On 5 September 2012 16:03, Ian Lance Taylor <iant@google.com> wrote:
> On Wed, Sep 5, 2012 at 6:56 AM, Simon Baldwin <simonb@google.com> wrote:
>> Add a configure option to disable system header canonicalizations.
>
> Why should this be a configure option rather than a command-line option?

The underlying problem is a niche one, likely to affect a
(vanishingly?) small number of users.  It is hard in widely
distributed build systems that combine make and non-make schemes to
ensure that a given flag is passed to every compiler invocation every
time.  A configure option is a relatively non-invasive libcpp change.
Gcc already has too many command line flags.

Do you have a strong reason for why this should be a command line
option and not a configure flag?


>
>
>
>> Libcpp may canonicalize system header paths with lrealpath() for
diagnostics,
>> dependency output, and similar.  If gcc is held in a symlink farm the
>> canonicalized paths may be meaningless to users, and will also conflict with
>> build frameworks that (for example) disallow absolute paths to header files.
>>
>> This change adds --[en/dis]able-canonical-system-headers, allowing configure
>> to select whether or not to implement r186991.  See also PR c++/52974.
>>
>> Tested for regressions with bootstrap builds of C and C++, both with and
>> without configure --disable-canonical-system-headers.
>>
>> Okay for trunk?
>>
>> libcpp/ChangeLog.google-integration
>> 2012-09-05  Simon Baldwin  <simonb@google.com>
>>
>>         * files.c (maybe_shorter_path): Suppress function definition if
>>         ENABLE_CANONICAL_SYSTEM_HEADERS is not defined.
>>         * (find_file_in_dir): Call maybe_shorter_path() only if
>>         ENABLE_CANONICAL_SYSTEM_HEADERS is defined.
>>         * configure.ac: Add new --enable-canonical-system-headers.
>>         * configure: Regenerate.
>>         * config.in: Regenerate.
>>
>> gcc/ChangeLog.google-integration
>> 2012-09-05  Simon Baldwin  <simonb@google.com>
>>
>>         * doc/install.texi: Document --enable-canonical-system-headers.
>>
>>
>> Index: gcc/doc/install.texi
>> ===================================================================
>> --- gcc/doc/install.texi        (revision 190968)
>> +++ gcc/doc/install.texi        (working copy)
>> @@ -1710,6 +1710,14 @@ link time when @option{-fuse-linker-plug
>>  This linker should have plugin support such as gold starting with
>>  version 2.20 or GNU ld starting with version 2.21.
>>  See @option{-fuse-linker-plugin} for details.
>> +
>> +@item --enable-canonical-system-headers
>> +@itemx --disable-canonical-system-headers
>> +Enable system header path canonicalization for @file{libcpp}.  This can
>> +produce shorter header file paths in diagnostics and dependency output
>> +files, but these changed header paths may conflict with some compilation
>> +environments.  Enabled by default, and may be disabled using
>> +@option{--disable-canonical-system-headers}.
>>  @end table
>>
>>  @subheading Cross-Compiler-Specific Options
>> Index: libcpp/configure
>> ===================================================================
>> --- libcpp/configure    (revision 190968)
>> +++ libcpp/configure    (working copy)
>> @@ -700,6 +700,7 @@ enable_rpath
>>  with_libiconv_prefix
>>  enable_maintainer_mode
>>  enable_checking
>> +enable_canonical_system_headers
>>  '
>>        ac_precious_vars='build_alias
>>  host_alias
>> @@ -1333,6 +1334,8 @@ Optional Features:
>>    --disable-rpath         do not hardcode runtime library paths
>>    --enable-maintainer-mode enable rules only needed by maintainers
>>    --enable-checking      enable expensive run-time checks
>> +  --enable-canonical-system-headers
>> +                          enable or disable system headers canonicalization
>>
>>  Optional Packages:
>>    --with-PACKAGE[=ARG]    use PACKAGE [ARG=yes]
>> @@ -7094,6 +7097,19 @@ $as_echo "#define ENABLE_CHECKING 1" >>c
>>
>>  fi
>>
>> +# Check whether --enable-canonical-system-headers was given.
>> +if test "${enable_canonical_system_headers+set}" = set; then :
>> +  enableval=$enable_canonical_system_headers;
>> +else
>> +  enable_canonical_system_headers=yes
>> +fi
>> +
>> +if test $enable_canonical_system_headers != no; then
>> +
>> +$as_echo "#define ENABLE_CANONICAL_SYSTEM_HEADERS 1" >>confdefs.h
>> +
>> +fi
>> +
>>
>>  case $target in
>>         alpha*-*-* | \
>> Index: libcpp/files.c
>> ===================================================================
>> --- libcpp/files.c      (revision 190968)
>> +++ libcpp/files.c      (working copy)
>> @@ -345,6 +345,7 @@ pch_open_file (cpp_reader *pfile, _cpp_f
>>     shorter, otherwise return NULL.  This function does NOT free the
>>     memory pointed by FILE.  */
>>
>> +#ifdef ENABLE_CANONICAL_SYSTEM_HEADERS
>>  static char *
>>  maybe_shorter_path (const char * file)
>>  {
>> @@ -359,6 +360,7 @@ maybe_shorter_path (const char * file)
>>        return NULL;
>>      }
>>  }
>> +#endif
>>
>>  /* Try to open the path FILE->name appended to FILE->dir.  This is
>>     where remap and PCH intercept the file lookup process.  Return true
>> @@ -384,6 +386,7 @@ find_file_in_dir (cpp_reader *pfile, _cp
>>        char *copy;
>>        void **pp;
>>
>> +#ifdef ENABLE_CANONICAL_SYSTEM_HEADERS
>>        /* We try to canonicalize system headers.  */
>>        if (file->dir->sysp)
>>         {
>> @@ -396,6 +399,7 @@ find_file_in_dir (cpp_reader *pfile, _cp
>>               path = canonical_path;
>>             }
>>         }
>> +#endif
>>
>>        hv = htab_hash_string (path);
>>        if (htab_find_with_hash (pfile->nonexistent_file_hash, path, hv) !=
NULL)
>> Index: libcpp/configure.ac
>> ===================================================================
>> --- libcpp/configure.ac (revision 190968)
>> +++ libcpp/configure.ac (working copy)
>> @@ -132,6 +132,16 @@ if test $enable_checking != no ; then
>>  [Define if you want more run-time sanity checks.])
>>  fi
>>
>> +AC_ARG_ENABLE(canonical-system-headers,
>> +[  --enable-canonical-system-headers
>> +                          enable or disable system headers
canonicalization],
>> +[],
>> +enable_canonical_system_headers=yes)
>> +if test $enable_canonical_system_headers != no; then
>> +  AC_DEFINE(ENABLE_CANONICAL_SYSTEM_HEADERS,
>> +            1, [Define to enable system headers canonicalization.])
>> +fi
>> +
>>  m4_changequote(,)
>>  case $target in
>>         alpha*-*-* | \
>> Index: libcpp/config.in
>> ===================================================================
>> --- libcpp/config.in    (revision 190968)
>> +++ libcpp/config.in    (working copy)
>> @@ -11,6 +11,9 @@
>>  /* Define to 1 if using `alloca.c'. */
>>  #undef C_ALLOCA
>>
>> +/* Define to enable system headers canonicalization. */
>> +#undef ENABLE_CANONICAL_SYSTEM_HEADERS
>> +
>>  /* Define if you want more run-time sanity checks. */
>>  #undef ENABLE_CHECKING
>>
>>
>> --
>> This patch is available for review at http://codereview.appspot.com/6495088



-- 
Google UK Limited | Registered Office: Belgrave House, 76 Buckingham
Palace Road, London SW1W 9TQ | Registered in England Number: 3977902

[iant2, 2012-09-05 15:01:58]

On Wed, Sep 5, 2012 at 7:23 AM, Simon Baldwin <simonb@google.com> wrote:
> On 5 September 2012 16:03, Ian Lance Taylor <iant@google.com> wrote:
>> On Wed, Sep 5, 2012 at 6:56 AM, Simon Baldwin <simonb@google.com> wrote:
>>> Add a configure option to disable system header canonicalizations.
>>
>> Why should this be a configure option rather than a command-line option?
>
> The underlying problem is a niche one, likely to affect a
> (vanishingly?) small number of users.  It is hard in widely
> distributed build systems that combine make and non-make schemes to
> ensure that a given flag is passed to every compiler invocation every
> time.  A configure option is a relatively non-invasive libcpp change.
> Gcc already has too many command line flags.
>
> Do you have a strong reason for why this should be a command line
> option and not a configure flag?

I don't know if it's a strong reason, but the problem seems to be one
that is characteristic of a specific invocation of a compiler, rather
than characteristic of the compiler in general.  The same compiler may
be invoked in multiple ways.  In only some of those ways is it
appropriate to avoid canonicalizing paths.

Ian

>>> Libcpp may canonicalize system header paths with lrealpath() for
diagnostics,
>>> dependency output, and similar.  If gcc is held in a symlink farm the
>>> canonicalized paths may be meaningless to users, and will also conflict
with
>>> build frameworks that (for example) disallow absolute paths to header
files.
>>>
>>> This change adds --[en/dis]able-canonical-system-headers, allowing
configure
>>> to select whether or not to implement r186991.  See also PR c++/52974.
>>>
>>> Tested for regressions with bootstrap builds of C and C++, both with and
>>> without configure --disable-canonical-system-headers.
>>>
>>> Okay for trunk?
>>>
>>> libcpp/ChangeLog.google-integration
>>> 2012-09-05  Simon Baldwin  <simonb@google.com>
>>>
>>>         * files.c (maybe_shorter_path): Suppress function definition if
>>>         ENABLE_CANONICAL_SYSTEM_HEADERS is not defined.
>>>         * (find_file_in_dir): Call maybe_shorter_path() only if
>>>         ENABLE_CANONICAL_SYSTEM_HEADERS is defined.
>>>         * configure.ac: Add new --enable-canonical-system-headers.
>>>         * configure: Regenerate.
>>>         * config.in: Regenerate.
>>>
>>> gcc/ChangeLog.google-integration
>>> 2012-09-05  Simon Baldwin  <simonb@google.com>
>>>
>>>         * doc/install.texi: Document --enable-canonical-system-headers.
>>>
>>>
>>> Index: gcc/doc/install.texi
>>> ===================================================================
>>> --- gcc/doc/install.texi        (revision 190968)
>>> +++ gcc/doc/install.texi        (working copy)
>>> @@ -1710,6 +1710,14 @@ link time when @option{-fuse-linker-plug
>>>  This linker should have plugin support such as gold starting with
>>>  version 2.20 or GNU ld starting with version 2.21.
>>>  See @option{-fuse-linker-plugin} for details.
>>> +
>>> +@item --enable-canonical-system-headers
>>> +@itemx --disable-canonical-system-headers
>>> +Enable system header path canonicalization for @file{libcpp}.  This can
>>> +produce shorter header file paths in diagnostics and dependency output
>>> +files, but these changed header paths may conflict with some compilation
>>> +environments.  Enabled by default, and may be disabled using
>>> +@option{--disable-canonical-system-headers}.
>>>  @end table
>>>
>>>  @subheading Cross-Compiler-Specific Options
>>> Index: libcpp/configure
>>> ===================================================================
>>> --- libcpp/configure    (revision 190968)
>>> +++ libcpp/configure    (working copy)
>>> @@ -700,6 +700,7 @@ enable_rpath
>>>  with_libiconv_prefix
>>>  enable_maintainer_mode
>>>  enable_checking
>>> +enable_canonical_system_headers
>>>  '
>>>        ac_precious_vars='build_alias
>>>  host_alias
>>> @@ -1333,6 +1334,8 @@ Optional Features:
>>>    --disable-rpath         do not hardcode runtime library paths
>>>    --enable-maintainer-mode enable rules only needed by maintainers
>>>    --enable-checking      enable expensive run-time checks
>>> +  --enable-canonical-system-headers
>>> +                          enable or disable system headers
canonicalization
>>>
>>>  Optional Packages:
>>>    --with-PACKAGE[=ARG]    use PACKAGE [ARG=yes]
>>> @@ -7094,6 +7097,19 @@ $as_echo "#define ENABLE_CHECKING 1" >>c
>>>
>>>  fi
>>>
>>> +# Check whether --enable-canonical-system-headers was given.
>>> +if test "${enable_canonical_system_headers+set}" = set; then :
>>> +  enableval=$enable_canonical_system_headers;
>>> +else
>>> +  enable_canonical_system_headers=yes
>>> +fi
>>> +
>>> +if test $enable_canonical_system_headers != no; then
>>> +
>>> +$as_echo "#define ENABLE_CANONICAL_SYSTEM_HEADERS 1" >>confdefs.h
>>> +
>>> +fi
>>> +
>>>
>>>  case $target in
>>>         alpha*-*-* | \
>>> Index: libcpp/files.c
>>> ===================================================================
>>> --- libcpp/files.c      (revision 190968)
>>> +++ libcpp/files.c      (working copy)
>>> @@ -345,6 +345,7 @@ pch_open_file (cpp_reader *pfile, _cpp_f
>>>     shorter, otherwise return NULL.  This function does NOT free the
>>>     memory pointed by FILE.  */
>>>
>>> +#ifdef ENABLE_CANONICAL_SYSTEM_HEADERS
>>>  static char *
>>>  maybe_shorter_path (const char * file)
>>>  {
>>> @@ -359,6 +360,7 @@ maybe_shorter_path (const char * file)
>>>        return NULL;
>>>      }
>>>  }
>>> +#endif
>>>
>>>  /* Try to open the path FILE->name appended to FILE->dir.  This is
>>>     where remap and PCH intercept the file lookup process.  Return true
>>> @@ -384,6 +386,7 @@ find_file_in_dir (cpp_reader *pfile, _cp
>>>        char *copy;
>>>        void **pp;
>>>
>>> +#ifdef ENABLE_CANONICAL_SYSTEM_HEADERS
>>>        /* We try to canonicalize system headers.  */
>>>        if (file->dir->sysp)
>>>         {
>>> @@ -396,6 +399,7 @@ find_file_in_dir (cpp_reader *pfile, _cp
>>>               path = canonical_path;
>>>             }
>>>         }
>>> +#endif
>>>
>>>        hv = htab_hash_string (path);
>>>        if (htab_find_with_hash (pfile->nonexistent_file_hash, path, hv) !=
NULL)
>>> Index: libcpp/configure.ac
>>> ===================================================================
>>> --- libcpp/configure.ac (revision 190968)
>>> +++ libcpp/configure.ac (working copy)
>>> @@ -132,6 +132,16 @@ if test $enable_checking != no ; then
>>>  [Define if you want more run-time sanity checks.])
>>>  fi
>>>
>>> +AC_ARG_ENABLE(canonical-system-headers,
>>> +[  --enable-canonical-system-headers
>>> +                          enable or disable system headers
canonicalization],
>>> +[],
>>> +enable_canonical_system_headers=yes)
>>> +if test $enable_canonical_system_headers != no; then
>>> +  AC_DEFINE(ENABLE_CANONICAL_SYSTEM_HEADERS,
>>> +            1, [Define to enable system headers canonicalization.])
>>> +fi
>>> +
>>>  m4_changequote(,)
>>>  case $target in
>>>         alpha*-*-* | \
>>> Index: libcpp/config.in
>>> ===================================================================
>>> --- libcpp/config.in    (revision 190968)
>>> +++ libcpp/config.in    (working copy)
>>> @@ -11,6 +11,9 @@
>>>  /* Define to 1 if using `alloca.c'. */
>>>  #undef C_ALLOCA
>>>
>>> +/* Define to enable system headers canonicalization. */
>>> +#undef ENABLE_CANONICAL_SYSTEM_HEADERS
>>> +
>>>  /* Define if you want more run-time sanity checks. */
>>>  #undef ENABLE_CHECKING
>>>
>>>
>>> --
>>> This patch is available for review at http://codereview.appspot.com/6495088
>
>
>
> --
> Google UK Limited | Registered Office: Belgrave House, 76 Buckingham
> Palace Road, London SW1W 9TQ | Registered in England Number: 3977902

[simonb, 2012-09-07 11:00:28]

On 5 September 2012 16:01, Ian Lance Taylor <iant@google.com> wrote:
>...
>
> I don't know if it's a strong reason, but the problem seems to be one
> that is characteristic of a specific invocation of a compiler, rather
> than characteristic of the compiler in general.  The same compiler may
> be invoked in multiple ways.  In only some of those ways is it
> appropriate to avoid canonicalizing paths.

Revised version below.  Adds new gcc command line flags to override
any default set by configure.  Please take another look when ready.

----------

Add flags to disable system header canonicalizations.

Libcpp may canonicalize system header paths with lrealpath() for diagnostics,
dependency output, and similar.  If gcc is held in a symlink farm the
canonicalized paths may be meaningless to users, and will also conflict with
build frameworks that (for example) disallow absolute paths to header files.

This change adds -f[no-]canonical-system-headers to the gcc command line, and
a configure option --[en/dis]able-canonical-system-headers to set default
behaviour, allowing the user to select whether or not to implement r186991.
Default is enabled.  See also PR c++/52974.

Tested for regressions with bootstrap builds of C and C++, both with and
without configure --disable-canonical-system-headers.

Okay for trunk?

libcpp/ChangeLog
2012-09-07  Simon Baldwin  <simonb@google.com>

    * include/cpplib.h (struct cpp_options): Add canonical_system_headers.
    * files.c (find_file_in_dir): Call maybe_shorter_path() only if
    canonical_system_headers is set.
    * init.c (cpp_create_reader): Initialize canonical_system_headers.
    * configure.ac: Add new --enable-canonical-system-headers.
    * configure: Regenerate.
    * config.in: Regenerate.

gcc/ChangeLog
2012-09-07  Simon Baldwin  <simonb@google.com>

    * doc/cppopts.texi: Document -f[no-]canonical-system-headers.
    * doc/install.texi: Document --enable-canonical-system-headers.

gcc/c-family/ChangeLog
2012-09-07  Simon Baldwin  <simonb@google.com>

    * c.opt: Add f[no-]canonical-system-headers.
    * c-opts.c (c_common_handle_option): Handle
    OPT_fcanonical_system_headers.


Index: gcc/doc/cppopts.texi
===================================================================
--- gcc/doc/cppopts.texi    (revision 191054)
+++ gcc/doc/cppopts.texi    (working copy)
@@ -564,6 +564,10 @@ Accept universal character names in iden
 experimental; in a future version of GCC, it will be enabled by
 default for C99 and C++.

+@item -fno-canonical-system-headers
+@opindex fno-canonical-system-headers
+When preprocessing, do not shorten system header paths with canonicalization.
+
 @item -fpreprocessed
 @opindex fpreprocessed
 Indicate to the preprocessor that the input file has already been
Index: gcc/doc/install.texi
===================================================================
--- gcc/doc/install.texi    (revision 191054)
+++ gcc/doc/install.texi    (working copy)
@@ -1710,6 +1710,14 @@ link time when @option{-fuse-linker-plug
 This linker should have plugin support such as gold starting with
 version 2.20 or GNU ld starting with version 2.21.
 See @option{-fuse-linker-plugin} for details.
+
+@item --enable-canonical-system-headers
+@itemx --disable-canonical-system-headers
+Enable system header path canonicalization for @file{libcpp}.  This can
+produce shorter header file paths in diagnostics and dependency output
+files, but these changed header paths may conflict with some compilation
+environments.  Enabled by default, and may be disabled using
+@option{--disable-canonical-system-headers}.
 @end table

 @subheading Cross-Compiler-Specific Options
Index: gcc/c-family/c.opt
===================================================================
--- gcc/c-family/c.opt    (revision 191054)
+++ gcc/c-family/c.opt    (working copy)
@@ -755,6 +755,10 @@ Recognize built-in functions
 fbuiltin-
 C ObjC C++ ObjC++ Joined

+fcanonical-system-headers
+C ObjC C++ ObjC++
+Where shorter, use canonicalized paths to systems headers.
+
 fcheck-new
 C++ ObjC++ Var(flag_check_new)
 Check the return value of new
Index: gcc/c-family/c-opts.c
===================================================================
--- gcc/c-family/c-opts.c    (revision 191054)
+++ gcc/c-family/c-opts.c    (working copy)
@@ -552,6 +552,10 @@ c_common_handle_option (size_t scode, co
       handle_OPT_d (arg);
       break;

+    case OPT_fcanonical_system_headers:
+      cpp_opts->canonical_system_headers = value;
+      break;
+
     case OPT_fcond_mismatch:
       if (!c_dialect_cxx ())
     {
Index: libcpp/configure
===================================================================
--- libcpp/configure    (revision 191054)
+++ libcpp/configure    (working copy)
@@ -700,6 +700,7 @@ enable_rpath
 with_libiconv_prefix
 enable_maintainer_mode
 enable_checking
+enable_canonical_system_headers
 '
       ac_precious_vars='build_alias
 host_alias
@@ -1333,6 +1334,8 @@ Optional Features:
   --disable-rpath         do not hardcode runtime library paths
   --enable-maintainer-mode enable rules only needed by maintainers
   --enable-checking      enable expensive run-time checks
+  --enable-canonical-system-headers
+                          enable or disable system headers canonicalization

 Optional Packages:
   --with-PACKAGE[=ARG]    use PACKAGE [ARG=yes]
@@ -7094,6 +7097,19 @@ $as_echo "#define ENABLE_CHECKING 1" >>c

 fi

+# Check whether --enable-canonical-system-headers was given.
+if test "${enable_canonical_system_headers+set}" = set; then :
+  enableval=$enable_canonical_system_headers;
+else
+  enable_canonical_system_headers=yes
+fi
+
+if test $enable_canonical_system_headers != no; then
+
+$as_echo "#define ENABLE_CANONICAL_SYSTEM_HEADERS 1" >>confdefs.h
+
+fi
+

 case $target in
     alpha*-*-* | \
Index: libcpp/files.c
===================================================================
--- libcpp/files.c    (revision 191054)
+++ libcpp/files.c    (working copy)
@@ -385,7 +385,7 @@ find_file_in_dir (cpp_reader *pfile, _cp
       void **pp;

       /* We try to canonicalize system headers.  */
-      if (file->dir->sysp)
+      if (CPP_OPTION (pfile, canonical_system_headers) && file->dir->sysp)
     {
       char * canonical_path = maybe_shorter_path (path);
       if (canonical_path)
Index: libcpp/include/cpplib.h
===================================================================
--- libcpp/include/cpplib.h    (revision 191054)
+++ libcpp/include/cpplib.h    (working copy)
@@ -489,6 +489,9 @@ struct cpp_options

   /* True disables tokenization outside of preprocessing directives. */
   bool directives_only;
+
+  /* True enables canonicalization of system header file paths. */
+  bool canonical_system_headers;
 };

 /* Callback for header lookup for HEADER, which is the name of a
Index: libcpp/init.c
===================================================================
--- libcpp/init.c    (revision 191054)
+++ libcpp/init.c    (working copy)
@@ -28,6 +28,10 @@ along with this program; see the file CO
 #include "localedir.h"
 #include "filenames.h"

+#ifndef ENABLE_CANONICAL_SYSTEM_HEADERS
+#define ENABLE_CANONICAL_SYSTEM_HEADERS 0
+#endif
+
 static void init_library (void);
 static void mark_named_operators (cpp_reader *, int);
 static void read_original_filename (cpp_reader *);
@@ -182,6 +186,8 @@ cpp_create_reader (enum c_lang lang, cpp
   CPP_OPTION (pfile, track_macro_expansion) = 2;
   CPP_OPTION (pfile, warn_normalize) = normalized_C;
   CPP_OPTION (pfile, warn_literal_suffix) = 1;
+  CPP_OPTION (pfile, canonical_system_headers)
+      = ENABLE_CANONICAL_SYSTEM_HEADERS;

   /* Default CPP arithmetic to something sensible for the host for the
      benefit of dumb users like fix-header.  */
Index: libcpp/configure.ac
===================================================================
--- libcpp/configure.ac    (revision 191054)
+++ libcpp/configure.ac    (working copy)
@@ -132,6 +132,16 @@ if test $enable_checking != no ; then
 [Define if you want more run-time sanity checks.])
 fi

+AC_ARG_ENABLE(canonical-system-headers,
+[  --enable-canonical-system-headers
+                          enable or disable system headers canonicalization],
+[],
+enable_canonical_system_headers=yes)
+if test $enable_canonical_system_headers != no; then
+  AC_DEFINE(ENABLE_CANONICAL_SYSTEM_HEADERS,
+            1, [Define to enable system headers canonicalization.])
+fi
+
 m4_changequote(,)
 case $target in
     alpha*-*-* | \
Index: libcpp/config.in
===================================================================
--- libcpp/config.in    (revision 191054)
+++ libcpp/config.in    (working copy)
@@ -11,6 +11,9 @@
 /* Define to 1 if using `alloca.c'. */
 #undef C_ALLOCA

+/* Define to enable system headers canonicalization. */
+#undef ENABLE_CANONICAL_SYSTEM_HEADERS
+
 /* Define if you want more run-time sanity checks. */
 #undef ENABLE_CHECKING


--
Google UK Limited | Registered Office: Belgrave House, 76 Buckingham
Palace Road, London SW1W 9TQ | Registered in England Number: 3977902

[simonb, 2012-09-21 10:45:47]

Ping.

http://gcc.gnu.org/ml/gcc-patches/2012-09/msg00459.html

Full text of previous message and context at URL above.  No comments
or code changes since.  Patch description left below for convenience.

>
> Add flags to disable system header canonicalizations.
>
> Libcpp may canonicalize system header paths with lrealpath() for diagnostics,
> dependency output, and similar.  If gcc is held in a symlink farm the
> canonicalized paths may be meaningless to users, and will also conflict with
> build frameworks that (for example) disallow absolute paths to header files.
>
> This change adds -f[no-]canonical-system-headers to the gcc command line, and
> a configure option --[en/dis]able-canonical-system-headers to set default
> behaviour, allowing the user to select whether or not to implement r186991.
> Default is enabled.  See also PR c++/52974.
>
> Tested for regressions with bootstrap builds of C and C++, both with and
> without configure --disable-canonical-system-headers.

--
Google UK Limited | Registered Office: Belgrave House, 76 Buckingham
Palace Road, London SW1W 9TQ | Registered in England Number: 3977902

[simonb, 2012-10-01 14:56:44]

Ping, again.


On 21 September 2012 12:45, Simon Baldwin <simonb@google.com> wrote:
>
> Ping.
>
> http://gcc.gnu.org/ml/gcc-patches/2012-09/msg00459.html
>
> Full text of previous message and context at URL above.  No comments
> or code changes since.  Patch description left below for convenience.
>
> >
> > Add flags to disable system header canonicalizations.
> >
> > Libcpp may canonicalize system header paths with lrealpath() for
diagnostics,
> > dependency output, and similar.  If gcc is held in a symlink farm the
> > canonicalized paths may be meaningless to users, and will also conflict
with
> > build frameworks that (for example) disallow absolute paths to header
files.
> >
> > This change adds -f[no-]canonical-system-headers to the gcc command line,
and
> > a configure option --[en/dis]able-canonical-system-headers to set default
> > behaviour, allowing the user to select whether or not to implement r186991.
> > Default is enabled.  See also PR c++/52974.
> >
> > Tested for regressions with bootstrap builds of C and C++, both with and
> > without configure --disable-canonical-system-headers.
>
> --
> Google UK Limited | Registered Office: Belgrave House, 76 Buckingham
> Palace Road, London SW1W 9TQ | Registered in England Number: 3977902




--
Google UK Limited | Registered Office: Belgrave House, 76 Buckingham
Palace Road, London SW1W 9TQ | Registered in England Number: 3977902

[Ollie Wild, 2012-10-03 01:11:54]

Tom, this is mainly a libcpp change.  Would you mind taking a look?

Thanks,
Ollie

On Mon, Oct 1, 2012 at 9:56 AM, Simon Baldwin <simonb@google.com> wrote:
>
> Ping, again.
>
>
> On 21 September 2012 12:45, Simon Baldwin <simonb@google.com> wrote:
> >
> > Ping.
> >
> > http://gcc.gnu.org/ml/gcc-patches/2012-09/msg00459.html
> >
> > Full text of previous message and context at URL above.  No comments
> > or code changes since.  Patch description left below for convenience.
> >
> > >
> > > Add flags to disable system header canonicalizations.
> > >
> > > Libcpp may canonicalize system header paths with lrealpath() for
> > > diagnostics,
> > > dependency output, and similar.  If gcc is held in a symlink farm the
> > > canonicalized paths may be meaningless to users, and will also
> > > conflict with
> > > build frameworks that (for example) disallow absolute paths to header
> > > files.
> > >
> > > This change adds -f[no-]canonical-system-headers to the gcc command
> > > line, and
> > > a configure option --[en/dis]able-canonical-system-headers to set
> > > default
> > > behaviour, allowing the user to select whether or not to implement
> > > r186991.
> > > Default is enabled.  See also PR c++/52974.
> > >
> > > Tested for regressions with bootstrap builds of C and C++, both with
> > > and
> > > without configure --disable-canonical-system-headers.
> >
> > --
> > Google UK Limited | Registered Office: Belgrave House, 76 Buckingham
> > Palace Road, London SW1W 9TQ | Registered in England Number: 3977902
>
>
>
>
> --
> Google UK Limited | Registered Office: Belgrave House, 76 Buckingham
> Palace Road, London SW1W 9TQ | Registered in England Number: 3977902

[simonb, 2012-10-08 15:46:16]

Ping, again.


On 1 October 2012 16:56, Simon Baldwin <simonb@google.com> wrote:

> Ping, again.
>
>
> On 21 September 2012 12:45, Simon Baldwin <simonb@google.com> wrote:
> >
> > Ping.
> >
> > http://gcc.gnu.org/ml/gcc-patches/2012-09/msg00459.html
> >
> > Full text of previous message and context at URL above.  No comments
> > or code changes since.  Patch description left below for convenience.
> >
> > >
> > > Add flags to disable system header canonicalizations.
> > >
> > > Libcpp may canonicalize system header paths with lrealpath() for
> diagnostics,
> > > dependency output, and similar.  If gcc is held in a symlink farm the
> > > canonicalized paths may be meaningless to users, and will also
> conflict with
> > > build frameworks that (for example) disallow absolute paths to header
> files.
> > >
> > > This change adds -f[no-]canonical-system-headers to the gcc command
> line, and
> > > a configure option --[en/dis]able-canonical-system-headers to set
> default
> > > behaviour, allowing the user to select whether or not to implement
> r186991.
> > > Default is enabled.  See also PR c++/52974.
> > >
> > > Tested for regressions with bootstrap builds of C and C++, both with
> and
> > > without configure --disable-canonical-system-headers.


-- 
Google UK Limited | Registered Office: Belgrave House, 76 Buckingham
Palace Road, London SW1W 9TQ | Registered in England Number: 3977902

[simonb, 2012-10-08 15:50:10]

Ping, again.

On 1 October 2012 16:56, Simon Baldwin <simonb@google.com> wrote:
>
> Ping, again.
>
>
> On 21 September 2012 12:45, Simon Baldwin <simonb@google.com> wrote:
> >
> > Ping.
> >
> > http://gcc.gnu.org/ml/gcc-patches/2012-09/msg00459.html
> >
> > Full text of previous message and context at URL above.  No comments
> > or code changes since.  Patch description left below for convenience.
> >
> > >
> > > Add flags to disable system header canonicalizations.
> > >
> > > Libcpp may canonicalize system header paths with lrealpath() for
> > > diagnostics,
> > > dependency output, and similar.  If gcc is held in a symlink farm the
> > > canonicalized paths may be meaningless to users, and will also
> > > conflict with
> > > build frameworks that (for example) disallow absolute paths to header
> > > files.
> > >
> > > This change adds -f[no-]canonical-system-headers to the gcc command
> > > line, and
> > > a configure option --[en/dis]able-canonical-system-headers to set
> > > default
> > > behaviour, allowing the user to select whether or not to implement
> > > r186991.
> > > Default is enabled.  See also PR c++/52974.
> > >
> > > Tested for regressions with bootstrap builds of C and C++, both with
> > > and
> > > without configure --disable-canonical-system-headers.

--
Google UK Limited | Registered Office: Belgrave House, 76 Buckingham
Palace Road, London SW1W 9TQ | Registered in England Number: 3977902

[simonb, 2012-10-16 08:46:48]

Ping, again.


On 8 October 2012 17:46, Simon Baldwin <simonb@google.com> wrote:
>
> Ping, again.
>
>
> On 1 October 2012 16:56, Simon Baldwin <simonb@google.com> wrote:
>>
>> Ping, again.
>>
>>
>> On 21 September 2012 12:45, Simon Baldwin <simonb@google.com> wrote:
>> >
>> > Ping.
>> >
>> > http://gcc.gnu.org/ml/gcc-patches/2012-09/msg00459.html
>> >
>> > Full text of previous message and context at URL above.  No comments
>> > or code changes since.  Patch description left below for convenience.
>> >
>> > >
>> > > Add flags to disable system header canonicalizations.
>> > >
>> > > Libcpp may canonicalize system header paths with lrealpath() for
diagnostics,
>> > > dependency output, and similar.  If gcc is held in a symlink farm the
>> > > canonicalized paths may be meaningless to users, and will also conflict
with
>> > > build frameworks that (for example) disallow absolute paths to header
files.
>> > >
>> > > This change adds -f[no-]canonical-system-headers to the gcc command line,
and
>> > > a configure option --[en/dis]able-canonical-system-headers to set
default
>> > > behaviour, allowing the user to select whether or not to implement
r186991.
>> > > Default is enabled.  See also PR c++/52974.
>> > >
>> > > Tested for regressions with bootstrap builds of C and C++, both with and
>> > > without configure --disable-canonical-system-headers.

--
Google UK Limited | Registered Office: Belgrave House, 76 Buckingham
Palace Road, London SW1W 9TQ | Registered in England Number: 3977902

[simonb, 2012-10-16 08:50:19]

Ping, again

http://gcc.gnu.org/ml/gcc-patches/2012-09/msg00459.html

--
Google UK Limited | Registered Office: Belgrave House, 76 Buckingham
Palace Road, London SW1W 9TQ | Registered in England Number: 3977902

[stevenb.gcc, 2012-10-16 09:03:55]

On Tue, Oct 16, 2012 at 10:50 AM, Simon Baldwin wrote:
> Ping, again
>
> http://gcc.gnu.org/ml/gcc-patches/2012-09/msg00459.html

Probably you mean the revised patch here:

http://gcc.gnu.org/ml/gcc-patches/2012-09/msg00459.html

The patch look OK to me but I can't approve it.
Technically you're fixing a regression. It'd be helpful if you can
file a PR about it and add the PR number to the ChangeLog entry. That
helps for traceability, and might, eh, motivate release managers to
review your patch :-)

Ciao!
Steven

[tromey_redhat.com, 2012-10-23 19:38:08]

Steven> Probably you mean the revised patch here:
Steven> http://gcc.gnu.org/ml/gcc-patches/2012-09/msg00459.html

Steven> The patch look OK to me but I can't approve it.

I'm sorry about the delay on this.

The libcpp bits are ok.
I can't approve the other parts.

I think new configure options should be documented in install.texi.

Tom

[iant2, 2012-10-23 19:41:30]

On Tue, Oct 23, 2012 at 12:38 PM, Tom Tromey <tromey@redhat.com> wrote:
> Steven> Probably you mean the revised patch here:
> Steven> http://gcc.gnu.org/ml/gcc-patches/2012-09/msg00459.html
>
> Steven> The patch look OK to me but I can't approve it.
>
> I'm sorry about the delay on this.
>
> The libcpp bits are ok.
> I can't approve the other parts.

The rest of the patch is fine.

> I think new configure options should be documented in install.texi.

Looks to me like it is.

Thanks.

Ian