| Message ID | 20200826151006.80-1-luoyonggang@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v5,1/6] meson: Fixes the ninjatool issue that E$$: are generated in Makefile.ninja | expand |
On Wed, Aug 26, 2020 at 5:12 PM <luoyonggang@gmail.com> wrote: > > From: Yonggang Luo <luoyonggang@gmail.com> > > SIMPLE_PATH_RE should match the full path token. > Or the $ and : contained in path would not matched if the path are start with C:/ and E:/ I don't understand this, SIMPLE_PATH_RE is used with re.match which only matches at the beginning of the string. Can you send me your build.ninja file offlist? Thanks, Paolo
I can tell you build.ninja can contains $: symbol, that's the escape for Ninja, when ninjatool parse it, it will convert $: -> :, so that's not a problem. This is part of the build.ninja on my computer ``` build version.rc_version.o: CUSTOM_COMMAND_DEP ../qemu.org/version.rc | C$:/CI-Tools/msys64/mingw64/bin/x86_64-w64-mingw32-windres.EXE ../ qemu.org/pc-bios/qemu-nsis.ico DEPFILE = "version.rc_version.o.d" DEPFILE_UNQUOTED = version.rc_version.o.d COMMAND = "C:/CI-Tools/msys64/mingw64/bin/x86_64-w64-mingw32-windres.EXE" "-I./." "-I../qemu.org/." "../qemu.org/version.rc" "version.rc_version.o" "--preprocessor-arg=-MD" "--preprocessor-arg=-MQversion.rc_version.o" "--preprocessor-arg=-MFversion.rc_version.o.d" description = Generating$ Windows$ resource$ for$ file$ 'version.rc'$ with$ a$ custom$ command ``` On Wed, Aug 26, 2020 at 11:28 PM Paolo Bonzini <pbonzini@redhat.com> wrote: > On Wed, Aug 26, 2020 at 5:12 PM <luoyonggang@gmail.com> wrote: > > > > From: Yonggang Luo <luoyonggang@gmail.com> > > > > SIMPLE_PATH_RE should match the full path token. > > Or the $ and : contained in path would not matched if the path are start > with C:/ and E:/ > > I don't understand this, SIMPLE_PATH_RE is used with re.match which > only matches at the beginning of the string. Can you send me your > build.ninja file offlist? > > Thanks, > > Paolo > >
On Wed, Aug 26, 2020 at 5:31 PM 罗勇刚(Yonggang Luo) <luoyonggang@gmail.com> wrote: > > I can tell you build.ninja can contains $: symbol, that's the escape for Ninja, > when ninjatool parse it, it will convert $: -> :, so that's not a problem. > This is part of the build.ninja on my computer Ok, that's useful. But can you just send the whole file (it's huge but you can gzip it or something similar)? Paolo > ``` > build version.rc_version.o: CUSTOM_COMMAND_DEP ../qemu.org/version.rc | C$:/CI-Tools/msys64/mingw64/bin/x86_64-w64-mingw32-windres.EXE ../qemu.org/pc-bios/qemu-nsis.ico > DEPFILE = "version.rc_version.o.d" > DEPFILE_UNQUOTED = version.rc_version.o.d > COMMAND = "C:/CI-Tools/msys64/mingw64/bin/x86_64-w64-mingw32-windres.EXE" "-I./." "-I../qemu.org/." "../qemu.org/version.rc" "version.rc_version.o" "--preprocessor-arg=-MD" "--preprocessor-arg=-MQversion.rc_version.o" "--preprocessor-arg=-MFversion.rc_version.o.d" > description = Generating$ Windows$ resource$ for$ file$ 'version.rc'$ with$ a$ custom$ command > ``` > > On Wed, Aug 26, 2020 at 11:28 PM Paolo Bonzini <pbonzini@redhat.com> wrote: >> >> On Wed, Aug 26, 2020 at 5:12 PM <luoyonggang@gmail.com> wrote: >> > >> > From: Yonggang Luo <luoyonggang@gmail.com> >> > >> > SIMPLE_PATH_RE should match the full path token. >> > Or the $ and : contained in path would not matched if the path are start with C:/ and E:/ >> >> I don't understand this, SIMPLE_PATH_RE is used with re.match which >> only matches at the beginning of the string. Can you send me your >> build.ninja file offlist? >> >> Thanks, >> >> Paolo >> > > > -- > 此致 > 礼 > 罗勇刚 > Yours > sincerely, > Yonggang Luo
On Wed, Aug 26, 2020 at 11:37 PM Paolo Bonzini <pbonzini@redhat.com> wrote: > On Wed, Aug 26, 2020 at 5:31 PM 罗勇刚(Yonggang Luo) <luoyonggang@gmail.com> > wrote: > > > > I can tell you build.ninja can contains $: symbol, that's the escape for > Ninja, > > when ninjatool parse it, it will convert $: -> :, so that's not a > problem. > > This is part of the build.ninja on my computer > > Ok, that's useful. But can you just send the whole file (it's huge but > you can gzip it or something similar)? > > Paolo > I am OK with that, but where should I post > > > ``` > > build version.rc_version.o: CUSTOM_COMMAND_DEP ../qemu.org/version.rc | > C$:/CI-Tools/msys64/mingw64/bin/x86_64-w64-mingw32-windres.EXE ../ > qemu.org/pc-bios/qemu-nsis.ico > > DEPFILE = "version.rc_version.o.d" > > DEPFILE_UNQUOTED = version.rc_version.o.d > > COMMAND = > "C:/CI-Tools/msys64/mingw64/bin/x86_64-w64-mingw32-windres.EXE" "-I./." > "-I../qemu.org/." "../qemu.org/version.rc" "version.rc_version.o" > "--preprocessor-arg=-MD" "--preprocessor-arg=-MQversion.rc_version.o" > "--preprocessor-arg=-MFversion.rc_version.o.d" > > description = Generating$ Windows$ resource$ for$ file$ 'version.rc'$ > with$ a$ custom$ command > > ``` > > > > On Wed, Aug 26, 2020 at 11:28 PM Paolo Bonzini <pbonzini@redhat.com> > wrote: > >> > >> On Wed, Aug 26, 2020 at 5:12 PM <luoyonggang@gmail.com> wrote: > >> > > >> > From: Yonggang Luo <luoyonggang@gmail.com> > >> > > >> > SIMPLE_PATH_RE should match the full path token. > >> > Or the $ and : contained in path would not matched if the path are > start with C:/ and E:/ > >> > >> I don't understand this, SIMPLE_PATH_RE is used with re.match which > >> only matches at the beginning of the string. Can you send me your > >> build.ninja file offlist? > >> > >> Thanks, > >> > >> Paolo > >> > > > > > > -- > > 此致 > > 礼 > > 罗勇刚 > > Yours > > sincerely, > > Yonggang Luo > >
On Wed, Aug 26, 2020 at 5:39 PM 罗勇刚(Yonggang Luo) <luoyonggang@gmail.com> wrote: >> Ok, that's useful. But can you just send the whole file (it's huge but >> you can gzip it or something similar)? >> >> Paolo > > I am OK with that, but where should I post Just pbonzini@redhat.com. Regarding the version.rc_version.o target in build.ninja, I see it translated to version.rc_version.o: private .var.command := "C:/CI-Tools/msys64/mingw64/bin/x86_64-w64-mingw32-windres.EXE" "-I./." "-I../qemu.org/." "../qemu.org/version.rc" "version.rc_version.o" "--preprocessor-arg=-MD" "--preprocessor-arg=-MQversion.rc_version.o" "--preprocessor-arg=-MFversion.rc_version.o.d" version.rc_version.o: private .var.description := Generating Windows resource for file 'version.rc' with a custom command version.rc_version.o: private .var.out := version.rc_version.o Is this wrong? Paolo
On Wed, Aug 26, 2020 at 11:41 PM Paolo Bonzini <pbonzini@redhat.com> wrote: > On Wed, Aug 26, 2020 at 5:39 PM 罗勇刚(Yonggang Luo) <luoyonggang@gmail.com> > wrote: > >> Ok, that's useful. But can you just send the whole file (it's huge but > >> you can gzip it or something similar)? > >> > >> Paolo > > > > I am OK with that, but where should I post > > Just pbonzini@redhat.com. > > Regarding the version.rc_version.o target in build.ninja, I see it > translated to > > version.rc_version.o: private .var.command := > "C:/CI-Tools/msys64/mingw64/bin/x86_64-w64-mingw32-windres.EXE" > "-I./." "-I../qemu.org/." "../qemu.org/version.rc" > "version.rc_version.o" "--preprocessor-arg=-MD" > "--preprocessor-arg=-MQversion.rc_version.o" > "--preprocessor-arg=-MFversion.rc_version.o.d" > version.rc_version.o: private .var.description := Generating Windows > resource for file 'version.rc' with a custom command > version.rc_version.o: private .var.out := version.rc_version.o > > Is this wrong? > This is fine > > > Paolo > >
How about enabling github actions to enable MSYS2 builds in CI. So we won't break msys2 silently.
Yes that would be great. We are tracking all CI "holes" that we didn't catch before committing the Meson transition, and MSYS is certainly one of them. You can send a patch and it will be reviewed and included. Thanks, Paolo On Wed, Aug 26, 2020 at 5:48 PM 罗勇刚(Yonggang Luo) <luoyonggang@gmail.com> wrote: > > How about enabling github actions to enable MSYS2 builds in CI. > So we won't break msys2 silently.
On Wed, Aug 26, 2020 at 11:56 PM Paolo Bonzini <pbonzini@redhat.com> wrote: > Yes that would be great. We are tracking all CI "holes" that we didn't > catch before committing the Meson transition, and MSYS is certainly > one of them. > > You can send a patch and it will be reviewed and included. > > Thanks, > After these patches merged, I'll do that > > Paolo > > On Wed, Aug 26, 2020 at 5:48 PM 罗勇刚(Yonggang Luo) <luoyonggang@gmail.com> > wrote: > > > > How about enabling github actions to enable MSYS2 builds in CI. > > So we won't break msys2 silently. > >
Still very big, I zip it for you. seems meson are not generating build.ninja in a stable very On Thu, Aug 27, 2020 at 12:09 AM 罗勇刚(Yonggang Luo) <luoyonggang@gmail.com> wrote: > > > On Wed, Aug 26, 2020 at 11:56 PM Paolo Bonzini <pbonzini@redhat.com> > wrote: > >> Yes that would be great. We are tracking all CI "holes" that we didn't >> catch before committing the Meson transition, and MSYS is certainly >> one of them. >> >> You can send a patch and it will be reviewed and included. >> >> Thanks, >> > After these patches merged, I'll do that > >> >> Paolo >> >> On Wed, Aug 26, 2020 at 5:48 PM 罗勇刚(Yonggang Luo) <luoyonggang@gmail.com> >> wrote: >> > >> > How about enabling github actions to enable MSYS2 builds in CI. >> > So we won't break msys2 silently. >> >> > > -- > 此致 > 礼 > 罗勇刚 > Yours > sincerely, > Yonggang Luo >
diff --git a/scripts/ninjatool.py b/scripts/ninjatool.py index cc77d51aa8..6ca8be6f10 100755 --- a/scripts/ninjatool.py +++ b/scripts/ninjatool.py @@ -55,7 +55,7 @@ else: PATH_RE = r"[^$\s:|]+|\$[$ :]|\$[a-zA-Z0-9_-]+|\$\{[a-zA-Z0-9_.-]+\}" -SIMPLE_PATH_RE = re.compile(r"[^$\s:|]+") +SIMPLE_PATH_RE = re.compile(r"^[^$\s:|]+$") IDENT_RE = re.compile(r"[a-zA-Z0-9_.-]+$") STRING_RE = re.compile(r"(" + PATH_RE + r"|[\s:|])(?:\r?\n)?|.") TOPLEVEL_RE = re.compile(r"([=:#]|\|\|?|^ +|(?:" + PATH_RE + r")+)\s*|.")