Re: [PATCH v8 27/27] Revert "configure: add --ninja option"
On Tue, Sep 15, 2020 at 7:44 PM Thomas Huth wrote: > On 13/09/2020 16.08, Paolo Bonzini wrote: > > On 13/09/20 00:44, Yonggang Luo wrote: > >> This reverts commit 48328880fddf0145bdccc499160fb24dfabfbd41. > >> > >> The --ninja option doesn't need anymore because of upgrade meson to > 0.55.2 > >> At that version we can use ninjatool > > > > We might actually get rid of ninjatool before QEMU 5.2 goes out, if we > > decide to make Ninja a mandatory build dependency. So we can hold on > > patches 26 and 27. Thanks for testing though! > > > > I'm also not sure about patch 16, since that's not my area, but Daniel > > and Ed both reviewed it so that's okay. > > > > Finally, instead of checking !_WIN32 it's better to check CONFIG_POSIX > > or CONFIG_WIN32. That can be changed on commit though. > > > > Everything else seems okay. I'll wait a couple days and queue the whole > > bunch up to patch 25. > > Patch 13 definitely needs a replacement, and I think patch 2 should > likely go through the block tree instead... > I am prepareing V9, and move nfs related things to the end patch 13 are re-implemented please wait for some minutes > > But I recently started to experiment with these patches, too, and I > think I got a reasonable subset now which should be OK for getting > merged (e.g. disabling NFS and the crypto test in the CI for now). I'll > take those through my testing tree. The remaining work can then be done > on top later. > > Thomas > > -- 此致 礼 罗勇刚 Yours sincerely, Yonggang Luo
Re: [PATCH v8 27/27] Revert "configure: add --ninja option"
On 13/09/2020 16.08, Paolo Bonzini wrote: > On 13/09/20 00:44, Yonggang Luo wrote: >> This reverts commit 48328880fddf0145bdccc499160fb24dfabfbd41. >> >> The --ninja option doesn't need anymore because of upgrade meson to 0.55.2 >> At that version we can use ninjatool > > We might actually get rid of ninjatool before QEMU 5.2 goes out, if we > decide to make Ninja a mandatory build dependency. So we can hold on > patches 26 and 27. Thanks for testing though! > > I'm also not sure about patch 16, since that's not my area, but Daniel > and Ed both reviewed it so that's okay. > > Finally, instead of checking !_WIN32 it's better to check CONFIG_POSIX > or CONFIG_WIN32. That can be changed on commit though. > > Everything else seems okay. I'll wait a couple days and queue the whole > bunch up to patch 25. Patch 13 definitely needs a replacement, and I think patch 2 should likely go through the block tree instead... But I recently started to experiment with these patches, too, and I think I got a reasonable subset now which should be OK for getting merged (e.g. disabling NFS and the crypto test in the CI for now). I'll take those through my testing tree. The remaining work can then be done on top later. Thomas
Re: [PATCH v8 27/27] Revert "configure: add --ninja option"
On Mon, Sep 14, 2020 at 4:45 PM Paolo Bonzini wrote: > > On 13/09/20 18:14, 罗勇刚(Yonggang Luo) wrote: > > I am hurry to revert --ninja option because when the meson are changed, the > > make -j10 can not automatically re configure, that would raise ninja can > > not found error > > My understanding is that with 0.55.2 you don't need --ninja at all (the > default search works), and also the previously configured build tree > should work. > > What's the issue there? > Oh, I mis-understood the --ninja option, so the ninja option doesn't have to be revert, but upgrade meson to 0.55.2 are necessary > > Paolo > -- 此致 礼 罗勇刚 Yours sincerely, Yonggang Luo
Re: [PATCH v8 27/27] Revert "configure: add --ninja option"
On 13/09/20 18:14, 罗勇刚(Yonggang Luo) wrote: > I am hurry to revert --ninja option because when the meson are changed, the > make -j10 can not automatically re configure, that would raise ninja can > not found error My understanding is that with 0.55.2 you don't need --ninja at all (the default search works), and also the previously configured build tree should work. What's the issue there? Paolo
Re: [PATCH v8 27/27] Revert "configure: add --ninja option"
On Mon, Sep 14, 2020 at 12:12 AM Paolo Bonzini wrote:
> On 13/09/20 18:03, 罗勇刚(Yonggang Luo) wrote:
> >
> > _WIN32 are more precise and only depends on the compiler, on the
> > other hand, CONFIG_POSIX and CONFIG_WIN32 need configure
> > scripts. I prefer _WIN32 unless the compiler can not provide enough
> > information.
>
> That's not what the QEMU coding standards say; we generally don't test
> the preprocessor symbols. If we were to change to _WIN32, it should be
> done at once on the whole codebase (don't do it :)).>
>
CONFIG_WIN32 are rarely used, most of the are using _WIN32
Search CONFIG_WIN32
```
> 36 results - 20 files
>
> configure:
> 6511 if test "$mingw32" = "yes" ; then
> 6512: echo "CONFIG_WIN32=y" >> $config_host_mak
> 6513rc_version=$(cat $source_path/VERSION)
>
> Makefile:
> 274 @echo ''
> 275: ifdef CONFIG_WIN32
> 276 @echo 'Windows targets:'
>
> meson.build:
> 853 blockdev_ss.add(when: 'CONFIG_POSIX', if_true: files('os-posix.c'))
> 854: softmmu_ss.add(when: 'CONFIG_WIN32', if_true: [files('os-win32.c')])
> 855
>
> backends\qemu\configure:
> 6511 if test "$mingw32" = "yes" ; then
> 6512: echo "CONFIG_WIN32=y" >> $config_host_mak
> 6513rc_version=$(cat $source_path/VERSION)
>
> backends\qemu\Makefile:
> 272 @echo ''
> 273: ifdef CONFIG_WIN32
> 274 @echo 'Windows targets:'
>
> backends\qemu\meson.build:
> 856 blockdev_ss.add(when: 'CONFIG_POSIX', if_true: files('os-posix.c'))
> 857: softmmu_ss.add(when: 'CONFIG_WIN32', if_true: [files('os-win32.c')])
> 858
>
> block\meson.build:
> 58 block_ss.add(when: [libxml2, 'CONFIG_PARALLELS'], if_true:
> files('parallels.c'))
> 59: block_ss.add(when: 'CONFIG_WIN32', if_true: files('file-win32.c',
> 'win32-aio.c'))
> 60 block_ss.add(when: 'CONFIG_POSIX', if_true: [files('file-posix.c'),
> coref, iokit])
>
> chardev\meson.build:
> 20 ))
> 21: chardev_ss.add(when: 'CONFIG_WIN32', if_true: files(
> 22'char-console.c',
>
> hw\usb\host-libusb.c:
> 37 #include "qom/object.h"
> 38: #ifndef CONFIG_WIN32
> 39 #include
>
>228
>229: #ifndef CONFIG_WIN32
>230
>
>249
>250: #endif /* !CONFIG_WIN32 */
>251
>
>253 {
>254: #ifndef CONFIG_WIN32
>255 const struct libusb_pollfd **poll;
>
>270 #endif
>271: #ifdef CONFIG_WIN32
>272 /* FIXME: add support for Windows. */
>
>916 } else {
>917: #if LIBUSB_API_VERSION >= 0x01000107 && !defined(CONFIG_WIN32)
>918 trace_usb_host_open_hostfd(hostfd);
>
> 1145
> 1146: #if LIBUSB_API_VERSION >= 0x01000107 && !defined(CONFIG_WIN32)
> 1147 if (s->hostdevice) {
>
> io\channel-watch.c:
>32
>33: #ifdef CONFIG_WIN32
>34 typedef struct QIOChannelSocketSource QIOChannelSocketSource;
>
>98
>99: #ifdef CONFIG_WIN32
> 100 static gboolean
>
> 267
> 268: #ifdef CONFIG_WIN32
> 269 ssource->fd.fd = (gint64)_get_osfhandle(fd);
>
> 279
> 280: #ifdef CONFIG_WIN32
> 281 GSource *qio_channel_create_socket_watch(QIOChannel *ioc,
>
> 337
> 338: #ifdef CONFIG_WIN32
> 339 ssource->fdread.fd = (gint64)_get_osfhandle(fdread);
>
> net\meson.build:
> 36 softmmu_ss.add(when: 'CONFIG_POSIX', if_true: files(tap_posix))
> 37: softmmu_ss.add(when: 'CONFIG_WIN32', if_true: files('tap-win32.c'))
> 38 softmmu_ss.add(when: 'CONFIG_VHOST_NET_VDPA', if_true:
> files('vhost-vdpa.c'))
>
> qga\meson.build:
> 39'commands-posix.c'))
> 40: qga_ss.add(when: 'CONFIG_WIN32', if_true: files(
> 41'channel-win32.c',
>
> scripts\checkpatch.pl:
> 2775 # check of hardware specific defines
> 2776: # we have e.g. CONFIG_LINUX and CONFIG_WIN32 for common cases
> 2777 # where they might be necessary.
>
> target\i386\hax-i386.h:
> 22
> 23: #ifdef CONFIG_WIN32
> 24 typedef HANDLE hax_fd;
>
> 87
> 88: #ifdef CONFIG_WIN32
> 89 #include "target/i386/hax-windows.h"
>
> target\i386\meson.build:
> 34 i386_softmmu_ss.add(when: ['CONFIG_POSIX', 'CONFIG_HAX'], if_true:
> files('hax-all.c', 'hax-mem.c', 'hax-posix.c'))
> 35: i386_softmmu_ss.add(when: ['CONFIG_WIN32', 'CONFIG_HAX'], if_true:
> files('hax-all.c', 'hax-mem.c', 'hax-windows.c'))
> 36
>
> ui\gtk.c:
> 1171 {
> 1172: #ifdef CONFIG_WIN32
> 1173 /*
>
> ui\meson.build:
> 48 if config_host.has_key('CONFIG_GTK')
> 49: softmmu_ss.add(when: 'CONFIG_WIN32', if_true:
> files('win32-kbd-hook.c'))
> 50
>
> 59 if sdl.found()
> 60: softmmu_ss.add(when: 'CONFIG_WIN32', if_true:
> files('win32-kbd-hook.c'))
> 61
>
> ui\sdl2.c:
> 332 {
> 333: #ifdef CONFIG_WIN32
> 334 SDL_SysWMinfo info;
>
> util\meson.build:
> 14 util_ss.add(when: 'CONFIG_POSIX', if_true: files('memfd.c'))
> 15: util_ss.add(when: 'CONFIG_WIN32', if_true: files('aio-win32.c'))
> 16: util_ss.add(when: 'CONFIG_WIN32', if_true:
> files('event_notifier-win32.c'))
> 17: util_ss.add(when: 'CONFIG_WIN32', if_true: files('oslib-win32.c'))
>
Re: [PATCH v8 27/27] Revert "configure: add --ninja option"
On Sun, Sep 13, 2020 at 10:08 PM Paolo Bonzini wrote: > On 13/09/20 00:44, Yonggang Luo wrote: > > This reverts commit 48328880fddf0145bdccc499160fb24dfabfbd41. > > > > The --ninja option doesn't need anymore because of upgrade meson to > 0.55.2 > > At that version we can use ninjatool > > We might actually get rid of ninjatool before QEMU 5.2 goes out, if we > decide to make Ninja a mandatory build dependency. So we can hold on > patches 26 and 27. Thanks for testing though! > I am hurry to revert --ninja option because when the meson are changed, the make -j10 can not automatically re configure, that would raise ninja can not found error > > I'm also not sure about patch 16, since that's not my area, but Daniel > and Ed both reviewed it so that's okay. > > Finally, instead of checking !_WIN32 it's better to check CONFIG_POSIX > or CONFIG_WIN32. That can be changed on commit though. > > Everything else seems okay. I'll wait a couple days and queue the whole > bunch up to patch 25. > > Paolo > > -- 此致 礼 罗勇刚 Yours sincerely, Yonggang Luo
Re: [PATCH v8 27/27] Revert "configure: add --ninja option"
On 13/09/20 18:03, 罗勇刚(Yonggang Luo) wrote: > > _WIN32 are more precise and only depends on the compiler, on the > other hand, CONFIG_POSIX and CONFIG_WIN32 need configure > scripts. I prefer _WIN32 unless the compiler can not provide enough > information. That's not what the QEMU coding standards say; we generally don't test the preprocessor symbols. If we were to change to _WIN32, it should be done at once on the whole codebase (don't do it :)). Paolo
Re: [PATCH v8 27/27] Revert "configure: add --ninja option"
On Sun, Sep 13, 2020 at 10:08 PM Paolo Bonzini wrote: > On 13/09/20 00:44, Yonggang Luo wrote: > > This reverts commit 48328880fddf0145bdccc499160fb24dfabfbd41. > > > > The --ninja option doesn't need anymore because of upgrade meson to > 0.55.2 > > At that version we can use ninjatool > > We might actually get rid of ninjatool before QEMU 5.2 goes out, if we > decide to make Ninja a mandatory build dependency. So we can hold on > patches 26 and 27. Thanks for testing though! > > I'm also not sure about patch 16, since that's not my area, but Daniel > and Ed both reviewed it so that's okay. > > Finally, instead of checking !_WIN32 it's better to check CONFIG_POSIX > or CONFIG_WIN32. That can be changed on commit though. > > Everything else seems okay. I'll wait a couple days and queue the whole > bunch up to patch 25. > > > Paolo > > _WIN32 are more precise and only depends on the compiler, on the other hand,CONFIG_POSIX and CONFIG_WIN32 need configure scripts. I prefer _WIN32 unless the compiler can not provide enough information. -- 此致 礼 罗勇刚 Yours sincerely, Yonggang Luo
Re: [PATCH v8 27/27] Revert "configure: add --ninja option"
On 13/09/20 00:44, Yonggang Luo wrote: > This reverts commit 48328880fddf0145bdccc499160fb24dfabfbd41. > > The --ninja option doesn't need anymore because of upgrade meson to 0.55.2 > At that version we can use ninjatool We might actually get rid of ninjatool before QEMU 5.2 goes out, if we decide to make Ninja a mandatory build dependency. So we can hold on patches 26 and 27. Thanks for testing though! I'm also not sure about patch 16, since that's not my area, but Daniel and Ed both reviewed it so that's okay. Finally, instead of checking !_WIN32 it's better to check CONFIG_POSIX or CONFIG_WIN32. That can be changed on commit though. Everything else seems okay. I'll wait a couple days and queue the whole bunch up to patch 25. Paolo
