On Tue 29-05-18 11:39:14, Dave Chinner wrote:
> On Thu, May 24, 2018 at 08:30:55PM +0200, Jan Kara wrote:
> > Currently 'new' script sources common/config which tries to find mkfs
> > and fails if not found (which is likely for non-root user). This is
> > inconvenient as development usually does not happen as root. In fact the
> > vast majority of setup in common/config and common/rc is not necessary
> > for 'new'. Split out the necessary bits into new common/config-base and
> > use it in 'new'. Cleanup common/rc and common/config now that it's only
> > used from 'check'.
>
> FWIW, common/config is also called from setup.
Fixed up.
> > diff --git a/check b/check
> > index 96198ac4714e..4c6e8285bc16 100755
> > --- a/check
> > +++ b/check
> > @@ -331,10 +331,11 @@ while [ $# -gt 0 ]; do
> > shift
> > done
> >
> > -# we need common/config, source it after processing args, overlay needs FSTYP
> > -# set before sourcing common/config
> > -if ! . ./common/config; then
> > - echo "$iam: failed to source common/config"
> > +# we need common/rc, that also sources common/config. We need to source it
> > +# after processing args, overlay needs FSTYP set before sourcing common/config
> > +if ! . ./common/rc
> > +then
> > + echo "check: failed to source common/rc"
> > exit 1
> > fi
>
> Can we keep the existing formatting of "if foo ; then", please?
Will do.
> > diff --git a/common/config b/common/config
> > index af360cefc804..fa07a6799824 100644
> > --- a/common/config
> > +++ b/common/config
> > @@ -47,6 +47,8 @@
> > # validity or mountedness.
> > #
> >
> > +. common/config-base
> > +
> > # all tests should use a common language setting to prevent golden
> > # output mismatches.
> > export LANG=C
> > @@ -88,12 +90,6 @@ export LOCAL_CONFIGURE_OPTIONS=${LOCAL_CONFIGURE_OPTIONS:=--enable-readline=yes}
> >
> > export RECREATE_TEST_DEV=false
> >
> > -# $1 = prog to look for
> > -set_prog_path()
> > -{
> > - type -P $1
> > -}
>
> Can we just get rid of set_prog_path() and replace it with direct
> calls to $(type -P foo) as an initial patch? This goes away at that
> point, because new can then just use a locally coded version....
I've added your patch to the series. Thanks!
> > --- /dev/null
> > +++ b/common/config-base
> > @@ -0,0 +1,21 @@
> > +##/bin/bash
> > +
> > +# Valid test names start with 3 digits "NNN":
> > +# "[0-9]\{3\}"
> > +# followed by an optional "-":
> > +# "-\?"
> > +# followed by an optional combination of alphanumeric and "-" chars:
> > +# "[[:alnum:]-]*"
> > +# e.g. 999-the-mark-of-fstests
> > +#
> > +VALID_TEST_ID="[0-9]\{3\}"
> > +VALID_TEST_NAME="$VALID_TEST_ID-\?[[:alnum:]-]*"
>
> This, then, is really the only thing shared between check and new,
> right? So perhaps rename the file to common/test_names so it doesn't
> get confused with stuff to do with configuration?
OK, there's also the AWK_PROG thing but I've decided to just have that
directly in 'new'. There's not much to break there.
Thanks for review!
Honza
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
--
To unsubscribe from this list: send the line "unsubscribe fstests" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
