[Qemu-devel] [PATCH] configure: Ensure tests/qemu-iotests exists before writing common.env

[Qemu-devel] [PATCH] configure: Ensure tests/qemu-iotests exists before writing common.env

[Peter Maydell]

Before we write common.env to the tests/qemu-iotests directory, ensure
that it exists. This fixes out-of-tree builds from clean.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
If somebody would like to review this I'll apply it to master as
a buildfix...

 configure | 1 +
 1 file changed, 1 insertion(+)

diff --git a/configure b/configure
index 6adfa72..c4e43ed 100755
--- a/configure
+++ b/configure
@@ -4770,6 +4770,7 @@ fi
 
 iotests_common_env="tests/qemu-iotests/common.env"
 
+mkdir -p "$(dirname "$iotests_common_env")"
 echo "# Automatically generated by configure - do not modify" > $iotests_common_env
 echo >> $iotests_common_env
 echo "PYTHON='$python'" >> $iotests_common_env
-- 
1.9.2

Re: [Qemu-devel] [PATCH] configure: Ensure tests/qemu-iotests exists before writing common.env

[Kevin Wolf]

Am 14.05.2014 um 16:26 hat Peter Maydell geschrieben:
> Before we write common.env to the tests/qemu-iotests directory, ensure
> that it exists. This fixes out-of-tree builds from clean.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> If somebody would like to review this I'll apply it to master as
> a buildfix...
> 
>  configure | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/configure b/configure
> index 6adfa72..c4e43ed 100755
> --- a/configure
> +++ b/configure
> @@ -4770,6 +4770,7 @@ fi
>  
>  iotests_common_env="tests/qemu-iotests/common.env"
>  
> +mkdir -p "$(dirname "$iotests_common_env")"
>  echo "# Automatically generated by configure - do not modify" > $iotests_common_env
>  echo >> $iotests_common_env
>  echo "PYTHON='$python'" >> $iotests_common_env

We already have an mkdir -p, in the section at line 5161ff. Perhaps we
should just move that to further above?

But as this is a build fix, I don't want to start bikeshedding, so I'm
not objecting to your patch.

Kevin

Re: [Qemu-devel] [PATCH] configure: Ensure tests/qemu-iotests exists before writing common.env

[Peter Maydell]

On 14 May 2014 15:32, Kevin Wolf <kwolf@redhat.com> wrote:
> Am 14.05.2014 um 16:26 hat Peter Maydell geschrieben:
>> Before we write common.env to the tests/qemu-iotests directory, ensure
>> that it exists. This fixes out-of-tree builds from clean.
>>
>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>> ---
>> If somebody would like to review this I'll apply it to master as
>> a buildfix...
>>
>>  configure | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/configure b/configure
>> index 6adfa72..c4e43ed 100755
>> --- a/configure
>> +++ b/configure
>> @@ -4770,6 +4770,7 @@ fi
>>
>>  iotests_common_env="tests/qemu-iotests/common.env"
>>
>> +mkdir -p "$(dirname "$iotests_common_env")"
>>  echo "# Automatically generated by configure - do not modify" > $iotests_common_env
>>  echo >> $iotests_common_env
>>  echo "PYTHON='$python'" >> $iotests_common_env
>
> We already have an mkdir -p, in the section at line 5161ff. Perhaps we
> should just move that to further above?
>
> But as this is a build fix, I don't want to start bikeshedding, so I'm
> not objecting to your patch.

Yes, there are other mkdirs scattered through the configure
script that we could perhaps collect up and move to the top
with that whole section of mkdir/symlink code, as a later
cleanup.

thanks
-- PMM

Re: [Qemu-devel] [PATCH] configure: Ensure tests/qemu-iotests exists before writing common.env

[Kevin Wolf]

