1. Patchew
  2. QEMU
  3. View series

[PATCH] configure: add support for psuedo-"in source tree" builds

Daniel P. Berrangé posted 1 patch 3 years, 8 months ago
Test docker-quick@centos7 passed
Test docker-mingw@fedora passed
Test checkpatch passed
Test FreeBSD passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20200820165543.215372-1-berrange@redhat.com
.gitignore |  2 ++
configure  | 40 ++++++++++++++++++++++++++++++++--------
2 files changed, 34 insertions(+), 8 deletions(-)
[PATCH] configure: add support for psuedo-"in source tree" builds
Posted by Daniel P. Berrangé 3 years, 8 months ago 
Meson requires the build dir to be separate from the source tree. Many
people are used to just running "./configure && make" though and the
meson conversion breaks that.

This introduces some backcompat support to make it appear as if an
"in source tree" build is being done, but with the the results in the
"build/" directory. This allows "./configure && make" to work as it
did historically, albeit with the output binaries staying under build/.

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

This is a simple integration of Eric's proposal from

  https://lists.gnu.org/archive/html/qemu-devel/2020-03/msg07290.html

with a bit of configure magic. It is enough to enable

   ./configure
   make
   make check

I've not tested it beyond that. Note it blows away the "build/"
dir each time ./configure is run so it is pristine each time.

We could optionally symlink binaries from build/ into $PWD
if poeople think that is important, eg by changing GNUmakefile
to have:

recurse: all
	for bin in `find build -maxdepth 1 -type f -executable | grep -v -E '(ninjatool|config.status)'`; \
	do \
	  ln -f -s $$bin . ; \
	done

and some cleanup logic to purge the symlinks for "make clean"

This goes on top of Paolo's most recent meson port v175 posting,
or whatever number it is upto now :-)

 .gitignore |  2 ++
 configure  | 40 ++++++++++++++++++++++++++++++++--------
 2 files changed, 34 insertions(+), 8 deletions(-)

diff --git a/.gitignore b/.gitignore
index 92311284ef..4ccb9ed975 100644
--- a/.gitignore
+++ b/.gitignore
@@ -1,3 +1,5 @@
+/GNUmakefile
+/build/
 /.doctrees
 /config-devices.*
 /config-all-devices.*
diff --git a/configure b/configure
index cc5f58f31a..a5c88ad1ac 100755
--- a/configure
+++ b/configure
@@ -11,6 +11,38 @@ unset CLICOLOR_FORCE GREP_OPTIONS
 # Don't allow CCACHE, if present, to use cached results of compile tests!
 export CCACHE_RECACHE=yes
 
+source_path=$(cd "$(dirname -- "$0")"; pwd)
+
+if printf %s\\n "$source_path" "$PWD" | grep -q "[[:space:]:]";
+then
+  error_exit "main directory cannot contain spaces nor colons"
+fi
+
+if test "$PWD" == "$source_path"
+then
+    echo "Using './build' as the directory for build output"
+    rm -rf build
+    mkdir -p build
+    cat > GNUmakefile <<EOF
+
+ifeq (\$(MAKECMDGOALS),)
+recurse: all
+endif
+
+.NOTPARALLEL: %
+%: force
+	@echo 'changing dir to build for \$(MAKE) "\$(MAKECMDGOALS)"...'
+	@\$(MAKE) -C build -f Makefile \$(MAKECMDGOALS)
+	if test "\$(MAKECMDGOALS)" = "distclean" ; then rm -rf build ; fi
+force: ;
+.PHONY: force
+GNUmakefile: ;
+
+EOF
+    cd build
+    exec $source_path/configure "$@"
+fi
+
 # Temporary directory used for files created while
 # configure runs. Since it is in the build directory
 # we can safely blow away any previous version of it
@@ -297,14 +329,6 @@ ld_has() {
     $ld --help 2>/dev/null | grep ".$1" >/dev/null 2>&1
 }
 
-# make source path absolute
-source_path=$(cd "$(dirname -- "$0")"; pwd)
-
-if printf %s\\n "$source_path" "$PWD" | grep -q "[[:space:]:]";
-then
-  error_exit "main directory cannot contain spaces nor colons"
-fi
-
 # default parameters
 cpu=""
 iasl="iasl"
-- 
2.26.2
Re: [PATCH] configure: add support for psuedo-"in source tree" builds
Posted by Peter Maydell 3 years, 8 months ago 
On Thu, 20 Aug 2020 at 17:56, Daniel P. Berrangé <berrange@redhat.com> wrote:
> Meson requires the build dir to be separate from the source tree. Many
> people are used to just running "./configure && make" though and the
> meson conversion breaks that.
>
> This introduces some backcompat support to make it appear as if an
> "in source tree" build is being done, but with the the results in the
> "build/" directory. This allows "./configure && make" to work as it
> did historically, albeit with the output binaries staying under build/.

> +if test "$PWD" == "$source_path"
> +then
> +    echo "Using './build' as the directory for build output"
> +    rm -rf build
> +    mkdir -p build

Can we put in a mollyguard here so we only blow away build/
if we previously auto-created it? Something like

      if ! -e build || -e build/created-by-configure; then
          rm -rf build
          mkdir -p build
          touch build/created-by-configure
      else
          echo "some helpful error message here"
      fi

