[PATCH v5 for-5.0] configure: warn if not using a separate build directory

[PATCH v5 for-5.0] configure: warn if not using a separate build directory

[Daniel P. Berrangé]

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.

Reviewed-by: Aleksandar Markovic <aleksandar.qemu.devel@gmail.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---

Changed in v5:

 - Use PeterM's suggested wording instead.
 - Dropped previous R-bs due to wording change

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

diff --git a/configure b/configure
index 22870f3867..b8f1d26293 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"
@@ -6751,6 +6761,23 @@ if test "$supported_os" = "no"; then
     echo "us upstream at qemu-devel@nongnu.org."
 fi
 
+if test "$in_srcdir" = "yes"; then
+    echo
+    echo "NOTE: we recommend against building in the source directory"
+    echo
+    echo "You've run the 'configure' script directly from the source"
+    echo "directory. This will work, but we recommend using a separate"
+    echo "build directory, especially if you plan to work with the QEMU"
+    echo "sources rather than just building it once. You can switch to"
+    echo "a separate build directory like this:"
+    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

Re: [PATCH v5 for-5.0] configure: warn if not using a separate build directory

[Eric Blake]

On 4/6/20 10:33 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

Calling it a deprecation warning may be overkill now that we've toned 
down the language.

> the user attempts to use an in-srcdir build setup, so that they are
> aware that they're building QEMU in an undesirable manner.
> 
> Reviewed-by: Aleksandar Markovic <aleksandar.qemu.devel@gmail.com>
> Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
> 

> +if test "$in_srcdir" = "yes"; then
> +    echo
> +    echo "NOTE: we recommend against building in the source directory"
> +    echo
> +    echo "You've run the 'configure' script directly from the source"
> +    echo "directory. This will work, but we recommend using a separate"
> +    echo "build directory, especially if you plan to work with the QEMU"
> +    echo "sources rather than just building it once. You can switch to"
> +    echo "a separate build directory like this:"
> +    echo
> +    echo "  $ mkdir build"

As I pointed out on v4, this is missing a step.  Since this is just a 
warning and not fatal, './configure' completed and polluted the in-tree 
directories to the point that following these instructions will fail 
unless they start with 'make distclean' prior to the other steps.

> +    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
> 

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

Re: [PATCH v5 for-5.0] configure: warn if not using a separate build directory

[Daniel P. Berrangé]

On Mon, Apr 06, 2020 at 10:38:45AM -0500, Eric Blake wrote:
> On 4/6/20 10:33 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
> 
> Calling it a deprecation warning may be overkill now that we've toned down
> the language.
> 
> > the user attempts to use an in-srcdir build setup, so that they are
> > aware that they're building QEMU in an undesirable manner.
> > 
> > Reviewed-by: Aleksandar Markovic <aleksandar.qemu.devel@gmail.com>
> > Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
> > 
> 
> > +if test "$in_srcdir" = "yes"; then
> > +    echo
> > +    echo "NOTE: we recommend against building in the source directory"
> > +    echo
> > +    echo "You've run the 'configure' script directly from the source"
> > +    echo "directory. This will work, but we recommend using a separate"
> > +    echo "build directory, especially if you plan to work with the QEMU"
> > +    echo "sources rather than just building it once. You can switch to"
> > +    echo "a separate build directory like this:"
> > +    echo
> > +    echo "  $ mkdir build"
> 
> As I pointed out on v4, this is missing a step.  Since this is just a
> warning and not fatal, './configure' completed and polluted the in-tree
> directories to the point that following these instructions will fail unless
> they start with 'make distclean' prior to the other steps.

Hmm, I was thinking this wasn't needed because we would assume this  was
a fresh checkout, but I had forgot that this very run of configure will
have polluted it.

> 
> > +    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
> > 
> 
> -- 
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.           +1-919-301-3226
> Virtualization:  qemu.org | libvirt.org
> 
> 

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 :|

Re: [PATCH v5 for-5.0] configure: warn if not using a separate build directory

[Peter Maydell]

On Mon, 6 Apr 2020 at 16:33, Daniel P. Berrangé <berrange@redhat.com> 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.
>
> Reviewed-by: Aleksandar Markovic <aleksandar.qemu.devel@gmail.com>
> Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---

