[libvirt] [PATCH 0/8] Remove some unnecessary compatibility stuff

Andrea Bolognani (8): tests: Drop CONFIG_HEADER from TESTS_ENVIRONMENT tests: Drop LIBVIRT_DRIVER_DIR from TESTS_ENVIRONMENT tests: Don't use TEST_DRIVER_DIR in virTestCaptureProgramExecChild() tests: Don't define TEST_DRIVER_DIR tests: Stop looking for abs_top_srcdir in the environment Fix names for abs_top_{src,build}dir variables tests: Don't redefine variables for TESTS_ENVIRONMENT Don't define abs_* variables ourselves src/Makefile.am | 10 ++-------- src/conf/domain_conf.c | 2 +- src/cpu/cpu_map.c | 4 ++-- src/driver.c | 2 +- src/locking/lock_driver_lockd.c | 2 +- src/locking/lock_manager.c | 2 +- src/logging/log_manager.c | 2 +- src/lxc/lxc_conf.c | 2 +- src/network/bridge_driver.c | 2 +- src/remote/remote_driver.c | 2 +- src/storage/storage_backend.c | 2 +- src/storage/storage_backend_disk.c | 4 ++-- src/util/virfile.c | 4 ++-- src/util/virstoragefilebackend.c | 2 +- tests/Makefile.am | 31 ++++++------------------------ tests/libxlxml2domconfigtest.c | 5 ----- tests/networkxml2firewalltest.c | 6 ------ tests/nwfilterxml2firewalltest.c | 6 ------ tests/qemumemlocktest.c | 5 ----- tests/qemuxml2argvtest.c | 5 ----- tests/testutils.c | 1 - tests/virschematest.c | 2 +- tests/virscsitest.c | 5 ----- tests/virshtest.c | 4 ++-- tests/virtestmock.c | 4 ++-- 25 files changed, 29 insertions(+), 87 deletions(-) -- 2.20.1

It's no longer used as of commit a9694a8e183e. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- tests/Makefile.am | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/Makefile.am b/tests/Makefile.am index a7f1b39a5e..52a64c9eb1 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -486,7 +486,6 @@ TESTS_ENVIRONMENT = \ abs_top_srcdir=`cd '$(top_srcdir)'; pwd` \ abs_builddir=$(abs_builddir) \ abs_srcdir=$(abs_srcdir) \ - CONFIG_HEADER="$(lv_abs_top_builddir)/config.h" \ SHELL="$(SHELL)" \ LIBVIRT_DRIVER_DIR="$(lv_abs_top_builddir)/src/.libs" \ LIBVIRT_AUTOSTART=0 \ -- 2.20.1

LIBVIRT_DRIVER_DIR is defined as (what is for all intents and purposes equivalent to) "$(abs_top_builddir)/src/.libs"; however, as of commit bc6e206322ae, virDriverLoadModule() will search that directory automatically, which means passing it through the environment is no longer necessary. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- tests/Makefile.am | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/Makefile.am b/tests/Makefile.am index 52a64c9eb1..adbe2f3d5e 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -487,7 +487,6 @@ TESTS_ENVIRONMENT = \ abs_builddir=$(abs_builddir) \ abs_srcdir=$(abs_srcdir) \ SHELL="$(SHELL)" \ - LIBVIRT_DRIVER_DIR="$(lv_abs_top_builddir)/src/.libs" \ LIBVIRT_AUTOSTART=0 \ LC_ALL=C \ VIR_TEST_EXPENSIVE=$(VIR_TEST_EXPENSIVE) \ -- 2.20.1

TEST_DRIVER_DIR is defined as "$(top_builddir)/src/.libs"; however, as of commit bc6e206322ae, virDriverLoadModule() will search (the absolute version of) that directory automatically, which means passing it through the environment is no longer necessary. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- tests/testutils.c | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/testutils.c b/tests/testutils.c index d2aa4e5d49..245b1832f6 100644 --- a/tests/testutils.c +++ b/tests/testutils.c @@ -443,7 +443,6 @@ void virTestCaptureProgramExecChild(const char *const argv[], int stdinfd = -1; const char *const env[] = { "LANG=C", - "LIBVIRT_DRIVER_DIR=" TEST_DRIVER_DIR, NULL }; -- 2.20.1

It's no longer used anywhere. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- tests/Makefile.am | 3 --- 1 file changed, 3 deletions(-) diff --git a/tests/Makefile.am b/tests/Makefile.am index adbe2f3d5e..237336d648 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -63,9 +63,6 @@ DRIVERLIB_LDFLAGS = \ -rpath /evil/libtool/hack/to/force/shared/lib/creation \ $(MINGW_EXTRA_LDFLAGS) -AM_CPPFLAGS += \ - -DTEST_DRIVER_DIR=\"$(top_builddir)/src/.libs\" - PROBES_O = if WITH_DTRACE_PROBES PROBES_O += ../src/libvirt_probes.lo -- 2.20.1

This code snippet has clearly been cargo-culted, and all its instances can be safely dropped seeing as 1) a much better way to handle the scenario in C programs would be to pass the value via the preprocessor, and 2) the value is actually not used anywhere after being defined. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- tests/libxlxml2domconfigtest.c | 5 ----- tests/networkxml2firewalltest.c | 6 ------ tests/nwfilterxml2firewalltest.c | 6 ------ tests/qemumemlocktest.c | 5 ----- tests/qemuxml2argvtest.c | 5 ----- tests/virscsitest.c | 5 ----- 6 files changed, 32 deletions(-) diff --git a/tests/libxlxml2domconfigtest.c b/tests/libxlxml2domconfigtest.c index 863d8cded5..a96422b3f9 100644 --- a/tests/libxlxml2domconfigtest.c +++ b/tests/libxlxml2domconfigtest.c @@ -41,7 +41,6 @@ # define VIR_FROM_THIS VIR_FROM_LIBXL -static const char *abs_top_srcdir; static virCapsPtr caps; static int @@ -172,10 +171,6 @@ mymain(void) { int ret = 0; - abs_top_srcdir = getenv("abs_top_srcdir"); - if (!abs_top_srcdir) - abs_top_srcdir = abs_srcdir "/.."; - /* Set the timezone because we are mocking the time() function. * If we don't do that, then localtime() may return unpredictable * results. In order to detect things that just work by a blind diff --git a/tests/networkxml2firewalltest.c b/tests/networkxml2firewalltest.c index 91bb6e6b8a..575b68379a 100644 --- a/tests/networkxml2firewalltest.c +++ b/tests/networkxml2firewalltest.c @@ -36,8 +36,6 @@ # define VIR_FROM_THIS VIR_FROM_NONE -static const char *abs_top_srcdir; - # ifdef __linux__ # define RULESTYPE "linux" # else @@ -136,10 +134,6 @@ mymain(void) { int ret = 0; - abs_top_srcdir = getenv("abs_top_srcdir"); - if (!abs_top_srcdir) - abs_top_srcdir = abs_srcdir "/.."; - # define DO_TEST(name) \ do { \ static struct testInfo info = { \ diff --git a/tests/nwfilterxml2firewalltest.c b/tests/nwfilterxml2firewalltest.c index b4cfe5769e..bea267f0fc 100644 --- a/tests/nwfilterxml2firewalltest.c +++ b/tests/nwfilterxml2firewalltest.c @@ -35,8 +35,6 @@ # define VIR_FROM_THIS VIR_FROM_NONE -static const char *abs_top_srcdir; - # ifdef __linux__ # define RULESTYPE "linux" # else @@ -459,10 +457,6 @@ mymain(void) { int ret = 0; - abs_top_srcdir = getenv("abs_top_srcdir"); - if (!abs_top_srcdir) - abs_top_srcdir = abs_srcdir "/.."; - # define DO_TEST(name) \ do { \ static struct testInfo info = { \ diff --git a/tests/qemumemlocktest.c b/tests/qemumemlocktest.c index 786f647cf0..42a4643338 100644 --- a/tests/qemumemlocktest.c +++ b/tests/qemumemlocktest.c @@ -19,7 +19,6 @@ # define VIR_FROM_THIS VIR_FROM_QEMU -static const char *abs_top_srcdir; static virQEMUDriver driver; struct testInfo { @@ -74,10 +73,6 @@ mymain(void) setenv("LIBVIRT_FAKE_ROOT_DIR", fakerootdir, 1); - abs_top_srcdir = getenv("abs_top_srcdir"); - if (!abs_top_srcdir) - abs_top_srcdir = abs_srcdir "/.."; - if (qemuTestDriverInit(&driver) < 0) { VIR_FREE(fakerootdir); return EXIT_FAILURE; diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 03e8d79595..67c5c74ec5 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -33,7 +33,6 @@ # define VIR_FROM_THIS VIR_FROM_QEMU -static const char *abs_top_srcdir; static virQEMUDriver driver; static unsigned char * @@ -653,10 +652,6 @@ mymain(void) setenv("LIBVIRT_FAKE_ROOT_DIR", fakerootdir, 1); - abs_top_srcdir = getenv("abs_top_srcdir"); - if (!abs_top_srcdir) - abs_top_srcdir = abs_srcdir "/.."; - /* Set the timezone because we are mocking the time() function. * If we don't do that, then localtime() may return unpredictable * results. In order to detect things that just work by a blind diff --git a/tests/virscsitest.c b/tests/virscsitest.c index 665f70e28b..be3ef6234e 100644 --- a/tests/virscsitest.c +++ b/tests/virscsitest.c @@ -30,7 +30,6 @@ VIR_LOG_INIT("tests.scsitest"); -static const char *abs_top_srcdir; static char *virscsi_prefix; static int @@ -195,10 +194,6 @@ mymain(void) char *tmpdir = NULL; char template[] = "/tmp/libvirt_XXXXXX"; - abs_top_srcdir = getenv("abs_top_srcdir"); - if (!abs_top_srcdir) - abs_top_srcdir = abs_srcdir "/.."; - if (virAsprintf(&virscsi_prefix, "%s" VIR_SCSI_DATA, abs_srcdir) < 0) { ret = -1; goto cleanup; -- 2.20.1

According to the official documentation for autoconf[1], the correct names for these variables are abs_top_{src,build}dir rather than abs_top{src,build}dir; in fact, we're already using the correct names in various places, so let's just make everything nice and consistent. [1] https://www.gnu.org/software/autoconf/manual/autoconf-2.69/html_node/Preset-... Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/Makefile.am | 8 ++++---- src/conf/domain_conf.c | 2 +- src/cpu/cpu_map.c | 4 ++-- src/driver.c | 2 +- src/locking/lock_driver_lockd.c | 2 +- src/locking/lock_manager.c | 2 +- src/logging/log_manager.c | 2 +- src/lxc/lxc_conf.c | 2 +- src/network/bridge_driver.c | 2 +- src/remote/remote_driver.c | 2 +- src/storage/storage_backend.c | 2 +- src/storage/storage_backend_disk.c | 4 ++-- src/util/virfile.c | 4 ++-- src/util/virstoragefilebackend.c | 2 +- tests/Makefile.am | 8 ++++---- tests/virschematest.c | 2 +- tests/virshtest.c | 4 ++-- tests/virtestmock.c | 4 ++-- 18 files changed, 29 insertions(+), 29 deletions(-) diff --git a/src/Makefile.am b/src/Makefile.am index 8c8dfe3dcf..0d3d231889 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -18,9 +18,9 @@ # old automake does not provide abs_{src,build}dir variables abs_builddir = $(shell pwd) -abs_topbuilddir = $(shell cd .. && pwd) +abs_top_builddir = $(shell cd .. && pwd) abs_srcdir = $(shell cd $(srcdir) && pwd) -abs_topsrcdir = $(shell cd $(top_srcdir) && pwd) +abs_top_srcdir = $(shell cd $(top_srcdir) && pwd) # No libraries with the exception of LIBXML should be listed # here. List them against the individual XXX_la_CFLAGS targets @@ -33,8 +33,8 @@ AM_CPPFLAGS = -I../gnulib/lib \ -I$(srcdir)/util \ -I./util \ -DIN_LIBVIRT \ - -Dabs_topbuilddir="\"$(abs_topbuilddir)\"" \ - -Dabs_topsrcdir="\"$(abs_topsrcdir)\"" \ + -Dabs_top_builddir="\"$(abs_top_builddir)\"" \ + -Dabs_top_srcdir="\"$(abs_top_srcdir)\"" \ $(NULL) WARN_CFLAGS += $(STRICT_FRAME_LIMIT_CFLAGS) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index cfd7221538..0eee982989 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -19396,7 +19396,7 @@ virDomainDefParseXML(xmlDocPtr xml, VIR_AUTOFREE(char *) schema = NULL; schema = virFileFindResource("domain.rng", - abs_topsrcdir "/docs/schemas", + abs_top_srcdir "/docs/schemas", PKGDATADIR "/schemas"); if (!schema) return NULL; diff --git a/src/cpu/cpu_map.c b/src/cpu/cpu_map.c index 83151c1c54..cb9f7c243f 100644 --- a/src/cpu/cpu_map.c +++ b/src/cpu/cpu_map.c @@ -96,7 +96,7 @@ cpuMapLoadInclude(const char *filename, char *mapfile; if (!(mapfile = virFileFindResource(filename, - abs_topsrcdir "/src/cpu_map", + abs_top_srcdir "/src/cpu_map", PKGDATADIR "/cpu_map"))) return -1; @@ -185,7 +185,7 @@ int cpuMapLoad(const char *arch, char *mapfile; if (!(mapfile = virFileFindResource("index.xml", - abs_topsrcdir "/src/cpu_map", + abs_top_srcdir "/src/cpu_map", PKGDATADIR "/cpu_map"))) return -1; diff --git a/src/driver.c b/src/driver.c index 8b5ade763f..5e8f68f6df 100644 --- a/src/driver.c +++ b/src/driver.c @@ -54,7 +54,7 @@ virDriverLoadModule(const char *name, if (!(modfile = virFileFindResourceFull(name, "libvirt_driver_", ".so", - abs_topbuilddir "/src/.libs", + abs_top_builddir "/src/.libs", DEFAULT_DRIVER_DIR, "LIBVIRT_DRIVER_DIR"))) return -1; diff --git a/src/locking/lock_driver_lockd.c b/src/locking/lock_driver_lockd.c index 4ffa92fc9b..f6371f3050 100644 --- a/src/locking/lock_driver_lockd.c +++ b/src/locking/lock_driver_lockd.c @@ -221,7 +221,7 @@ static virNetClientPtr virLockManagerLockDaemonConnectionNew(bool privileged, if (!privileged && !(daemonPath = virFileFindResourceFull("virtlockd", NULL, NULL, - abs_topbuilddir "/src", + abs_top_builddir "/src", SBINDIR, "VIRTLOCKD_PATH"))) goto error; diff --git a/src/locking/lock_manager.c b/src/locking/lock_manager.c index d421b6acfc..1c8705ab11 100644 --- a/src/locking/lock_manager.c +++ b/src/locking/lock_manager.c @@ -141,7 +141,7 @@ virLockManagerPluginPtr virLockManagerPluginNew(const char *name, if (!(modfile = virFileFindResourceFull(name, NULL, ".so", - abs_topbuilddir "/src/.libs", + abs_top_builddir "/src/.libs", LIBDIR "/libvirt/lock-driver", "LIBVIRT_LOCK_MANAGER_PLUGIN_DIR"))) goto cleanup; diff --git a/src/logging/log_manager.c b/src/logging/log_manager.c index cd7935802d..eb0a32b4b6 100644 --- a/src/logging/log_manager.c +++ b/src/logging/log_manager.c @@ -80,7 +80,7 @@ virLogManagerConnect(bool privileged, if (!privileged && !(daemonPath = virFileFindResourceFull("virtlogd", NULL, NULL, - abs_topbuilddir "/src", + abs_top_builddir "/src", SBINDIR, "VIRTLOGD_PATH"))) goto error; diff --git a/src/lxc/lxc_conf.c b/src/lxc/lxc_conf.c index 4720c99793..2e4cc4f51a 100644 --- a/src/lxc/lxc_conf.c +++ b/src/lxc/lxc_conf.c @@ -94,7 +94,7 @@ virCapsPtr virLXCDriverCapsInit(virLXCDriverPtr driver) } if (!(lxc_path = virFileFindResource("libvirt_lxc", - abs_topbuilddir "/src", + abs_top_builddir "/src", LIBEXECDIR))) goto error; diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index c3e1381124..6789dafd15 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -1488,7 +1488,7 @@ networkBuildDhcpDaemonCommandLine(virNetworkDriverStatePtr driver, /* This helper is used to create custom leases file for libvirt */ if (!(leaseshelper_path = virFileFindResource("libvirt_leaseshelper", - abs_topbuilddir "/src", + abs_top_builddir "/src", LIBEXECDIR))) goto cleanup; diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index eabe7a3823..5c4dd41227 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -1079,7 +1079,7 @@ doRemoteOpen(virConnectPtr conn, if ((flags & VIR_DRV_OPEN_REMOTE_AUTOSTART) && !(daemonPath = virFileFindResourceFull("libvirtd", NULL, NULL, - abs_topbuilddir "/src", + abs_top_builddir "/src", SBINDIR, "LIBVIRTD_PATH"))) goto failed; diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index 295d8de479..ed5bdd5bad 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -92,7 +92,7 @@ virStorageDriverLoadBackendModule(const char *name, if (!(modfile = virFileFindResourceFull(name, "libvirt_storage_backend_", ".so", - abs_topbuilddir "/src/.libs", + abs_top_builddir "/src/.libs", STORAGE_BACKEND_MODULE_DIR, "LIBVIRT_STORAGE_BACKEND_DIR"))) return -1; diff --git a/src/storage/storage_backend_disk.c b/src/storage/storage_backend_disk.c index 4c5b784ca7..9b94d26cf9 100644 --- a/src/storage/storage_backend_disk.c +++ b/src/storage/storage_backend_disk.c @@ -363,7 +363,7 @@ virStorageBackendDiskReadPartitions(virStoragePoolObjPtr pool, VIR_AUTOPTR(virCommand) cmd = NULL; if (!(parthelper_path = virFileFindResource("libvirt_parthelper", - abs_topbuilddir "/src", + abs_top_builddir "/src", LIBEXECDIR))) return -1; @@ -417,7 +417,7 @@ virStorageBackendDiskReadGeometry(virStoragePoolObjPtr pool) VIR_AUTOPTR(virCommand) cmd = NULL; if (!(parthelper_path = virFileFindResource("libvirt_parthelper", - abs_topbuilddir "/src", + abs_top_builddir "/src", LIBEXECDIR))) return -1; diff --git a/src/util/virfile.c b/src/util/virfile.c index a4c6d184af..ec8d85929c 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -260,7 +260,7 @@ virFileWrapperFdNew(int *fd, const char *name, unsigned int flags) } if (!(iohelper_path = virFileFindResource("libvirt_iohelper", - abs_topbuilddir "/src", + abs_top_builddir "/src", LIBEXECDIR))) goto error; @@ -1704,7 +1704,7 @@ static bool useDirOverride; * @prefix: optional string to prepend to filename * @suffix: optional string to append to filename * @builddir: location of the filename in the build tree including - * abs_topsrcdir or abs_topbuilddir prefix + * abs_top_srcdir or abs_top_builddir prefix * @installdir: location of the installed binary * @envname: environment variable used to override all dirs * diff --git a/src/util/virstoragefilebackend.c b/src/util/virstoragefilebackend.c index 9acaea2f81..f362436d9b 100644 --- a/src/util/virstoragefilebackend.c +++ b/src/util/virstoragefilebackend.c @@ -57,7 +57,7 @@ virStorageFileLoadBackendModule(const char *name, if (!(modfile = virFileFindResourceFull(name, "libvirt_storage_file_", ".so", - abs_topbuilddir "/src/.libs", + abs_top_builddir "/src/.libs", STORAGE_FILE_MODULE_DIR, "LIBVIRT_STORAGE_FILE_DIR"))) return -1; diff --git a/tests/Makefile.am b/tests/Makefile.am index 237336d648..42ec64755d 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -18,9 +18,9 @@ # old automake does not provide abs_{src,build}dir variables abs_builddir = $(shell pwd) -abs_topbuilddir = $(shell cd .. && pwd) +abs_top_builddir = $(shell cd .. && pwd) abs_srcdir = $(shell cd $(srcdir) && pwd) -abs_topsrcdir = $(shell cd $(top_srcdir) && pwd) +abs_top_srcdir = $(shell cd $(top_srcdir) && pwd) SHELL = $(PREFERABLY_POSIX_SHELL) @@ -37,9 +37,9 @@ WARN_CFLAGS += $(RELAXED_FRAME_LIMIT_CFLAGS) AM_CFLAGS = \ -Dabs_builddir="\"$(abs_builddir)\"" \ - -Dabs_topbuilddir="\"$(abs_topbuilddir)\"" \ + -Dabs_top_builddir="\"$(abs_top_builddir)\"" \ -Dabs_srcdir="\"$(abs_srcdir)\"" \ - -Dabs_topsrcdir="\"$(abs_topsrcdir)\"" \ + -Dabs_top_srcdir="\"$(abs_top_srcdir)\"" \ $(LIBXML_CFLAGS) \ $(LIBNL_CFLAGS) \ $(GNUTLS_CFLAGS) \ diff --git a/tests/virschematest.c b/tests/virschematest.c index 8dfab5e2a3..56bdcb2f88 100644 --- a/tests/virschematest.c +++ b/tests/virschematest.c @@ -148,7 +148,7 @@ testSchemaGrammar(const void *opaque) int ret = -1; if (virAsprintf(&schema_path, "%s/docs/schemas/%s", - abs_topsrcdir, data->schema) < 0) + abs_top_srcdir, data->schema) < 0) return -1; if (!(data->validator = virXMLValidatorInit(schema_path))) diff --git a/tests/virshtest.c b/tests/virshtest.c index d77d3d81d4..7e59ad7da6 100644 --- a/tests/virshtest.c +++ b/tests/virshtest.c @@ -82,13 +82,13 @@ testCompareOutputLit(const char *expectData, return result; } -# define VIRSH_DEFAULT abs_topbuilddir "/tools/virsh", \ +# define VIRSH_DEFAULT abs_top_builddir "/tools/virsh", \ "--connect", \ "test:///default" static char *custom_uri; -# define VIRSH_CUSTOM abs_topbuilddir "/tools/virsh", \ +# define VIRSH_CUSTOM abs_top_builddir "/tools/virsh", \ "--connect", \ custom_uri diff --git a/tests/virtestmock.c b/tests/virtestmock.c index 1cafbec0d5..3049c90789 100644 --- a/tests/virtestmock.c +++ b/tests/virtestmock.c @@ -161,8 +161,8 @@ checkPath(const char *path, } - if (!STRPREFIX(path, abs_topsrcdir) && - !STRPREFIX(path, abs_topbuilddir)) { + if (!STRPREFIX(path, abs_top_srcdir) && + !STRPREFIX(path, abs_top_builddir)) { printFile(path, func); } -- 2.20.1

