Patchwork [trunk] RFS: translate built-in include paths for sysroot (issue5394041)

login
register
mail settings
Submitter Han Shen
Date Dec. 6, 2011, 7:23 p.m.
Message ID <CACkGtrgCCyynKzTGJg+JeWr+FMTsub=s_Edftkn0L_PPfHUCkw@mail.gmail.com>
Download mbox | patch
Permalink /patch/129800/
State New
Headers show

Comments

Han Shen - Dec. 6, 2011, 7:23 p.m.
Hi, Joseph, thanks!

Yeah, I see where the problem is - I posted all these
(patchset,Changelog, rationale and previous gcc-patches discussion.)
on http://codereview.appspot.com/5394041

So in addition to that, I include them all here
==================================
This is a follow up for issue "http://codereview.appspot.com/4641076".

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

ChangeLog
       * Makefile.in (gcc_gxx_include_dir_add_sysroot): New.
         (PREPROCESSOR_DEFINES): Define GPLUSPLUS_INCLUDE_DIR_ADD_SYSROOT.

       * cppdefault.c (cpp_include_defaults): replace hard coded "1" with
         GPLUSPLUS_INCLUDE_DIR_ADD_SYSROOT for "add_sysroot" field.

       * configure.ac (AC_SUBST): Add gcc_gxx_include_dir_add_sysroot to
         control whether sysroot should be prepended to gxx include dir.

       * configure: Regenerate.

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.

Patch: attached.

Regards,
-Han


On Mon, Dec 5, 2011 at 1:33 PM, Joseph S. Myers <joseph@codesourcery.com> wrote:
> On Fri, 18 Nov 2011, Han Shen 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.
>
> Please make sure to follow the style of existing ChangeLog entries.  For
> Makefile.in for example it might be:
>
>        * Makefile.in (gcc_gxx_include_dir_add_sysroot): New.
>        (PREPROCESSOR_DEFINES): Define GPLUSPLUS_INCLUDE_DIR_ADD_SYSROOT.
>
> I went to review the change as I had it queued to look at, but found that
> this message, which I had noted as the message to look at, did not
> actually contain a patch - and I cannot find any posting of the
> configure.ac changes in any of your messages to gcc-patches since at least
> the start of November.  So it's still not ready to review.  Please post a
> complete, self-contained patch submission, that includes all of:
>
> * the patch itself, including configure.ac changes;
> * the ChangeLog entries;
> * self-contained rationale;
> * URLs to previous gcc-patches discussion
>
> in the one gcc-patches message.  If you have in fact already posted such a
> submission and I've failed to locate it, could you give the gcc-patches
> URL?
>
> --
> Joseph S. Myers
> joseph@codesourcery.com
Joseph S. Myers - Dec. 20, 2011, 10:44 p.m.
On Tue, 6 Dec 2011, Han Shen wrote:

> +gcc_gxx_include_dir_add_sysroot=0
> +if test "${with_sysroot+set}" = set; then :
> +  gcc_gxx_without_sysroot=`expr "${gcc_gxx_include_dir}" : "${with_sysroot}"'\(.*\)'`
> +  if test "${gcc_gxx_without_sysroot}"; then :

No need for the " :" after "then" in two places here (the only reason I 
could see for having it would be if the contents of the "if" were the 
expansion of an autoconf macro that might expand to empty, so you needed 
to make sure the contents were nonempty).

The patch is OK with that fixed.

Patch

Index: gcc/Makefile.in
diff --git a/gcc/Makefile.in b/gcc/Makefile.in
index ae4f4da..0a05783 100644
--- a/gcc/Makefile.in
+++ b/gcc/Makefile.in
@@ -615,6 +615,7 @@  gcc_tooldir = @gcc_tooldir@
 build_tooldir = $(exec_prefix)/$(target_noncanonical)
 # Directory in which the compiler finds target-independent g++ includes.
 gcc_gxx_include_dir = @gcc_gxx_include_dir@
+gcc_gxx_include_dir_add_sysroot = @gcc_gxx_include_dir_add_sysroot@
 # Directory to search for site-specific includes.
 local_includedir = $(local_prefix)/include
 includedir = $(prefix)/include
