* [PATCH 0/2] xfstests: extended names fixes
@ 2016-07-01 16:09 Jan Tulak
2016-07-01 16:09 ` [PATCH 1/2] xfstests: Fix installation for extended names Jan Tulak
2016-07-01 16:09 ` [PATCH 2/2] xfstests: filename handling for extended names in ./check was on a wrong place Jan Tulak
0 siblings, 2 replies; 9+ messages in thread
From: Jan Tulak @ 2016-07-01 16:09 UTC (permalink / raw)
To: fstests; +Cc: david, eguan, Jan Tulak
Two fixes for extended names for tests (tests/foo/123-bar-baz).
Jan Tulak (2):
xfstests: Fix installation for extended names
xfstests: filename handling for extended names in ./check was on a
wrong place
check | 27 ++++++++++++++-------------
tests/btrfs/Makefile | 21 +++++++++++++++++----
tests/cifs/Makefile | 21 +++++++++++++++++----
tests/ext4/Makefile | 21 +++++++++++++++++----
tests/f2fs/Makefile | 24 ++++++++++++++++++------
tests/generic/Makefile | 21 +++++++++++++++++----
tests/overlay/Makefile | 23 ++++++++++++++++++-----
tests/shared/Makefile | 21 +++++++++++++++++----
tests/udf/Makefile | 21 +++++++++++++++++----
tests/xfs/Makefile | 21 +++++++++++++++++----
10 files changed, 169 insertions(+), 52 deletions(-)
--
2.5.5
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/2] xfstests: Fix installation for extended names
2016-07-01 16:09 [PATCH 0/2] xfstests: extended names fixes Jan Tulak
@ 2016-07-01 16:09 ` Jan Tulak
2016-07-04 1:40 ` Dave Chinner
2016-07-01 16:09 ` [PATCH 2/2] xfstests: filename handling for extended names in ./check was on a wrong place Jan Tulak
1 sibling, 1 reply; 9+ messages in thread
From: Jan Tulak @ 2016-07-01 16:09 UTC (permalink / raw)
To: fstests; +Cc: david, eguan, Jan Tulak
xfstests supports extended test names like 314-foo-bar, but installation of
these tests was skipped (not matching a regexp). So this patch fixes the
makefiles in tests/*/
Signed-off-by: Jan Tulak <jtulak@redhat.com>
---
UPDATE:
Rewritten, now it doesn't fail without the extended name, and doesn't
suppress anything. Thanks Dave.
---
tests/btrfs/Makefile | 21 +++++++++++++++++----
tests/cifs/Makefile | 21 +++++++++++++++++----
tests/ext4/Makefile | 21 +++++++++++++++++----
tests/f2fs/Makefile | 24 ++++++++++++++++++------
tests/generic/Makefile | 21 +++++++++++++++++----
tests/overlay/Makefile | 23 ++++++++++++++++++-----
tests/shared/Makefile | 21 +++++++++++++++++----
tests/udf/Makefile | 21 +++++++++++++++++----
tests/xfs/Makefile | 21 +++++++++++++++++----
9 files changed, 155 insertions(+), 39 deletions(-)
diff --git a/tests/btrfs/Makefile b/tests/btrfs/Makefile
index e1a5be1..0301d78 100644
--- a/tests/btrfs/Makefile
+++ b/tests/btrfs/Makefile
@@ -5,16 +5,29 @@
TOPDIR = ../..
include $(TOPDIR)/include/builddefs
-BTRFS_DIR = btrfs
-TARGET_DIR = $(PKG_LIB_DIR)/$(TESTS_DIR)/$(BTRFS_DIR)
+GROUP_DIR = btrfs
+TARGET_DIR = $(PKG_LIB_DIR)/$(TESTS_DIR)/$(GROUP_DIR)
include $(BUILDRULES)
+# note: the second TESTS wildcard gets also .out files, but these will be
+# overwritten with correct permissions a moment later, as they are also in
+# OUTPUTS. Regexps here are really limited, so I didn't find a better
+# solution...
+TESTS = $(wildcard [0-9][0-9][0-9])
+TESTS += $(wildcard [0-9][0-9][0-9]-*)
+
+
+OUTPUTS = $(wildcard [0-9][0-9][0-9].*)
+OUTPUTS += $(wildcard [0-9][0-9][0-9]-*.*)
+
+
+
install:
$(INSTALL) -m 755 -d $(TARGET_DIR)
- $(INSTALL) -m 755 [0-9]?? $(TARGET_DIR)
$(INSTALL) -m 644 group $(TARGET_DIR)
- $(INSTALL) -m 644 [0-9]??.* $(TARGET_DIR)
+ $(INSTALL) -m 755 $(TESTS) $(TARGET_DIR)
+ $(INSTALL) -m 644 $(OUTPUTS) $(TARGET_DIR)
# Nothing.
install-dev install-lib:
diff --git a/tests/cifs/Makefile b/tests/cifs/Makefile
index 9176e5c..b0575c2 100644
--- a/tests/cifs/Makefile
+++ b/tests/cifs/Makefile
@@ -5,16 +5,29 @@
TOPDIR = ../..
include $(TOPDIR)/include/builddefs
-CIFS_DIR = cifs
-TARGET_DIR = $(PKG_LIB_DIR)/$(TESTS_DIR)/$(CIFS_DIR)
+GROUP_DIR = cifs
+TARGET_DIR = $(PKG_LIB_DIR)/$(TESTS_DIR)/$(GROUP_DIR)
include $(BUILDRULES)
+# note: the second TESTS wildcard gets also .out files, but these will be
+# overwritten with correct permissions a moment later, as they are also in
+# OUTPUTS. Regexps here are really limited, so I didn't find a better
+# solution...
+TESTS = $(wildcard [0-9][0-9][0-9])
+TESTS += $(wildcard [0-9][0-9][0-9]-*)
+
+
+OUTPUTS = $(wildcard [0-9][0-9][0-9].*)
+OUTPUTS += $(wildcard [0-9][0-9][0-9]-*.*)
+
+
+
install:
$(INSTALL) -m 755 -d $(TARGET_DIR)
- $(INSTALL) -m 755 [0-9]?? $(TARGET_DIR)
$(INSTALL) -m 644 group $(TARGET_DIR)
- $(INSTALL) -m 644 [0-9]??.* $(TARGET_DIR)
+ $(INSTALL) -m 755 $(TESTS) $(TARGET_DIR)
+ $(INSTALL) -m 644 $(OUTPUTS) $(TARGET_DIR)
# Nothing.
install-dev install-lib:
diff --git a/tests/ext4/Makefile b/tests/ext4/Makefile
index 7a3c8e1..5c562a8 100644
--- a/tests/ext4/Makefile
+++ b/tests/ext4/Makefile
@@ -5,16 +5,29 @@
TOPDIR = ../..
include $(TOPDIR)/include/builddefs
-EXT4_DIR = ext4
-TARGET_DIR = $(PKG_LIB_DIR)/$(TESTS_DIR)/$(EXT4_DIR)
+GROUP_DIR = ext4
+TARGET_DIR = $(PKG_LIB_DIR)/$(TESTS_DIR)/$(GROUP_DIR)
include $(BUILDRULES)
+# note: the second TESTS wildcard gets also .out files, but these will be
+# overwritten with correct permissions a moment later, as they are also in
+# OUTPUTS. Regexps here are really limited, so I didn't find a better
+# solution...
+TESTS = $(wildcard [0-9][0-9][0-9])
+TESTS += $(wildcard [0-9][0-9][0-9]-*)
+
+
+OUTPUTS = $(wildcard [0-9][0-9][0-9].*)
+OUTPUTS += $(wildcard [0-9][0-9][0-9]-*.*)
+
+
+
install:
$(INSTALL) -m 755 -d $(TARGET_DIR)
- $(INSTALL) -m 755 [0-9]?? $(TARGET_DIR)
$(INSTALL) -m 644 group $(TARGET_DIR)
- $(INSTALL) -m 644 [0-9]??.* $(TARGET_DIR)
+ $(INSTALL) -m 755 $(TESTS) $(TARGET_DIR)
+ $(INSTALL) -m 644 $(OUTPUTS) $(TARGET_DIR)
# Nothing.
install-dev install-lib:
diff --git a/tests/f2fs/Makefile b/tests/f2fs/Makefile
index 4d00e9e..bc0a7fd 100644
--- a/tests/f2fs/Makefile
+++ b/tests/f2fs/Makefile
@@ -1,21 +1,33 @@
#
-# Copyright (c) 2015 Fujitsu Ltd.
-# Author: Xiao Yang <yangx.jy@cn.fujitsu.com>
+# Copyright (c) 2003-2005 Silicon Graphics, Inc. All Rights Reserved.
#
TOPDIR = ../..
include $(TOPDIR)/include/builddefs
-F2FS_DIR = f2fs
-TARGET_DIR = $(PKG_LIB_DIR)/$(TESTS_DIR)/$(F2FS_DIR)
+GROUP_DIR = f2fs
+TARGET_DIR = $(PKG_LIB_DIR)/$(TESTS_DIR)/$(GROUP_DIR)
include $(BUILDRULES)
+# note: the second TESTS wildcard gets also .out files, but these will be
+# overwritten with correct permissions a moment later, as they are also in
+# OUTPUTS. Regexps here are really limited, so I didn't find a better
+# solution...
+TESTS = $(wildcard [0-9][0-9][0-9])
+TESTS += $(wildcard [0-9][0-9][0-9]-*)
+
+
+OUTPUTS = $(wildcard [0-9][0-9][0-9].*)
+OUTPUTS += $(wildcard [0-9][0-9][0-9]-*.*)
+
+
+
install:
$(INSTALL) -m 755 -d $(TARGET_DIR)
- $(INSTALL) -m 755 [0-9]?? $(TARGET_DIR)
$(INSTALL) -m 644 group $(TARGET_DIR)
- $(INSTALL) -m 644 [0-9]??.* $(TARGET_DIR)
+ $(INSTALL) -m 755 $(TESTS) $(TARGET_DIR)
+ $(INSTALL) -m 644 $(OUTPUTS) $(TARGET_DIR)
# Nothing.
install-dev install-lib:
diff --git a/tests/generic/Makefile b/tests/generic/Makefile
index 9529fb8..6ca5df8 100644
--- a/tests/generic/Makefile
+++ b/tests/generic/Makefile
@@ -5,16 +5,29 @@
TOPDIR = ../..
include $(TOPDIR)/include/builddefs
-GENERIC_DIR = generic
-TARGET_DIR = $(PKG_LIB_DIR)/$(TESTS_DIR)/$(GENERIC_DIR)
+GROUP_DIR = generic
+TARGET_DIR = $(PKG_LIB_DIR)/$(TESTS_DIR)/$(GROUP_DIR)
include $(BUILDRULES)
+# note: the second TESTS wildcard gets also .out files, but these will be
+# overwritten with correct permissions a moment later, as they are also in
+# OUTPUTS. Regexps here are really limited, so I didn't find a better
+# solution...
+TESTS = $(wildcard [0-9][0-9][0-9])
+TESTS += $(wildcard [0-9][0-9][0-9]-*)
+
+
+OUTPUTS = $(wildcard [0-9][0-9][0-9].*)
+OUTPUTS += $(wildcard [0-9][0-9][0-9]-*.*)
+
+
+
install:
$(INSTALL) -m 755 -d $(TARGET_DIR)
- $(INSTALL) -m 755 [0-9]?? $(TARGET_DIR)
$(INSTALL) -m 644 group $(TARGET_DIR)
- $(INSTALL) -m 644 [0-9]??.* $(TARGET_DIR)
+ $(INSTALL) -m 755 $(TESTS) $(TARGET_DIR)
+ $(INSTALL) -m 644 $(OUTPUTS) $(TARGET_DIR)
# Nothing.
install-dev install-lib:
diff --git a/tests/overlay/Makefile b/tests/overlay/Makefile
index 63c9878..2a49585 100644
--- a/tests/overlay/Makefile
+++ b/tests/overlay/Makefile
@@ -1,20 +1,33 @@
#
-# Copyright (c) 2016 Red Hat Inc. All Rights Reserved.
+# Copyright (c) 2003-2005 Silicon Graphics, Inc. All Rights Reserved.
#
TOPDIR = ../..
include $(TOPDIR)/include/builddefs
-TEST_DIR = overlay
-TARGET_DIR = $(PKG_LIB_DIR)/$(TESTS_DIR)/$(TEST_DIR)
+GROUP_DIR = overlay
+TARGET_DIR = $(PKG_LIB_DIR)/$(TESTS_DIR)/$(GROUP_DIR)
include $(BUILDRULES)
+# note: the second TESTS wildcard gets also .out files, but these will be
+# overwritten with correct permissions a moment later, as they are also in
+# OUTPUTS. Regexps here are really limited, so I didn't find a better
+# solution...
+TESTS = $(wildcard [0-9][0-9][0-9])
+TESTS += $(wildcard [0-9][0-9][0-9]-*)
+
+
+OUTPUTS = $(wildcard [0-9][0-9][0-9].*)
+OUTPUTS += $(wildcard [0-9][0-9][0-9]-*.*)
+
+
+
install:
$(INSTALL) -m 755 -d $(TARGET_DIR)
- $(INSTALL) -m 755 [0-9]?? $(TARGET_DIR)
$(INSTALL) -m 644 group $(TARGET_DIR)
- $(INSTALL) -m 644 [0-9]??.* $(TARGET_DIR)
+ $(INSTALL) -m 755 $(TESTS) $(TARGET_DIR)
+ $(INSTALL) -m 644 $(OUTPUTS) $(TARGET_DIR)
# Nothing.
install-dev install-lib:
diff --git a/tests/shared/Makefile b/tests/shared/Makefile
index cbd87f9..381abe9 100644
--- a/tests/shared/Makefile
+++ b/tests/shared/Makefile
@@ -5,16 +5,29 @@
TOPDIR = ../..
include $(TOPDIR)/include/builddefs
-SHARED_DIR = shared
-TARGET_DIR = $(PKG_LIB_DIR)/$(TESTS_DIR)/$(SHARED_DIR)
+GROUP_DIR = shared
+TARGET_DIR = $(PKG_LIB_DIR)/$(TESTS_DIR)/$(GROUP_DIR)
include $(BUILDRULES)
+# note: the second TESTS wildcard gets also .out files, but these will be
+# overwritten with correct permissions a moment later, as they are also in
+# OUTPUTS. Regexps here are really limited, so I didn't find a better
+# solution...
+TESTS = $(wildcard [0-9][0-9][0-9])
+TESTS += $(wildcard [0-9][0-9][0-9]-*)
+
+
+OUTPUTS = $(wildcard [0-9][0-9][0-9].*)
+OUTPUTS += $(wildcard [0-9][0-9][0-9]-*.*)
+
+
+
install:
$(INSTALL) -m 755 -d $(TARGET_DIR)
- $(INSTALL) -m 755 [0-9]?? $(TARGET_DIR)
$(INSTALL) -m 644 group $(TARGET_DIR)
- $(INSTALL) -m 644 [0-9]??.* $(TARGET_DIR)
+ $(INSTALL) -m 755 $(TESTS) $(TARGET_DIR)
+ $(INSTALL) -m 644 $(OUTPUTS) $(TARGET_DIR)
# Nothing.
install-dev install-lib:
diff --git a/tests/udf/Makefile b/tests/udf/Makefile
index 1d96658..d9e388c 100644
--- a/tests/udf/Makefile
+++ b/tests/udf/Makefile
@@ -5,16 +5,29 @@
TOPDIR = ../..
include $(TOPDIR)/include/builddefs
-UDF_DIR = udf
-TARGET_DIR = $(PKG_LIB_DIR)/$(TESTS_DIR)/$(UDF_DIR)
+GROUP_DIR = udf
+TARGET_DIR = $(PKG_LIB_DIR)/$(TESTS_DIR)/$(GROUP_DIR)
include $(BUILDRULES)
+# note: the second TESTS wildcard gets also .out files, but these will be
+# overwritten with correct permissions a moment later, as they are also in
+# OUTPUTS. Regexps here are really limited, so I didn't find a better
+# solution...
+TESTS = $(wildcard [0-9][0-9][0-9])
+TESTS += $(wildcard [0-9][0-9][0-9]-*)
+
+
+OUTPUTS = $(wildcard [0-9][0-9][0-9].*)
+OUTPUTS += $(wildcard [0-9][0-9][0-9]-*.*)
+
+
+
install:
$(INSTALL) -m 755 -d $(TARGET_DIR)
- $(INSTALL) -m 755 [0-9]?? $(TARGET_DIR)
$(INSTALL) -m 644 group $(TARGET_DIR)
- $(INSTALL) -m 644 [0-9]??.* $(TARGET_DIR)
+ $(INSTALL) -m 755 $(TESTS) $(TARGET_DIR)
+ $(INSTALL) -m 644 $(OUTPUTS) $(TARGET_DIR)
# Nothing.
install-dev install-lib:
diff --git a/tests/xfs/Makefile b/tests/xfs/Makefile
index db94be0..824e32c 100644
--- a/tests/xfs/Makefile
+++ b/tests/xfs/Makefile
@@ -5,16 +5,29 @@
TOPDIR = ../..
include $(TOPDIR)/include/builddefs
-XFS_DIR = xfs
-TARGET_DIR = $(PKG_LIB_DIR)/$(TESTS_DIR)/$(XFS_DIR)
+GROUP_DIR = xfs
+TARGET_DIR = $(PKG_LIB_DIR)/$(TESTS_DIR)/$(GROUP_DIR)
include $(BUILDRULES)
+# note: the second TESTS wildcard gets also .out files, but these will be
+# overwritten with correct permissions a moment later, as they are also in
+# OUTPUTS. Regexps here are really limited, so I didn't find a better
+# solution...
+TESTS = $(wildcard [0-9][0-9][0-9])
+TESTS += $(wildcard [0-9][0-9][0-9]-*)
+
+
+OUTPUTS = $(wildcard [0-9][0-9][0-9].*)
+OUTPUTS += $(wildcard [0-9][0-9][0-9]-*.*)
+
+
+
install:
$(INSTALL) -m 755 -d $(TARGET_DIR)
- $(INSTALL) -m 755 [0-9]?? $(TARGET_DIR)
$(INSTALL) -m 644 group $(TARGET_DIR)
- $(INSTALL) -m 644 [0-9]??.* $(TARGET_DIR)
+ $(INSTALL) -m 755 $(TESTS) $(TARGET_DIR)
+ $(INSTALL) -m 644 $(OUTPUTS) $(TARGET_DIR)
# Nothing.
install-dev install-lib:
--
2.5.5
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/2] xfstests: filename handling for extended names in ./check was on a wrong place
2016-07-01 16:09 [PATCH 0/2] xfstests: extended names fixes Jan Tulak
2016-07-01 16:09 ` [PATCH 1/2] xfstests: Fix installation for extended names Jan Tulak
@ 2016-07-01 16:09 ` Jan Tulak
2016-07-04 1:46 ` Dave Chinner
1 sibling, 1 reply; 9+ messages in thread
From: Jan Tulak @ 2016-07-01 16:09 UTC (permalink / raw)
To: fstests; +Cc: david, eguan, Jan Tulak
The code handling "./check foo/123", when the real test is "foo/123-bar-baz"
was moved at the earliest position, so everything working with the test
name/path will know the full name, and no 123/123-bar-baz mix is possible.
When moving the code, put qutation marks around a wildcard name to prevent
early expansion
Signed-off-by: Jan Tulak <jtulak@redhat.com>
---
check | 27 ++++++++++++++-------------
1 file changed, 14 insertions(+), 13 deletions(-)
diff --git a/check b/check
index 5be183f..10ae375 100755
--- a/check
+++ b/check
@@ -543,6 +543,20 @@ for section in $HOST_OPTIONS_SECTIONS; do
for seq in $list
do
err=false
+ if [ ! -f $seq ]; then
+ # Try to get full name in case the user supplied only seq id
+ # and the test has a name. A bit of hassle to find really
+ # the test and not its sample output or helping files.
+ bname=$(basename $seq)
+
+ full_seq=$(find $(dirname $seq) -name "$bname*" -executable |
+ awk '(NR == 1 || length < length(shortest)) { shortest = $0 }\
+ END { print shortest }')
+ if [ -f $full_seq ] \
+ && [ x$(echo $bname | grep -o "^$VALID_TEST_ID") != x ]; then
+ seq=$full_seq
+ fi
+ fi
# the filename for the test and the name output are different.
# we don't include the tests/ directory in the name output.
@@ -566,19 +580,6 @@ for section in $HOST_OPTIONS_SECTIONS; do
if $showme; then
echo
continue
- elif [ ! -f $seq ]; then
- # Try to get full name in case the user supplied only seq id
- # and the test has a name. A bit of hassle to find really
- # the test and not its sample output or helping files.
- bname=$(basename $seq)
- full_seq=$(find $(dirname $seq) -name $bname* -executable |
- awk '(NR == 1 || length < length(shortest)) { shortest = $0 }\
- END { print shortest }')
- if [ -f $full_seq ] \
- && [ x$(echo $bname | grep -o "^$VALID_TEST_ID") != x ]; then
- seq=$full_seq
- seqnum=${full_seq#*/}
- fi
fi
if [ ! -f $seq ]; then
--
2.5.5
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] xfstests: Fix installation for extended names
2016-07-01 16:09 ` [PATCH 1/2] xfstests: Fix installation for extended names Jan Tulak
@ 2016-07-04 1:40 ` Dave Chinner
2016-07-11 9:29 ` Jan Tulak
0 siblings, 1 reply; 9+ messages in thread
From: Dave Chinner @ 2016-07-04 1:40 UTC (permalink / raw)
To: Theodore Ts'o; +Cc: fstests, eguan
On Fri, Jul 01, 2016 at 06:09:06PM +0200, Jan Tulak wrote:
> xfstests supports extended test names like 314-foo-bar, but installation of
> these tests was skipped (not matching a regexp). So this patch fixes the
> makefiles in tests/*/
>
> Signed-off-by: Jan Tulak <jtulak@redhat.com>
> ---
> UPDATE:
> Rewritten, now it doesn't fail without the extended name, and doesn't
> suppress anything. Thanks Dave.
> ---
> tests/btrfs/Makefile | 21 +++++++++++++++++----
> tests/cifs/Makefile | 21 +++++++++++++++++----
> tests/ext4/Makefile | 21 +++++++++++++++++----
> tests/f2fs/Makefile | 24 ++++++++++++++++++------
> tests/generic/Makefile | 21 +++++++++++++++++----
> tests/overlay/Makefile | 23 ++++++++++++++++++-----
> tests/shared/Makefile | 21 +++++++++++++++++----
> tests/udf/Makefile | 21 +++++++++++++++++----
> tests/xfs/Makefile | 21 +++++++++++++++++----
> 9 files changed, 155 insertions(+), 39 deletions(-)
>
> diff --git a/tests/btrfs/Makefile b/tests/btrfs/Makefile
> index e1a5be1..0301d78 100644
> --- a/tests/btrfs/Makefile
> +++ b/tests/btrfs/Makefile
> @@ -5,16 +5,29 @@
> TOPDIR = ../..
> include $(TOPDIR)/include/builddefs
>
> -BTRFS_DIR = btrfs
> -TARGET_DIR = $(PKG_LIB_DIR)/$(TESTS_DIR)/$(BTRFS_DIR)
> +GROUP_DIR = btrfs
> +TARGET_DIR = $(PKG_LIB_DIR)/$(TESTS_DIR)/$(GROUP_DIR)
What's this change for?
Please don't make random undocumented changes when updating a
patchset. It is best not to change anything unrelated to the problem
being solved.
> include $(BUILDRULES)
>
> +# note: the second TESTS wildcard gets also .out files, but these will be
> +# overwritten with correct permissions a moment later, as they are also in
> +# OUTPUTS. Regexps here are really limited, so I didn't find a better
> +# solution...
> +TESTS = $(wildcard [0-9][0-9][0-9])
> +TESTS += $(wildcard [0-9][0-9][0-9]-*)
Ugh. The hacks need to be constrained to the makefile and not do
nasty things with installed files.
What I think should be done here is to turn the selection around the
other way because we can select the output files directly and
correctly. We can then use that to build the test list via stripping
and filtering as all tests must have an output file associated with
them.
So:
# Start with all test related files:
ALLFILES = $(wildcard [0-9]??*)
# Now build a list of known output files. Unfortunately, the
# multiple output test files are poorly handled as makefiles don't
# handle wildcarded multi-suffix matching. Hence we separate the
# processing of these right now.
EXTENDED_OUTFILES = $(wildcard [0-9]??*.out.*)
BASIC_OUTFILES = $(wildcard [0-9]??*.out)
OUTFILES = $(EXTENDED_OUTFILES) $(BASIC_OUTFILES)
# Strip suffix to get matching tests. We want to strip from the
# first "." to the end, but makefiles don't have a built in
# operative for that. So:
#
# Hack: strip the multiple segments after .out with repeated basename calls.
EXTFILTER1 = $(basename $(EXTENDED_OUTFILES))
EXTFILTER2 = $(basename $(EXTFILTER1))
EXTFILTER3 = $(basename $(EXTFILTER2))
EXTFILTER4 = $(basename $(EXTFILTER3))
# final filter list
FILTER = $(basename $(EXTFILTER4) $(BASIC_OUTFILES))
# finally, select the test files by filtering against against the
# stripped output files and sort them to remove duplicates.
TESTS = $(sort $(filter $(ALLFILES), $(FILTER)))
That will give you two lists - "OUTFILES" which are all the output
files that need to be installed, and "TESTS" which are all the tests
that should be installed. There will be no overlap between these two
lists.
I'd also suggest that this can probably be put in the BUILDRULES
file, as it will be common to all makefiles and doesn't need to be
repeated in every makefile.
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] xfstests: filename handling for extended names in ./check was on a wrong place
2016-07-01 16:09 ` [PATCH 2/2] xfstests: filename handling for extended names in ./check was on a wrong place Jan Tulak
@ 2016-07-04 1:46 ` Dave Chinner
2016-07-11 9:27 ` Jan Tulak
0 siblings, 1 reply; 9+ messages in thread
From: Dave Chinner @ 2016-07-04 1:46 UTC (permalink / raw)
To: Jan Tulak; +Cc: fstests, eguan
On Fri, Jul 01, 2016 at 06:09:07PM +0200, Jan Tulak wrote:
> The code handling "./check foo/123", when the real test is "foo/123-bar-baz"
> was moved at the earliest position, so everything working with the test
> name/path will know the full name, and no 123/123-bar-baz mix is possible.
>
> When moving the code, put qutation marks around a wildcard name to prevent
> early expansion
Can you put this in a separate patch so it's easy to see and review?
i.e. move the code in one patch, change the code in a second?
> Signed-off-by: Jan Tulak <jtulak@redhat.com>
> ---
> check | 27 ++++++++++++++-------------
> 1 file changed, 14 insertions(+), 13 deletions(-)
>
> diff --git a/check b/check
> index 5be183f..10ae375 100755
> --- a/check
> +++ b/check
> @@ -543,6 +543,20 @@ for section in $HOST_OPTIONS_SECTIONS; do
> for seq in $list
> do
> err=false
> + if [ ! -f $seq ]; then
> + # Try to get full name in case the user supplied only seq id
> + # and the test has a name. A bit of hassle to find really
> + # the test and not its sample output or helping files.
> + bname=$(basename $seq)
> +
> + full_seq=$(find $(dirname $seq) -name "$bname*" -executable |
> + awk '(NR == 1 || length < length(shortest)) { shortest = $0 }\
> + END { print shortest }')
> + if [ -f $full_seq ] \
> + && [ x$(echo $bname | grep -o "^$VALID_TEST_ID") != x ]; then
> + seq=$full_seq
> + fi
> + fi
>
> # the filename for the test and the name output are different.
> # we don't include the tests/ directory in the name output.
This now has whitespace issues - the indentation does not match the
surrounding context.
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] xfstests: filename handling for extended names in ./check was on a wrong place
2016-07-04 1:46 ` Dave Chinner
@ 2016-07-11 9:27 ` Jan Tulak
0 siblings, 0 replies; 9+ messages in thread
From: Jan Tulak @ 2016-07-11 9:27 UTC (permalink / raw)
To: Dave Chinner; +Cc: fstests, Eryu Guan
Hi,
On Mon, Jul 4, 2016 at 3:46 AM, Dave Chinner <david@fromorbit.com> wrote:
> On Fri, Jul 01, 2016 at 06:09:07PM +0200, Jan Tulak wrote:
>> The code handling "./check foo/123", when the real test is "foo/123-bar-baz"
>> was moved at the earliest position, so everything working with the test
>> name/path will know the full name, and no 123/123-bar-baz mix is possible.
>>
>> When moving the code, put qutation marks around a wildcard name to prevent
>> early expansion
>
> Can you put this in a separate patch so it's easy to see and review?
> i.e. move the code in one patch, change the code in a second?
>
>> Signed-off-by: Jan Tulak <jtulak@redhat.com>
>> ---
>> check | 27 ++++++++++++++-------------
>> 1 file changed, 14 insertions(+), 13 deletions(-)
>>
>> diff --git a/check b/check
>> index 5be183f..10ae375 100755
>> --- a/check
>> +++ b/check
>> @@ -543,6 +543,20 @@ for section in $HOST_OPTIONS_SECTIONS; do
>> for seq in $list
>> do
>> err=false
>> + if [ ! -f $seq ]; then
>> + # Try to get full name in case the user supplied only seq id
>> + # and the test has a name. A bit of hassle to find really
>> + # the test and not its sample output or helping files.
>> + bname=$(basename $seq)
>> +
>> + full_seq=$(find $(dirname $seq) -name "$bname*" -executable |
>> + awk '(NR == 1 || length < length(shortest)) { shortest = $0 }\
>> + END { print shortest }')
>> + if [ -f $full_seq ] \
>> + && [ x$(echo $bname | grep -o "^$VALID_TEST_ID") != x ]; then
>> + seq=$full_seq
>> + fi
>> + fi
>>
>> # the filename for the test and the name output are different.
>> # we don't include the tests/ directory in the name output.
>
> This now has whitespace issues - the indentation does not match the
> surrounding context.
>
I'm back from holiday. I split the patch and the whitespaces should
now match the code around.
Thanks,
Jan
--
Jan Tulak
jtulak@redhat.com / jan@tulak.me
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] xfstests: Fix installation for extended names
2016-07-04 1:40 ` Dave Chinner
@ 2016-07-11 9:29 ` Jan Tulak
2016-07-12 2:32 ` Dave Chinner
0 siblings, 1 reply; 9+ messages in thread
From: Jan Tulak @ 2016-07-11 9:29 UTC (permalink / raw)
To: Dave Chinner; +Cc: fstests, Eryu Guan
Hi,
On Mon, Jul 4, 2016 at 3:40 AM, Dave Chinner <david@fromorbit.com> wrote:
> On Fri, Jul 01, 2016 at 06:09:06PM +0200, Jan Tulak wrote:
>> xfstests supports extended test names like 314-foo-bar, but installation of
>> these tests was skipped (not matching a regexp). So this patch fixes the
>> makefiles in tests/*/
>>
>> Signed-off-by: Jan Tulak <jtulak@redhat.com>
>> ---
>> UPDATE:
>> Rewritten, now it doesn't fail without the extended name, and doesn't
>> suppress anything. Thanks Dave.
>> ---
>> tests/btrfs/Makefile | 21 +++++++++++++++++----
>> tests/cifs/Makefile | 21 +++++++++++++++++----
>> tests/ext4/Makefile | 21 +++++++++++++++++----
>> tests/f2fs/Makefile | 24 ++++++++++++++++++------
>> tests/generic/Makefile | 21 +++++++++++++++++----
>> tests/overlay/Makefile | 23 ++++++++++++++++++-----
>> tests/shared/Makefile | 21 +++++++++++++++++----
>> tests/udf/Makefile | 21 +++++++++++++++++----
>> tests/xfs/Makefile | 21 +++++++++++++++++----
>> 9 files changed, 155 insertions(+), 39 deletions(-)
>>
>> diff --git a/tests/btrfs/Makefile b/tests/btrfs/Makefile
>> index e1a5be1..0301d78 100644
>> --- a/tests/btrfs/Makefile
>> +++ b/tests/btrfs/Makefile
>> @@ -5,16 +5,29 @@
>> TOPDIR = ../..
>> include $(TOPDIR)/include/builddefs
>>
>> -BTRFS_DIR = btrfs
>> -TARGET_DIR = $(PKG_LIB_DIR)/$(TESTS_DIR)/$(BTRFS_DIR)
>> +GROUP_DIR = btrfs
>> +TARGET_DIR = $(PKG_LIB_DIR)/$(TESTS_DIR)/$(GROUP_DIR)
>
> What's this change for?
>
> Please don't make random undocumented changes when updating a
> patchset. It is best not to change anything unrelated to the problem
> being solved.
>
>
>> include $(BUILDRULES)
>>
>> +# note: the second TESTS wildcard gets also .out files, but these will be
>> +# overwritten with correct permissions a moment later, as they are also in
>> +# OUTPUTS. Regexps here are really limited, so I didn't find a better
>> +# solution...
>> +TESTS = $(wildcard [0-9][0-9][0-9])
>> +TESTS += $(wildcard [0-9][0-9][0-9]-*)
>
> Ugh. The hacks need to be constrained to the makefile and not do
> nasty things with installed files.
>
> What I think should be done here is to turn the selection around the
> other way because we can select the output files directly and
> correctly. We can then use that to build the test list via stripping
> and filtering as all tests must have an output file associated with
> them.
>
> So:
>
> # Start with all test related files:
> ALLFILES = $(wildcard [0-9]??*)
>
> # Now build a list of known output files. Unfortunately, the
> # multiple output test files are poorly handled as makefiles don't
> # handle wildcarded multi-suffix matching. Hence we separate the
> # processing of these right now.
> EXTENDED_OUTFILES = $(wildcard [0-9]??*.out.*)
> BASIC_OUTFILES = $(wildcard [0-9]??*.out)
> OUTFILES = $(EXTENDED_OUTFILES) $(BASIC_OUTFILES)
>
> # Strip suffix to get matching tests. We want to strip from the
> # first "." to the end, but makefiles don't have a built in
> # operative for that. So:
> #
> # Hack: strip the multiple segments after .out with repeated basename calls.
> EXTFILTER1 = $(basename $(EXTENDED_OUTFILES))
> EXTFILTER2 = $(basename $(EXTFILTER1))
> EXTFILTER3 = $(basename $(EXTFILTER2))
> EXTFILTER4 = $(basename $(EXTFILTER3))
>
> # final filter list
> FILTER = $(basename $(EXTFILTER4) $(BASIC_OUTFILES))
>
> # finally, select the test files by filtering against against the
> # stripped output files and sort them to remove duplicates.
> TESTS = $(sort $(filter $(ALLFILES), $(FILTER)))
>
> That will give you two lists - "OUTFILES" which are all the output
> files that need to be installed, and "TESTS" which are all the tests
> that should be installed. There will be no overlap between these two
> lists.
>
> I'd also suggest that this can probably be put in the BUILDRULES
> file, as it will be common to all makefiles and doesn't need to be
> repeated in every makefile.
>
Thanks for your code. I submitted a modified patch, though I have a question...
Should I put you into Signed-of-by in this case, as you wrote most of
the new version of this patch?
Thanks,
Jan
--
Jan Tulak
jtulak@redhat.com / jan@tulak.me
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] xfstests: Fix installation for extended names
2016-07-11 9:29 ` Jan Tulak
@ 2016-07-12 2:32 ` Dave Chinner
2016-07-12 7:29 ` Jan Tulak
0 siblings, 1 reply; 9+ messages in thread
From: Dave Chinner @ 2016-07-12 2:32 UTC (permalink / raw)
To: Jan Tulak; +Cc: fstests, Eryu Guan
On Mon, Jul 11, 2016 at 11:29:42AM +0200, Jan Tulak wrote:
> Hi,
>
> On Mon, Jul 4, 2016 at 3:40 AM, Dave Chinner <david@fromorbit.com> wrote:
> > On Fri, Jul 01, 2016 at 06:09:06PM +0200, Jan Tulak wrote:
> >> xfstests supports extended test names like 314-foo-bar, but installation of
> >> these tests was skipped (not matching a regexp). So this patch fixes the
> >> makefiles in tests/*/
> >>
> >> Signed-off-by: Jan Tulak <jtulak@redhat.com>
> >> ---
> >> UPDATE:
> >> Rewritten, now it doesn't fail without the extended name, and doesn't
> >> suppress anything. Thanks Dave.
> >> ---
> >> tests/btrfs/Makefile | 21 +++++++++++++++++----
> >> tests/cifs/Makefile | 21 +++++++++++++++++----
> >> tests/ext4/Makefile | 21 +++++++++++++++++----
> >> tests/f2fs/Makefile | 24 ++++++++++++++++++------
> >> tests/generic/Makefile | 21 +++++++++++++++++----
> >> tests/overlay/Makefile | 23 ++++++++++++++++++-----
> >> tests/shared/Makefile | 21 +++++++++++++++++----
> >> tests/udf/Makefile | 21 +++++++++++++++++----
> >> tests/xfs/Makefile | 21 +++++++++++++++++----
> >> 9 files changed, 155 insertions(+), 39 deletions(-)
> >>
> >> diff --git a/tests/btrfs/Makefile b/tests/btrfs/Makefile
> >> index e1a5be1..0301d78 100644
> >> --- a/tests/btrfs/Makefile
> >> +++ b/tests/btrfs/Makefile
> >> @@ -5,16 +5,29 @@
> >> TOPDIR = ../..
> >> include $(TOPDIR)/include/builddefs
> >>
> >> -BTRFS_DIR = btrfs
> >> -TARGET_DIR = $(PKG_LIB_DIR)/$(TESTS_DIR)/$(BTRFS_DIR)
> >> +GROUP_DIR = btrfs
> >> +TARGET_DIR = $(PKG_LIB_DIR)/$(TESTS_DIR)/$(GROUP_DIR)
> >
> > What's this change for?
> >
> > Please don't make random undocumented changes when updating a
> > patchset. It is best not to change anything unrelated to the problem
> > being solved.
> >
> >
> >> include $(BUILDRULES)
> >>
> >> +# note: the second TESTS wildcard gets also .out files, but these will be
> >> +# overwritten with correct permissions a moment later, as they are also in
> >> +# OUTPUTS. Regexps here are really limited, so I didn't find a better
> >> +# solution...
> >> +TESTS = $(wildcard [0-9][0-9][0-9])
> >> +TESTS += $(wildcard [0-9][0-9][0-9]-*)
> >
> > Ugh. The hacks need to be constrained to the makefile and not do
> > nasty things with installed files.
> >
> > What I think should be done here is to turn the selection around the
> > other way because we can select the output files directly and
> > correctly. We can then use that to build the test list via stripping
> > and filtering as all tests must have an output file associated with
> > them.
> >
> > So:
> >
> > # Start with all test related files:
> > ALLFILES = $(wildcard [0-9]??*)
> >
> > # Now build a list of known output files. Unfortunately, the
> > # multiple output test files are poorly handled as makefiles don't
> > # handle wildcarded multi-suffix matching. Hence we separate the
> > # processing of these right now.
> > EXTENDED_OUTFILES = $(wildcard [0-9]??*.out.*)
> > BASIC_OUTFILES = $(wildcard [0-9]??*.out)
> > OUTFILES = $(EXTENDED_OUTFILES) $(BASIC_OUTFILES)
> >
> > # Strip suffix to get matching tests. We want to strip from the
> > # first "." to the end, but makefiles don't have a built in
> > # operative for that. So:
> > #
> > # Hack: strip the multiple segments after .out with repeated basename calls.
> > EXTFILTER1 = $(basename $(EXTENDED_OUTFILES))
> > EXTFILTER2 = $(basename $(EXTFILTER1))
> > EXTFILTER3 = $(basename $(EXTFILTER2))
> > EXTFILTER4 = $(basename $(EXTFILTER3))
> >
> > # final filter list
> > FILTER = $(basename $(EXTFILTER4) $(BASIC_OUTFILES))
> >
> > # finally, select the test files by filtering against against the
> > # stripped output files and sort them to remove duplicates.
> > TESTS = $(sort $(filter $(ALLFILES), $(FILTER)))
> >
> > That will give you two lists - "OUTFILES" which are all the output
> > files that need to be installed, and "TESTS" which are all the tests
> > that should be installed. There will be no overlap between these two
> > lists.
> >
> > I'd also suggest that this can probably be put in the BUILDRULES
> > file, as it will be common to all makefiles and doesn't need to be
> > repeated in every makefile.
> >
>
> Thanks for your code. I submitted a modified patch, though I have a question...
> Should I put you into Signed-of-by in this case, as you wrote most of
> the new version of this patch?
No. You wrote the patch, so it needs your SOB. The usual thing to do
here is add a line to the commit message saying something like:
"Based on work/idea/prototype/code from ..."
that way it's clear where it originally came from. If the maintainer
requires further verification of the source of the code, I can ack
the patch you sent to the list.
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] xfstests: Fix installation for extended names
2016-07-12 2:32 ` Dave Chinner
@ 2016-07-12 7:29 ` Jan Tulak
0 siblings, 0 replies; 9+ messages in thread
From: Jan Tulak @ 2016-07-12 7:29 UTC (permalink / raw)
To: Dave Chinner; +Cc: fstests, Eryu Guan
On Tue, Jul 12, 2016 at 4:32 AM, Dave Chinner <david@fromorbit.com> wrote:
>> Thanks for your code. I submitted a modified patch, though I have a question...
>> Should I put you into Signed-of-by in this case, as you wrote most of
>> the new version of this patch?
>
> No. You wrote the patch, so it needs your SOB. The usual thing to do
> here is add a line to the commit message saying something like:
>
> "Based on work/idea/prototype/code from ..."
>
> that way it's clear where it originally came from. If the maintainer
> requires further verification of the source of the code, I can ack
> the patch you sent to the list.
>
OK, thanks. Exactly as I did it. :-)
Thanks,
Jan
--
Jan Tulak
jtulak@redhat.com / jan@tulak.me
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2016-07-12 7:29 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-07-01 16:09 [PATCH 0/2] xfstests: extended names fixes Jan Tulak
2016-07-01 16:09 ` [PATCH 1/2] xfstests: Fix installation for extended names Jan Tulak
2016-07-04 1:40 ` Dave Chinner
2016-07-11 9:29 ` Jan Tulak
2016-07-12 2:32 ` Dave Chinner
2016-07-12 7:29 ` Jan Tulak
2016-07-01 16:09 ` [PATCH 2/2] xfstests: filename handling for extended names in ./check was on a wrong place Jan Tulak
2016-07-04 1:46 ` Dave Chinner
2016-07-11 9:27 ` Jan Tulak
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.