We already have code that defines all abs_* variables at the top of tests/Makefile.am, so there is no point in redefining them a second time (using a slightly different shell incantation to boot). Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- tests/Makefile.am | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/tests/Makefile.am b/tests/Makefile.am index 42ec64755d..858ed047e8 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -469,20 +469,12 @@ endif ! WITH_TESTS TESTS = $(test_programs) \ $(test_scripts) -# NB, automake < 1.10 does not provide the real -# abs_top_{src/build}dir or builddir variables, so don't rely -# on them here. Fake them with 'pwd' -# Also, BSD sh doesn't like 'a=b b=$$a', so we can't use an -# intermediate shell variable, but must do all the expansion in make - -lv_abs_top_builddir=$(shell cd '$(top_builddir)' && pwd) - VIR_TEST_EXPENSIVE ?= $(VIR_TEST_EXPENSIVE_DEFAULT) TESTS_ENVIRONMENT = \ - abs_top_builddir=$(lv_abs_top_builddir) \ - abs_top_srcdir=`cd '$(top_srcdir)'; pwd` \ - abs_builddir=$(abs_builddir) \ - abs_srcdir=$(abs_srcdir) \ + abs_top_builddir="$(abs_top_builddir)" \ + abs_top_srcdir="$(abs_top_srcdir)" \ + abs_builddir="$(abs_builddir)" \ + abs_srcdir="$(abs_srcdir)" \ SHELL="$(SHELL)" \ LIBVIRT_AUTOSTART=0 \ LC_ALL=C \ -- 2.20.1