(shell syntax probably wrong but you get the idea)

My current setup has multiple build-directories like
build/x86, build/clang, ... and I'd like a guard against
configure blowing them all away if I accidentally run it
from the source tree some day.

thanks
-- PMM
Re: [PATCH] configure: add support for psuedo-"in source tree" builds
Posted by Daniel P. Berrangé 3 years, 8 months ago 
On Thu, Aug 20, 2020 at 06:10:34PM +0100, Peter Maydell wrote:
> On Thu, 20 Aug 2020 at 17:56, Daniel P. Berrangé <berrange@redhat.com> wrote:
> > Meson requires the build dir to be separate from the source tree. Many
> > people are used to just running "./configure && make" though and the
> > meson conversion breaks that.
> >
> > This introduces some backcompat support to make it appear as if an
> > "in source tree" build is being done, but with the the results in the
> > "build/" directory. This allows "./configure && make" to work as it
> > did historically, albeit with the output binaries staying under build/.
> 
> > +if test "$PWD" == "$source_path"
> > +then
> > +    echo "Using './build' as the directory for build output"
> > +    rm -rf build
> > +    mkdir -p build
> 
> Can we put in a mollyguard here so we only blow away build/
> if we previously auto-created it? Something like
> 
>       if ! -e build || -e build/created-by-configure; then
>           rm -rf build
>           mkdir -p build
>           touch build/created-by-configure
>       else
>           echo "some helpful error message here"
>       fi
> 
> (shell syntax probably wrong but you get the idea)
> 
> My current setup has multiple build-directories like
> build/x86, build/clang, ... and I'd like a guard against
> configure blowing them all away if I accidentally run it
> from the source tree some day.

Sure, that makes sense.

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] configure: add support for psuedo-"in source tree" builds
Posted by Peter Maydell 3 years, 8 months ago 
On Thu, 20 Aug 2020 at 17:56, Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> Meson requires the build dir to be separate from the source tree. Many
> people are used to just running "./configure && make" though and the
> meson conversion breaks that.
>
> This introduces some backcompat support to make it appear as if an
> "in source tree" build is being done, but with the the results in the
> "build/" directory. This allows "./configure && make" to work as it
> did historically, albeit with the output binaries staying under build/.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>
> This is a simple integration of Eric's proposal from
>
>   https://lists.gnu.org/archive/html/qemu-devel/2020-03/msg07290.html
>
> with a bit of configure magic. It is enough to enable
>
>    ./configure
>    make
>    make check
>
> I've not tested it beyond that. Note it blows away the "build/"
> dir each time ./configure is run so it is pristine each time.
>
> We could optionally symlink binaries from build/ into $PWD
> if poeople think that is important, eg by changing GNUmakefile
> to have:
>
> recurse: all
>         for bin in `find build -maxdepth 1 -type f -executable | grep -v -E '(ninjatool|config.status)'`; \
>         do \
>           ln -f -s $$bin . ; \
>         done
>
> and some cleanup logic to purge the symlinks for "make clean"
>
> This goes on top of Paolo's most recent meson port v175 posting,
> or whatever number it is upto now :-)
>
>  .gitignore |  2 ++
>  configure  | 40 ++++++++++++++++++++++++++++++++--------
>  2 files changed, 34 insertions(+), 8 deletions(-)
>
> diff --git a/.gitignore b/.gitignore
> index 92311284ef..4ccb9ed975 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -1,3 +1,5 @@
> +/GNUmakefile
> +/build/
>  /.doctrees
>  /config-devices.*
>  /config-all-devices.*
> diff --git a/configure b/configure
> index cc5f58f31a..a5c88ad1ac 100755
> --- a/configure
> +++ b/configure
> @@ -11,6 +11,38 @@ unset CLICOLOR_FORCE GREP_OPTIONS
>  # Don't allow CCACHE, if present, to use cached results of compile tests!
>  export CCACHE_RECACHE=yes
>
> +source_path=$(cd "$(dirname -- "$0")"; pwd)
> +
> +if printf %s\\n "$source_path" "$PWD" | grep -q "[[:space:]:]";
> +then
> +  error_exit "main directory cannot contain spaces nor colons"
> +fi
> +
> +if test "$PWD" == "$source_path"
> +then
> +    echo "Using './build' as the directory for build output"
> +    rm -rf build
> +    mkdir -p build
> +    cat > GNUmakefile <<EOF
> +

Since this created file ends up in the source directory it would
be useful to have it start with a brief comment saying
(a) that it's autogenerated by configure (b) what it's for.

thanks
-- PMM
Re: [PATCH] configure: add support for psuedo-"in source tree" builds
Posted by Philippe Mathieu-Daudé 3 years, 8 months ago 
Typo "pseudo" in subject.

On 8/20/20 6:55 PM, Daniel P. Berrangé wrote:
> Meson requires the build dir to be separate from the source tree. Many
> people are used to just running "./configure && make" though and the
> meson conversion breaks that.
> 
> This introduces some backcompat support to make it appear as if an
> "in source tree" build is being done, but with the the results in the
> "build/" directory. This allows "./configure && make" to work as it
> did historically, albeit with the output binaries staying under build/.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
> 
> This is a simple integration of Eric's proposal from
> 
>   https://lists.gnu.org/archive/html/qemu-devel/2020-03/msg07290.html
> 
> with a bit of configure magic. It is enough to enable
> 
>    ./configure
>    make
>    make check

