This patchset fixes a problem with our build infrastructure
that meant that MST's recent 'pci, pc, virtio' pullreq failed
tests.

Currently our configure script has a wildcard loop that creates
symlinks for every data file in tests/acpi-test-data from the
source tree to the build tree. However, if a new data file is
added in git, there is nothing that causes configure to be rerun,
and so it is not available in the build tree, which can cause
test failures.

The core of the change here is that instead of linking every
individual data file, we just symlink the directory itself (which
we do in patch 2 for this and also tests/hex-loader-check-data).
The rest of the patchset deals with:
 * renaming a shell variable to make it clearer that it's OK
   to put directories to symlink in it, not just files
 * adding a comment about why wildcarding is a bad idea
 * cleaning up a couple of places that did their own symlinking
   when they could have just used the existing code

We do still use wildcarding to construct a list of files in
pc-bios to be symlinked; we get away with this because we don't
in practice add new BIOS images often and if we do there's also
usually a change that means configure is rerun anyway. We can't
just symlink all of pc-bios into the build tree because it
contains other things than just generated binaries. There's
scope for fixing this, I think, but I wanted to get this fix out.

thanks
-- PMM

Peter Maydell (3):
  configure: Rename FILES variable to LINKS
  configure: Symlink entire test directories rather than individual
    files
  configure: Use FILES loop for all build tree symlinks

 configure | 58 ++++++++++++++++++++++++-------------------------------
 1 file changed, 25 insertions(+), 33 deletions(-)

-- 
2.19.1

On 30 October 2018 at 12:50, Peter Maydell <peter.maydell@linaro.org> wrote:
> This patchset fixes a problem with our build infrastructure
> that meant that MST's recent 'pci, pc, virtio' pullreq failed
> tests.
>
> Currently our configure script has a wildcard loop that creates
> symlinks for every data file in tests/acpi-test-data from the
> source tree to the build tree. However, if a new data file is
> added in git, there is nothing that causes configure to be rerun,
> and so it is not available in the build tree, which can cause
> test failures.
>
> The core of the change here is that instead of linking every
> individual data file, we just symlink the directory itself (which
> we do in patch 2 for this and also tests/hex-loader-check-data).
> The rest of the patchset deals with:
>  * renaming a shell variable to make it clearer that it's OK
>    to put directories to symlink in it, not just files
>  * adding a comment about why wildcarding is a bad idea
>  * cleaning up a couple of places that did their own symlinking
>    when they could have just used the existing code
>
> We do still use wildcarding to construct a list of files in
> pc-bios to be symlinked; we get away with this because we don't
> in practice add new BIOS images often and if we do there's also
> usually a change that means configure is rerun anyway. We can't
> just symlink all of pc-bios into the build tree because it
> contains other things than just generated binaries. There's
> scope for fixing this, I think, but I wanted to get this fix out.

...having sent this out and moved on to something else, I have
discovered a problem with it. If you configure a build tree
using this, and then switch back to a git branch which has
the older configure, then that configure will end up trashing
all the tests/acpi-test-data and tests/hex-loader-check-data
files in the source tree by replacing them with symlinks to
themselves... This is fixable by doing "git checkout tests"
but it's a bit of a landmine for bisection :-(

thanks
-- PMM

On 30 October 2018 at 13:01, Peter Maydell <peter.maydell@linaro.org> wrote:
> ...having sent this out and moved on to something else, I have
> discovered a problem with it. If you configure a build tree
> using this, and then switch back to a git branch which has
> the older configure, then that configure will end up trashing
> all the tests/acpi-test-data and tests/hex-loader-check-data
> files in the source tree by replacing them with symlinks to
> themselves... This is fixable by doing "git checkout tests"
> but it's a bit of a landmine for bisection :-(

Maybe we should standardize on having all data files for tests
be in tests/data, which is and has always been a symlink to
a directory. That would avoid this problem with old versions
of configure mangling the source directory by accident.

thanks
-- PMM

On 30/10/18 14:08, Peter Maydell wrote:
> On 30 October 2018 at 13:01, Peter Maydell <peter.maydell@linaro.org> wrote:
>> ...having sent this out and moved on to something else, I have
>> discovered a problem with it. If you configure a build tree
>> using this, and then switch back to a git branch which has
>> the older configure, then that configure will end up trashing
>> all the tests/acpi-test-data and tests/hex-loader-check-data
>> files in the source tree by replacing them with symlinks to
>> themselves... This is fixable by doing "git checkout tests"
>> but it's a bit of a landmine for bisection :-(

Got there now switching to another series after testing yours ;)

> Maybe we should standardize on having all data files for tests
> be in tests/data, which is and has always been a symlink to
> a directory. That would avoid this problem with old versions
> of configure mangling the source directory by accident.

This seems the clever move.

> 
> thanks
> -- PMM
>