From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id 30EF13858D3C for ; Fri, 18 Mar 2022 16:23:25 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 30EF13858D3C Received: from mail-wm1-f69.google.com (mail-wm1-f69.google.com [209.85.128.69]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-621-jBtXFaTSOXm5vTiwuHRmpg-1; Fri, 18 Mar 2022 12:23:22 -0400 X-MC-Unique: jBtXFaTSOXm5vTiwuHRmpg-1 Received: by mail-wm1-f69.google.com with SMTP id 2-20020a1c0202000000b0038c71e8c49cso3775353wmc.1 for ; Fri, 18 Mar 2022 09:23:22 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=SJbNJsT35MZ/pbGV625Pl8mbdT2I5ff1HNgX8Cwvmjs=; b=2SvnS69qRiDWiTAR4hpHJr+Tmzw2KSn7nWGo1TFpqg6k6Hyhzcus38FNDUBlJgz6bO aMlmGCcjY3LzHjn107RGunvGw1dkAkeiyzOExpLeA2wS9ZHf5HOOOnZEemEQUnJ6y5jJ 5Hkj9pm+yqj3OMFMFGuHwpn/eFZOUbYhJ4jONDwkIGwGYJHxpifEU8fUrmRllBLVXZJb VBXbYX12d+knco1MzzyvJt7Wnb7uw492itOxcxXi7Xzr8WJGbk+dt6dF1G0+gOHj18lg M2PF/b9iUkty9fepq+98H6CnR+FYsVPf4qAfxj5GKtaciknSQunJuZSqiVUvl01DDvOK EWuQ== X-Gm-Message-State: AOAM530GgnKJVrpI+4/+St8QhHrxwYkWKZUbNoDe1yuQbvtmmMogXTJR 1OHSEIgVErMVZfJ7wn1/e0B+tokgn4dwzyKNSUhapdG76lJ/+zbey34ENtIZxNdXv/R1vG5OEuK Ie6pE+jz5YGFleqGa2kiQpg== X-Received: by 2002:a05:600c:3d8d:b0:38c:6f6e:e61a with SMTP id bi13-20020a05600c3d8d00b0038c6f6ee61amr11245905wmb.101.1647620600769; Fri, 18 Mar 2022 09:23:20 -0700 (PDT) X-Google-Smtp-Source: ABdhPJwgZvtcNESF+tDDa+D/TEUfijQTCBy4Sq3VJBfzpUzUbls7Ta1Lz007JAhVJLql/CJmKRwjeg== X-Received: by 2002:a05:600c:3d8d:b0:38c:6f6e:e61a with SMTP id bi13-20020a05600c3d8d00b0038c6f6ee61amr11245879wmb.101.1647620600349; Fri, 18 Mar 2022 09:23:20 -0700 (PDT) Received: from localhost (host109-155-5-90.range109-155.btcentralplus.com. [109.155.5.90]) by smtp.gmail.com with ESMTPSA id o11-20020adf9d4b000000b001f0077ea337sm6750916wre.22.2022.03.18.09.23.19 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 18 Mar 2022 09:23:19 -0700 (PDT) Date: Fri, 18 Mar 2022 16:23:18 +0000 From: Andrew Burgess To: Pedro Alves Cc: gdb-patches@sourceware.org Subject: Re: [PATCH] gdb/testsuite: Installed-GDB testing & data-directory Message-ID: <20220318162318.GR1212730@redhat.com> References: <20220318161036.3290639-1-pedro@palves.net> MIME-Version: 1.0 In-Reply-To: <20220318161036.3290639-1-pedro@palves.net> X-Operating-System: Linux/5.8.18-100.fc31.x86_64 (x86_64) X-Uptime: 16:14:31 up 19 days, 5:52, X-Editor: GNU Emacs [ http://www.gnu.org/software/emacs ] X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=us-ascii Content-Disposition: inline X-Spam-Status: No, score=-11.1 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_BARRACUDACENTRAL, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H4, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_NONE, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 18 Mar 2022 16:23:28 -0000 * Pedro Alves [2022-03-18 16:10:36 +0000]: > In testsuite/README, we suggest that you can run the testsuite against > some other GDB binary by using: > > make check RUNTESTFLAGS=GDB=/usr/bin/gdb > > However, that example isn't fully correct, because with that command > line, the testsuite will still pass > > -data-directory=[pwd]/../data-directory > > to /usr/bin/gdb, like e.g.: > > ... > builtin_spawn /usr/bin/gdb -nw -nx -data-directory /home/pedro/gdb/build/gdb/testsuite/../data-directory -iex set height 0 -iex set width 0 > ... > > while if you're testing an installed GDB (the system GDB being the > most usual scenario), then you should normally let it use its own > configured directory, not the just-built GDB's data directory. > > This commit improves the status quo with the following two changes: > > - if the user specifies GDB on the command line, then by default, > don't start GDB with the -data-directory command line option. > I.e., let the tested GDB use its own configured data directory. Not sure I like that. First, I'm normally not pointing at a fully installed GDB, so a --data-directory is needed anyway. I usually have ~10 different gdb development trees in use, 1 of which is an upstream build. Normally I use this option to point at a GDB built (but not installed) in one of these other build trees. Second, mostly it usually doesn't matter which data-directory is used, but if I've changed anything in a python module then I'm definitely going to want to pick up the data-directory from the local build tree rather than anything else. I get that with the feature below I can still achieve the same end result, but (for me) the most common use case (using the local data-directory) now requires more typing. I guess you hit a case where using the local data-directory was a bad thing? I'm guessing maybe a change to something in the installed Python code maybe? I'm just trying to understand the motivation for this change. Thanks, Andrew > > - let the user override the data directory, via a new > GDB_DATA_DIRECTORY global. This replaces the existing > BUILD_DATA_DIRECTORY variable in testsuite/lib/gdb.exp, which > wasn't overridable, and was a bit misnamed for the new purpose. > > So after this, the following commands I believe behave intuitively: > > # Test the non-installed GDB in some build dir: > > make check \ > RUNTESTFLAGS="GDB=/path/to/other/build/gdb \ > GDB_DATA_DIRECTORY=/path/to/other/build/gdb/data-directory" > > # Test the GDB installed in some prefix: > > make check \ > RUNTESTFLAGS="GDB=/opt/gdb/bin/gdb" > > # Test the built GDB with some alternative data directory, e.g., the > system GDB's data directory: > > make check \ > RUNTESTFLAGS="GDB_DATA_DIRECTORY=/usr/share/gdb" > > Change-Id: Icdc21c85219155d9564a9900961997e6624b78fb > --- > gdb/testsuite/README | 13 ++++++++++++ > gdb/testsuite/lib/gdb.exp | 44 +++++++++++++++++++++++++++++++++------ > 2 files changed, 51 insertions(+), 6 deletions(-) > > diff --git a/gdb/testsuite/README b/gdb/testsuite/README > index c2f659a7188..3a34dcdd154 100644 > --- a/gdb/testsuite/README > +++ b/gdb/testsuite/README > @@ -164,6 +164,19 @@ instance > > checks both the installed GDB and GDBserver. > > +GDB_DATA_DIRECTORY > + > +If you set GDB, then by default the testsuite assumes you are > +exercising an installed GDB, and thus the testsuite lets GDB use its > +configured data directory. Otherwise, if you don't set GDB, then by > +default the tested GDB uses the data directory found under the GDB > +build directory. You can override this by setting GDB_DATA_DIRECTORY. > +For instance: > + > + make check \ > + RUNTESTFLAGS="GDB=/path/to/other/build/gdb \ > + GDB_DATA_DIRECTORY=/path/to/other/build/gdb/data-directory" > + > INTERNAL_GDBFLAGS > > Command line options passed to all GDB invocations. > diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp > index 08726f78563..729bded2950 100644 > --- a/gdb/testsuite/lib/gdb.exp > +++ b/gdb/testsuite/lib/gdb.exp > @@ -92,8 +92,13 @@ load_lib gdb-utils.exp > load_lib memory.exp > load_lib check-test-names.exp > > +# The path to the GDB binary to test. > global GDB > > +# The data directory to use for testing. If this is the empty string, > +# then we let GDB use its own configured data directory. > +global GDB_DATA_DIRECTORY > + > # The spawn ID used for I/O interaction with the inferior. For native > # targets, or remote targets that can do I/O through GDB > # (semi-hosting) this will be the same as the host/GDB's spawn ID. > @@ -111,9 +116,24 @@ if ![info exists GDB] { > } else { > set GDB [transform gdb] > } > +} else { > + # If the user specifies GDB on the command line, and doesn't > + # specify GDB_DATA_DIRECTORY, then assume we're testing an > + # installed GDB, and let it use its own configured data directory. > + if ![info exists GDB_DATA_DIRECTORY] { > + set GDB_DATA_DIRECTORY "" > + } > } > verbose "using GDB = $GDB" 2 > > +# The data directory the testing GDB will use. By default, assume > +# we're testing a non-installed GDB in the build directory. Users may > +# also explictly override the -data-directory from the command line. > +if ![info exists GDB_DATA_DIRECTORY] { > + set GDB_DATA_DIRECTORY "[pwd]/../data-directory" > +} > +verbose "using GDB_DATA_DIRECTORY = $GDB_DATA_DIRECTORY" 2 > + > # GDBFLAGS is available for the user to set on the command line. > # E.g. make check RUNTESTFLAGS=GDBFLAGS=mumble > # Testcases may use it to add additional flags, but they must: > @@ -125,23 +145,35 @@ if ![info exists GDBFLAGS] { > } > verbose "using GDBFLAGS = $GDBFLAGS" 2 > > -# Make the build data directory available to tests. > -set BUILD_DATA_DIRECTORY "[pwd]/../data-directory" > +# Append the -data-directory option to pass to GDB to CMDLINE and > +# return the resulting string. If GDB_DATA_DIRECTORY is empty, > +# nothing is appended. > +proc append_gdb_data_directory_option {cmdline} { > + global GDB_DATA_DIRECTORY > + > + if { $GDB_DATA_DIRECTORY != "" } { > + return "$cmdline -data-directory $GDB_DATA_DIRECTORY" > + } else { > + return $cmdline > + } > +} > > # INTERNAL_GDBFLAGS contains flags that the testsuite requires. > # `-nw' disables any of the windowed interfaces. > # `-nx' disables ~/.gdbinit, so that it doesn't interfere with the tests. > -# `-data-directory' points to the data directory in the build directory. > # `-iex "set {height,width} 0"' disables pagination. > +# `-data-directory' points to the data directory, usually in the build > +# directory. > global INTERNAL_GDBFLAGS > if ![info exists INTERNAL_GDBFLAGS] { > set INTERNAL_GDBFLAGS \ > [join [list \ > "-nw" \ > "-nx" \ > - "-data-directory $BUILD_DATA_DIRECTORY" \ > {-iex "set height 0"} \ > {-iex "set width 0"}]] > + > + set INTERNAL_GDBFLAGS [append_gdb_data_directory_option $INTERNAL_GDBFLAGS] > } > > # The variable gdb_prompt is a regexp which matches the gdb prompt. > @@ -8021,9 +8053,9 @@ proc verify_psymtab_expanded { filename readin } { > # string (the default) means .gdb_index; "-dwarf-5" means .debug_names. > > proc add_gdb_index { program {style ""} } { > - global srcdir GDB env BUILD_DATA_DIRECTORY > + global srcdir GDB env > set contrib_dir "$srcdir/../contrib" > - set env(GDB) "$GDB --data-directory=$BUILD_DATA_DIRECTORY" > + set env(GDB) [append_gdb_data_directory_option $GDB] > set result [catch "exec $contrib_dir/gdb-add-index.sh $style $program" output] > if { $result != 0 } { > verbose -log "result is $result" > > base-commit: 0a30596cfad9cd221a81eea984b6fe3fabb20b95 > -- > 2.26.2 >