| Message ID | 1400077612-28488-1-git-send-email-peter.maydell@linaro.org |
|---|---|
| State | Rejected |
| Headers | show |
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
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
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
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
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
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
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(+)