On 5 September 2012 16:03, Ian Lance Taylor <i...@google.com> wrote:
> On Wed, Sep 5, 2012 at 6:56 AM, Simon Baldwin <sim...@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 <sim...@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 <sim...@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
