Re: [Qemu-devel] [PATCH 1/2] build-sys: define QEMU_SRC_PATH

[Marc-André Lureau]

Hi

On Tue, Jun 14, 2016 at 11:59 PM, Michael Roth
<address@hidden> wrote:
> Quoting address@hidden (2016-06-14 08:16:53)
>> From: Marc-André Lureau <address@hidden>
>>
>> Define QEMU_SRC_PATH in config-host.h, to ease accessing of tests data
>> files.
>>
>> Signed-off-by: Marc-André Lureau <address@hidden>
>
> I know this avoids the need to define environment variables for
> individual test targets to pass on SRC_PATH, but the fact that
> we didn't rely on this before made me a bit apprehensive about
> suggesting it. qemu-iotests for instance relies on a symlink
> back to SRC_PATH, and check-qapi-schema tests feed individual
> schema files to a script via tests/Makefile.include. Both can
> be made to check against a different/changing SRC_PATH, but
> if we bake it into the code that's not possible.

Right, so let's use a symlink too?

+if [ ! -e tests/test-qga-config ]; then
+    symlink "$source_path/tests/test-qga-config" tests/test-qga-config
+fi

>
> I'm not sure if there's a valid use-case for needing to do
> so, but it seems to be bad form to not have any mechanism
> to change them without recompiling. Personally I think I'd
> prefer the environment variable approach, even if it means
> needing to add it per-target. I think if we wanted to get
> fancy we could do this via a recipe that exports environment
> variables added to a lazily-evaluated Makefile variable by
> each target's dependencies, but it's probably not worth it
> outside of a more general cleanup to how we handle
> SRC_PATH/BUILD_DIR dependencies throughout tests/

I agree an environment variable would be nice (along with helper
functions to lookup test data files), but I think we should stick with
the common symlink way for now.

I'll resend the patch, thanks

-- 
Marc-André Lureau