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

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

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.