Lovely :)

> 
> I've not tested it beyond that. Note it blows away the "build/"
> dir each time ./configure is run so it is pristine each time.
> 
> We could optionally symlink binaries from build/ into $PWD
> if poeople think that is important, eg by changing GNUmakefile

Typo "people".

> to have:
> 
> recurse: all
> 	for bin in `find build -maxdepth 1 -type f -executable | grep -v -E '(ninjatool|config.status)'`; \
> 	do \
> 	  ln -f -s $$bin . ; \
> 	done
> 
> and some cleanup logic to purge the symlinks for "make clean"
> 
> This goes on top of Paolo's most recent meson port v175 posting,
> or whatever number it is upto now :-)
> 
>  .gitignore |  2 ++
>  configure  | 40 ++++++++++++++++++++++++++++++++--------
>  2 files changed, 34 insertions(+), 8 deletions(-)
> 
> diff --git a/.gitignore b/.gitignore
> index 92311284ef..4ccb9ed975 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -1,3 +1,5 @@
> +/GNUmakefile
> +/build/
>  /.doctrees
>  /config-devices.*
>  /config-all-devices.*
> diff --git a/configure b/configure
> index cc5f58f31a..a5c88ad1ac 100755
> --- a/configure
> +++ b/configure
> @@ -11,6 +11,38 @@ unset CLICOLOR_FORCE GREP_OPTIONS
>  # Don't allow CCACHE, if present, to use cached results of compile tests!
>  export CCACHE_RECACHE=yes
>  
> +source_path=$(cd "$(dirname -- "$0")"; pwd)
> +
> +if printf %s\\n "$source_path" "$PWD" | grep -q "[[:space:]:]";
> +then
> +  error_exit "main directory cannot contain spaces nor colons"
> +fi
> +
> +if test "$PWD" == "$source_path"
> +then
> +    echo "Using './build' as the directory for build output"
> +    rm -rf build
> +    mkdir -p build
> +    cat > GNUmakefile <<EOF
> +
> +ifeq (\$(MAKECMDGOALS),)
> +recurse: all
> +endif
> +
> +.NOTPARALLEL: %
> +%: force
> +	@echo 'changing dir to build for \$(MAKE) "\$(MAKECMDGOALS)"...'
> +	@\$(MAKE) -C build -f Makefile \$(MAKECMDGOALS)
> +	if test "\$(MAKECMDGOALS)" = "distclean" ; then rm -rf build ; fi

Can we use '@' to silent the last line too?

  make[1]: Leaving directory '/tmp/qemu/build'
  if test "help" = "distclean" ; then rm -rf build ; fi

Similarly to Peter, I also use the ./build/{conf1,conf2,...} each
subdir ./configured differently, so I wouldn't like to blow my
subdirs. Otherwise:

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> +force: ;
> +.PHONY: force
> +GNUmakefile: ;
> +
> +EOF
> +    cd build
> +    exec $source_path/configure "$@"
> +fi
> +
>  # Temporary directory used for files created while
>  # configure runs. Since it is in the build directory
>  # we can safely blow away any previous version of it
> @@ -297,14 +329,6 @@ ld_has() {
>      $ld --help 2>/dev/null | grep ".$1" >/dev/null 2>&1
>  }
>  
> -# make source path absolute
> -source_path=$(cd "$(dirname -- "$0")"; pwd)
> -
> -if printf %s\\n "$source_path" "$PWD" | grep -q "[[:space:]:]";
> -then
> -  error_exit "main directory cannot contain spaces nor colons"
> -fi
> -
>  # default parameters
>  cpu=""
>  iasl="iasl"
>
Re: [PATCH] configure: add support for psuedo-"in source tree" builds
Posted by Eric Blake 3 years, 8 months ago 
On 8/20/20 11:55 AM, Daniel P. Berrangé wrote:
> Meson requires the build dir to be separate from the source tree. Many
> people are used to just running "./configure && make" though and the
> meson conversion breaks that.
> 
> This introduces some backcompat support to make it appear as if an
> "in source tree" build is being done, but with the the results in the
> "build/" directory. This allows "./configure && make" to work as it
> did historically, albeit with the output binaries staying under build/.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---

In addition to reviews you already have,


> I've not tested it beyond that. Note it blows away the "build/"
> dir each time ./configure is run so it is pristine each time.

I definitely like the idea of only blowing away what we created - but if 
we created build, then recreating it for each new configure run is nice.

> 
> We could optionally symlink binaries from build/ into $PWD
> if poeople think that is important, eg by changing GNUmakefile
> to have:
> 
> recurse: all
> 	for bin in `find build -maxdepth 1 -type f -executable | grep -v -E '(ninjatool|config.status)'`; \

Using -maxdepth gets rid of the need to pre-create empty directories for 
nested binaries, but also loses out on binaries such as 
x86_64-softmmu/qemu-system-x86_64.  Oh, it looks like meson creates 
qemu-system-x86_64 as a binary in the top level, then a symlink in its 
old location.  Populating symlinks to ALL old locations is thus trickier 
than what you are proposing here, so it is fine to save that for a 
followup patch (let's get the bare minimum in first, so that at least 
./configure && make works, before we worry about back-compat symlinks).

