From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 102817 invoked by alias); 17 Jan 2020 11:54:30 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 102806 invoked by uid 89); 17 Jan 2020 11:54:30 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-5.5 required=5.0 tests=AWL,BAYES_00,SPF_HELO_PASS,SPF_PASS autolearn=ham version=3.3.1 spammy=indirections X-HELO: simark.ca Received: from simark.ca (HELO simark.ca) (158.69.221.121) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 17 Jan 2020 11:54:19 +0000 Received: from [10.0.0.11] (unknown [192.222.164.54]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id 0AA9F1E47D; Fri, 17 Jan 2020 06:54:18 -0500 (EST) Subject: Re: [PATCH v2 0/6] Move gdbsupport to top level To: Pedro Alves , Tom Tromey , gdb-patches@sourceware.org References: <20200109005807.7314-1-tom@tromey.com> <8a8de6a9-37b8-cad3-c818-be903037fe48@redhat.com> <437c1b86-0aa8-57b9-53e2-f21567e2bb14@redhat.com> <87c733a2-2b25-a954-88a1-9bfb1a7eca12@redhat.com> <4fed38dc-aaa7-b76b-880f-bab0b1b5add2@redhat.com> <053b43db-381c-a72d-ea5d-9bcb74a8440c@simark.ca> <78d5599b-9773-954f-ea06-86f65a1c6842@redhat.com> From: Simon Marchi Message-ID: Date: Fri, 17 Jan 2020 12:20:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.3.1 MIME-Version: 1.0 In-Reply-To: <78d5599b-9773-954f-ea06-86f65a1c6842@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-SW-Source: 2020-01/txt/msg00514.txt.bz2 On 2020-01-16 9:29 a.m., Pedro Alves wrote: > On 1/16/20 4:23 AM, Simon Marchi wrote: >> On 2020-01-15 4:35 p.m., Pedro Alves wrote: >>> Here's an improved version, which fixes gdbserver's standalone >>> build, simplifies gdbsupport's config.h (there's no need for >>> #ifdef GDBSERVER stuff since gdbserver doesn't use gdbsupport >>> as a library yet), and adds copyright/intro comments. >> >> I have to admit I'm a bit lost in the spaghetti of config.h inclusion. I don't >> understand, for example, why GDB needs to include gdbsupport's config. All the >> features that GDB needs are checked by its own configure script, aren't they? > > Good question. I guess the idea is that instead of having all of gdb, gdbserver, > and gdbsupport share tests via gdbsupport/common.m4, having run them more > than once, we can run the tests only once by gdbsupport's configure, and then > include the support-config.h file. Ok, but that's not what is done today, that's a possibility for the future, right? common.m4 defines a macro used by all three configure scripts, so each configure script ends up doing its own checks. > > I've given this another thought, and went through the process of thinking > that this more complicated patch isn't really necessary, and then concluded > that having it makes things more normalized between all the dirs. But > we can simplify it -- don't need to generate the "trampoline" config.h. > > My initial thought that led to the more complicated patch was to make > sure that that "#include " in gnulib's libc-config.h picks up > gnulib config.h. But it's really harmless if that picks some other config.h > instead as long as we're sure that the gnulib config.h is also included > somehow. Which we are sure of, from the fact that common-defs.h > is always included as first thing in every .c file. > > So while we're building gdbsupport, we don't have any config.h in the > include path other than the BFD one, and we hit the problem. We could > add an empty gdbsupport/config/config.h file in the > source and add -Igdbsupport/config/ to gdbsupport's Makefile, or, > add gnulib's build dir to the include path, like in my original patch. > > So the original solution of adding the gnulib dir to the include path > isn't that bad, though I call it a hack. I would have to see it, but it sounds like the simplest solution. > The gdbsupport/config/config.h approach is pedantically less of a hack > and more in line with the "solution" that we happen to have in gdb > and gdbserver's dirs currently, by chance. > > OK, so I tried that. And then, I realized, well, if we have that > new config.h file, then we could as well move the gnulib config.h inclusion > to that file, like I was doing in the complicated patch. Essentially > it's the same as the complicated patch, except the "trampoline" config.h > isn't generated in the build dir, but instead is put in the source tree. > We'd do the same to gdb and gdbserver, for consistency. > > (I actually named the new dirs "config.h", as there's already a config > dir under gdb. It's more to the point, anyway.) I think it's a bit confusing to have directories named "config.h", but not really a big deal. >> The gdb-config.h above should be gdbserver-config.h. > Here's v2. WDYT? Which of all the approaches discussed would > you prefer? Probably the solution that involves the least files and indirections. But the one proposed in the latest patch is fine with me too. It's set and forget, once it builds again we won't think about it, at least until the next time it breaks. The buildbot is struggling with this (sending many breakage emails), so I think you should choose one and push it. Then we can cancel all the builds on the master branch until that commit. Simon