| Message ID | 20180626210834.24220-1-mcgrof@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Jun 26, 2018 at 02:08:34PM -0700, Luis R. Rodriguez wrote: > 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? > 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? > 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. > Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org> > --- > tests/Makefile | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/tests/Makefile b/tests/Makefile > index 2611b3b845f5..084135da0487 100644 > --- a/tests/Makefile > +++ b/tests/Makefile > @@ -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.
On Wed, Jun 27, 2018 at 05:42:19PM +1000, Dave Chinner wrote: > 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: > > -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 -- To unsubscribe from this list: send the line "unsubscribe fstests" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/tests/Makefile b/tests/Makefile index 2611b3b845f5..084135da0487 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -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)
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. Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org> --- tests/Makefile | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)