All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] configure: warn if not using a separate build directory
@ 2020-03-31 15:51 Daniel P. Berrangé
  2020-03-31 16:05 ` Eric Blake
  2020-03-31 16:16 ` Markus Armbruster
  0 siblings, 2 replies; 6+ messages in thread
From: Daniel P. Berrangé @ 2020-03-31 15:51 UTC (permalink / raw)
  To: qemu-devel
  Cc: Kevin Wolf, Peter Maydell, Daniel P. Berrangé,
	Stefan Hajnoczi, Markus Armbruster, Liviu Ionescu, Paolo Bonzini,
	Michal Suchánek, Philippe Mathieu-Daudé,
	Aleksandar Markovic

Running configure directly from the source directory is a build
configuration that will go away in future. It is also not currently
covered by any automated testing. Display a deprecation warning if
the user attempts to use an in-srcdir build setup, so that they are
aware that they're building QEMU in an undesirable manner.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---

Changed in v2:

 - Use existing $source_path variable (Eric)
 - Remove bash-ism in comparison (Eric)
 - Safe quoting of directory paths (Eric)
 - Rename variables to reflect that we're applying canonicalization (Eric)

 configure | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/configure b/configure
index e225a1e3ff..5991e0e6e5 100755
--- a/configure
+++ b/configure
@@ -285,6 +285,16 @@ then
   error_exit "main directory cannot contain spaces nor colons"
 fi
 
+canon_build_path=$(realpath -- "$PWD")
+canon_source_path=$(realpath -- "$source_path")
+
+in_srcdir=no
+if [ "$canon_build_path" = "$canon_source_path" ]
+then
+    in_srcdir=yes
+fi
+
+
 # default parameters
 cpu=""
 iasl="iasl"
@@ -6799,6 +6809,23 @@ if test "$supported_os" = "no"; then
     echo "us upstream at qemu-devel@nongnu.org."
 fi
 
+if test "$in_srcdir" = "yes"; then
+    echo
+    echo "WARNING: SUPPORT FOR IN SOURCE DIR BUILDS IS DEPRECATED"
+    echo
+    echo "Support for running the 'configure' script directly from the"
+    echo "source directory is deprecated and will go away in a future"
+    echo "release. In source dir builds are not covered by automated"
+    echo "testing and are liable to break without warning. Users are"
+    echo "strongly recommended to switch to a separate build directory:"
+    echo
+    echo "  $ mkdir build"
+    echo "  $ cd build"
+    echo "  $ ../configure"
+    echo "  $ make"
+    echo
+fi
+
 config_host_mak="config-host.mak"
 
 echo "# Automatically generated by configure - do not modify" >config-all-disas.mak
-- 
2.24.1



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] configure: warn if not using a separate build directory
  2020-03-31 15:51 [PATCH v2] configure: warn if not using a separate build directory Daniel P. Berrangé