> 
> This goes on top of Paolo's most recent meson port v175 posting,
> or whatever number it is upto now :-)

Nice comment for reviewers, but doesn't quite need to be preserved in git.

> 
>   .gitignore |  2 ++
>   configure  | 40 ++++++++++++++++++++++++++++++++--------
>   2 files changed, 34 insertions(+), 8 deletions(-)
> 
> diff --git a/.gitignore b/.gitignore
> index 92311284ef..4ccb9ed975 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -1,3 +1,5 @@
> +/GNUmakefile
> +/build/
>   /.doctrees
>   /config-devices.*
>   /config-all-devices.*
> diff --git a/configure b/configure
> index cc5f58f31a..a5c88ad1ac 100755
> --- a/configure
> +++ b/configure
> @@ -11,6 +11,38 @@ unset CLICOLOR_FORCE GREP_OPTIONS
>   # Don't allow CCACHE, if present, to use cached results of compile tests!
>   export CCACHE_RECACHE=yes
>   
> +source_path=$(cd "$(dirname -- "$0")"; pwd)

This behaves wrong if CDPATH is set in the environment.  We should 
really include CDPATH in our environment sanitization at the top of the 
file.

> +
> +if printf %s\\n "$source_path" "$PWD" | grep -q "[[:space:]:]";
> +then
> +  error_exit "main directory cannot contain spaces nor colons"
> +fi
> +
> +if test "$PWD" == "$source_path"

bashism; s/==/=/ or you will break configure on dash systems

> +then
> +    echo "Using './build' as the directory for build output"

Do we want a way for a user to type './configure builddir=build/' and 
'make builddir=build/' so they can specify different builddir overrides 
per invocation (of course, where builddir defaults to 'build/' if not 
specified)?  But hardcoding to _just_ ./build/ for getting this patch in 
quickly is fine.

> +    rm -rf build
> +    mkdir -p build
> +    cat > GNUmakefile <<EOF

If you use 'EOF' or \EOF here, then...

> +
> +ifeq (\$(MAKECMDGOALS),)

you wouldn't have to escape all these $.  Looking through the file...

> +recurse: all
> +endif
> +
> +.NOTPARALLEL: %
> +%: force
> +	@echo 'changing dir to build for \$(MAKE) "\$(MAKECMDGOALS)"...'
> +	@\$(MAKE) -C build -f Makefile \$(MAKECMDGOALS)
> +	if test "\$(MAKECMDGOALS)" = "distclean" ; then rm -rf build ; fi
> +force: ;
> +.PHONY: force
> +GNUmakefile: ;
> +
> +EOF

...I didn't see any use of $ that was not supposed to be literally in 
the generated GNUmakefile.

> +    cd build
> +    exec $source_path/configure "$@"
> +fi
> +
>   # Temporary directory used for files created while
>   # configure runs. Since it is in the build directory
>   # we can safely blow away any previous version of it

Now that we are guaranteeing configure is run in a build directory, this 
part of configure might have some cleanups possible.  But that can be a 
separate patch.

> @@ -297,14 +329,6 @@ ld_has() {
>       $ld --help 2>/dev/null | grep ".$1" >/dev/null 2>&1
>   }
>   
> -# make source path absolute
> -source_path=$(cd "$(dirname -- "$0")"; pwd)
> -
> -if printf %s\\n "$source_path" "$PWD" | grep -q "[[:space:]:]";
> -then
> -  error_exit "main directory cannot contain spaces nor colons"
> -fi
> -
>   # default parameters
>   cpu=""
>   iasl="iasl"
> 

Looking forward to v2.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org
Re: [PATCH] configure: add support for psuedo-"in source tree" builds
Posted by Philippe Mathieu-Daudé 3 years, 8 months ago 
On 8/20/20 7:42 PM, Eric Blake wrote:
> On 8/20/20 11:55 AM, Daniel P. Berrangé wrote:
>> Meson requires the build dir to be separate from the source tree. Many
>> people are used to just running "./configure && make" though and the
>> meson conversion breaks that.
>>
>> This introduces some backcompat support to make it appear as if an
>> "in source tree" build is being done, but with the the results in the
>> "build/" directory. This allows "./configure && make" to work as it
>> did historically, albeit with the output binaries staying under build/.
>>
>> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
>> ---

[*]

> 
> In addition to reviews you already have,
> 
> 
>> I've not tested it beyond that. Note it blows away the "build/"
>> dir each time ./configure is run so it is pristine each time.
> 
> I definitely like the idea of only blowing away what we created - but if
> we created build, then recreating it for each new configure run is nice.
> 
>>
>> We could optionally symlink binaries from build/ into $PWD
>> if poeople think that is important, eg by changing GNUmakefile
>> to have:
>>
>> recurse: all
>>     for bin in `find build -maxdepth 1 -type f -executable | grep -v
>> -E '(ninjatool|config.status)'`; \
> 
> Using -maxdepth gets rid of the need to pre-create empty directories for
> nested binaries, but also loses out on binaries such as
> x86_64-softmmu/qemu-system-x86_64.  Oh, it looks like meson creates
> qemu-system-x86_64 as a binary in the top level, then a symlink in its
> old location.  Populating symlinks to ALL old locations is thus trickier
> than what you are proposing here, so it is fine to save that for a
> followup patch (let's get the bare minimum in first, so that at least
> ./configure && make works, before we worry about back-compat symlinks).
> 
>>
>> This goes on top of Paolo's most recent meson port v175 posting,
>> or whatever number it is upto now :-)
> 
> Nice comment for reviewers, but doesn't quite need to be preserved in git.