Apparently this was necessary in the past because old versions of autoconf/automake didn't make them available, but these days all of the platforms we target include recent enough autotools - as evidenced by the fact that, for example, we already use abs_top_srcdir in tools/ despite the fact that tools/Makefile.am is missing the same boilerplate. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/Makefile.am | 6 ------ tests/Makefile.am | 6 ------ 2 files changed, 12 deletions(-) diff --git a/src/Makefile.am b/src/Makefile.am index 0d3d231889..4294bd1c6c 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -16,12 +16,6 @@ ## License along with this library. If not, see ## <http://www.gnu.org/licenses/>. -# old automake does not provide abs_{src,build}dir variables -abs_builddir = $(shell pwd) -abs_top_builddir = $(shell cd .. && pwd) -abs_srcdir = $(shell cd $(srcdir) && pwd) -abs_top_srcdir = $(shell cd $(top_srcdir) && pwd) - # No libraries with the exception of LIBXML should be listed # here. List them against the individual XXX_la_CFLAGS targets # that actually use them. diff --git a/tests/Makefile.am b/tests/Makefile.am index 858ed047e8..7ce327c9e8 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -16,12 +16,6 @@ ## License along with this library. If not, see ## <http://www.gnu.org/licenses/>. -# old automake does not provide abs_{src,build}dir variables -abs_builddir = $(shell pwd) -abs_top_builddir = $(shell cd .. && pwd) -abs_srcdir = $(shell cd $(srcdir) && pwd) -abs_top_srcdir = $(shell cd $(top_srcdir) && pwd) - SHELL = $(PREFERABLY_POSIX_SHELL) AM_CPPFLAGS = \ -- 2.20.1