@ 2020-03-31 16:05 ` Eric Blake
  2020-03-31 16:12   ` Peter Maydell
  2020-03-31 16:16 ` Markus Armbruster
  1 sibling, 1 reply; 6+ messages in thread
From: Eric Blake @ 2020-03-31 16:05 UTC (permalink / raw)
  To: Daniel P. Berrangé, qemu-devel
  Cc: Kevin Wolf, Peter Maydell, Stefan Hajnoczi, Markus Armbruster,
	Liviu Ionescu, Paolo Bonzini, Michal Suchánek,
	Philippe Mathieu-Daudé,
	Aleksandar Markovic

On 3/31/20 10:51 AM, Daniel P. Berrangé wrote:
> Running configure directly from the source directory is a build
> configuration that will go away in future. It is also not currently
> covered by any automated testing. Display a deprecation warning if
> the user attempts to use an in-srcdir build setup, so that they are
> aware that they're building QEMU in an undesirable manner.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
> 
> Changed in v2:
> 
>   - Use existing $source_path variable (Eric)

Phillipe, actually - but this line isn't permanent.

>   - Remove bash-ism in comparison (Eric)
>   - Safe quoting of directory paths (Eric)
>   - Rename variables to reflect that we're applying canonicalization (Eric)
> 
>   configure | 27 +++++++++++++++++++++++++++
>   1 file changed, 27 insertions(+)
> 
> diff --git a/configure b/configure
> index e225a1e3ff..5991e0e6e5 100755
> --- a/configure
> +++ b/configure
> @@ -285,6 +285,16 @@ then
>     error_exit "main directory cannot contain spaces nor colons"
>   fi
>   
> +canon_build_path=$(realpath -- "$PWD")
> +canon_source_path=$(realpath -- "$source_path")
> +
> +in_srcdir=no
> +if [ "$canon_build_path" = "$canon_source_path" ]
> +then
> +    in_srcdir=yes
> +fi

This part is good.

> +
> +
>   # default parameters
>   cpu=""
>   iasl="iasl"
> @@ -6799,6 +6809,23 @@ if test "$supported_os" = "no"; then
>       echo "us upstream at qemu-devel@nongnu.org."
>   fi
>   
> +if test "$in_srcdir" = "yes"; then
> +    echo
> +    echo "WARNING: SUPPORT FOR IN SOURCE DIR BUILDS IS DEPRECATED"
> +    echo
> +    echo "Support for running the 'configure' script directly from the"
> +    echo "source directory is deprecated and will go away in a future"
> +    echo "release. In source dir builds are not covered by automated"
> +    echo "testing and are liable to break without warning. Users are"
> +    echo "strongly recommended to switch to a separate build directory:"

Per Kevin's response, we may want to tweak this wording slightly; maybe:

Support for running the 'configure' script directly from the source 
directory is deprecated.  In-tree builds are not covered by automated 
testing and are liable to break without warning.  Future releases may 
change the default location of built executables for an in-tree build, 
or drop in-tree build support altogether.  Users are strongly 
recommended to switch to a separate build directory:

> +    echo
> +    echo "  $ mkdir build"
> +    echo "  $ cd build"
> +    echo "  $ ../configure"
> +    echo "  $ make"
> +    echo
> +fi
> +
>   config_host_mak="config-host.mak"
>   
>   echo "# Automatically generated by configure - do not modify" >config-all-disas.mak
> 

Whether we keep your wording or switch to somthing based on mine,
Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] configure: warn if not using a separate build directory
  2020-03-31 16:05 ` Eric Blake
@ 2020-03-31 16:12   ` Peter Maydell
  2020-03-31 16:18     ` Daniel P. Berrangé
  0 siblings, 1 reply; 6+ messages in thread
From: Peter Maydell @ 2020-03-31 16:12 UTC (permalink / raw)
  To: Eric Blake
  Cc: Kevin Wolf, Daniel P. Berrangé,
	Liviu Ionescu, QEMU Developers, Markus Armbruster,
	Stefan Hajnoczi, Paolo Bonzini, Michal Suchánek,
	Philippe Mathieu-Daudé,
	Aleksandar Markovic

On Tue, 31 Mar 2020 at 17:05, Eric Blake <eblake@redhat.com> wrote:
>
> On 3/31/20 10:51 AM, Daniel P. Berrangé wrote:
> > +if test "$in_srcdir" = "yes"; then
> > +    echo
> > +    echo "WARNING: SUPPORT FOR IN SOURCE DIR BUILDS IS DEPRECATED"
> > +    echo
> > +    echo "Support for running the 'configure' script directly from the"
> > +    echo "source directory is deprecated and will go away in a future"
> > +    echo "release. In source dir builds are not covered by automated"
> > +    echo "testing and are liable to break without warning. Users are"
> > +    echo "strongly recommended to switch to a separate build directory:"
>
> Per Kevin's response, we may want to tweak this wording slightly; maybe:
>
> Support for running the 'configure' script directly from the source
> directory is deprecated.  In-tree builds are not covered by automated
> testing and are liable to break without warning.  Future releases may
> change the default location of built executables for an in-tree build,
> or drop in-tree build support altogether.  Users are strongly
> recommended to switch to a separate build directory:

I now feel that, given that we've had various people say they
want to retain at least the basic in-tree build commands and are
willing to put in a wrapper to make it keep working, that this
deprecation wording is a bit strong. I don't think that it
much serves our users to say "this will go away" and then
implement the compatibility wrapper -- we've caused them
to unnecessarily change what they're doing, and then reduced
the value of the compat-wrapper work we do. A more gently
phrased note that we recommend use of out-of-tree builds
would be sufficient I think.

