From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 89928 invoked by alias); 14 Apr 2017 11:11:22 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 89756 invoked by uid 89); 14 Apr 2017 11:11:14 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.7 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_NONE,RCVD_IN_SORBS_SPAM,SPF_PASS autolearn=no version=3.3.2 spammy=4.2, 42, imminent X-HELO: mail-yw0-f177.google.com Received: from mail-yw0-f177.google.com (HELO mail-yw0-f177.google.com) (209.85.161.177) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 14 Apr 2017 11:11:12 +0000 Received: by mail-yw0-f177.google.com with SMTP id k13so34729897ywk.1 for ; Fri, 14 Apr 2017 04:11:14 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=tiqljUMD0Ws4NUJDZYfMKUXWpUfFpF7EIfIEf2H72gQ=; b=fB7OTHVWpE3CjQCyabiKhzz/XJKWlr7nVukjGGK3d5TTU27hw9nCob49jiIpAzqPOa utk7J5IZSehuIew/vvT6UIzzuUo3R0zNSjAztB5xpSlGuNV1ZaLywQcLPF/VO4CpQOJc 9XHEzRvjNMZGZqLePwehpMIeG6NYSUucLtvyrVJyfnNMeZRx1vWBGW0g3+a1NWfGUol/ VAHX4VZzcpsGMPut8RfM6ayqUIODYtECh9XYI5/1W7tH6C3haRVM8BsKW2mBM1vau3Xz 67+I5W6so0dnQhn8PIyKev28E76SEyjjo8HFXs1Gl/loPfpeTR5Dl3IXdCBZ73sS/xiE f8/g== X-Gm-Message-State: AN3rC/7dAjLzvyccDQZwjzVfaAdAweEueQyqK6kpUQONadUOSK3klzDL nwlRvue2cYU8cjchPghp4uVNxc3GiRbe X-Received: by 10.129.102.130 with SMTP id a124mr5571969ywc.266.1492168272524; Fri, 14 Apr 2017 04:11:12 -0700 (PDT) MIME-Version: 1.0 Received: by 10.37.42.19 with HTTP; Fri, 14 Apr 2017 04:11:12 -0700 (PDT) In-Reply-To: References: <4b114aec-1ab7-7054-1086-a06d3b358a27@gnu.org> From: Yvan Roux Date: Fri, 14 Apr 2017 11:20:00 -0000 Message-ID: Subject: Re: [PATCH] Fix fixincludes for canadian cross builds To: Bernd Edlinger Cc: Bruce Korb , "gcc-patches@gcc.gnu.org" , Richard Biener , Jakub Jelinek , Jeff Law Content-Type: text/plain; charset=UTF-8 X-IsSubscribed: yes X-SW-Source: 2017-04/txt/msg00660.txt.bz2 On 14 April 2017 at 12:29, Bernd Edlinger wrote: > Hi Yvan, > > On 04/14/17 10:24, Yvan Roux wrote: >> Hi Bernd, >> >> On 14 April 2017 at 06:18, Bernd Edlinger wrote: >>> On 04/12/17 17:58, Yvan Roux wrote: >>>> Hi, >>>> >>>> On 20 February 2017 at 18:53, Bruce Korb wrote: >>>>> On 02/18/17 01:01, Bernd Edlinger wrote: >>>>>> On 02/18/17 00:37, Bruce Korb wrote: >>>>>>> On 02/06/17 10:44, Bernd Edlinger wrote: >>>>>>>> I tested this change with different arm-linux-gnueabihf cross >>>>>>>> compilers, and verified that mkheaders still works on the host system. >>>>>>>> >>>>>>>> Bootstrapped and reg-tested on x86_64-pc-linux-gnu. >>>>>>>> Is it OK for trunk? >>>>>>> >>>>>>> As long as you certify that this is correct for all systems we care about: >>>>>>> >>>>>>> +BUILD_SYSTEM_HEADER_DIR = ` >>>>>>> + echo $(CROSS_SYSTEM_HEADER_DIR) | \ >>>>>>> + sed -e :a -e 's,[^/]*/\.\.\/,,' -e ta` >>>>>>> >>>>>>> that is pretty obtuse sed-speak to me. I suggest a comment >>>>>>> explaining what sed is supposed to be doing. What should >>>>>>> "$(CROSS_SYSTEM_HEADER_DIR)" look like? >>>>>>> >>>>>> >>>>>> I took it just from a few lines above, so I thought that comment would >>>>>> sufficiently explain the syntax: >>>>> >>>>> I confess, I didn't pull a new copy of gcc, sorry. >>>>> So it looks good to me. >>>> >>>> >>>> We just noticed that this patch brakes canadian cross builds when >>>> configured with --with-build-sysroot, since headers are searched into >>>> the target sysroot instead of the one specified for builds. >>>> >>>> Maybe there's a cleaner way to fix this and avoid the duplication but >>>> I didn't find another to test if --with-build-sysroot is used. The >>>> attached patch fixes the issue. Tested with a Full canadian cross >>>> build for i686-w64-mingw32 host and arm-linux-gnueabihf. >>>> >>>> Thanks >>>> Yvan >>>> >>>> 2017-04-12 Yvan Roux >>>> >>>> * Makefile.in (BUILD_SYSTEM_HEADER_DIR): Set to SYSTEM_HEADER_DIR >>>> when configured with --with-build-sysroot. >>>> >>> >>> Oops, sorry for the breakage... >>> >>> However I think the patch simply restores the previous behavior, because >>> ifdef SYSROOT_CFLAGS_FOR_TARGET is always true, even if it's empty, >>> but it does still not work correctly. >> >> hmm, according to Makefile manual ifdef VAR is true if VAR is >> non-empty, but maybe SYSROOT_CFLAGS_FOR_TARGET is set to an empty >> string when --with-build-sysroot is not used, sorry for not having >> tested this case :/ >> > > Yes, interesting point, the reason must have been somewhere else then. > Actually the "else ifdef" syntax appears to be working to my surprise: > I use gnu make 3.81 here. But the manual says "else" has to be on a > line of it's own, and I have never seen that syntax being used before. Oh, maybe the syntax was changed, I use gnu make 4.2 and the syntax for complex conditional in the manual (https://www.gnu.org/software/make/manual/make.html#Conditional-Syntax) contains: conditional-directive-one text-if-one-is-true else conditional-directive-two ... I made more tests, and when configured without --with-build-sysroot, SYSROOT_CFLAGS_FOR_TARGET really is empty in gcc/Makefile, so I don't understand why my patch didn't work... But anyway, your version is better than mine ;) >>> I tried to build a cross with your patch and a --with-build-sysroot >>> but the target compiler does fix the wrong includes for me: >>> >>> ../gcc-trunk/configure --prefix=/home/ed/gnu/arm-linux-gnueabihf-cross >>> --host=arm-linux-gnueabihf --target=arm-linux-gnueabihf >>> --enable-languages=c,c++,ada,fortran --with-arch=armv7-a >>> --with-tune=cortex-a9 --with-fpu=vfpv3-d16 --with-float=hard >>> --with-build-sysroot=/home/ed/gnu/arm-linux-gnueabihf-3 >>> => >>> Fixing headers into >>> /home/ed/gnu/gcc-build-arm-linux-gnueabihf-2/gcc/include-fixed for >>> arm-unknown-linux-gnueabihf target >>> Forbidden identifiers: linux unix >>> Finding directories and links to directories >>> Searching /usr/include/. >>> Searching /usr/include/./c++/4.8.4 >>> Searching /usr/include/./numpy >>> Searching /usr/include/./python2.7/numpy >>> Making symbolic directory links >>> Fixing directory /usr/include into >>> /home/ed/gnu/gcc-build-arm-linux-gnueabihf-2/gcc/include-fixed >>> >>> but it should fix headers in .../arm-linux-gnueabihf-3/usr/include >>> >>> I think it would work if I use --with-sysroot together with >>> --with-build-sysroot in the config above, but why should the >>> target compiler use --with-sysroot ? >>> There is no need for that on the target, just the cross-compiler >>> might need to support that. >> >> Ah yes, sorry ! we use both together, my understanding was that >> --with-build-sysroot only makes sense we --with-sysroot is used. >> > > No problem, there are certainly many situations when that is true. > >>> I tried to fix some possible combinations of --with-sysroot/ >>> --with-build-sysroot, and moved the logic to the configure >>> script. When I did that I noticed also some more glitches >>> when grabbing the TARGET_GLIBC_MAJOR/MINOR defines from sysroot >>> files. >>> >>> This updated patch seems to work for me, could you give it a try? >> >> The patch looks good to me, and works fine. >> >> Thanks a lot Bernd. >> >> Cheers, >> Yvan >> > > Hi RMs: > > I am sorry that this happened so close to the imminent gcc-7 release > date. And sorry for not having caught it sooner. > To my best knowledge it would be fine to apply this update patch on the > trunk: https://gcc.gnu.org/ml/gcc-patches/2017-04/msg00649.html > > But if you decide otherwise, I am also ready to revert my original > commit: https://gcc.gnu.org/viewcvs/gcc?view=revision&revision=245613 > > > Thanks > Bernd.