Already below '---' in [*] ;)

> 
>>
>>   .gitignore |  2 ++
>>   configure  | 40 ++++++++++++++++++++++++++++++++--------
>>   2 files changed, 34 insertions(+), 8 deletions(-)
>>
>> diff --git a/.gitignore b/.gitignore
>> index 92311284ef..4ccb9ed975 100644
>> --- a/.gitignore
>> +++ b/.gitignore
>> @@ -1,3 +1,5 @@
>> +/GNUmakefile
>> +/build/
>>   /.doctrees
>>   /config-devices.*
>>   /config-all-devices.*
>> diff --git a/configure b/configure
>> index cc5f58f31a..a5c88ad1ac 100755
>> --- a/configure
>> +++ b/configure
>> @@ -11,6 +11,38 @@ unset CLICOLOR_FORCE GREP_OPTIONS
>>   # Don't allow CCACHE, if present, to use cached results of compile
>> tests!
>>   export CCACHE_RECACHE=yes
>>   +source_path=$(cd "$(dirname -- "$0")"; pwd)
> 
> This behaves wrong if CDPATH is set in the environment.  We should
> really include CDPATH in our environment sanitization at the top of the
> file.
> 
>> +
>> +if printf %s\\n "$source_path" "$PWD" | grep -q "[[:space:]:]";
>> +then
>> +  error_exit "main directory cannot contain spaces nor colons"
>> +fi
>> +
>> +if test "$PWD" == "$source_path"
> 
> bashism; s/==/=/ or you will break configure on dash systems
> 
>> +then
>> +    echo "Using './build' as the directory for build output"
> 
> Do we want a way for a user to type './configure builddir=build/' and
> 'make builddir=build/' so they can specify different builddir overrides
> per invocation (of course, where builddir defaults to 'build/' if not
> specified)?  But hardcoding to _just_ ./build/ for getting this patch in
> quickly is fine.
> 
>> +    rm -rf build
>> +    mkdir -p build
>> +    cat > GNUmakefile <<EOF
> 
> If you use 'EOF' or \EOF here, then...
> 
>> +
>> +ifeq (\$(MAKECMDGOALS),)
> 
> you wouldn't have to escape all these $.  Looking through the file...
> 
>> +recurse: all
>> +endif
>> +
>> +.NOTPARALLEL: %
>> +%: force
>> +    @echo 'changing dir to build for \$(MAKE) "\$(MAKECMDGOALS)"...'
>> +    @\$(MAKE) -C build -f Makefile \$(MAKECMDGOALS)
>> +    if test "\$(MAKECMDGOALS)" = "distclean" ; then rm -rf build ; fi
>> +force: ;
>> +.PHONY: force
>> +GNUmakefile: ;
>> +
>> +EOF
> 
> ...I didn't see any use of $ that was not supposed to be literally in
> the generated GNUmakefile.
> 
>> +    cd build
>> +    exec $source_path/configure "$@"
>> +fi
>> +
>>   # Temporary directory used for files created while
>>   # configure runs. Since it is in the build directory
>>   # we can safely blow away any previous version of it
> 
> Now that we are guaranteeing configure is run in a build directory, this
> part of configure might have some cleanups possible.  But that can be a
> separate patch.
> 
>> @@ -297,14 +329,6 @@ ld_has() {
>>       $ld --help 2>/dev/null | grep ".$1" >/dev/null 2>&1
>>   }
>>   -# make source path absolute
>> -source_path=$(cd "$(dirname -- "$0")"; pwd)
>> -
>> -if printf %s\\n "$source_path" "$PWD" | grep -q "[[:space:]:]";
>> -then
>> -  error_exit "main directory cannot contain spaces nor colons"
>> -fi
>> -
>>   # default parameters
>>   cpu=""
>>   iasl="iasl"
>>
> 
> Looking forward to v2.
>
Re: [PATCH] configure: add support for psuedo-"in source tree" builds
Posted by Paolo Bonzini 3 years, 8 months ago 
On 20/08/20 19:42, Eric Blake wrote:
>>
>> This goes on top of Paolo's most recent meson port v175 posting,
>> or whatever number it is upto now :-)
> 
> Nice comment for reviewers, but doesn't quite need to be preserved in git.

I for one don't mind. :)