Given where we are in the release cycle, I think this isn't
going to go in for 5.0; and it's not really that urgent now
we've decided we don't want to actually deprecate in-tree builds.
I've removed the text I put into the changelog about this earlier.

thanks
-- PMM

Re: [PATCH v5 for-5.0] configure: warn if not using a separate build directory

[Markus Armbruster]

Peter Maydell <peter.maydell@linaro.org> writes:

> On Mon, 6 Apr 2020 at 16:33, Daniel P. Berrangé <berrange@redhat.com> 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.

The warning text has evolved since v5, but the commit message hasn't
quite kept up, I think.

>>
>> Reviewed-by: Aleksandar Markovic <aleksandar.qemu.devel@gmail.com>
>> Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
>> ---
>
> Given where we are in the release cycle, I think this isn't
> going to go in for 5.0; and it's not really that urgent now
> we've decided we don't want to actually deprecate in-tree builds.

Have we?

We had a Aleksandar assert that out-of-tree builds can't do certain
things, which led to the decision to soften this patch's warning from
"deprecated; better use the grace period to adjust, and here's how to"
to "not recommended; here's the recommended way".  Since we know in-tree
builds are more fragile, we owe our users such a warning.  We should've
added it long ago.

We also had a few people telling us that in-tree builds are so much more
convenient for them that we doing extra work to keep them working for
them is totally worth it for them.  SCNR.

Whether we want to keep sinking time & energy into an extra way to build
will become irrelevant once we move to Meson, unless Meson deviates from
its "this is an opinionated build tool, not a 'give users all the rope
they may possibly want, and then some'" approach in a surprising lapse
of judgement.

> I've removed the text I put into the changelog about this earlier.

Pity.

If we can't reach consensus in time for 5.0, that's regrettable, but I
accept it.  Our decision making process is open and slow.  Hard to get
one without the other.

Much harder to accept is us once again defaulting to do nothing because
deciding what to do involves a tradeoff.

Re: [PATCH v5 for-5.0] configure: warn if not using a separate build directory

[Peter Maydell]

On Wed, 15 Apr 2020 at 07:13, Markus Armbruster <armbru@redhat.com> wrote:
> Peter Maydell <peter.maydell@linaro.org> writes:
> > Given where we are in the release cycle, I think this isn't
> > going to go in for 5.0; and it's not really that urgent now
> > we've decided we don't want to actually deprecate in-tree builds.
>
> Have we?
>
> We had a Aleksandar assert that out-of-tree builds can't do certain
> things, which led to the decision to soften this patch's warning from
> "deprecated; better use the grace period to adjust, and here's how to"
> to "not recommended; here's the recommended way".

This is not how I recall the discussion. The reason we decided to
soften this patche's warning was because there was a sizeable
contingent of people who said "I want the basic './configure; make'
commands to keep working and am willing to write the wrapper
makefile that wil cause those to automatically create and use
a build directory under the hood". If the commands will keep working
then there's nothing to deprecate.

("Stuff doesn't currently work with out-of-tree builds" is something
that I argued at that point in the thread was definitely not a reason
not to deprecate.)

> Whether we want to keep sinking time & energy into an extra way to build
> will become irrelevant once we move to Meson, unless Meson deviates from
> its "this is an opinionated build tool, not a 'give users all the rope
> they may possibly want, and then some'" approach in a surprising lapse
> of judgement.

And Meson is the other part of this -- if Meson is coming soon-ish
and will mean users having to change their build commands in some
way anyway, it's better for them if we only make them change once,
when Meson arrives, rather than once now and then again later.

> If we can't reach consensus in time for 5.0, that's regrettable, but I
> accept it.  Our decision making process is open and slow.  Hard to get
> one without the other.
>
> Much harder to accept is us once again defaulting to do nothing because
> deciding what to do involves a tradeoff.

My understanding of the consensus position was "we should stop
supporting in-tree build in the main makefile machinery but should
have a trivial wrapper that creates and uses a build directory
under the hood if the user does run configure/make from the
source tree". I think that should be much less painful to maintain
than handling both setups through the whole of our makefile system.
(I wouldn't personally bother to implement it, but several people
volunteered to do so.)

thanks
-- PMM