All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Marc-André Lureau" <marcandre.lureau@gmail.com>
To: Michael Roth <mdroth@linux.vnet.ibm.com>
Cc: QEMU <qemu-devel@nongnu.org>, Peter Maydell <peter.maydell@linaro.org>
Subject: Re: [Qemu-devel] [PATCH 1/2] build-sys: define QEMU_SRC_PATH
Date: Wed, 15 Jun 2016 12:46:31 +0200	[thread overview]
Message-ID: <CAJ+F1CKgijEzXsEDxWpY4TqWpbn+XnUTXkH4RxgujSMOPUNkxA@mail.gmail.com> (raw)
In-Reply-To: <20160614215949.6421.413@loki>

Hi

On Tue, Jun 14, 2016 at 11:59 PM, Michael Roth
<mdroth@linux.vnet.ibm.com> wrote:
> Quoting marcandre.lureau@redhat.com (2016-06-14 08:16:53)
>> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>>
>> Define QEMU_SRC_PATH in config-host.h, to ease accessing of tests data
>> files.
>>
>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> 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

  reply	other threads:[~2016-06-15 10:46 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-06-14 13:16 [Qemu-devel] [PATCH 0/2] Remove test-qga temporary file marcandre.lureau
2016-06-14 13:16 ` [Qemu-devel] [PATCH 1/2] build-sys: define QEMU_SRC_PATH marcandre.lureau
2016-06-14 21:59   ` Michael Roth
2016-06-15 10:46     ` Marc-André Lureau [this message]
2016-06-14 13:16 ` [Qemu-devel] [PATCH 2/2] tests: use static qga config file marcandre.lureau

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAJ+F1CKgijEzXsEDxWpY4TqWpbn+XnUTXkH4RxgujSMOPUNkxA@mail.gmail.com \
    --to=marcandre.lureau@gmail.com \
    --cc=mdroth@linux.vnet.ibm.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.