Paolo
Re: [PATCH] configure: add support for psuedo-"in source tree" builds
Posted by Daniel P. Berrangé 3 years, 8 months ago 
On Thu, Aug 20, 2020 at 12:42:20PM -0500, Eric Blake wrote:
> On 8/20/20 11:55 AM, Daniel P. Berrangé wrote:
> > Meson requires the build dir to be separate from the source tree. Many
> > people are used to just running "./configure && make" though and the
> > meson conversion breaks that.
> > 
> > This introduces some backcompat support to make it appear as if an
> > "in source tree" build is being done, but with the the results in the
> > "build/" directory. This allows "./configure && make" to work as it
> > did historically, albeit with the output binaries staying under build/.
> > 
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
> 
> In addition to reviews you already have,
> 
> 
> > I've not tested it beyond that. Note it blows away the "build/"
> > dir each time ./configure is run so it is pristine each time.
> 
> I definitely like the idea of only blowing away what we created - but if we
> created build, then recreating it for each new configure run is nice.
> 
> > 
> > We could optionally symlink binaries from build/ into $PWD
> > if poeople think that is important, eg by changing GNUmakefile
> > to have:
> > 
> > recurse: all
> > 	for bin in `find build -maxdepth 1 -type f -executable | grep -v -E '(ninjatool|config.status)'`; \
> 
> Using -maxdepth gets rid of the need to pre-create empty directories for
> nested binaries, but also loses out on binaries such as
> x86_64-softmmu/qemu-system-x86_64.  Oh, it looks like meson creates
> qemu-system-x86_64 as a binary in the top level, then a symlink in its old
> location.  Populating symlinks to ALL old locations is thus trickier than
> what you are proposing here, so it is fine to save that for a followup patch
> (let's get the bare minimum in first, so that at least ./configure && make
> works, before we worry about back-compat symlinks).
> 
> > 
> > This goes on top of Paolo's most recent meson port v175 posting,
> > or whatever number it is upto now :-)
> 
> Nice comment for reviewers, but doesn't quite need to be preserved in git.
> 
> > 
> >   .gitignore |  2 ++
> >   configure  | 40 ++++++++++++++++++++++++++++++++--------
> >   2 files changed, 34 insertions(+), 8 deletions(-)
> > 
> > diff --git a/.gitignore b/.gitignore
> > index 92311284ef..4ccb9ed975 100644
> > --- a/.gitignore
> > +++ b/.gitignore
> > @@ -1,3 +1,5 @@
> > +/GNUmakefile
> > +/build/
> >   /.doctrees
> >   /config-devices.*
> >   /config-all-devices.*
> > diff --git a/configure b/configure
> > index cc5f58f31a..a5c88ad1ac 100755
> > --- a/configure
> > +++ b/configure
> > @@ -11,6 +11,38 @@ unset CLICOLOR_FORCE GREP_OPTIONS
> >   # Don't allow CCACHE, if present, to use cached results of compile tests!
> >   export CCACHE_RECACHE=yes
> > +source_path=$(cd "$(dirname -- "$0")"; pwd)
> 
> This behaves wrong if CDPATH is set in the environment.  We should really
> include CDPATH in our environment sanitization at the top of the file.

This code is pre-existing, so any fixes wrt CDPATH can be done by
a different patch.

> 
> > +
> > +if printf %s\\n "$source_path" "$PWD" | grep -q "[[:space:]:]";
> > +then
> > +  error_exit "main directory cannot contain spaces nor colons"
> > +fi
> > +
> > +if test "$PWD" == "$source_path"
> 
> bashism; s/==/=/ or you will break configure on dash systems
> 
> > +then
> > +    echo "Using './build' as the directory for build output"
> 
> Do we want a way for a user to type './configure builddir=build/' and 'make
> builddir=build/' so they can specify different builddir overrides per
> invocation (of course, where builddir defaults to 'build/' if not
> specified)?  But hardcoding to _just_ ./build/ for getting this patch in
> quickly is fine.

Adding "builddir=build" as an option would be introducing new
functionality that doesn't already exist. The goal is just to
do some minimal work to preserve existing functionality. If
people want control over the name of their build dir, then
they can do that by using a out-of-source tree build already.

> 
> > +    rm -rf build
> > +    mkdir -p build
> > +    cat > GNUmakefile <<EOF
> 
> If you use 'EOF' or \EOF here, then...
> 
> > +
> > +ifeq (\$(MAKECMDGOALS),)
> 
> you wouldn't have to escape all these $.  Looking through the file...

Ah, subtle, but nice.


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] configure: add support for psuedo-"in source tree" builds
Posted by Kevin Wolf 3 years, 8 months ago 
Am 20.08.2020 um 19:42 hat Eric Blake geschrieben:
> On 8/20/20 11:55 AM, Daniel P. Berrangé wrote:
> > Meson requires the build dir to be separate from the source tree. Many
> > people are used to just running "./configure && make" though and the
> > meson conversion breaks that.
> > 
> > This introduces some backcompat support to make it appear as if an
> > "in source tree" build is being done, but with the the results in the
> > "build/" directory. This allows "./configure && make" to work as it
> > did historically, albeit with the output binaries staying under build/.
> > 
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
> 
> In addition to reviews you already have,
> 
> 
> > I've not tested it beyond that. Note it blows away the "build/"
> > dir each time ./configure is run so it is pristine each time.
> 
> I definitely like the idea of only blowing away what we created - but if we
> created build, then recreating it for each new configure run is nice.

I think I actually wouldn't automatically remove anything on configure.
It can be surprising behaviour for configure to delete directories, and
the old setup didn't do an automatic "make clean" either. By having a
separate build directory, manually emptying as needed has already become
easier.