(That is, the idea of notes in the release notes and configure
is to signpost to our users the direction we think the
project is going in, so they are not unpleasantly surprised
in future. For this to work the signpost should be pointing
reasonably in the direction we plan to head, rather than somewhat
askew from it :-))

thanks
-- PMM


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] configure: warn if not using a separate build directory
  2020-03-31 15:51 [PATCH v2] configure: warn if not using a separate build directory Daniel P. Berrangé
  2020-03-31 16:05 ` Eric Blake
@ 2020-03-31 16:16 ` Markus Armbruster
  2020-03-31 17:12   ` Eric Blake
  1 sibling, 1 reply; 6+ messages in thread
From: Markus Armbruster @ 2020-03-31 16:16 UTC (permalink / raw)
  To: Daniel P. Berrangé
  Cc: Kevin Wolf, Peter Maydell, Liviu Ionescu, qemu-devel,
	Stefan Hajnoczi, Paolo Bonzini, Michal Suchánek,
	Philippe Mathieu-Daudé,
	Aleksandar Markovic

Daniel P. Berrangé <berrange@redhat.com> writes:

> Running configure directly from the source directory is a build
> configuration that will go away in future. It is also not currently
> covered by any automated testing. Display a deprecation warning if
> the user attempts to use an in-srcdir build setup, so that they are
> aware that they're building QEMU in an undesirable manner.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>
> Changed in v2:
>
>  - Use existing $source_path variable (Eric)
>  - Remove bash-ism in comparison (Eric)
>  - Safe quoting of directory paths (Eric)
>  - Rename variables to reflect that we're applying canonicalization (Eric)
>
>  configure | 27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
>
> diff --git a/configure b/configure
> index e225a1e3ff..5991e0e6e5 100755
> --- a/configure
> +++ b/configure
> @@ -285,6 +285,16 @@ then
>    error_exit "main directory cannot contain spaces nor colons"
>  fi
>  
> +canon_build_path=$(realpath -- "$PWD")
> +canon_source_path=$(realpath -- "$source_path")
> +
> +in_srcdir=no
> +if [ "$canon_build_path" = "$canon_source_path" ]
> +then
> +    in_srcdir=yes
> +fi
> +
> +
>  # default parameters
>  cpu=""
>  iasl="iasl"
> @@ -6799,6 +6809,23 @@ if test "$supported_os" = "no"; then
>      echo "us upstream at qemu-devel@nongnu.org."
>  fi
>  
> +if test "$in_srcdir" = "yes"; then
> +    echo
> +    echo "WARNING: SUPPORT FOR IN SOURCE DIR BUILDS IS DEPRECATED"

I don't like shouted warnings, but it's consistent with what we have.

> +    echo
> +    echo "Support for running the 'configure' script directly from the"
> +    echo "source directory is deprecated and will go away in a future"
> +    echo "release. In source dir builds are not covered by automated"
> +    echo "testing and are liable to break without warning. Users are"
> +    echo "strongly recommended to switch to a separate build directory:"
> +    echo
> +    echo "  $ mkdir build"
> +    echo "  $ cd build"
> +    echo "  $ ../configure"
> +    echo "  $ make"
> +    echo
> +fi
> +
>  config_host_mak="config-host.mak"
>  
>  echo "# Automatically generated by configure - do not modify" >config-all-disas.mak

Output is suboptimal because we run $source_path/tests/tcg/configure.sh
after emitting warnings, and it's chatty:

    [...]
    plugin support    no
    fuzzing support   no
    gdb               /usr/bin/gdb

    WARNING: SUPPORT FOR IN SOURCE DIR BUILDS IS DEPRECATED

    Support for running the 'configure' script directly from the
    source directory is deprecated and will go away in a future
    release. In source dir builds are not covered by automated
    testing and are liable to break without warning. Users are
    strongly recommended to switch to a separate build directory:

      $ mkdir build
      $ cd build
      $ ../configure
      $ make

--> cross containers  podman

    NOTE: guest cross-compilers enabled: cc cc

Not this patch's fault.

Reviewed-by: Markus Armbruster <armbru@redhat.com>



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] configure: warn if not using a separate build directory
  2020-03-31 16:12   ` Peter Maydell
