* [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
* 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 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
* [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 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
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 a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox