| Message ID | 20120905135632.AA650A0D91@xoom.chi.corp.google.com |
|---|---|
| State | New |
| Headers | show |
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
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
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
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