* [PATCH] xfstests: fix install target using sudo
@ 2018-06-26 21:08 Luis R. Rodriguez
2018-06-27 7:42 ` Dave Chinner
0 siblings, 1 reply; 3+ messages in thread
From: Luis R. Rodriguez @ 2018-06-26 21:08 UTC (permalink / raw)
To: fstests; +Cc: 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.
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(-)
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)
--
2.16.3
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] xfstests: fix install target using sudo
2018-06-26 21:08 [PATCH] xfstests: fix install target using sudo Luis R. Rodriguez
@ 2018-06-27 7:42 ` Dave Chinner
2018-06-27 16:03 ` Luis R. Rodriguez
0 siblings, 1 reply; 3+ messages in thread
From: Dave Chinner @ 2018-06-27 7:42 UTC (permalink / raw)
To: Luis R. Rodriguez; +Cc: fstests
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.
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] xfstests: fix install target using sudo
2018-06-27 7:42 ` Dave Chinner
@ 2018-06-27 16:03 ` Luis R. Rodriguez
0 siblings, 0 replies; 3+ messages in thread
From: Luis R. Rodriguez @ 2018-06-27 16:03 UTC (permalink / raw)
To: Dave Chinner; +Cc: Luis R. Rodriguez, fstests
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
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-06-27 16:03 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-06-26 21:08 [PATCH] xfstests: fix install target using sudo Luis R. Rodriguez
2018-06-27 7:42 ` Dave Chinner
2018-06-27 16:03 ` Luis R. Rodriguez
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.