@@ -3979,6 +3980,7 @@  PREPROCESSOR_DEFINES = \
   -DGCC_INCLUDE_DIR=\"$(libsubdir)/include\" \
   -DFIXED_INCLUDE_DIR=\"$(libsubdir)/include-fixed\" \
   -DGPLUSPLUS_INCLUDE_DIR=\"$(gcc_gxx_include_dir)\" \
+  -DGPLUSPLUS_INCLUDE_DIR_ADD_SYSROOT=$(gcc_gxx_include_dir_add_sysroot) \
   -DGPLUSPLUS_TOOL_INCLUDE_DIR=\"$(gcc_gxx_include_dir)/$(target_noncanonical)\" \
   -DGPLUSPLUS_BACKWARD_INCLUDE_DIR=\"$(gcc_gxx_include_dir)/backward\" \
   -DLOCAL_INCLUDE_DIR=\"$(local_includedir)\" \
Index: gcc/configure.ac
diff --git a/gcc/configure.ac b/gcc/configure.ac
index 12492ff..46a7d13 100644
--- a/gcc/configure.ac
+++ b/gcc/configure.ac
@@ -145,6 +145,15 @@  if test x${gcc_gxx_include_dir} = x; then
   fi
 fi
 
+gcc_gxx_include_dir_add_sysroot=0
+if test "${with_sysroot+set}" = set; then :
+  gcc_gxx_without_sysroot=`expr "${gcc_gxx_include_dir}" : "${with_sysroot}"'\(.*\)'`
+  if test "${gcc_gxx_without_sysroot}"; then :
+    gcc_gxx_include_dir="${gcc_gxx_without_sysroot}"
+    gcc_gxx_include_dir_add_sysroot=1
+  fi
+fi
+
 AC_ARG_WITH(cpp_install_dir,
 [AC_HELP_STRING([--with-cpp-install-dir=DIR],
                 [install the user visible C preprocessor in DIR
@@ -4927,6 +4936,7 @@  AC_SUBST(extra_programs)
 AC_SUBST(float_h_file)
 AC_SUBST(gcc_config_arguments)
 AC_SUBST(gcc_gxx_include_dir)
+AC_SUBST(gcc_gxx_include_dir_add_sysroot)
 AC_SUBST(host_exeext)
 AC_SUBST(host_xm_file_list)
 AC_SUBST(host_xm_include_list)
Index: gcc/cppdefault.c
diff --git a/gcc/cppdefault.c b/gcc/cppdefault.c
index 099899a..927083e 100644
--- a/gcc/cppdefault.c
+++ b/gcc/cppdefault.c
@@ -44,15 +44,18 @@  const struct default_include cpp_include_defaults[]
 = {
 #ifdef GPLUSPLUS_INCLUDE_DIR
     /* Pick up GNU C++ generic include files.  */
-    { GPLUSPLUS_INCLUDE_DIR, "G++", 1, 1, 0, 0 },
+    { GPLUSPLUS_INCLUDE_DIR, "G++", 1, 1,
+      GPLUSPLUS_INCLUDE_DIR_ADD_SYSROOT, 0 },
 #endif
 #ifdef GPLUSPLUS_TOOL_INCLUDE_DIR
     /* Pick up GNU C++ target-dependent include files.  */
-    { GPLUSPLUS_TOOL_INCLUDE_DIR, "G++", 1, 1, 0, 1 },
+    { GPLUSPLUS_TOOL_INCLUDE_DIR, "G++", 1, 1,
+      GPLUSPLUS_INCLUDE_DIR_ADD_SYSROOT, 1 },
 #endif
 #ifdef GPLUSPLUS_BACKWARD_INCLUDE_DIR
     /* Pick up GNU C++ backward and deprecated include files.  */
-    { GPLUSPLUS_BACKWARD_INCLUDE_DIR, "G++", 1, 1, 0, 0 },
+    { GPLUSPLUS_BACKWARD_INCLUDE_DIR, "G++", 1, 1,
+      GPLUSPLUS_INCLUDE_DIR_ADD_SYSROOT, 0 },
 #endif
 #ifdef GCC_INCLUDE_DIR
     /* This is the dir for gcc's private headers.  */