* [PATCH] Update scripts/meson-buildoptions.sh
@ 2023-01-02 10:41 Alessandro Di Federico via
2023-01-03 14:14 ` Thomas Huth
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Alessandro Di Federico via @ 2023-01-02 10:41 UTC (permalink / raw)
To: qemu-devel
Cc: Thomas Huth, Taylor Simpson, Anton Johansson, philmd,
peter.maydell, bcain, quic_mathbern, stefanha,
Alessandro Di Federico
Note: `Makefile` relies on modification dates in the source tree to
detect changes to `meson_options.txt`. However, git does not track
those. Therefore, the following was necessary to regenerate
`meson-buildoptions.sh`:
touch meson_options.txt
cd "$BUILD_DIR"
make update-buildoptions
Signed-off-by: Alessandro Di Federico <ale@rev.ng>
---
scripts/meson-buildoptions.sh | 18 ++++++++++++------
1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/scripts/meson-buildoptions.sh b/scripts/meson-buildoptions.sh
index aa6e30ea911..0f71e92dcba 100644
--- a/scripts/meson-buildoptions.sh
+++ b/scripts/meson-buildoptions.sh
@@ -10,6 +10,9 @@ meson_options_help() {
printf "%s\n" ' affects only QEMU, not tools like qemu-img)'
printf "%s\n" ' --datadir=VALUE Data file directory [share]'
printf "%s\n" ' --disable-coroutine-pool coroutine freelist (better performance)'
+ printf "%s\n" ' --disable-hexagon-idef-parser'
+ printf "%s\n" ' use idef-parser to automatically generate TCG'
+ printf "%s\n" ' code for the Hexagon frontend'
printf "%s\n" ' --disable-install-blobs install provided firmware blobs'
printf "%s\n" ' --docdir=VALUE Base directory for documentation installation'
printf "%s\n" ' (can be empty) [share/doc]'
@@ -40,7 +43,8 @@ meson_options_help() {
printf "%s\n" ' --enable-trace-backends=CHOICES'
printf "%s\n" ' Set available tracing backends [log] (choices:'
printf "%s\n" ' dtrace/ftrace/log/nop/simple/syslog/ust)'
- printf "%s\n" ' --firmwarepath=VALUES search PATH for firmware files [share/qemu-firmware]'
+ printf "%s\n" ' --firmwarepath=VALUES search PATH for firmware files [share/qemu-'
+ printf "%s\n" ' firmware]'
printf "%s\n" ' --iasl=VALUE Path to ACPI disassembler'
printf "%s\n" ' --includedir=VALUE Header file directory [include]'
printf "%s\n" ' --interp-prefix=VALUE where to find shared libraries etc., use %M for'
@@ -93,7 +97,7 @@ meson_options_help() {
printf "%s\n" ' glusterfs Glusterfs block device driver'
printf "%s\n" ' gnutls GNUTLS cryptography support'
printf "%s\n" ' gtk GTK+ user interface'
- printf "%s\n" ' gtk-clipboard clipboard support for GTK (EXPERIMENTAL, MAY HANG)'
+ printf "%s\n" ' gtk-clipboard clipboard support for the gtk UI (EXPERIMENTAL, MAY HANG)'
printf "%s\n" ' guest-agent Build QEMU Guest Agent'
printf "%s\n" ' guest-agent-msi Build MSI package for the QEMU Guest Agent'
printf "%s\n" ' hax HAX acceleration support'
@@ -156,6 +160,8 @@ meson_options_help() {
printf "%s\n" ' usb-redir libusbredir support'
printf "%s\n" ' vde vde network backend support'
printf "%s\n" ' vdi vdi image format support'
+ printf "%s\n" ' vduse-blk-export'
+ printf "%s\n" ' VDUSE block export support'
printf "%s\n" ' vfio-user-server'
printf "%s\n" ' vfio-user server support'
printf "%s\n" ' vhost-crypto vhost-user crypto backend support'
@@ -164,8 +170,6 @@ meson_options_help() {
printf "%s\n" ' vhost-user vhost-user backend support'
printf "%s\n" ' vhost-user-blk-server'
printf "%s\n" ' build vhost-user-blk server'
- printf "%s\n" ' vduse-blk-export'
- printf "%s\n" ' VDUSE block export support'
printf "%s\n" ' vhost-vdpa vhost-vdpa kernel backend support'
printf "%s\n" ' virglrenderer virgl rendering support'
printf "%s\n" ' virtfs virtio-9p support'
@@ -283,6 +287,8 @@ _meson_option_parse() {
--disable-guest-agent-msi) printf "%s" -Dguest_agent_msi=disabled ;;
--enable-hax) printf "%s" -Dhax=enabled ;;
--disable-hax) printf "%s" -Dhax=disabled ;;
+ --enable-hexagon-idef-parser) printf "%s" -Dhexagon_idef_parser=true ;;
+ --disable-hexagon-idef-parser) printf "%s" -Dhexagon_idef_parser=false ;;
--enable-hvf) printf "%s" -Dhvf=enabled ;;
--disable-hvf) printf "%s" -Dhvf=disabled ;;
--iasl=*) quote_sh "-Diasl=$2" ;;
@@ -429,6 +435,8 @@ _meson_option_parse() {
--disable-vde) printf "%s" -Dvde=disabled ;;
--enable-vdi) printf "%s" -Dvdi=enabled ;;
--disable-vdi) printf "%s" -Dvdi=disabled ;;
+ --enable-vduse-blk-export) printf "%s" -Dvduse_blk_export=enabled ;;
+ --disable-vduse-blk-export) printf "%s" -Dvduse_blk_export=disabled ;;
--enable-vfio-user-server) printf "%s" -Dvfio_user_server=enabled ;;
--disable-vfio-user-server) printf "%s" -Dvfio_user_server=disabled ;;
--enable-vhost-crypto) printf "%s" -Dvhost_crypto=enabled ;;
@@ -441,8 +449,6 @@ _meson_option_parse() {
--disable-vhost-user) printf "%s" -Dvhost_user=disabled ;;
--enable-vhost-user-blk-server) printf "%s" -Dvhost_user_blk_server=enabled ;;
--disable-vhost-user-blk-server) printf "%s" -Dvhost_user_blk_server=disabled ;;
- --enable-vduse-blk-export) printf "%s" -Dvduse_blk_export=enabled ;;
- --disable-vduse-blk-export) printf "%s" -Dvduse_blk_export=disabled ;;
--enable-vhost-vdpa) printf "%s" -Dvhost_vdpa=enabled ;;
--disable-vhost-vdpa) printf "%s" -Dvhost_vdpa=disabled ;;
--enable-virglrenderer) printf "%s" -Dvirglrenderer=enabled ;;
--
2.38.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] Update scripts/meson-buildoptions.sh
2023-01-02 10:41 [PATCH] Update scripts/meson-buildoptions.sh Alessandro Di Federico via
@ 2023-01-03 14:14 ` Thomas Huth
2023-01-03 14:37 ` Stefan Hajnoczi
2023-01-04 14:39 ` Stefan Hajnoczi
2 siblings, 0 replies; 11+ messages in thread
From: Thomas Huth @ 2023-01-03 14:14 UTC (permalink / raw)
To: Alessandro Di Federico, qemu-devel
Cc: Taylor Simpson, Anton Johansson, philmd, peter.maydell, bcain,
quic_mathbern, stefanha, QEMU Trivial, Paolo Bonzini
On 02/01/2023 11.41, Alessandro Di Federico wrote:
> Note: `Makefile` relies on modification dates in the source tree to
> detect changes to `meson_options.txt`. However, git does not track
> those. Therefore, the following was necessary to regenerate
> `meson-buildoptions.sh`:
>
> touch meson_options.txt
> cd "$BUILD_DIR"
> make update-buildoptions
>
> Signed-off-by: Alessandro Di Federico <ale@rev.ng>
> ---
> scripts/meson-buildoptions.sh | 18 ++++++++++++------
> 1 file changed, 12 insertions(+), 6 deletions(-)
>
> diff --git a/scripts/meson-buildoptions.sh b/scripts/meson-buildoptions.sh
> index aa6e30ea911..0f71e92dcba 100644
> --- a/scripts/meson-buildoptions.sh
> +++ b/scripts/meson-buildoptions.sh
> @@ -10,6 +10,9 @@ meson_options_help() {
> printf "%s\n" ' affects only QEMU, not tools like qemu-img)'
> printf "%s\n" ' --datadir=VALUE Data file directory [share]'
> printf "%s\n" ' --disable-coroutine-pool coroutine freelist (better performance)'
> + printf "%s\n" ' --disable-hexagon-idef-parser'
> + printf "%s\n" ' use idef-parser to automatically generate TCG'
> + printf "%s\n" ' code for the Hexagon frontend'
> printf "%s\n" ' --disable-install-blobs install provided firmware blobs'
> printf "%s\n" ' --docdir=VALUE Base directory for documentation installation'
> printf "%s\n" ' (can be empty) [share/doc]'
> @@ -40,7 +43,8 @@ meson_options_help() {
> printf "%s\n" ' --enable-trace-backends=CHOICES'
> printf "%s\n" ' Set available tracing backends [log] (choices:'
> printf "%s\n" ' dtrace/ftrace/log/nop/simple/syslog/ust)'
> - printf "%s\n" ' --firmwarepath=VALUES search PATH for firmware files [share/qemu-firmware]'
> + printf "%s\n" ' --firmwarepath=VALUES search PATH for firmware files [share/qemu-'
> + printf "%s\n" ' firmware]'
> printf "%s\n" ' --iasl=VALUE Path to ACPI disassembler'
> printf "%s\n" ' --includedir=VALUE Header file directory [include]'
> printf "%s\n" ' --interp-prefix=VALUE where to find shared libraries etc., use %M for'
> @@ -93,7 +97,7 @@ meson_options_help() {
> printf "%s\n" ' glusterfs Glusterfs block device driver'
> printf "%s\n" ' gnutls GNUTLS cryptography support'
> printf "%s\n" ' gtk GTK+ user interface'
> - printf "%s\n" ' gtk-clipboard clipboard support for GTK (EXPERIMENTAL, MAY HANG)'
> + printf "%s\n" ' gtk-clipboard clipboard support for the gtk UI (EXPERIMENTAL, MAY HANG)'
> printf "%s\n" ' guest-agent Build QEMU Guest Agent'
> printf "%s\n" ' guest-agent-msi Build MSI package for the QEMU Guest Agent'
> printf "%s\n" ' hax HAX acceleration support'
> @@ -156,6 +160,8 @@ meson_options_help() {
> printf "%s\n" ' usb-redir libusbredir support'
> printf "%s\n" ' vde vde network backend support'
> printf "%s\n" ' vdi vdi image format support'
> + printf "%s\n" ' vduse-blk-export'
> + printf "%s\n" ' VDUSE block export support'
> printf "%s\n" ' vfio-user-server'
> printf "%s\n" ' vfio-user server support'
> printf "%s\n" ' vhost-crypto vhost-user crypto backend support'
> @@ -164,8 +170,6 @@ meson_options_help() {
> printf "%s\n" ' vhost-user vhost-user backend support'
> printf "%s\n" ' vhost-user-blk-server'
> printf "%s\n" ' build vhost-user-blk server'
> - printf "%s\n" ' vduse-blk-export'
> - printf "%s\n" ' VDUSE block export support'
> printf "%s\n" ' vhost-vdpa vhost-vdpa kernel backend support'
> printf "%s\n" ' virglrenderer virgl rendering support'
> printf "%s\n" ' virtfs virtio-9p support'
> @@ -283,6 +287,8 @@ _meson_option_parse() {
> --disable-guest-agent-msi) printf "%s" -Dguest_agent_msi=disabled ;;
> --enable-hax) printf "%s" -Dhax=enabled ;;
> --disable-hax) printf "%s" -Dhax=disabled ;;
> + --enable-hexagon-idef-parser) printf "%s" -Dhexagon_idef_parser=true ;;
> + --disable-hexagon-idef-parser) printf "%s" -Dhexagon_idef_parser=false ;;
> --enable-hvf) printf "%s" -Dhvf=enabled ;;
> --disable-hvf) printf "%s" -Dhvf=disabled ;;
> --iasl=*) quote_sh "-Diasl=$2" ;;
> @@ -429,6 +435,8 @@ _meson_option_parse() {
> --disable-vde) printf "%s" -Dvde=disabled ;;
> --enable-vdi) printf "%s" -Dvdi=enabled ;;
> --disable-vdi) printf "%s" -Dvdi=disabled ;;
> + --enable-vduse-blk-export) printf "%s" -Dvduse_blk_export=enabled ;;
> + --disable-vduse-blk-export) printf "%s" -Dvduse_blk_export=disabled ;;
> --enable-vfio-user-server) printf "%s" -Dvfio_user_server=enabled ;;
> --disable-vfio-user-server) printf "%s" -Dvfio_user_server=disabled ;;
> --enable-vhost-crypto) printf "%s" -Dvhost_crypto=enabled ;;
> @@ -441,8 +449,6 @@ _meson_option_parse() {
> --disable-vhost-user) printf "%s" -Dvhost_user=disabled ;;
> --enable-vhost-user-blk-server) printf "%s" -Dvhost_user_blk_server=enabled ;;
> --disable-vhost-user-blk-server) printf "%s" -Dvhost_user_blk_server=disabled ;;
> - --enable-vduse-blk-export) printf "%s" -Dvduse_blk_export=enabled ;;
> - --disable-vduse-blk-export) printf "%s" -Dvduse_blk_export=disabled ;;
> --enable-vhost-vdpa) printf "%s" -Dvhost_vdpa=enabled ;;
> --disable-vhost-vdpa) printf "%s" -Dvhost_vdpa=disabled ;;
> --enable-virglrenderer) printf "%s" -Dvirglrenderer=enabled ;;
Reviewed-by: Thomas Huth <thuth@redhat.com>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Update scripts/meson-buildoptions.sh
2023-01-02 10:41 [PATCH] Update scripts/meson-buildoptions.sh Alessandro Di Federico via
2023-01-03 14:14 ` Thomas Huth
@ 2023-01-03 14:37 ` Stefan Hajnoczi
2023-01-03 15:26 ` Alessandro Di Federico via
2023-01-04 14:39 ` Stefan Hajnoczi
2 siblings, 1 reply; 11+ messages in thread
From: Stefan Hajnoczi @ 2023-01-03 14:37 UTC (permalink / raw)
To: Alessandro Di Federico
Cc: qemu-devel, Thomas Huth, Taylor Simpson, Anton Johansson, philmd,
peter.maydell, bcain, quic_mathbern, stefanha
On Mon, 2 Jan 2023 at 05:42, Alessandro Di Federico via
<qemu-devel@nongnu.org> wrote:
>
> Note: `Makefile` relies on modification dates in the source tree to
> detect changes to `meson_options.txt`. However, git does not track
> those. Therefore, the following was necessary to regenerate
> `meson-buildoptions.sh`:
>
> touch meson_options.txt
> cd "$BUILD_DIR"
> make update-buildoptions
I don't understand the issue. Can you describe the steps that cause
meson-buildoptions.sh to become out-of-sync with meson_options.txt?
This will continue to be a problem in the future. Is there a way to
fix it permanently?
Stefan
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Update scripts/meson-buildoptions.sh
2023-01-03 14:37 ` Stefan Hajnoczi
@ 2023-01-03 15:26 ` Alessandro Di Federico via
2023-01-03 15:51 ` Stefan Hajnoczi
0 siblings, 1 reply; 11+ messages in thread
From: Alessandro Di Federico via @ 2023-01-03 15:26 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: qemu-devel, Thomas Huth, Taylor Simpson, Anton Johansson, philmd,
peter.maydell, bcain, quic_mathbern, stefanha, Paolo Bonzini
On Tue, 3 Jan 2023 09:37:51 -0500
Stefan Hajnoczi <stefanha@gmail.com> wrote:
> I don't understand the issue. Can you describe the steps that cause
> meson-buildoptions.sh to become out-of-sync with meson_options.txt?
>
> This will continue to be a problem in the future. Is there a way to
> fix it permanently?
In Makefile we have:
$(SRC_PATH)/scripts/meson-buildoptions.sh: $(SRC_PATH)/meson_options.txt
(Cc'ing Paolo since he's the author of this line)
This means make will regenerate
`$(SRC_PATH)/scripts/meson-buildoptions.sh` if its last modification
date is older than `$(SRC_PATH)/meson_options.txt`.
However these files are in the source directory, so this will behave
properly only under certain circumstances.
For instance if, for some reason, someone committed a new version of
`meson_options.txt` but not of `meson-buildoptions.sh`, a fresh clone
of the repo will not have the dates set correctly to trigger the
Makefile rule above:
$ ls -ln scripts/meson*
-rw-r--r-- 1 1000 1000 28913 Jan 3 15:58 scripts/meson-buildoptions.sh
-rw-r--r-- 1 1000 1000 91 Jan 3 15:58 scripts/meson.build
This is because git does not update file dates depending on the last
commit changing them.
This, on top of the fact that invoking `ninja` does not trigger
regeneration (which works for most other use cases), leads to a good
chance of forgetting to update meson-buildoptions.sh.
We could add the target to ninja to mitigate the risk, but still, the
dates problem remains.
An alternative solution would be to avoid committing generated files and
simply regenerating it every time.
On my machine `meson.py introspect --buildoptions` +
`scripts/meson-buildoptions.py` take 1.070s.
`./configure --help` takes 0.162s, so it's a bit sad.
On the other hand an actual invocation of configure can take
significantly longer (`./configure` takes 29.150s on my machine).
To avoid re-running it every time we could invoke `make
update-buildoptions` in `configure` but keep
`scripts/meson-buildoptions.sh` in the build directory.
--
Alessandro Di Federico
rev.ng Labs
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Update scripts/meson-buildoptions.sh
2023-01-03 15:26 ` Alessandro Di Federico via
@ 2023-01-03 15:51 ` Stefan Hajnoczi
2023-01-03 16:11 ` Alessandro Di Federico via
0 siblings, 1 reply; 11+ messages in thread
From: Stefan Hajnoczi @ 2023-01-03 15:51 UTC (permalink / raw)
To: Alessandro Di Federico
Cc: qemu-devel, Thomas Huth, Taylor Simpson, Anton Johansson, philmd,
peter.maydell, bcain, quic_mathbern, stefanha, Paolo Bonzini
On Tue, 3 Jan 2023 at 10:26, Alessandro Di Federico <ale@rev.ng> wrote:
>
> On Tue, 3 Jan 2023 09:37:51 -0500
> Stefan Hajnoczi <stefanha@gmail.com> wrote:
>
> > I don't understand the issue. Can you describe the steps that cause
> > meson-buildoptions.sh to become out-of-sync with meson_options.txt?
> >
> > This will continue to be a problem in the future. Is there a way to
> > fix it permanently?
>
> In Makefile we have:
>
> $(SRC_PATH)/scripts/meson-buildoptions.sh: $(SRC_PATH)/meson_options.txt
>
> (Cc'ing Paolo since he's the author of this line)
>
> This means make will regenerate
> `$(SRC_PATH)/scripts/meson-buildoptions.sh` if its last modification
> date is older than `$(SRC_PATH)/meson_options.txt`.
>
> However these files are in the source directory, so this will behave
> properly only under certain circumstances.
>
> For instance if, for some reason, someone committed a new version of
> `meson_options.txt` but not of `meson-buildoptions.sh`, a fresh clone
> of the repo will not have the dates set correctly to trigger the
> Makefile rule above:
>
> $ ls -ln scripts/meson*
> -rw-r--r-- 1 1000 1000 28913 Jan 3 15:58 scripts/meson-buildoptions.sh
> -rw-r--r-- 1 1000 1000 91 Jan 3 15:58 scripts/meson.build
>
> This is because git does not update file dates depending on the last
> commit changing them.
>
> This, on top of the fact that invoking `ninja` does not trigger
> regeneration (which works for most other use cases), leads to a good
> chance of forgetting to update meson-buildoptions.sh.
>
> We could add the target to ninja to mitigate the risk, but still, the
> dates problem remains.
>
> An alternative solution would be to avoid committing generated files and
> simply regenerating it every time.
>
> On my machine `meson.py introspect --buildoptions` +
> `scripts/meson-buildoptions.py` take 1.070s.
> `./configure --help` takes 0.162s, so it's a bit sad.
> On the other hand an actual invocation of configure can take
> significantly longer (`./configure` takes 29.150s on my machine).
>
> To avoid re-running it every time we could invoke `make
> update-buildoptions` in `configure` but keep
> `scripts/meson-buildoptions.sh` in the build directory.
QEMU's Makefile used to a use a technique where it generated
"timestamp" files and used cmp(1) to check if rebuilding was
necessary:
1. Always generate meson-buildoptions.sh-timestamp.
2. If cmp meson-buildoptions.sh-timestamp meson-buildoptions.sh
detects a difference, cp meson-buildoptions.sh-timestamp
meson-buildoptions.sh.
3. Let make handle dependencies on meson-buildoptions.sh as usual.
You can find examples by grepping for -timestamp in the git log -p output.
I think this would solve the problem?
Stefan
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Update scripts/meson-buildoptions.sh
2023-01-03 15:51 ` Stefan Hajnoczi
@ 2023-01-03 16:11 ` Alessandro Di Federico via
2023-01-03 17:30 ` Peter Maydell
0 siblings, 1 reply; 11+ messages in thread
From: Alessandro Di Federico via @ 2023-01-03 16:11 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: qemu-devel, Thomas Huth, Taylor Simpson, Anton Johansson, philmd,
peter.maydell, bcain, quic_mathbern, stefanha, Paolo Bonzini
On Tue, 3 Jan 2023 10:51:36 -0500
Stefan Hajnoczi <stefanha@gmail.com> wrote:
> QEMU's Makefile used to a use a technique where it generated
> "timestamp" files and used cmp(1) to check if rebuilding was
> necessary:
> 1. Always generate meson-buildoptions.sh-timestamp.
`meson-buildoptions.sh-timestamp` would be the full expected output,
right? It's not just a date or something.
AFAIU that would make sure that if nothing changed in the output you
don't trigger other targets depending on `meson-buildoptions.sh`. It's
a solution for a different problem.
The problem with always rebuilding `meson-buildoptions.sh` is that we
spend 1 extra second on every build, even those that doesn't need to
rebuild anything else.
Not unacceptable, but I think we should strive not to commit generated
files and move the file to the build directory, unless there's a reason
why this is not viable that I'm not seeing.
--
Alessandro Di Federico
rev.ng Labs
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Update scripts/meson-buildoptions.sh
2023-01-03 16:11 ` Alessandro Di Federico via
@ 2023-01-03 17:30 ` Peter Maydell
2023-01-03 19:31 ` Stefan Hajnoczi
0 siblings, 1 reply; 11+ messages in thread
From: Peter Maydell @ 2023-01-03 17:30 UTC (permalink / raw)
To: Alessandro Di Federico
Cc: Stefan Hajnoczi, qemu-devel, Thomas Huth, Taylor Simpson,
Anton Johansson, philmd, bcain, quic_mathbern, stefanha,
Paolo Bonzini
On Tue, 3 Jan 2023 at 16:12, Alessandro Di Federico <ale@rev.ng> wrote:
>
> On Tue, 3 Jan 2023 10:51:36 -0500
> Stefan Hajnoczi <stefanha@gmail.com> wrote:
>
> > QEMU's Makefile used to a use a technique where it generated
> > "timestamp" files and used cmp(1) to check if rebuilding was
> > necessary:
> > 1. Always generate meson-buildoptions.sh-timestamp.
>
> `meson-buildoptions.sh-timestamp` would be the full expected output,
> right? It's not just a date or something.
> AFAIU that would make sure that if nothing changed in the output you
> don't trigger other targets depending on `meson-buildoptions.sh`. It's
> a solution for a different problem.
>
> The problem with always rebuilding `meson-buildoptions.sh` is that we
> spend 1 extra second on every build, even those that doesn't need to
> rebuild anything else.
> Not unacceptable, but I think we should strive not to commit generated
> files and move the file to the build directory, unless there's a reason
> why this is not viable that I'm not seeing.
The other problem with this file is that it appears to
be generated differently depending on the host distro
(specifically the default value for the --libdir option).
That also would seem to nudge towards "don't commit a
generated file".
thanks
-- PMM
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Update scripts/meson-buildoptions.sh
2023-01-03 17:30 ` Peter Maydell
@ 2023-01-03 19:31 ` Stefan Hajnoczi
2023-01-07 18:02 ` Paolo Bonzini
0 siblings, 1 reply; 11+ messages in thread
From: Stefan Hajnoczi @ 2023-01-03 19:31 UTC (permalink / raw)
To: Peter Maydell
Cc: Alessandro Di Federico, qemu-devel, Thomas Huth, Taylor Simpson,
Anton Johansson, philmd, bcain, quic_mathbern, stefanha,
Paolo Bonzini
On Tue, 3 Jan 2023 at 12:31, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Tue, 3 Jan 2023 at 16:12, Alessandro Di Federico <ale@rev.ng> wrote:
> >
> > On Tue, 3 Jan 2023 10:51:36 -0500
> > Stefan Hajnoczi <stefanha@gmail.com> wrote:
> >
> > > QEMU's Makefile used to a use a technique where it generated
> > > "timestamp" files and used cmp(1) to check if rebuilding was
> > > necessary:
> > > 1. Always generate meson-buildoptions.sh-timestamp.
> >
> > `meson-buildoptions.sh-timestamp` would be the full expected output,
> > right? It's not just a date or something.
> > AFAIU that would make sure that if nothing changed in the output you
> > don't trigger other targets depending on `meson-buildoptions.sh`. It's
> > a solution for a different problem.
> >
> > The problem with always rebuilding `meson-buildoptions.sh` is that we
> > spend 1 extra second on every build, even those that doesn't need to
> > rebuild anything else.
> > Not unacceptable, but I think we should strive not to commit generated
> > files and move the file to the build directory, unless there's a reason
> > why this is not viable that I'm not seeing.
>
> The other problem with this file is that it appears to
> be generated differently depending on the host distro
> (specifically the default value for the --libdir option).
> That also would seem to nudge towards "don't commit a
> generated file".
Paolo: Is the meson-buildoptions.sh approach a temporary solution or
something long-term? Maybe everything can be migrated to meson
eventually so that ./configure and meson-buildoptions.sh are no longer
necessary?
Stefan
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Update scripts/meson-buildoptions.sh
2023-01-03 19:31 ` Stefan Hajnoczi
@ 2023-01-07 18:02 ` Paolo Bonzini
2023-01-09 1:59 ` Richard Henderson
0 siblings, 1 reply; 11+ messages in thread
From: Paolo Bonzini @ 2023-01-07 18:02 UTC (permalink / raw)
To: Stefan Hajnoczi, Peter Maydell
Cc: Alessandro Di Federico, qemu-devel, Thomas Huth, Taylor Simpson,
Anton Johansson, philmd, bcain, quic_mathbern, stefanha
On 1/3/23 20:31, Stefan Hajnoczi wrote:
>> The other problem with this file is that it appears to
>> be generated differently depending on the host distro
>> (specifically the default value for the --libdir option).
>> That also would seem to nudge towards "don't commit a
>> generated file".
I wasn't aware of the libdir default that Peter mentioned, but the same
issue would happen for release tarballs so "do not commit it" would have
to be extended to "do not ship it", that is do everything in Python.
This in turn goes back to the reason for the buildoptions.sh approach:
the path to the Python binary is not known when "configure --help"
prints the help message. If the user does not have a python3 or meson
binary in the path, requiring "configure --meson=... --help" or
"configure --python=... --help" is not hideous I guess, but not pretty
either.
> Paolo: Is the meson-buildoptions.sh approach a temporary solution or
> something long-term? Maybe everything can be migrated to meson
> eventually so that ./configure and meson-buildoptions.sh are no longer
> necessary?
It is long-term. Meson is only used to build the emulators and that
part will move entirely out of configure soon, but other parts of the
build (especially tests/tcg and firmware, which need to build docker
containers for cross-compilation) are separate and there's no plan to
use anything but configure/Makefile for the overall orchestration.
While I have noticed stale meson-buildoptions.sh it's never happened to
me. I ascribed it to someone not noticing the dirty file rather than a
bug; it should be possible to add a test to CI that catches it.
Paolo
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Update scripts/meson-buildoptions.sh
2023-01-07 18:02 ` Paolo Bonzini
@ 2023-01-09 1:59 ` Richard Henderson
0 siblings, 0 replies; 11+ messages in thread
From: Richard Henderson @ 2023-01-09 1:59 UTC (permalink / raw)
To: Paolo Bonzini, Stefan Hajnoczi, Peter Maydell
Cc: Alessandro Di Federico, qemu-devel, Thomas Huth, Taylor Simpson,
Anton Johansson, philmd, bcain, quic_mathbern, stefanha
On 1/7/23 10:02, Paolo Bonzini wrote:
> On 1/3/23 20:31, Stefan Hajnoczi wrote:
>>> The other problem with this file is that it appears to
>>> be generated differently depending on the host distro
>>> (specifically the default value for the --libdir option).
>>> That also would seem to nudge towards "don't commit a
>>> generated file".
>
> I wasn't aware of the libdir default that Peter mentioned, but the same issue would happen
> for release tarballs so "do not commit it" would have to be extended to "do not ship it",
> that is do everything in Python.
>
> This in turn goes back to the reason for the buildoptions.sh approach: the path to the
> Python binary is not known when "configure --help" prints the help message. If the user
> does not have a python3 or meson binary in the path, requiring "configure --meson=...
> --help" or "configure --python=... --help" is not hideous I guess, but not pretty either.
I think an extremely early error message for missing python (and pointer to --python) is
perfectly reasonable. I imagine it would be very rare, a case of forgetting to install
all of the build dependencies into a fresh container.
r~
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Update scripts/meson-buildoptions.sh
2023-01-02 10:41 [PATCH] Update scripts/meson-buildoptions.sh Alessandro Di Federico via
2023-01-03 14:14 ` Thomas Huth
2023-01-03 14:37 ` Stefan Hajnoczi
@ 2023-01-04 14:39 ` Stefan Hajnoczi
2 siblings, 0 replies; 11+ messages in thread
From: Stefan Hajnoczi @ 2023-01-04 14:39 UTC (permalink / raw)
To: Alessandro Di Federico
Cc: qemu-devel, Thomas Huth, Taylor Simpson, Anton Johansson, philmd,
peter.maydell, bcain, quic_mathbern
[-- Attachment #1: Type: text/plain, Size: 686 bytes --]
On Mon, Jan 02, 2023 at 11:41:13AM +0100, Alessandro Di Federico wrote:
> Note: `Makefile` relies on modification dates in the source tree to
> detect changes to `meson_options.txt`. However, git does not track
> those. Therefore, the following was necessary to regenerate
> `meson-buildoptions.sh`:
>
> touch meson_options.txt
> cd "$BUILD_DIR"
> make update-buildoptions
>
> Signed-off-by: Alessandro Di Federico <ale@rev.ng>
> ---
> scripts/meson-buildoptions.sh | 18 ++++++++++++------
> 1 file changed, 12 insertions(+), 6 deletions(-)
The discussion we're having is independent of this patch:
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2023-01-09 1:59 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-01-02 10:41 [PATCH] Update scripts/meson-buildoptions.sh Alessandro Di Federico via
2023-01-03 14:14 ` Thomas Huth
2023-01-03 14:37 ` Stefan Hajnoczi
2023-01-03 15:26 ` Alessandro Di Federico via
2023-01-03 15:51 ` Stefan Hajnoczi
2023-01-03 16:11 ` Alessandro Di Federico via
2023-01-03 17:30 ` Peter Maydell
2023-01-03 19:31 ` Stefan Hajnoczi
2023-01-07 18:02 ` Paolo Bonzini
2023-01-09 1:59 ` Richard Henderson
2023-01-04 14:39 ` Stefan Hajnoczi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).