> > We could optionally symlink binaries from build/ into $PWD
> > if poeople think that is important, eg by changing GNUmakefile
> > to have:
> > 
> > recurse: all
> > 	for bin in `find build -maxdepth 1 -type f -executable | grep -v -E '(ninjatool|config.status)'`; \
> 
> Using -maxdepth gets rid of the need to pre-create empty directories for
> nested binaries, but also loses out on binaries such as
> x86_64-softmmu/qemu-system-x86_64.  Oh, it looks like meson creates
> qemu-system-x86_64 as a binary in the top level, then a symlink in its old
> location.  Populating symlinks to ALL old locations is thus trickier than
> what you are proposing here, so it is fine to save that for a followup patch
> (let's get the bare minimum in first, so that at least ./configure && make
> works, before we worry about back-compat symlinks).

Having the system emulator symlinks in the top level would be a change,
but even more convenient than the original places. I'd vote for adding
the auto-symlinking at least for the tools; if the top-level symlinks
for system emulators get also symlinked by this, that's fine, too.

I was actually surprised that Dan reports "make check" from the source
tree to be working without the symlinks. Some code must be cleverer than
I thought!

Kevin
Re: [PATCH] configure: add support for psuedo-"in source tree" builds
Posted by Daniel P. Berrangé 3 years, 8 months ago 
On Fri, Aug 21, 2020 at 11:58:21AM +0200, Kevin Wolf wrote:
> Am 20.08.2020 um 19:42 hat Eric Blake geschrieben:
> > On 8/20/20 11:55 AM, Daniel P. Berrangé wrote:
> > > Meson requires the build dir to be separate from the source tree. Many
> > > people are used to just running "./configure && make" though and the
> > > meson conversion breaks that.
> > > 
> > > This introduces some backcompat support to make it appear as if an
> > > "in source tree" build is being done, but with the the results in the
> > > "build/" directory. This allows "./configure && make" to work as it
> > > did historically, albeit with the output binaries staying under build/.
> > > 
> > > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > > ---
> > 
> > In addition to reviews you already have,
> > 
> > 
> > > I've not tested it beyond that. Note it blows away the "build/"
> > > dir each time ./configure is run so it is pristine each time.
> > 
> > I definitely like the idea of only blowing away what we created - but if we
> > created build, then recreating it for each new configure run is nice.
> 
> I think I actually wouldn't automatically remove anything on configure.
> It can be surprising behaviour for configure to delete directories, and
> the old setup didn't do an automatic "make clean" either. By having a
> separate build directory, manually emptying as needed has already become
> easier.

The issue is that previously you could do

  ./configure
  make
  ./configure
  make

This isn't possible with the new system because meson will refuse
to use the "build/" directory if it already contains a previous
configured build.

Doing "rm -rf build" in configure lets the above sequence work.

I can remove the "rm -rf biuld" in configure if we are happy
to require


  ./configure
  make
  make distclean
  ./configure
  make

because the "GNUmakefile" wires up "distclean" to purge the
build/ directory.

> > > We could optionally symlink binaries from build/ into $PWD
> > > if poeople think that is important, eg by changing GNUmakefile
> > > to have:
> > > 
> > > recurse: all
> > > 	for bin in `find build -maxdepth 1 -type f -executable | grep -v -E '(ninjatool|config.status)'`; \
> > 
> > Using -maxdepth gets rid of the need to pre-create empty directories for
> > nested binaries, but also loses out on binaries such as
> > x86_64-softmmu/qemu-system-x86_64.  Oh, it looks like meson creates
> > qemu-system-x86_64 as a binary in the top level, then a symlink in its old
> > location.  Populating symlinks to ALL old locations is thus trickier than
> > what you are proposing here, so it is fine to save that for a followup patch
> > (let's get the bare minimum in first, so that at least ./configure && make
> > works, before we worry about back-compat symlinks).
> 
> Having the system emulator symlinks in the top level would be a change,
> but even more convenient than the original places. I'd vote for adding
> the auto-symlinking at least for the tools; if the top-level symlinks
> for system emulators get also symlinked by this, that's fine, too.
> 
> I was actually surprised that Dan reports "make check" from the source
> tree to be working without the symlinks. Some code must be cleverer than
> I thought!

That's because "make check" is not actually running from the source
tree. When you run "make check" in the source tree, what's acutally
happening is

  cd build
  make check

so it is actually running from build-dir context.


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] configure: add support for psuedo-"in source tree" builds
Posted by Paolo Bonzini 3 years, 8 months ago 
On 21/08/20 12:14, Daniel P. Berrangé wrote:
> I can remove the "rm -rf biuld" in configure if we are happy
> to require
> 
> 
>   ./configure
>   make
>   make distclean
>   ./configure
>   make
> 
> because the "GNUmakefile" wires up "distclean" to purge the
> build/ directory.
> 

I have already included this version in v8 of the meson pull request,
actually (to place it early in the series).  Any other refinements[1]
and bugfixes can be done on top.

Paolo

