| Message ID | 20200912224431.1428-28-luoyonggang@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | W32, W64 msys2/mingw patches | expand |
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
On Sun, Sep 13, 2020 at 10:08 PM Paolo Bonzini <pbonzini@redhat.com> 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.
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
On Sun, Sep 13, 2020 at 10:08 PM Paolo Bonzini <pbonzini@redhat.com> 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 > >
On Mon, Sep 14, 2020 at 12:12 AM Paolo Bonzini <pbonzini@redhat.com> 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 > 6513 rc_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 > 6513 rc_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 <poll.h> > > 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')) > 18: util_ss.add(when: 'CONFIG_WIN32', if_true: > files('qemu-thread-win32.c')) > 19: util_ss.add(when: 'CONFIG_WIN32', if_true: winmm) > 20 util_ss.add(files('envlist.c', 'path.c', 'module.c')) > > util\sys_membarrier.c: > 25 { > 26: #if defined CONFIG_WIN32 > 27 FlushProcessWriteBuffers(); > ``` > > Paolo > > Search _WIN32 ``` 561 results - 257 files block.c: 59 60: #ifdef _WIN32 61 #include <windows.h> 85 86: #ifdef _WIN32 87 static int is_windows_drive_prefix(const char *filename) 130 131: #ifdef _WIN32 132 if (is_windows_drive(path) || ``` 145 {-- 此致 礼 罗勇刚 Yours sincerely, Yonggang Luo
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
On Mon, Sep 14, 2020 at 4:45 PM Paolo Bonzini <pbonzini@redhat.com> 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
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
On Tue, Sep 15, 2020 at 7:44 PM Thomas Huth <thuth@redhat.com> 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 > >
diff --git a/configure b/configure index af86ba1db3..dc7cc0f411 100755 --- a/configure +++ b/configure @@ -535,7 +535,6 @@ rng_none="no" secret_keyring="" libdaxctl="" meson="" -ninja="" skip_meson=no gettext="" @@ -1003,8 +1002,6 @@ for opt do ;; --meson=*) meson="$optarg" ;; - --ninja=*) ninja="$optarg" - ;; --smbd=*) smbd="$optarg" ;; --extra-cflags=*) @@ -1777,7 +1774,6 @@ Advanced options (experts only): --python=PYTHON use specified python [$python] --sphinx-build=SPHINX use specified sphinx-build [$sphinx_build] --meson=MESON use specified meson [$meson] - --ninja=NINJA use specified ninja [$ninja] --smbd=SMBD use specified smbd [$smbd] --with-git=GIT use specified git [$git] --static enable static build [$static] @@ -2014,16 +2010,6 @@ case "$meson" in *) meson=$(command -v meson) ;; esac -# Probe for ninja (used for compdb) - -if test -z "$ninja"; then - for c in ninja ninja-build samu; do - if has $c; then - ninja=$(command -v "$c") - break - fi - done -fi # Check that the C compiler works. Doing this here before testing # the host CPU ensures that we had a valid CC to autodetect the @@ -7952,7 +7938,7 @@ fi mv $cross config-meson.cross rm -rf meson-private meson-info meson-logs -NINJA=${ninja:-$PWD/ninjatool} $meson setup \ +NINJA=$PWD/ninjatool $meson setup \ --prefix "${pre_prefix}$prefix" \ --libdir "${pre_prefix}$libdir" \ --libexecdir "${pre_prefix}$libexecdir" \
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 Signed-off-by: Yonggang Luo <luoyonggang@gmail.com> --- configure | 16 +--------------- 1 file changed, 1 insertion(+), 15 deletions(-)