Re: [Qemu-devel] [PATCH v2 1/2] make: move generated headers to qemu-build/

["Michael S. Tsirkin" <mst@redhat.com>]

On Fri, Mar 23, 2018 at 10:22:27AM +0000, Daniel P. Berrangé wrote:
> On Thu, Mar 22, 2018 at 09:27:55PM +0200, Michael S. Tsirkin wrote:
> > Make sure all generated files go into qemu-build subdirectory.
> > We can then include them like this:
> >  #include "qemu-build/trace.h"
> > 
> > This serves two purposes:
> > - make it easy to detect which files are in the source
> >   directory (a bit more work for writers, easier for readers)
> > - reduce chances of conflicts with possible stale files in source
> >   directory (which could be left over from e.g. old patches, etc)
> 
> If people care about this, then they can just be doing a build
> with  srcdir != builddir config.

Helps with the second point but I do not see how this will help address
the first point.

I see include "x.h" in source, it's natural to ask where it is.

I look for it in the c file directory. Not there.

I look for it in the include, it's not there. Where then?

qemu source root on the include path. Not there.

Oh tcg directory is in the include path for some reason. Not there.

Some other tree coming from a submodule maybe?

Oh *maybe* it will be generated by some script, I need to run
build to find out.

For the record, more remains to be done.
Here's a list from and oot build of virtio, with some comments:

cc -iquote /home/mst/qemu-oot/hw/virtio


 -iquote hw/virtio 

<- same directory repeated twice.

<- also this is here just for trace.h . Better to just change code to point at the right trace header -
   will look into it.

-iquote /home/mst/qemu/tcg

<- why does everyone want tcg? better to just include with tcg/

 -iquote /home/mst/qemu/tcg/i386

<- and i386 specifically?

 -I/home/mst/qemu/linux-headers

<- ok this is so we can override with our own version of headers

 -I/home/mst/qemu-oot/linux-headers

<- why do we have these at all?
   oh it's because we have asm-<arch> tricks like linux used to use years ago,
   then we copy the correct asm headers:
     ls linux-headers/asm/
     bitsperlong.h  hyperv.h  kvm.h  kvm_para.h  unistd_32.h  unistd_64.h  unistd.h  unistd_x32.h

   a better strategy would be to have headers in arch/<arch>/asm like Linux does now,
   no code generation tricks.

 -iquote .

<- build directory. No headers there at all.

 -iquote /home/mst/qemu 

<- turns out we still have headers in source root. Why not move them to include/ ?

-iquote /home/mst/qemu/accel/tcg 

<- more tcg goodness for everyone?

-iquote /home/mst/qemu/include 

<- ok that's expected. Why isn't this first on the list though?

-I/usr/include/pixman-1

<- should be limited to ui files I guess?

  -I/usr/include/glib-2.0 

<- ok we need that

-I/usr/lib64/glib-2.0/include 

<- but that one does not exist

  -I/usr/include/p11-kit-1     -I/usr/include/libpng16  -I/usr/include/libdrm

<- more GUI things?

  -I/home/mst/qemu/capstone/include

<- ok a bunch of disasm things.
	ls capstone/include/
	arm64.h  arm.h  capstone.h  mips.h  platform.h  ppc.h  sparc.h  systemz.h  x86.h  xcore.h
   can we add this just to disas.c?

  -I../linux-headers

<- all together now: same as qemu-oot but with a relative path now.

 -iquote ..

<- build directory - no headers there

 -iquote /home/mst/qemu/target/arm

<- this is a good idea why?

 -iquote /home/mst/qemu/include

<- deja vu

Wow that was fun.

>  If people are using srcdir == builddir
> then they likely *want* all the generated files in their srcdir.
> 
> IMHO it would be valid for us to consider if we could just mandate
> srcdir != builddir, but if people object to such a proposal, then I
> don't think we should arbitrarily move all generated source files
> in this way, as that's effectively the same thing forced onto devs.
> 
> Regards,
> Daniel

People don't really care.





> -- 
> |: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org         -o-            https://fstop138.berrange.com :|
> |: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|