[PATCH 0/3] windows installer: Fix warnings, mouse-over descriptions, item order

[PATCH 0/3] windows installer: Fix warnings, mouse-over descriptions, item order

[Peter Maydell]

When we build our Windows installer, there are some warnings
generated:

 warning 7998: ANSI targets are deprecated
 warning 6000: unknown variable/constant "{Section_i386}" detected, ignoring (macro:_==:1)
 warning 6000: unknown variable/constant "{Section_i386w}" detected, ignoring (macro:_==:1)

(you can see these in the relevant gitlab CI job logs).
This series fixes them; one of them is flagging up an actual bug.
It also fixes the basically-random order we were listing the
emulators in.

I noticed while working on this that we list the emulators in
basically random order (whatever glob.glob() returns); so I start off
with fixing that in patch 1 by adding a sorted() call.

The "ANSI targets" warning is just complaining that we don't declare
our installer to be a Unicode installer. Doing so silences the
warning, and would allow us to include non-ASCII text in messages
(which we don't currently need). The resulting installer won't run on
Win95/98/ME, but we don't support those anyway. Fixed in patch 2.

The "unknown variable" warnings are about a real problem: we generate
installer sections for each emulator executable using the nsis.py
script, but still have a fixed and very out of date list of mouse-over
description text in the qemu.nsi file. So if you build the installer
for a configuration which doesn't include either i386 or alpha then
you get warnings about the mouse-over text defined for the
non-existent sections; and we don't provide any mouse-over text for
anything other than i386 and alpha. Patch 3 makes the nsis.py script
also generate a second installer script fragment with mouseover text
for all and only the sections it generates in the existing fragment.
The generated text is not particularly pretty or informative, but at
least it's there and clarifies the "GUI" vs "not GUI" versions of each
executable.

Tested by building the installer executable via the gitlab
'cross-win64-system' CI job and test-installing it on a Windows 10 VM
I had to hand.

thanks
-- PMM

Peter Maydell (3):
  nsis installer: List emulators in alphabetical order
  nsis installer: Suppress "ANSI targets are deprecated" warning
  nsis installer: Fix mouse-over descriptions for emulators

 qemu.nsi        |  8 ++++----
 scripts/nsis.py | 17 ++++++++++++++---
 2 files changed, 18 insertions(+), 7 deletions(-)

-- 
2.25.1

[PATCH 1/3] nsis installer: List emulators in alphabetical order

[Peter Maydell]

We currently list the emulators in the Windows installer's dialog
in an essentially random order (it's whatever glob.glob() returns
them to, which is filesystem-implementation-dependent). Add a
call to sorted() so they appear in alphabetical order.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 scripts/nsis.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/scripts/nsis.py b/scripts/nsis.py
index 5135a058316..383bef70332 100644
--- a/scripts/nsis.py
+++ b/scripts/nsis.py
@@ -34,9 +34,9 @@ def main():
         with open(
             os.path.join(destdir + args.prefix, "system-emulations.nsh"), "w"
         ) as nsh:
-            for exe in glob.glob(
+            for exe in sorted(glob.glob(
                 os.path.join(destdir + args.prefix, "qemu-system-*.exe")
-            ):
+            )):
                 exe = os.path.basename(exe)
                 arch = exe[12:-4]
                 nsh.write(
-- 
2.25.1

Re: [PATCH 1/3] nsis installer: List emulators in alphabetical order

[Philippe Mathieu-Daudé]

On 5/3/22 11:57, Peter Maydell wrote:
> We currently list the emulators in the Windows installer's dialog
> in an essentially random order (it's whatever glob.glob() returns
> them to, which is filesystem-implementation-dependent). Add a
> call to sorted() so they appear in alphabetical order.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   scripts/nsis.py | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Re: [PATCH 1/3] nsis installer: List emulators in alphabetical order

[Stefan Weil]

[-- Attachment #1.1.1: Type: text/plain, Size: 1138 bytes --]

Am 05.03.22 um 11:57 schrieb Peter Maydell:

> We currently list the emulators in the Windows installer's dialog
> in an essentially random order (it's whatever glob.glob() returns
> them to, which is filesystem-implementation-dependent). Add a
> call to sorted() so they appear in alphabetical order.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   scripts/nsis.py | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/scripts/nsis.py b/scripts/nsis.py
> index 5135a058316..383bef70332 100644
> --- a/scripts/nsis.py
> +++ b/scripts/nsis.py
> @@ -34,9 +34,9 @@ def main():
>           with open(
>               os.path.join(destdir + args.prefix, "system-emulations.nsh"), "w"
>           ) as nsh:
> -            for exe in glob.glob(
> +            for exe in sorted(glob.glob(
>                   os.path.join(destdir + args.prefix, "qemu-system-*.exe")
> -            ):
> +            )):
>                   exe = os.path.basename(exe)
>                   arch = exe[12:-4]
>                   nsh.write(


Reviewed-by: Stefan Weil <sw@weilnetz.de>



[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 15123 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

Re: [PATCH 1/3] nsis installer: List emulators in alphabetical order

[John Snow]

On Sat, Mar 5, 2022 at 5:57 AM Peter Maydell <peter.maydell@linaro.org> wrote:
>
> We currently list the emulators in the Windows installer's dialog
> in an essentially random order (it's whatever glob.glob() returns
> them to, which is filesystem-implementation-dependent). Add a
> call to sorted() so they appear in alphabetical order.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  scripts/nsis.py | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/scripts/nsis.py b/scripts/nsis.py
> index 5135a058316..383bef70332 100644
> --- a/scripts/nsis.py
> +++ b/scripts/nsis.py
> @@ -34,9 +34,9 @@ def main():
>          with open(
>              os.path.join(destdir + args.prefix, "system-emulations.nsh"), "w"
>          ) as nsh:
> -            for exe in glob.glob(
> +            for exe in sorted(glob.glob(
>                  os.path.join(destdir + args.prefix, "qemu-system-*.exe")
> -            ):
> +            )):
>                  exe = os.path.basename(exe)
>                  arch = exe[12:-4]
>                  nsh.write(
> --
> 2.25.1
>

Do you care if this order is dependent upon the locale settings of the
host machine?

If you don't:

Reviewed-by: John Snow <jsnow@redhat.com>

Re: [PATCH 1/3] nsis installer: List emulators in alphabetical order

[Peter Maydell]

On Mon, 7 Mar 2022 at 19:25, John Snow <jsnow@redhat.com> wrote:
>
> On Sat, Mar 5, 2022 at 5:57 AM Peter Maydell <peter.maydell@linaro.org> wrote:
> >
> > We currently list the emulators in the Windows installer's dialog
> > in an essentially random order (it's whatever glob.glob() returns
> > them to, which is filesystem-implementation-dependent). Add a
> > call to sorted() so they appear in alphabetical order.
> >
> > Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> > ---
> >  scripts/nsis.py | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/scripts/nsis.py b/scripts/nsis.py
> > index 5135a058316..383bef70332 100644
> > --- a/scripts/nsis.py
> > +++ b/scripts/nsis.py
> > @@ -34,9 +34,9 @@ def main():
> >          with open(
> >              os.path.join(destdir + args.prefix, "system-emulations.nsh"), "w"
> >          ) as nsh:
> > -            for exe in glob.glob(
> > +            for exe in sorted(glob.glob(
> >                  os.path.join(destdir + args.prefix, "qemu-system-*.exe")
> > -            ):
> > +            )):
> >                  exe = os.path.basename(exe)
> >                  arch = exe[12:-4]
> >                  nsh.write(
> > --
> > 2.25.1
> >
>
> Do you care if this order is dependent upon the locale settings of the
> host machine?

Everything we're sorting is lower-case letters only, so the locale
shouldn't matter.

-- PMM

[PATCH 2/3] nsis installer: Suppress "ANSI targets are deprecated" warning

[Peter Maydell]

When we build our Windows installer, it emits the warning:

   warning 7998: ANSI targets are deprecated

Fix this by making our installer a Unicode installer instead.  These
won't work on Win95/98/ME, but we already do not support those.

See
https://nsis.sourceforge.io/Docs/Chapter4.html#aunicodetarget
for the documentation of the Unicode directive.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 qemu.nsi | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/qemu.nsi b/qemu.nsi
index c3df8c9d3b0..a44d2be32a2 100644
--- a/qemu.nsi
+++ b/qemu.nsi
@@ -35,6 +35,9 @@
 !define OUTFILE "qemu-setup.exe"
 !endif
 
+; Build a unicode installer
+Unicode True
+
 ; Use maximum compression.
 SetCompressor /SOLID lzma
 
-- 
2.25.1

Re: [PATCH 2/3] nsis installer: Suppress "ANSI targets are deprecated" warning

[Philippe Mathieu-Daudé]

On 5/3/22 11:57, Peter Maydell wrote:
> When we build our Windows installer, it emits the warning:
> 
>     warning 7998: ANSI targets are deprecated
> 
> Fix this by making our installer a Unicode installer instead.  These
> won't work on Win95/98/ME, but we already do not support those.
> 
> See
> https://nsis.sourceforge.io/Docs/Chapter4.html#aunicodetarget
> for the documentation of the Unicode directive.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   qemu.nsi | 3 +++
>   1 file changed, 3 insertions(+)

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Re: [PATCH 2/3] nsis installer: Suppress "ANSI targets are deprecated" warning

[Stefan Weil]

[-- Attachment #1.1.1: Type: text/plain, Size: 1111 bytes --]

Am 05.03.22 um 11:57 schrieb Peter Maydell:

> When we build our Windows installer, it emits the warning:
>
>     warning 7998: ANSI targets are deprecated
>
> Fix this by making our installer a Unicode installer instead.  These
> won't work on Win95/98/ME, but we already do not support those.
>
> See
> https://nsis.sourceforge.io/Docs/Chapter4.html#aunicodetarget
> for the documentation of the Unicode directive.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   qemu.nsi | 3 +++
>   1 file changed, 3 insertions(+)
>
> diff --git a/qemu.nsi b/qemu.nsi
> index c3df8c9d3b0..a44d2be32a2 100644
> --- a/qemu.nsi
> +++ b/qemu.nsi
> @@ -35,6 +35,9 @@
>   !define OUTFILE "qemu-setup.exe"
>   !endif
>   
> +; Build a unicode installer
> +Unicode True
> +
>   ; Use maximum compression.
>   SetCompressor /SOLID lzma


Please use true instead of True to match the description. With this fix

Reviewed-by: Stefan Weil <sw@weilnetz.de>

( I should have sent 
https://repo.or.cz/qemu/ar7.git/commitdiff/171008240c2, the patch which 
I use for building)


[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 15123 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

[PATCH 3/3] nsis installer: Fix mouse-over descriptions for emulators

[Peter Maydell]

We use the nsis.py script to write out an installer script Section
for each emulator executable, so the exact set of Sections depends on
which executables were built.  However the part of qemu.nsi which
specifies mouse-over descriptions for each Section still has a
hard-coded and very outdated list (with just i386 and alpha).  This
causes two problems.  Firstly, if you build the installer for a
configuration where you didn't build the i386 binaries you get
warnings like this:
  warning 6000: unknown variable/constant "{Section_i386}" detected, ignoring (macro:_==:1)
  warning 6000: unknown variable/constant "{Section_i386w}" detected, ignoring (macro:_==:1)
(this happens in our gitlab CI jobs, for instance).
Secondly, most of the emulators in the generated installer don't have
any mouseover text.

Make nsis.py generate a second output file which has the necessary
MUI_DESCRIPTION_TEXT lines for each Section it creates, so we can
include that at the right point in qemu.nsi to set the mouse-over
text.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
I opted to put the logic into the Python script, which I think
many QEMU contributors understand, rather than investigating
whether NSIS installer script magic might for instance allow
us to write out only one file rather than two, since I think
very few of us understand the NSIS installer...
---
 qemu.nsi        |  5 +----
 scripts/nsis.py | 13 ++++++++++++-
 2 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/qemu.nsi b/qemu.nsi
index a44d2be32a2..aa93adee396 100644
--- a/qemu.nsi
+++ b/qemu.nsi
@@ -228,10 +228,7 @@ SectionEnd
 ; Descriptions (mouse-over).
 !insertmacro MUI_FUNCTION_DESCRIPTION_BEGIN
     !insertmacro MUI_DESCRIPTION_TEXT ${SectionSystem}  "System emulation."
-    !insertmacro MUI_DESCRIPTION_TEXT ${Section_alpha}  "Alpha system emulation."
-    !insertmacro MUI_DESCRIPTION_TEXT ${Section_alphaw} "Alpha system emulation (GUI)."
-    !insertmacro MUI_DESCRIPTION_TEXT ${Section_i386}   "PC i386 system emulation."
-    !insertmacro MUI_DESCRIPTION_TEXT ${Section_i386w}  "PC i386 system emulation (GUI)."
+!include "${BINDIR}\system-mui-text.nsh"
     !insertmacro MUI_DESCRIPTION_TEXT ${SectionTools} "Tools."
 !ifdef DLLDIR
     !insertmacro MUI_DESCRIPTION_TEXT ${SectionDll}   "Runtime Libraries (DLL)."
diff --git a/scripts/nsis.py b/scripts/nsis.py
index 383bef70332..462d6cac3b6 100644
--- a/scripts/nsis.py
+++ b/scripts/nsis.py
@@ -33,7 +33,9 @@ def main():
         subprocess.run(["make", "install", "DESTDIR=" + destdir + os.path.sep])
         with open(
             os.path.join(destdir + args.prefix, "system-emulations.nsh"), "w"
-        ) as nsh:
+        ) as nsh, open(
+            os.path.join(destdir + args.prefix, "system-mui-text.nsh"), "w"
+        ) as muinsh:
             for exe in sorted(glob.glob(
                 os.path.join(destdir + args.prefix, "qemu-system-*.exe")
             )):
@@ -49,6 +51,15 @@ def main():
                         arch, exe
                     )
                 )
+                if arch.endswith('w'):
+                    desc = arch[:-1] + " emulation (GUI)."
+                else:
+                    desc = arch + " emulation."
+
+                muinsh.write(
+                    """
+                !insertmacro MUI_DESCRIPTION_TEXT ${{Section_{0}}} "{1}"
+                """.format(arch, desc))
 
         for exe in glob.glob(os.path.join(destdir + args.prefix, "*.exe")):
             signcode(exe)
-- 
2.25.1

Re: [PATCH 3/3] nsis installer: Fix mouse-over descriptions for emulators

[Philippe Mathieu-Daudé]

On 5/3/22 11:57, Peter Maydell wrote:
> We use the nsis.py script to write out an installer script Section
> for each emulator executable, so the exact set of Sections depends on
> which executables were built.  However the part of qemu.nsi which
> specifies mouse-over descriptions for each Section still has a
> hard-coded and very outdated list (with just i386 and alpha).  This
> causes two problems.  Firstly, if you build the installer for a
> configuration where you didn't build the i386 binaries you get
> warnings like this:
>    warning 6000: unknown variable/constant "{Section_i386}" detected, ignoring (macro:_==:1)
>    warning 6000: unknown variable/constant "{Section_i386w}" detected, ignoring (macro:_==:1)
> (this happens in our gitlab CI jobs, for instance).
> Secondly, most of the emulators in the generated installer don't have
> any mouseover text.
> 
> Make nsis.py generate a second output file which has the necessary
> MUI_DESCRIPTION_TEXT lines for each Section it creates, so we can
> include that at the right point in qemu.nsi to set the mouse-over
> text.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> I opted to put the logic into the Python script, which I think
> many QEMU contributors understand, rather than investigating
> whether NSIS installer script magic might for instance allow
> us to write out only one file rather than two, since I think
> very few of us understand the NSIS installer...

Very much appreciated.

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
>   qemu.nsi        |  5 +----
>   scripts/nsis.py | 13 ++++++++++++-
>   2 files changed, 13 insertions(+), 5 deletions(-)

Re: [PATCH 3/3] nsis installer: Fix mouse-over descriptions for emulators

[John Snow]

On Sat, Mar 5, 2022 at 5:57 AM Peter Maydell <peter.maydell@linaro.org> wrote:
>
> We use the nsis.py script to write out an installer script Section
> for each emulator executable, so the exact set of Sections depends on
> which executables were built.  However the part of qemu.nsi which
> specifies mouse-over descriptions for each Section still has a
> hard-coded and very outdated list (with just i386 and alpha).  This
> causes two problems.  Firstly, if you build the installer for a
> configuration where you didn't build the i386 binaries you get
> warnings like this:
>   warning 6000: unknown variable/constant "{Section_i386}" detected, ignoring (macro:_==:1)
>   warning 6000: unknown variable/constant "{Section_i386w}" detected, ignoring (macro:_==:1)
> (this happens in our gitlab CI jobs, for instance).
> Secondly, most of the emulators in the generated installer don't have
> any mouseover text.
>
> Make nsis.py generate a second output file which has the necessary
> MUI_DESCRIPTION_TEXT lines for each Section it creates, so we can
> include that at the right point in qemu.nsi to set the mouse-over
> text.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> I opted to put the logic into the Python script, which I think
> many QEMU contributors understand, rather than investigating
> whether NSIS installer script magic might for instance allow
> us to write out only one file rather than two, since I think
> very few of us understand the NSIS installer...

Makes sense.

Python bits:

Reviewed-by: John Snow <jsnow@redhat.com>


> ---
>  qemu.nsi        |  5 +----
>  scripts/nsis.py | 13 ++++++++++++-
>  2 files changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/qemu.nsi b/qemu.nsi
> index a44d2be32a2..aa93adee396 100644
> --- a/qemu.nsi
> +++ b/qemu.nsi
> @@ -228,10 +228,7 @@ SectionEnd
>  ; Descriptions (mouse-over).
>  !insertmacro MUI_FUNCTION_DESCRIPTION_BEGIN
>      !insertmacro MUI_DESCRIPTION_TEXT ${SectionSystem}  "System emulation."
> -    !insertmacro MUI_DESCRIPTION_TEXT ${Section_alpha}  "Alpha system emulation."
> -    !insertmacro MUI_DESCRIPTION_TEXT ${Section_alphaw} "Alpha system emulation (GUI)."
> -    !insertmacro MUI_DESCRIPTION_TEXT ${Section_i386}   "PC i386 system emulation."
> -    !insertmacro MUI_DESCRIPTION_TEXT ${Section_i386w}  "PC i386 system emulation (GUI)."
> +!include "${BINDIR}\system-mui-text.nsh"
>      !insertmacro MUI_DESCRIPTION_TEXT ${SectionTools} "Tools."
>  !ifdef DLLDIR
>      !insertmacro MUI_DESCRIPTION_TEXT ${SectionDll}   "Runtime Libraries (DLL)."
> diff --git a/scripts/nsis.py b/scripts/nsis.py
> index 383bef70332..462d6cac3b6 100644
> --- a/scripts/nsis.py
> +++ b/scripts/nsis.py
> @@ -33,7 +33,9 @@ def main():
>          subprocess.run(["make", "install", "DESTDIR=" + destdir + os.path.sep])
>          with open(
>              os.path.join(destdir + args.prefix, "system-emulations.nsh"), "w"
> -        ) as nsh:
> +        ) as nsh, open(
> +            os.path.join(destdir + args.prefix, "system-mui-text.nsh"), "w"
> +        ) as muinsh:
>              for exe in sorted(glob.glob(
>                  os.path.join(destdir + args.prefix, "qemu-system-*.exe")
>              )):
> @@ -49,6 +51,15 @@ def main():
>                          arch, exe
>                      )
>                  )
> +                if arch.endswith('w'):
> +                    desc = arch[:-1] + " emulation (GUI)."
> +                else:
> +                    desc = arch + " emulation."
> +
> +                muinsh.write(
> +                    """
> +                !insertmacro MUI_DESCRIPTION_TEXT ${{Section_{0}}} "{1}"
> +                """.format(arch, desc))
>
>          for exe in glob.glob(os.path.join(destdir + args.prefix, "*.exe")):
>              signcode(exe)
> --
> 2.25.1
>