Am 14.05.2014 um 16:46 hat Peter Maydell geschrieben:
> On 14 May 2014 15:32, Kevin Wolf <kwolf@redhat.com> wrote:
> > Am 14.05.2014 um 16:26 hat Peter Maydell geschrieben:
> >> Before we write common.env to the tests/qemu-iotests directory, ensure
> >> that it exists. This fixes out-of-tree builds from clean.
> >>
> >> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> >> ---
> >> If somebody would like to review this I'll apply it to master as
> >> a buildfix...
> >>
> >>  configure | 1 +
> >>  1 file changed, 1 insertion(+)
> >>
> >> diff --git a/configure b/configure
> >> index 6adfa72..c4e43ed 100755
> >> --- a/configure
> >> +++ b/configure
> >> @@ -4770,6 +4770,7 @@ fi
> >>
> >>  iotests_common_env="tests/qemu-iotests/common.env"
> >>
> >> +mkdir -p "$(dirname "$iotests_common_env")"
> >>  echo "# Automatically generated by configure - do not modify" > $iotests_common_env
> >>  echo >> $iotests_common_env
> >>  echo "PYTHON='$python'" >> $iotests_common_env
> >
> > We already have an mkdir -p, in the section at line 5161ff. Perhaps we
> > should just move that to further above?
> >
> > But as this is a build fix, I don't want to start bikeshedding, so I'm
> > not objecting to your patch.
> 
> Yes, there are other mkdirs scattered through the configure
> script that we could perhaps collect up and move to the top
> with that whole section of mkdir/symlink code, as a later
> cleanup.

Sounds good to me.

Kevin

Re: [Qemu-devel] [PATCH] configure: Ensure tests/qemu-iotests exists before writing common.env

[Max Reitz]

On 14.05.2014 16:26, Peter Maydell wrote:
> Before we write common.env to the tests/qemu-iotests directory, ensure
> that it exists. This fixes out-of-tree builds from clean.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> If somebody would like to review this I'll apply it to master as
> a buildfix...
>
>   configure | 1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/configure b/configure
> index 6adfa72..c4e43ed 100755
> --- a/configure
> +++ b/configure
> @@ -4770,6 +4770,7 @@ fi
>   
>   iotests_common_env="tests/qemu-iotests/common.env"
>   
> +mkdir -p "$(dirname "$iotests_common_env")"
>   echo "# Automatically generated by configure - do not modify" > $iotests_common_env
>   echo >> $iotests_common_env
>   echo "PYTHON='$python'" >> $iotests_common_env

If we do this, we'd have a commen.env which we cannot use, as all the 
tests are still in the original source tree.

I'd rather use "$source_path/tests/qemu_iotests/common.env" instead, or, 
if that is unacceptable as it modifies the original source tree (which 
is probably not desired when doing an out-of-tree build), write 
common.env only if this is an in-tree build.

Max

Re: [Qemu-devel] [PATCH] configure: Ensure tests/qemu-iotests exists before writing common.env

[Peter Maydell]

On 14 May 2014 23:47, Max Reitz <mreitz@redhat.com> wrote:
> On 14.05.2014 16:26, Peter Maydell wrote:
>>
>> Before we write common.env to the tests/qemu-iotests directory, ensure
>> that it exists. This fixes out-of-tree builds from clean.
>>
>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>> ---
>> If somebody would like to review this I'll apply it to master as
>> a buildfix...
>>
>>   configure | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/configure b/configure
>> index 6adfa72..c4e43ed 100755
>> --- a/configure
>> +++ b/configure
>> @@ -4770,6 +4770,7 @@ fi
>>     iotests_common_env="tests/qemu-iotests/common.env"
>>   +mkdir -p "$(dirname "$iotests_common_env")"
>>   echo "# Automatically generated by configure - do not modify" >
>> $iotests_common_env
>>   echo >> $iotests_common_env
>>   echo "PYTHON='$python'" >> $iotests_common_env
>
>
> If we do this, we'd have a commen.env which we cannot use, as all the tests
> are still in the original source tree.

OK, this is busted then. We need to revert your original patch;
when we've figured out how to do it properly we can apply
a corrected version.

> I'd rather use "$source_path/tests/qemu_iotests/common.env" instead, or, if
> that is unacceptable as it modifies the original source tree (which is
> probably not desired when doing an out-of-tree build), write common.env only
> if this is an in-tree build.

You're correct, we must not write to the original source tree.
We mustn't behave differently on in-tree and out-of-tree builds
either.

thanks
-- PMM