xfstests: fix install target using sudo
PATCH 1/1 xfstests: fix install target using sudo
If you install with:
sudo make install
Depending on the system, you may see /var/lib/xfstests/tests/ is empty. This is because $(PWD) can expand to be empty on certain systems and so the wildcard finds nothing.
PWD is only used on one target, the tests/*/ dir install target.
We can fix this by using $(CURDIR) however that does not suffice as we are also using the $(wildcard) and that needs its own careful expansion.
This issue is observed on both Fedora and OpenSUSE, but not on Debian.
--- tests/Makefile | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
tests/Makefile +2 -1
@@ -5,7 +5,8 @@ TOPDIR = .. include $(TOPDIR)/include/builddefs -TESTS_SUBDIRS = $(sort $(dir $(wildcard $(PWD)/$(TESTS_DIR)/[a-z]*/))) +TEST_DIR = $(dir $(CURDIR)/$(TESTS_DIR)) +TESTS_SUBDIRS = $(sort $(dir $(wildcard $(TEST_DIR)/[a-z]*/))) include $(BUILDRULES)
Signature · 1 line
2.16.3
Quoted 7 lines · Luis R. Rodriguez
If you install with:
sudo make install
Depending on the system, you may see
/var/lib/xfstests/tests/is empty. This is because $(PWD) can expand to be empty on certain systems and so the wildcard finds nothing.
When and why?
I'm guessing it's because the environment variable PWD is not exported by the shell that make is being run in? /bin/sh, /bin/bash and /bin/dash should always set PWD in the environment, so maybe you're using a different shell that doesn't set PWD correctly?
Quoted 4 lines
PWD is only used on one target, the tests/*/ dir install target.
We can fix this by using $(CURDIR) however that does not suffice as we are also using the $(wildcard) and that needs its own careful expansion.
$(CURDIR) is documented as an being the current absolute path, so AFAICT there's nothing to be careful about in terms of expansion. What are you trying to avoid here?
Quoted 1 line
This issue is observed on both Fedora and OpenSUSE, but not on Debian.
This really smells more of a shell/environment issue, not a distro issue.
Quoted 10 lines
Signed-off-by Luis R. Rodriguez <[email protected]>--- tests/Makefile | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)tests/Makefile +0
-0
tests/Makefile line 8
Quoted 5 lines
tests/Makefile +1
-1@@ -5,7 +5,8 @@ TOPDIR = .. include $(TOPDIR)/include/builddefs -TESTS_SUBDIRS = $(sort $(dir $(wildcard $(PWD)/$(TESTS_DIR)/[a-z]*/))) +TEST_DIR = $(dir $(CURDIR)/$(TESTS_DIR))
I think this is wrong - you're creating an invalid path:
curdir/tests_dir is /home/dave/src/xfstests-dev/tests/tests dir curdir/tests_dir is /home/dave/src/xfstests-dev/tests/
then using $(dir <path>) command to truncate away the invalid path segment, resulting in the original path $(CURDIR) gave you.
In fact, $(CURDIR) is basically what the current code intends - I didn't know this existed because it's not obviously searchable (i.e. not a CWD or PWD variant) so I hacked around it with environment variables.
So AFAICT, the change the needs to be made here is:
-TESTS_SUBDIRS = $(sort $(dir $(wildcard $(PWD)/$(TESTS_DIR)/[a-z]*/))) +TESTS_SUBDIRS = $(sort $(dir $(wildcard $(CURDIR)/[a-z]*/)))
Cheers,
Dave.
Signature · 2 lines
Dave Chinner [email protected]
Review
Quoted 11 lines · Dave Chinner
On Tue, Jun 26, 2018 at 02:08:34PM -0700, Luis R. Rodriguez wrote:
In fact, $(CURDIR) is basically what the current code intends - I didn't know this existed because it's not obviously searchable (i.e. not a CWD or PWD variant) so I hacked around it with environment variables.
So AFAICT, the change the needs to be made here is:
(quoted diff) +1
-1-TESTS_SUBDIRS = $(sort $(dir $(wildcard $(PWD)/$(TESTS_DIR)/[a-z]*/))) +TESTS_SUBDIRS = $(sort $(dir $(wildcard $(CURDIR)/[a-z]*/)))
You are right, I botched my test with it and used CURDIR with TEST_DIR, removing it as you did works. Will send a v2.
Luis