[1] such as requiring --i-really-want-intree-builds :D
Re: [PATCH] configure: add support for psuedo-"in source tree" builds
Posted by Kevin Wolf 3 years, 8 months ago 
Am 21.08.2020 um 12:14 hat Daniel P. Berrangé geschrieben:
> On Fri, Aug 21, 2020 at 11:58:21AM +0200, Kevin Wolf wrote:
> > Am 20.08.2020 um 19:42 hat Eric Blake geschrieben:
> > > On 8/20/20 11:55 AM, Daniel P. Berrangé wrote:
> > > > Meson requires the build dir to be separate from the source tree. Many
> > > > people are used to just running "./configure && make" though and the
> > > > meson conversion breaks that.
> > > > 
> > > > This introduces some backcompat support to make it appear as if an
> > > > "in source tree" build is being done, but with the the results in the
> > > > "build/" directory. This allows "./configure && make" to work as it
> > > > did historically, albeit with the output binaries staying under build/.
> > > > 
> > > > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > > > ---
> > > 
> > > In addition to reviews you already have,
> > > 
> > > 
> > > > I've not tested it beyond that. Note it blows away the "build/"
> > > > dir each time ./configure is run so it is pristine each time.
> > > 
> > > I definitely like the idea of only blowing away what we created - but if we
> > > created build, then recreating it for each new configure run is nice.
> > 
> > I think I actually wouldn't automatically remove anything on configure.
> > It can be surprising behaviour for configure to delete directories, and
> > the old setup didn't do an automatic "make clean" either. By having a
> > separate build directory, manually emptying as needed has already become
> > easier.
> 
> The issue is that previously you could do
> 
>   ./configure
>   make
>   ./configure
>   make
> 
> This isn't possible with the new system because meson will refuse
> to use the "build/" directory if it already contains a previous
> configured build.

Oh. So now you always have to create a new build directory if you want
to change configure options?

> Doing "rm -rf build" in configure lets the above sequence work.
> 
> I can remove the "rm -rf biuld" in configure if we are happy
> to require
> 
>   ./configure
>   make
>   make distclean
>   ./configure
>   make
> 
> because the "GNUmakefile" wires up "distclean" to purge the
> build/ directory.

Hm, I see. Then maybe better keep the 'rm'.

> > > > We could optionally symlink binaries from build/ into $PWD
> > > > if poeople think that is important, eg by changing GNUmakefile
> > > > to have:
> > > > 
> > > > recurse: all
> > > > 	for bin in `find build -maxdepth 1 -type f -executable | grep -v -E '(ninjatool|config.status)'`; \
> > > 
> > > Using -maxdepth gets rid of the need to pre-create empty directories for
> > > nested binaries, but also loses out on binaries such as
> > > x86_64-softmmu/qemu-system-x86_64.  Oh, it looks like meson creates
> > > qemu-system-x86_64 as a binary in the top level, then a symlink in its old
> > > location.  Populating symlinks to ALL old locations is thus trickier than
> > > what you are proposing here, so it is fine to save that for a followup patch
> > > (let's get the bare minimum in first, so that at least ./configure && make
> > > works, before we worry about back-compat symlinks).
> > 
> > Having the system emulator symlinks in the top level would be a change,
> > but even more convenient than the original places. I'd vote for adding
> > the auto-symlinking at least for the tools; if the top-level symlinks
> > for system emulators get also symlinked by this, that's fine, too.
> > 
> > I was actually surprised that Dan reports "make check" from the source
> > tree to be working without the symlinks. Some code must be cleverer than
> > I thought!
> 
> That's because "make check" is not actually running from the source
> tree. When you run "make check" in the source tree, what's acutally
> happening is
> 
>   cd build
>   make check
> 
> so it is actually running from build-dir context.

Yup, I wasn't aware that it would do this.

Kevin
Re: [PATCH] configure: add support for psuedo-"in source tree" builds
Posted by Peter Maydell 3 years, 8 months ago 
On Thu, 20 Aug 2020 at 17:56, Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> Meson requires the build dir to be separate from the source tree. Many
> people are used to just running "./configure && make" though and the
> meson conversion breaks that.
>
> This introduces some backcompat support to make it appear as if an
> "in source tree" build is being done, but with the the results in the
> "build/" directory. This allows "./configure && make" to work as it
> did historically, albeit with the output binaries staying under build/.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---

Oh, one more minor thing: 'make distclean' should remove
the created GNUMakefile (and also build/ if configure
created it? dunno).

thanks
-- PMM
Re: [PATCH] configure: add support for psuedo-"in source tree" builds
Posted by Daniel P. Berrangé 3 years, 8 months ago 
On Thu, Aug 20, 2020 at 07:15:03PM +0100, Peter Maydell wrote:
> On Thu, 20 Aug 2020 at 17:56, Daniel P. Berrangé <berrange@redhat.com> wrote:
> >
> > Meson requires the build dir to be separate from the source tree. Many
> > people are used to just running "./configure && make" though and the
> > meson conversion breaks that.
> >
> > This introduces some backcompat support to make it appear as if an
> > "in source tree" build is being done, but with the the results in the
> > "build/" directory. This allows "./configure && make" to work as it
> > did historically, albeit with the output binaries staying under build/.
> >
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
> 
> Oh, one more minor thing: 'make distclean' should remove
> the created GNUMakefile (and also build/ if configure
> created it? dunno).

Yes, I mistakenly put that logic for "make clean" in this v1. My v2
has fixed this, and in v3 i'll remove the "GNUMakefile" too.


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

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.