From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:51415) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eye1F-0001dx-9q for qemu-devel@nongnu.org; Wed, 21 Mar 2018 09:43:12 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1eye1E-0002eZ-3p for qemu-devel@nongnu.org; Wed, 21 Mar 2018 09:43:09 -0400 Date: Wed, 21 Mar 2018 15:42:22 +0200 From: "Michael S. Tsirkin" Message-ID: <20180321153510-mutt-send-email-mst@kernel.org> References: <1521510562-529051-1-git-send-email-mst@redhat.com> <8f06e1e3-4823-d8e8-43bd-c092c523849e@vivier.eu> <20180320135548-mutt-send-email-mst@kernel.org> <459fa95d-8f0a-22fa-80fb-4af22ccf49b3@redhat.com> <20180321150543-mutt-send-email-mst@kernel.org> <20180321132953.GK8551@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: <20180321132953.GK8551@redhat.com> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [Qemu-ppc] [PATCH] qemu: include generated files with <> and not "" List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Daniel =?iso-8859-1?Q?P=2E_Berrang=E9?= Cc: Thomas Huth , Laurent Vivier , Peter Maydell , Dmitry Fleytman , sheepdog@lists.wpkg.org, Ronnie Sahlberg , Li Zhijian , David Hildenbrand , Jeff Cody , Zhang Chen , qemu-devel@nongnu.org, BALATON Zoltan , Keith Busch , Max Filippov , Gerd Hoffmann , Jiri Pirko , Subbaraya Sundeep , Eric Blake , Michael Roth , Marcelo Tosatti , Josh Durgin , Stefano Stabellini , Alberto Garcia , zhanghailiang , Ben Warren , Marcel Apfelbaum , Yongbok Kim , Markus Armbruster , Stefan Berger , Christian Borntraeger , kvm@vger.kernel.org, =?iso-8859-1?Q?Herv=E9?= Poussineau , Shannon Zhao , Anthony Perard , Liu Yuan , David Gibson , Andrzej Zaborowski , Jason Wang , Artyom Tarasenko , Riku Voipio , Fam Zheng , Eduardo Habkost , Corey Minyard , Amit Shah , Pavel Dovgalyuk , Stefan Weil , Xie Changlong , Alistair Francis , Peter Lieven , "Dr. David Alan Gilbert" , Greg Kurz , =?iso-8859-1?Q?Marc-Andr=E9?= Lureau , Alex Williamson , qemu-arm@nongnu.org, Peter Chubb , Yuval Shaia , Stefan Hajnoczi , Paolo Bonzini , xen-devel@lists.xenproject.org, John Snow , Richard Henderson , Kevin Wolf , qemu-block@nongnu.org, Peter Crosthwaite , Hitoshi Mitake , Wen Congyang , qemu-s390x@nongnu.org, Cornelia Huck , "Richard W.M. Jones" , Juan Quintela , Max Reitz , Michael Walle , qemu-ppc@nongnu.org, Andreas =?iso-8859-1?Q?F=E4rber?= , Igor Mammedov , Hannes Reinecke , Philippe =?iso-8859-1?Q?Mathieu-Daud=E9?= On Wed, Mar 21, 2018 at 01:29:53PM +0000, Daniel P. Berrang=E9 wrote: > On Wed, Mar 21, 2018 at 03:08:36PM +0200, Michael S. Tsirkin wrote: > > On Wed, Mar 21, 2018 at 08:16:00AM +0100, Thomas Huth wrote: > > > On 20.03.2018 13:05, Michael S. Tsirkin wrote: > > > > On Tue, Mar 20, 2018 at 09:58:23AM +0100, Laurent Vivier wrote: > > > >> Le 20/03/2018 =E0 02:54, Michael S. Tsirkin a =E9crit=A0: > > > >>> QEMU coding style at the moment asks for all non-system > > > >>> include files to be used with #include "foo.h". > > > >>> However this rule actually does not make sense and > > > >>> creates issues for when the included file is generated. > > > >> > > > >> If you change that, we can have issue when a system include has = the same > > > >> name as our local include. With "", system header are take= n first. > > > >=20 > > > > Are you sure? I just tested and that is not the case with > > > > either gcc or clang. > > > >=20 > > > >>> In C, include "file" means look in current directory, > > > >>> then on include search path. Current directory here > > > >>> means the source file directory. > > > >>> By comparison include means look on include search path. > > > >> > > > >> Not exactly, there is the notion of "system header" too. > > > >> > > > >> https://gcc.gnu.org/onlinedocs/cpp/Include-Syntax.html > > > >> > > > >> #include > > > >> This variant is used for system header files. It searches for a = file > > > >> named file in a standard list of system directories. You can pre= pend > > > >> directories to this list with the -I option (see Invocation). > > > >=20 > > > > This is exactly what we do. > > > >=20 > > > >> #include "file" > > > >> This variant is used for header files of your own program. It se= arches > > > >> for a file named file first in the directory containing the curr= ent > > > >> file, then in the quote directories and then the same directorie= s used > > > >> for . You can prepend directories to the list of quote dir= ectories > > > >> with the -iquote option. > > > >=20 > > > > Since we do not use -iquote, "" just adds the current directory. > > >=20 > > > So why don't we simply switch to use -iquote instead of -I for addi= ng > > > search paths for our own headers? We then would get a clean separat= ion > > > of QEMU headers from system headers. > > >=20 > > > Thomas > >=20 > > It still leaves us with a host of problems e.g. the problem of stale > > headers in the source directory. >=20 > We have a patch on list which effectively solves the problem of stale > generated files in source directory, so that's largely a non-issue at > this point IMHO. >=20 > Regards, > Daniel That was just one, and the solution is just to fail build. I think we can strive to address at least some of the following: - make sure that an incorrect use of a header fails to build - make it easier for new developers to understand the codebase - build correctly rather than fail in as many configurations as possible - actually support a mix of in and out of tree builds I think my patch under discussion does not address all issues here. I'll post a new proposal now. > --=20 > |: https://berrange.com -o- https://www.flickr.com/photos/dberr= ange :| > |: https://libvirt.org -o- https://fstop138.berrange= .com :| > |: https://entangle-photo.org -o- https://www.instagram.com/dberr= ange :|