| Message ID | 1465910214-22516-2-git-send-email-marcandre.lureau@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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. 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/ If others are fine with the approach taken here though I wouldn't hold things up. > --- > scripts/create_config | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/scripts/create_config b/scripts/create_config > index 1dd6a35..2fbe126 100755 > --- a/scripts/create_config > +++ b/scripts/create_config > @@ -116,6 +116,9 @@ case $line in > DSOSUF=*) > echo "#define HOST_DSOSUF \"${line#*=}\"" > ;; > + SRC_PATH=*) > + echo "#define QEMU_SRC_PATH \"${line#*=}\"" > + ;; > esac > > done # read > -- > 2.7.4 >
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
diff --git a/scripts/create_config b/scripts/create_config index 1dd6a35..2fbe126 100755 --- a/scripts/create_config +++ b/scripts/create_config @@ -116,6 +116,9 @@ case $line in DSOSUF=*) echo "#define HOST_DSOSUF \"${line#*=}\"" ;; + SRC_PATH=*) + echo "#define QEMU_SRC_PATH \"${line#*=}\"" + ;; esac done # read