On Wed, Mar 13, 2019 at 05:51:23PM +0100, Andrea Bolognani wrote:
Apparently this was necessary in the past because old versions of autoconf/automake didn't make them available, but these days all of the platforms we target include recent enough autotools - as evidenced by the fact that, for example, we already use abs_top_srcdir in tools/ despite the fact that tools/Makefile.am is missing the same boilerplate.
Although it's used there only if building wireshark dissector, which less people are building, IMHO.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>

On Thu, 2019-03-14 at 09:45 +0100, Martin Kletzander wrote:
On Wed, Mar 13, 2019 at 05:51:23PM +0100, Andrea Bolognani wrote:
Apparently this was necessary in the past because old versions of autoconf/automake didn't make them available, but these days all of the platforms we target include recent enough autotools - as evidenced by the fact that, for example, we already use abs_top_srcdir in tools/ despite the fact that tools/Makefile.am is missing the same boilerplate.
Although it's used there only if building wireshark dissector, which less people are building, IMHO.
Fair point :) -- Andrea Bolognani / Red Hat / Virtualization

On 3/13/19 11:51 AM, Andrea Bolognani wrote:
Apparently this was necessary in the past because old versions of autoconf/automake didn't make them available, but these days all of the platforms we target include recent enough autotools - as evidenced by the fact that, for example, we already use abs_top_srcdir in tools/ despite the fact that tools/Makefile.am is missing the same boilerplate.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/Makefile.am | 6 ------ tests/Makefile.am | 6 ------ 2 files changed, 12 deletions(-)
Commit 9aef4d96 states that it was automake from RHEL 5 that caused the grief; as we no longer support that platform, we no longer have to maintain the back-compat hacks. The next back-compat hack to get rid of: we should stop using INCLUDES in Makefile.am, and instead spell it CPPFLAGS. INCLUDES was necessary for automake 1.9.6 (see commit 6ae3052c), but modern automake says it is outdated. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org

On Wed, Mar 13, 2019 at 05:51:15PM +0100, Andrea Bolognani wrote:
Andrea Bolognani (8): tests: Drop CONFIG_HEADER from TESTS_ENVIRONMENT tests: Drop LIBVIRT_DRIVER_DIR from TESTS_ENVIRONMENT tests: Don't use TEST_DRIVER_DIR in virTestCaptureProgramExecChild() tests: Don't define TEST_DRIVER_DIR tests: Stop looking for abs_top_srcdir in the environment Fix names for abs_top_{src,build}dir variables tests: Don't redefine variables for TESTS_ENVIRONMENT Don't define abs_* variables ourselves
The whole series is Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
participants (3)
-
Andrea Bolognani
-
Eric Blake
-
Martin Kletzander