From patchwork Wed Mar 8 14:26:22 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Amir Goldstein X-Patchwork-Id: 9611363 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 6106B6046A for ; Wed, 8 Mar 2017 14:58:01 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 3A5E0285B8 for ; Wed, 8 Mar 2017 14:58:01 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 2F4D2285BD; Wed, 8 Mar 2017 14:58:01 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.3 required=2.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, FREEMAIL_FROM, RCVD_IN_DNSWL_HI, RCVD_IN_SORBS_SPAM, T_DKIM_INVALID, T_TVD_MIME_EPI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id F119E285D6 for ; Wed, 8 Mar 2017 14:57:59 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751748AbdCHO5s (ORCPT ); Wed, 8 Mar 2017 09:57:48 -0500 Received: from mail-oi0-f49.google.com ([209.85.218.49]:36806 "EHLO mail-oi0-f49.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751735AbdCHO5r (ORCPT ); Wed, 8 Mar 2017 09:57:47 -0500 Received: by mail-oi0-f49.google.com with SMTP id 126so19807013oig.3 for ; Wed, 08 Mar 2017 06:57:47 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=nN1HxUEiSM6ehZB+FJPYAIryTU57z3ZGBYYj30/MrMY=; b=h0sk99k1tyf8cCBEhHxhoGD9xcaczO1Rgcu2LEHu5398qjQR/XOhlGaabG1Z5TYKVk yFLXSi/NuAcc1rieVXruAJbFcSNFx2aWUcZ+is1LI6R7vmCztLLKzzGTBfyfjOPv0h99 5ZSysS2SAWIZ44t9VKfEbyrSeUylkmsEhbav/uAUUubWTIDSiBawofoE9TMb083Fo3bS sLr0RbB28PQH8yfObOpXEauqbn6+RM4gvXV7HX/3e82TqYXC12URA98KVKrFLzyVFcFy 4xxoajSkjLimSaOPAt77naFFABlX1EZfowg6RqwaPASCGg6fZSIEeckNXpq1y5RLpUvG 1qvg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=nN1HxUEiSM6ehZB+FJPYAIryTU57z3ZGBYYj30/MrMY=; b=M46zFjmvcclPKJmHkCTlUyLOiqml0eTcbPEAXZOFIyFI6eCfXKe2S3PcFnFcPzPx0j g3iP/CC8vUkcg0CVpDAUCOg0cofYctlDVMw3F4Rp63tlBeBw8YDqV/U6fY6+6ndsfwkY YDZpSc0+DBxAXEmDxJNPv7a0W3OSSJjaMzJzWbNY+WXuwZv+uklxKQ40+HjMrPbkQ3sq WGhD/SXXmxMbBvIxtnbsEjNZl3MR+hhBdiAO2Ge635S+o/XnHuTDGTjkeFvWlalHtw7R jwft86F5iRaZ5YQeiQosavYfA5BiJ8GGK7IwFCG0huc2PtH2r56ni7YIfKtjhWC+9Zt6 N8NA== X-Gm-Message-State: AMke39mOG1oSZetM/zocml8JJVYvlSn4QGsvUVSQBq/OeJZmoniBqilB4266JbntkZbJqohhgm5ZdzG50esJ2Q== X-Received: by 10.202.232.210 with SMTP id f201mr3216079oih.60.1488983183013; Wed, 08 Mar 2017 06:26:23 -0800 (PST) MIME-Version: 1.0 Received: by 10.182.139.103 with HTTP; Wed, 8 Mar 2017 06:26:22 -0800 (PST) In-Reply-To: <20170308115136.GF14226@eguan.usersys.redhat.com> References: <1488957991-18194-1-git-send-email-zlang@redhat.com> <20170308115136.GF14226@eguan.usersys.redhat.com> From: Amir Goldstein Date: Wed, 8 Mar 2017 16:26:22 +0200 Message-ID: Subject: Re: [PATCH] generic/411: change sub-path name that's duplicate of TEST_DIR To: Eryu Guan Cc: Zorro Lang , fstests , "Darrick J. Wong" Sender: fstests-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: fstests@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Wed, Mar 8, 2017 at 1:51 PM, Eryu Guan wrote: > On Wed, Mar 08, 2017 at 12:01:33PM +0200, Amir Goldstein wrote: >> On Wed, Mar 8, 2017 at 9:26 AM, Zorro Lang wrote: >> > Darrick found generic/411 golden output mismatch if use >> > TEST_DIR=/mnt. Because g/411 use some test path named >> > /mnt/XXXX/mnt1/mnt2, _filter_test_dir will replace all >> > "/mnt" things to "TEST_DIR". >> > >> > For stop this failure, change all directory names to be >> > "$seq-XXX", that's less likely to be mistaken for TEST_* >> > and SCRATCH_*. >> > >> >> Although you have a right to choose whichever names you >> want top use for your test, this is papering over a bug. >> >> I re-read the docuemtnation for \B: >> http://www.rexegg.com/regex-boundaries.html#bengines >> >> To my understanding, the expression "\B$TEST_DIR" will >> match every instance of $TEST_DIR, where preceding character >> is NOT a letter, number or underscore. >> This is because $TEST_DIR must start with '/', which is not >> a letter, number or underscore. >> >> I think it should be safe to fix _filter_test_dir and _filter_scratch. > > I agreed, according to the document above, \B matches all positions > where \b doesn't match. And \b matches positions where "one side is a > word character and the other side is not", so \B matches "neither side > is a word character" and "both sides are a word character". > > This is also because we canonicalized all mount points, there's no path > like //mnt/mnt1 is allowed in fstests. And this leads me to wonder if we > should canonicalize all test devices (if they're block device), to avoid > something like //dev/sda5? The double "/" will break the \B match. > //dev/sda5 does to break \B match. \B$TEST_DEV match, as you wrote, means that character before $TEST_DEV *is not* a word character, because $TEST_DEV begins with a non-word character. it doesn't matter the the second character is a non-word as well. Attached a patch that I tested -g quick on xfs and overlayfs and did not see any regressions, but you better test it with a wider variety of fs types and a larger group of tests. > Further more, if we decide to use \B to improve _filter_test_dir and > _filter_scratch, it appears to me that the fix from commit 4e965d8 > ("fstests: fix test and scratch filters for overlapping DEV/MNT paths") > can be discarded, the order is not a problem anymore. > Well, not exactly. The case that commit 4e965d8 fixes is: TEST_DEV=/mnt TEST_DIR=/mnt/ovl-mnt So \B$TEST_DEV will still match the prefix of $TEST_DIR and this is wrong. However, while $TEST_DEV can really be a prefix of $TEST_DIR (and will always be a prefix with new -overlay) I doubt that $TEST_DIR can ever be a prefix of $TEST_DEV. So instead of the full blown test in commit 4e965d8, it would suffice to just match $TEST_DIR before $TEST_DEV (i.e. keep the if case and drop the else case). So before you test \B with all fs types, you may revert commit 4e965d8, but please make sure that the resulting filters are in the correct order: _filter_test_dir() { # TEST_DEV may be a prefix of TEST_DIR (e.g. /mnt and /mnt/ovl-mnt) # substitute TEST_DIR first sed -e "s,\B$TEST_DIR,TEST_DIR,g" \ -e "s,\B$TEST_DEV,TEST_DEV,g" } _filter_scratch() { # SCRATCH_DEV may be a prefix of SCRATCH_MNT # substitute SCRATCH_MNT first sed -e "s,\B$SCRATCH_MNT,SCRATCH_MNT,g" \ -e "s,\B$SCRATCH_DEV,SCRATCH_DEV,g" \ -e "/.use_space/d" } From f365bd5d60d9d5b82e02db17e615380bf37a74de Mon Sep 17 00:00:00 2001 From: Amir Goldstein Date: Wed, 8 Mar 2017 12:38:22 +0200 Subject: [PATCH] filter: match $TEST_* $SCRATCH_* in beginning of path string For example, if $TEST_DIR=/mnt, only replace instacnes of /mnt that are in the beginning of a path string, e.g.: "/mnt/mntA/mntB:/mnt/mntC" => "TEST_DIR/mntA/mntB:TEST_DIR/mntC" Signed-off-by: Amir Goldstein --- common/filter | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/common/filter b/common/filter index 1ceb346..2509483 100644 --- a/common/filter +++ b/common/filter @@ -283,13 +283,13 @@ _filter_test_dir() if ( echo $TEST_DIR | grep -q $TEST_DEV ); then # TEST_DEV is substr of TEST_DIR (e.g. /mnt and /mnt/ovl-mnt) # substitute TEST_DIR first - sed -e "s,$TEST_DIR,TEST_DIR,g" \ - -e "s,$TEST_DEV,TEST_DEV,g" + sed -e "s,\B$TEST_DIR,TEST_DIR,g" \ + -e "s,\B$TEST_DEV,TEST_DEV,g" else # TEST_DIR maybe a substr of TEST_DIR (e.g. /vdc and /dev/vdc) # substitute TEST_DEV first - sed -e "s,$TEST_DEV,TEST_DEV,g" \ - -e "s,$TEST_DIR,TEST_DIR,g" + sed -e "s,\B$TEST_DEV,TEST_DEV,g" \ + -e "s,\B$TEST_DIR,TEST_DIR,g" fi } @@ -297,13 +297,13 @@ _filter_scratch() { if ( echo $SCRATCH_MNT | grep -q $SCRATCH_DEV ); then # SCRATCH_DEV is substr of SCRATCH_MNT - sed -e "s,$SCRATCH_MNT,SCRATCH_MNT,g" \ - -e "s,$SCRATCH_DEV,SCRATCH_DEV,g" \ + sed -e "s,\B$SCRATCH_MNT,SCRATCH_MNT,g" \ + -e "s,\B$SCRATCH_DEV,SCRATCH_DEV,g" \ -e "/.use_space/d" else # SCRATCH_MNT maybe a substr of SCRATCH_DEV - sed -e "s,$SCRATCH_DEV,SCRATCH_DEV,g" \ - -e "s,$SCRATCH_MNT,SCRATCH_MNT,g" \ + sed -e "s,\B$SCRATCH_DEV,SCRATCH_DEV,g" \ + -e "s,\B$SCRATCH_MNT,SCRATCH_MNT,g" \ -e "/.use_space/d" fi } -- 2.7.4