Message ID | 20230324114651.2987-1-tdevries@suse.de |
---|---|
State | New |
Headers |
Return-Path: <gdb-patches-bounces+patchwork=sourceware.org@sourceware.org> X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 1689B3850435 for <patchwork@sourceware.org>; Fri, 24 Mar 2023 11:47:18 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 1689B3850435 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1679658438; bh=cf7rfAGUQeK545byz7gmw4G7Woi3F0oocXpnu8FhLmE=; h=To:Subject:Date:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:From; b=WUwwTTIz4P/DtNoK4lTYq8Ib5TQZtEWfvJ0ee0qkOFxOlHkO0GP8QbBSPGDE2Yj1o X6zgz6fslgg0dbQSbVJRoTxg3Rzdjjh6WyHOsmiV8QC4QGmVUUxEk53TRWZT0SJw2s 3QP8UxQPt12krWJ0WNLnGTePq0miiLEwyVgkir68= X-Original-To: gdb-patches@sourceware.org Delivered-To: gdb-patches@sourceware.org Received: from smtp-out2.suse.de (smtp-out2.suse.de [195.135.220.29]) by sourceware.org (Postfix) with ESMTPS id DFE233858D28 for <gdb-patches@sourceware.org>; Fri, 24 Mar 2023 11:46:53 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org DFE233858D28 Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out2.suse.de (Postfix) with ESMTPS id 21C9D1F898 for <gdb-patches@sourceware.org>; Fri, 24 Mar 2023 11:46:53 +0000 (UTC) Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id 0B9E6133E5 for <gdb-patches@sourceware.org>; Fri, 24 Mar 2023 11:46:53 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id mjCuAa2NHWTBbgAAMHmgww (envelope-from <tdevries@suse.de>) for <gdb-patches@sourceware.org>; Fri, 24 Mar 2023 11:46:53 +0000 To: gdb-patches@sourceware.org Subject: [RFC] [gdb/testsuite] Add gdb/contrib/testsuite-normalize.sh Date: Fri, 24 Mar 2023 12:46:51 +0100 Message-Id: <20230324114651.2987-1-tdevries@suse.de> X-Mailer: git-send-email 2.35.3 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-12.5 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list <gdb-patches.sourceware.org> List-Unsubscribe: <https://sourceware.org/mailman/options/gdb-patches>, <mailto:gdb-patches-request@sourceware.org?subject=unsubscribe> List-Archive: <https://sourceware.org/pipermail/gdb-patches/> List-Post: <mailto:gdb-patches@sourceware.org> List-Help: <mailto:gdb-patches-request@sourceware.org?subject=help> List-Subscribe: <https://sourceware.org/mailman/listinfo/gdb-patches>, <mailto:gdb-patches-request@sourceware.org?subject=subscribe> From: Tom de Vries via Gdb-patches <gdb-patches@sourceware.org> Reply-To: Tom de Vries <tdevries@suse.de> Errors-To: gdb-patches-bounces+patchwork=sourceware.org@sourceware.org Sender: "Gdb-patches" <gdb-patches-bounces+patchwork=sourceware.org@sourceware.org> |
Series |
[RFC,gdb/testsuite] Add gdb/contrib/testsuite-normalize.sh
|
|
Commit Message
Tom de Vries
March 24, 2023, 11:46 a.m. UTC
There are a number of different ways to express the same thing in TCL. For instance, $foo and ${foo}: ... % set foo "foo bar" foo bar % puts ${foo} foo bar % puts $foo foo bar ... The braces make things (IMO) harder to read, but are necessary in some cases though, for instance ${foo-bar} and $foo-bar are not the same thing: ... % set foo "foo var" foo var % set foo-bar "foo-bar var" foo-bar var % puts ${foo-bar} foo-bar var % puts $foo-bar foo var-bar ... Furthermore, there's the tendency to use "$foo" (as is often necessary in shell scripting), while in TCL using $foo is sufficient: ... % set foo "bar bar" bar bar % puts "$foo" bar bar % puts $foo bar bar ... Add a script gdb/contrib/testsuite-normalize.sh, which rewrites test-cases in a normal form, using $foo instead of ${foo}, and $foo instead of "$foo", where possible. This patch doesn't contain the effects of running it, because that would make the patch too large: ... $ git diff | wc 63705 273146 2457187 ... If this approach is acceptable, I'll commit the effects as a seperate patch. Tested on x86_64-linux. --- gdb/contrib/testsuite-normalize.sh | 44 ++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) create mode 100755 gdb/contrib/testsuite-normalize.sh base-commit: ca357a9359f5d09058d27fc1f84f1d53c2717ba1
Comments
On 3/24/23 07:46, Tom de Vries via Gdb-patches wrote: > There are a number of different ways to express the same thing in TCL. > > For instance, $foo and ${foo}: > ... > % set foo "foo bar" > foo bar > % puts ${foo} > foo bar > % puts $foo > foo bar > ... > > The braces make things (IMO) harder to read, but are necessary in some cases > though, for instance ${foo-bar} and $foo-bar are not the same thing: > ... > % set foo "foo var" > foo var > % set foo-bar "foo-bar var" > foo-bar var > % puts ${foo-bar} > foo-bar var > % puts $foo-bar > foo var-bar > ... > > Furthermore, there's the tendency to use "$foo" (as is often necessary in shell > scripting), while in TCL using $foo is sufficient: > ... > % set foo "bar bar" > bar bar > % puts "$foo" > bar bar > % puts $foo > bar bar > ... > > Add a script gdb/contrib/testsuite-normalize.sh, which rewrites test-cases > in a normal form, using $foo instead of ${foo}, and $foo instead of "$foo", > where possible. > > This patch doesn't contain the effects of running it, because that would make > the patch too large: > ... > $ git diff | wc > 63705 273146 2457187 > ... > > If this approach is acceptable, I'll commit the effects as a seperate patch. I have nothing against it. I like when the formatting is normalized (which is why is <3 black for Python). > Tested on x86_64-linux. ➜ gdb git:(master) ✗ pwd /home/smarchi/src/binutils-gdb/gdb ➜ gdb git:(master) ✗ ./contrib/testsuite-normalize.sh find: ‘gdb/testsuite’: No such file or directory My CWD is the gdb directory 99% of the time. IWBN if the script would work regardless of where you are in the source repo (it looks like you need to be in the root of the repo for it to work). Perhaps it could locate the root directory of the git repository and cd there? I tried it and looked at a bunch of files, the changes seemed fine. > +main () > +{ > + if [ $# -eq 0 ]; then > + mapfile files < <(find gdb/testsuite -type f -name "*.exp*") > + else > + files=("$@") > + fi > + > + for f in "${files[@]}"; do > + f=$(echo $f) What's the purpose of this echo? shellcheck says: In gdb/contrib/testsuite-normalize.sh line 31: f=$(echo $f) ^--------^ SC2116 (style): Useless echo? Instead of 'cmd $(echo foo)', just use 'cmd foo'. ^-- SC2086 (info): Double quote to prevent globbing and word splitting. Did you mean: f=$(echo "$f") Simon
>>>>> "Tom" == Tom de Vries via Gdb-patches <gdb-patches@sourceware.org> writes:
Tom> Furthermore, there's the tendency to use "$foo" (as is often necessary in shell
Tom> scripting), while in TCL using $foo is sufficient:
FWIW I sometimes find myself using unnecessary quotes in Tcl just to
make some things highlight differently in Emacs. So I'd imagine there's
a lot more quoting that could be removed if we cared to.
Tom> Add a script gdb/contrib/testsuite-normalize.sh, which rewrites test-cases
Tom> in a normal form, using $foo instead of ${foo}, and $foo instead of "$foo",
Tom> where possible.
The idea seems fine to me. I didn't really read the code.
Tom> +++ b/gdb/contrib/testsuite-normalize.sh
Tom> @@ -0,0 +1,44 @@
Tom> +#!/bin/bash
Tom> +
Should have a copyright header.
Tom
On 3/24/23 14:52, Simon Marchi wrote: > On 3/24/23 07:46, Tom de Vries via Gdb-patches wrote: >> There are a number of different ways to express the same thing in TCL. >> >> For instance, $foo and ${foo}: >> ... >> % set foo "foo bar" >> foo bar >> % puts ${foo} >> foo bar >> % puts $foo >> foo bar >> ... >> >> The braces make things (IMO) harder to read, but are necessary in some cases >> though, for instance ${foo-bar} and $foo-bar are not the same thing: >> ... >> % set foo "foo var" >> foo var >> % set foo-bar "foo-bar var" >> foo-bar var >> % puts ${foo-bar} >> foo-bar var >> % puts $foo-bar >> foo var-bar >> ... >> >> Furthermore, there's the tendency to use "$foo" (as is often necessary in shell >> scripting), while in TCL using $foo is sufficient: >> ... >> % set foo "bar bar" >> bar bar >> % puts "$foo" >> bar bar >> % puts $foo >> bar bar >> ... >> >> Add a script gdb/contrib/testsuite-normalize.sh, which rewrites test-cases >> in a normal form, using $foo instead of ${foo}, and $foo instead of "$foo", >> where possible. >> >> This patch doesn't contain the effects of running it, because that would make >> the patch too large: >> ... >> $ git diff | wc >> 63705 273146 2457187 >> ... >> >> If this approach is acceptable, I'll commit the effects as a seperate patch. > > I have nothing against it. I like when the formatting is normalized > (which is why is <3 black for Python). > Yeah, I wish there was an existing tool we could use, but I haven't found anything. >> Tested on x86_64-linux. > > ➜ gdb git:(master) ✗ pwd > /home/smarchi/src/binutils-gdb/gdb > ➜ gdb git:(master) ✗ ./contrib/testsuite-normalize.sh > find: ‘gdb/testsuite’: No such file or directory > > My CWD is the gdb directory 99% of the time. IWBN if the script would > work regardless of where you are in the source repo (it looks like you > need to be in the root of the repo for it to work). Perhaps it could > locate the root directory of the git repository and cd there? > Ack, fixed (FWIW, not using cd though). > I tried it and looked at a bunch of files, the changes seemed fine. > >> +main () >> +{ >> + if [ $# -eq 0 ]; then >> + mapfile files < <(find gdb/testsuite -type f -name "*.exp*") >> + else >> + files=("$@") >> + fi >> + >> + for f in "${files[@]}"; do >> + f=$(echo $f) > > What's the purpose of this echo? Strip trailing newline. > shellcheck says: > > > In gdb/contrib/testsuite-normalize.sh line 31: > f=$(echo $f) > ^--------^ SC2116 (style): Useless echo? Instead of 'cmd $(echo foo)', just use 'cmd foo'. > ^-- SC2086 (info): Double quote to prevent globbing and word splitting. > > Did you mean: > f=$(echo "$f") > > I've managed to fix this by using instead: ... f=${f%$'\n'} ... It passes shellcheck now. Also added copyright notice, as requested by Tom in the other reply. Thanks, - Tom
On 3/24/23 15:16, Tom Tromey wrote: >>>>>> "Tom" == Tom de Vries via Gdb-patches <gdb-patches@sourceware.org> writes: > > Tom> Furthermore, there's the tendency to use "$foo" (as is often necessary in shell > Tom> scripting), while in TCL using $foo is sufficient: > > FWIW I sometimes find myself using unnecessary quotes in Tcl just to > make some things highlight differently in Emacs. So I'd imagine there's > a lot more quoting that could be removed if we cared to. > > Tom> Add a script gdb/contrib/testsuite-normalize.sh, which rewrites test-cases > Tom> in a normal form, using $foo instead of ${foo}, and $foo instead of "$foo", > Tom> where possible. > > The idea seems fine to me. Great, thanks. > I didn't really read the code. > > Tom> +++ b/gdb/contrib/testsuite-normalize.sh > Tom> @@ -0,0 +1,44 @@ > Tom> +#!/bin/bash > Tom> + > > Should have a copyright header. Ack, fixed in this ( https://sourceware.org/pipermail/gdb-patches/2023-March/198278.html ) update. Thanks, - Tom
diff --git a/gdb/contrib/testsuite-normalize.sh b/gdb/contrib/testsuite-normalize.sh new file mode 100755 index 00000000000..0cbde3526ce --- /dev/null +++ b/gdb/contrib/testsuite-normalize.sh @@ -0,0 +1,44 @@ +#!/bin/bash + +handle_file () +{ + f="$1" + + # Rewrite '${foo}' -> '$foo''. + # Don't rewrite '\${foo}' -> '\$foo'. + # Don't rewrite '${foo}(bar)' -> '$foo(bar). + # Don't rewrite '${foo}bar' -> '$foobar'. + # Don't rewrite '${foo}::bar' -> '$foo::bar'. + # Two variants, first one matching '${foo}<eol>'. + sed -i 's/\([^\\]\)${\([a-zA-Z0-9_]*\)}$/\1$\2/g' "$f" + sed -i 's/\([^\\]\)${\([a-zA-Z0-9_]*\)}\([^a-zA-Z0-9_(:]\)/\1$\2\3/g' "$f" + + # Rewrite ' "$foo" ' -> ' $foo '. + # Rewrite ' "$foo"<eol>' -> ' $foo<eol>'. + sed -i 's/\([ \t][ \t]*\)"\$\([a-zA-Z0-9_]*\)"$/\1$\2/g' "$f" + sed -i 's/\([ \t][ \t]*\)"\$\([a-zA-Z0-9_]*\)"\([ \t][ \t]*\)/\1$\2\3/g' "$f" +} + +main () +{ + if [ $# -eq 0 ]; then + mapfile files < <(find gdb/testsuite -type f -name "*.exp*") + else + files=("$@") + fi + + for f in "${files[@]}"; do + f=$(echo $f) + sum=$(md5sum "$f") + while true; do + handle_file "$f" + prev=$sum + sum=$(md5sum "$f") + if [ "$sum" == "$prev" ]; then + break + fi + done + done +} + +main "$@"