Fwd: [PATCH] gcc: Fix sysroot relative paths for MinGW

[Tadek Kijkowski <tkijkowski at gmail dot com>]

Can I have plain-text mode, please, gmail?

---------- Forwarded message ----------
From: Tadek Kijkowski <tkijkowski@gmail.com>
Date: 2016-09-30 5:16 GMT+02:00
Subject: Re: [PATCH] gcc: Fix sysroot relative paths for MinGW
To: Jeff Law <law@redhat.com>
CC: gcc-patches@gcc.gnu.org


2016-09-29 21:16 GMT+02:00 Jeff Law <law@redhat.com>:
> On 09/23/2016 12:31 AM, Tadek Kijkowski wrote:
>>
>> Prevent paths relative to sysroot directory from being transformed to
>> Windows form with MSYS prefix.
>> See: http://www.mingw.org/wiki/Posix_path_conversion
>>
>> 2016-09-23  Tadek Kijkowski  <tkijkowski@gmail.com>
>>
>>         * gcc/Makefile.in: Fix sysroot relative paths for MinGW
>>
>>
>> Index: gcc/Makefile.in
>> ===================================================================
>> --- gcc/Makefile.in    (revision 240386)
>> +++ gcc/Makefile.in    (working copy)
>> @@ -603,6 +603,18 @@
>>  # UNSORTED
>>  # --------
>>
>> +# MSYS will zealously translate all paths to Windows form,
>> +# so /usr/include becomes c:/msysX/usr/include.
>> +# If sysroot is specified this is undesirable, so this function converts
>> +# /usr/include to //usr\include, which will become /usr/include
>> +# again when passed to gcc.
>> +ifneq ($(and @TARGET_SYSTEM_ROOT@,$(filter %-mingw32,$(host))),)
>> +sysroot_relative_path = $(if $(2),$$(echo '$(1)' | tr '/' '\\' | sed
>> 's,^\\,//,'),$(1))
>> +else
>> +sysroot_relative_path = $(1)
>> +endif
>
> I'd really like to see the documentation here improved.
>
> There's no mention of the second argument's purpose.  And one has to parse
> the ifneq line to understand what it's doing.  Those kinds of things should
> be made clear in a comment.
>

OK. I'll expand it.

>
>> +
>> +
>>  # Directory in which the compiler finds libraries etc.
>>  libsubdir =
>> $(libdir)/gcc/$(real_target_noncanonical)/$(version)$(accel_dir_suffix)
>>  # Directory in which the compiler finds executables
>> @@ -2751,14 +2763,14 @@
>>  PREPROCESSOR_DEFINES = \
>>    -DGCC_INCLUDE_DIR=\"$(libsubdir)/include\" \
>>    -DFIXED_INCLUDE_DIR=\"$(libsubdir)/include-fixed\" \
>> -  -DGPLUSPLUS_INCLUDE_DIR=\"$(gcc_gxx_include_dir)\" \
>> +  -DGPLUSPLUS_INCLUDE_DIR=\"$(call
>> sysroot_relative_path,$(gcc_gxx_include_dir),$(filter-out
>> 0,$(gcc_gxx_include_dir_add_sysroot)))\" \
>
> So why the $(filter-out 0, ....)?
>
> I'd really like to avoid being too clever here and write this code in the
> most straightforward way possible.
>

Hmm... that's partially leftover from the abandoned idea to pass
@TARGET_SYSTEM_ROOT@ as second parameter of sysroot_relative_path.
Sysroot is prepended to GPLUSPLUS_INCLUDE_DIR in runtime only if
$(gcc_gxx_include_dir) is 1.
Since sysroot_relative_path checks for non-empty string the easiest
way was to use filter-out. But I agree this way it's confusing.

How about if I change the sysroot_relative_path function to explicitly
check for 1? But still - since $(if) checks for empty string, it will
have to use filter or filter-out.

# MSYS will zealously translate all paths to Windows form, so
/usr/include becomes c:/msysX/usr/include.
# If sysroot is specified this is undesirable, so this function
converts /usr/include to //usr\include, which will become /usr/include
# again when passed to gcc.
# This function takes two parameters: first parameter is include
directory path, second parameter tells if the path is relative
# to TARGET_SYSTEM_ROOT.
# If TARGET_SYSTEM_ROOT is not configured or host is not MinGW32, this
function always expands to the unmodified first parameter
# if TARGET_SYSTEM_ROOT is configured and host is MinGW32, but second
parameter is not 1, this function again expands to the unmodified
first parameter
# otherwise, it expands to a shell expression which will transform the
first parameter as described above.
ifneq ($(and @TARGET_SYSTEM_ROOT@,$(filter %-mingw32,$(host))),)
sysroot_relative_path = $(if $(filter 1,$(2)),$$(echo '$(1)' | tr '/'
'\\' | sed 's,^\\,//,'),$(1))
else
sysroot_relative_path = $(1)
endif

...

PREPROCESSOR_DEFINES = \
  -DGCC_INCLUDE_DIR=\"$(libsubdir)/include\" \
  -DFIXED_INCLUDE_DIR=\"$(libsubdir)/include-fixed\" \
  -DGPLUSPLUS_INCLUDE_DIR=\"$(call
sysroot_relative_path,$(gcc_gxx_include_dir),$(gcc_gxx_include_dir_add_sysroot))\"
\
...
  -DNATIVE_SYSTEM_HEADER_DIR=\"$(call
sysroot_relative_path,$(NATIVE_SYSTEM_HEADER_DIR),1)\" \
...

What do you think?

N.B. I'd prefer to use backticks over "$()", but it could clash if
some include paths already contain backtick expressions.

Tadek