[trunk] RFS: translate built-in include paths for sysroot (issue 5394041)

[shenhan@google.com]

Hi, Joseph, get a chance to take a look? Thanks!
-Han

On 2011/11/19 00:10:20, shenhan wrote:
> Hi, Joseph, thanks!

> ChangeLog entries added to the issue description.

> ChangeLog
>          * Makefile.in (GPLUSPLUS_INCLUDE_DIR_ADD_SYSROOT): add a macro
>          definition to compile command.
>          * cppdefault.c (GPLUSPLUS_INCLUDE_DIR_ADD_SYSROOT): replace
hard
>          coded "add_sysroot" field with the control macro.
>          * configure.ac (gcc_gxx_include_dir_add_sysroot): add a flag
>          variable to control whether sysroot should be prepended to gxx
>          include dir.
>          * configure : Regenerate.

> Yeah, I should include the URL for the previous discussion in the
mail. (I
> previously put it in the issue description together with the
rationale.)

> This is a follow up for issue - http://codereview.appspot.com/4641076.
> The rationale for this patch is also copied here:
> ==========

> The setup:

> Configuring a toolchain targeting x86-64 GNU Linux (Ubuntu Lucid), as
a
> cross-compiler.  Using a sysroot to provide the Lucid
headers+libraries,
> with the sysroot path being within the GCC install tree.  Want to use
the
> Lucid system libstdc++ and headers, which means that I'm not
> building/installing libstdc++-v3.

> So, configuring with:
>    --with-sysroot="$SYSROOT"
>    --disable-libstdc++-v3 \
>    --with-gxx-include-dir="$SYSROOT/usr/include/c++/4.4" \
> (among other options).

> Hoping to support two usage models with this configuration, w.r.t. use
of
> the sysroot:

> (1) somebody installs the sysroot in the normal location relative to
the
> GCC install, and relocates the whole bundle (sysroot+GCC).  This works
> great AFAICT, GCC finds its includes (including the C++ includes)
thanks
> to the add_standard_paths iprefix handling.

> (2) somebody installs the sysroot in a non-standard location, and uses
> --sysroot to try to access it.  This works fine for the C headers, but
> doesn't work.

> For the C headers, add_standard_paths prepends the sysroot location to
> the /usr/include path (since that's what's specified in cppdefault.c
for
> that path).  It doesn't do the same for the C++ include path, though
> (again, as specified in cppdefault.c).

> add_standard_paths doesn't attempt to relocate built-in include paths
that
> start with the compiled-in sysroot location (e.g., the g++ include
dir, in
> this case).  This isn't surprising really: normally you either prepend
the
> sysroot location or you don't (as specified by cppdefault.c); none of
the
> built-in paths normally *start* with the sysroot location and need to
be
> relocated.  However, in this odd-ball case of trying to use the C++
headers
> from the sysroot, one of the paths *does* need to be relocated in this
way.
> =========


> Chris(cgd@) provided a patch for the issue, but there was a different
opinion,
> my patch here is just coded as suggested.

> Tested before/after on a x86_64-unknown-linux-gnu system, with
> --enable-languages=all,
RUNTESTFLAGS='--target_board=unix\{-m32,-m64\}'.
> No changes in test results.

> -Han


> On Fri, Nov 18, 2011 at 8:54 AM, Joseph S. Myers
<joseph@codesourcery.com>wrote:

> > On Tue, 15 Nov 2011, Han Shen wrote:
> >
> > > 2011-11-15   Han Shen  <mailto:shenhan@google.com>
> > >
> > >       * gcc/Makefile.in:
> > >       * gcc/configure:
> > >       * gcc/cppdefault.c:
> >
> > You need to include the full ChangeLog entries with your patch.
Note that
> > paths in ChangeLogs are relative to the directory with the
ChangeLog, so
> > no gcc/ in this case.
> >
> > Please also include the full rationale with your patch *and URLs for
the
> > previous discussion* (from June, it seems) so that reviewers don't
need to
> > track that down themselves.
> >
> > > diff --git a/gcc/configure b/gcc/configure
> > > index 99334ce..364d8c2 100755
> > > --- a/gcc/configure
> > > +++ b/gcc/configure
> >
> > It's not possible to review this patch as is because you've only
included
> > the changes to the generated file configure, not to its source file
> > configure.ac.  Please make sure that your resubmission includes all
the
> > source file changes.  (There is no need to include the changes to
the
> > generated file configure at all in the submission; the ChangeLog
entry can
> > just mention it as "* configure: Regenerate.".)
> >
> > --
> > Joseph S. Myers
> > mailto:joseph@codesourcery.com
> >



> --




http://codereview.appspot.com/5394041/