Re: [Qemu-devel] [PATCH] configure: Remove --source-path option

[Antonio Ospite]

On 18/03/19 14:40, Peter Maydell wrote:
> Normally configure identifies the source path by looking
> at the location where the configure script itself exists.
> We also provide a --source-path option which lets the user
> manually override this.
>
> There isn't really an obvious use case for the --source-path
> option, and in commit 927128222b0a91f56c13a in 2017 we
> accidentally added some logic that looks at $source_path
> before the command line option that overrides it has been
> processed.
>
> The fact that nobody complained suggests that there isn't
> any use of this option and we aren't testing it either;
> remove it. This allows us to move the "make $source_path
> absolute" logic up so that there is no window in the script
> where $source_path is set but not yet absolute.
>
> Signed-off-by: Peter Maydell <address@hidden>
> ---
> Since this is a "noticed while reading code" issue rather than
> one that's actually causing a problem, it's probably 4.1
> material at this point.
>
> Cc'ing Antonio since they also have a patch to configure
> which this will affect.

Thanks for CC-ing me, I will send a v2 of my patch after this one gets in.

A minor comment below.

> ---
>   configure | 10 ++--------
>   1 file changed, 2 insertions(+), 8 deletions(-)
>
> diff --git a/configure b/configure
> index 7071f525843..bc2953e6114 100755
> --- a/configure
> +++ b/configure
> @@ -278,6 +278,8 @@ ld_has() {
>   
>   # default parameters
>   source_path=$(dirname "$0")
> +# make source path absolute
> +source_path=$(cd "$source_path"; pwd)

While we are at it, can't source_path be set only once?

And probably $(dirname -- "$0") is a little more robust, it covers the 
case of directories starting with a dash, so maybe:

# default parameters
# make source path absolute
source_path=$(cd "$(dirname -- "$0")"; pwd)
...

>   cpu=""
>   iasl="iasl"
>   interp_prefix="/usr/gnemul/qemu-%M"
> @@ -519,8 +521,6 @@ for opt do
>     ;;
>     --cxx=*) CXX="$optarg"
>     ;;
> -  --source-path=*) source_path="$optarg"
> -  ;;
>     --cpu=*) cpu="$optarg"
>     ;;
>     --extra-cflags=*) QEMU_CFLAGS="$QEMU_CFLAGS $optarg"
> @@ -599,9 +599,6 @@ if test "$debug_info" = "yes"; then
>       LDFLAGS="-g $LDFLAGS"
>   fi
>   
> -# make source path absolute
> -source_path=$(cd "$source_path"; pwd)
> -
>   # running configure in the source tree?
>   # we know that's the case if configure is there.
>   if test -f "./configure"; then
> @@ -945,8 +942,6 @@ for opt do
>     ;;
>     --interp-prefix=*) interp_prefix="$optarg"
>     ;;
> -  --source-path=*)
> -  ;;
>     --cross-prefix=*)
>     ;;
>     --cc=*)
> @@ -1624,7 +1619,6 @@ $(echo Available targets: $default_target_list | \
>     fold -s -w 53 | sed -e 's/^/                           /')
>   
>   Advanced options (experts only):
> -  --source-path=PATH       path of source code [$source_path]
>     --cross-prefix=PREFIX    use PREFIX for compile tools [$cross_prefix]
>     --cc=CC                  use C compiler CC [$cc]
>     --iasl=IASL              use ACPI compiler IASL [$iasl]