@ 2020-03-31 16:18     ` Daniel P. Berrangé
  0 siblings, 0 replies; 6+ messages in thread
From: Daniel P. Berrangé @ 2020-03-31 16:18 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Kevin Wolf, Liviu Ionescu, Philippe Mathieu-Daudé,
	QEMU Developers, Markus Armbruster, Stefan Hajnoczi,
	Paolo Bonzini, Michal Suchánek, Aleksandar Markovic

On Tue, Mar 31, 2020 at 05:12:35PM +0100, Peter Maydell wrote:
> On Tue, 31 Mar 2020 at 17:05, Eric Blake <eblake@redhat.com> wrote:
> >
> > On 3/31/20 10:51 AM, Daniel P. Berrangé wrote:
> > > +if test "$in_srcdir" = "yes"; then
> > > +    echo
> > > +    echo "WARNING: SUPPORT FOR IN SOURCE DIR BUILDS IS DEPRECATED"
> > > +    echo
> > > +    echo "Support for running the 'configure' script directly from the"
> > > +    echo "source directory is deprecated and will go away in a future"
> > > +    echo "release. In source dir builds are not covered by automated"
> > > +    echo "testing and are liable to break without warning. Users are"
> > > +    echo "strongly recommended to switch to a separate build directory:"
> >
> > Per Kevin's response, we may want to tweak this wording slightly; maybe:
> >
> > Support for running the 'configure' script directly from the source
> > directory is deprecated.  In-tree builds are not covered by automated
> > testing and are liable to break without warning.  Future releases may
> > change the default location of built executables for an in-tree build,
> > or drop in-tree build support altogether.  Users are strongly
> > recommended to switch to a separate build directory:
> 
> I now feel that, given that we've had various people say they
> want to retain at least the basic in-tree build commands and are
> willing to put in a wrapper to make it keep working, that this
> deprecation wording is a bit strong. I don't think that it
> much serves our users to say "this will go away" and then
> implement the compatibility wrapper -- we've caused them
> to unnecessarily change what they're doing, and then reduced
> the value of the compat-wrapper work we do. A more gently
> phrased note that we recommend use of out-of-tree builds
> would be sufficient I think.

Ok, i'll resend with a simpler warning message which tries to avoid
fortelling the future and just focuses on today.


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] configure: warn if not using a separate build directory
  2020-03-31 16:16 ` Markus Armbruster
@ 2020-03-31 17:12   ` Eric Blake
  0 siblings, 0 replies; 6+ messages in thread
From: Eric Blake @ 2020-03-31 17:12 UTC (permalink / raw)
  To: Markus Armbruster, Daniel P. Berrangé
  Cc: Kevin Wolf, Peter Maydell, Stefan Hajnoczi, qemu-devel,
	Liviu Ionescu, Paolo Bonzini, Michal Suchánek,
	Philippe Mathieu-Daudé,
	Aleksandar Markovic

On 3/31/20 11:16 AM, Markus Armbruster wrote:
> Daniel P. Berrangé <berrange@redhat.com> writes:
> 
>> Running configure directly from the source directory is a build
>> configuration that will go away in future. It is also not currently
>> covered by any automated testing. Display a deprecation warning if
>> the user attempts to use an in-srcdir build setup, so that they are
>> aware that they're building QEMU in an undesirable manner.
>>
>> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
>> ---
>>

>> +if test "$in_srcdir" = "yes"; then
>> +    echo
>> +    echo "WARNING: SUPPORT FOR IN SOURCE DIR BUILDS IS DEPRECATED"
> 
> I don't like shouted warnings, but it's consistent with what we have.

The grammar is also a bit awkward to parse; maybe:

support for building in the source directory is deprecated

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org



^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-03-31 17:13 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-31 15:51 [PATCH v2] configure: warn if not using a separate build directory Daniel P. Berrangé
2020-03-31 16:05 ` Eric Blake
2020-03-31 16:12   ` Peter Maydell
2020-03-31 16:18     ` Daniel P. Berrangé
2020-03-31 16:16 ` Markus Armbruster
2020-03-31 17:12   ` Eric Blake

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.