Can I have plain-text mode, please, gmail? ---------- Forwarded message ---------- From: Tadek Kijkowski <tkijkow...@gmail.com> Date: 2016-09-30 5:16 GMT+02:00 Subject: Re: [PATCH] gcc: Fix sysroot relative paths for MinGW To: Jeff Law <l...@redhat.com> CC: gcc-patches@gcc.gnu.org
2016-09-29 21:16 GMT+02:00 Jeff Law <l...@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 <tkijkow...@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
