From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from NAM12-MW2-obe.outbound.protection.outlook.com (mail-mw2nam12on2047.outbound.protection.outlook.com [40.107.244.47]) by sourceware.org (Postfix) with ESMTPS id 9D6BE3858409 for ; Mon, 24 Jan 2022 12:59:42 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 9D6BE3858409 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=bwOADveABudWFymJHXX1FjhNbs6G5016KJYMijwyRy9wm1IAbMWr4XMoSMf288g2+nSg03PzHrm+l+Toj1gGmmcLrFT9v2Xa1pldRVnUPiQCPUMPHJicVIYIjiGpRNyG6tIkv2lb2brkY3SZUXgXi+5BygrcifX5TlI+Z1DsLrP7wALmtNBXuAQTEHa48HIkqqideV7qzsqWzBSTBhtkrjPlKOJdHmc5n0XcedYU5WcmtlsRI7cciGSD1zLBRYq9zFkAhPArlz17ie9R0XFIIbqXKkycvOTp5ld8Hn+P/cf0+3QmAVkpoHHNsRDtjsLRYLU+Dl6mxe33UuGaJ1mU6g== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=tx7j1woN9Fyf6u6hT9WzTRHzHxjfv7+rI3vSIq/wvo4=; b=TWxTk78NeOZXtwWT95W35TKrNdzkrvlCg91BexcjAagqXH+C5VGDHTIqsd+/WsMgpySXvFAqsqVqLsn4JUJOYl4CZPp4hbDjFtyl2SI6p+N8rlCnMjuUq274dcr//NBN9KYu7LwnG1lfO+0kPQq2FvY71feWmGvrfzZZPwQh/kWZ31/joUuPUMT8iiJDkoTvm8k/RBjhenA9jiOPk4f4sIbl+ovdV33SK57NiS0RGGfWUg9dfeJUaCvKE48Hm42NeJb7sEdMkYP5Ss47xiS79/rpvHFGXeoxC8NQk1ngcAhBSTwzcuOTX11CCThGR4FR4kYTQ1HonmlhQyFeSgUqkA== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=none; dmarc=none; dkim=none; arc=none Received: from DM6PR17MB3113.namprd17.prod.outlook.com (2603:10b6:5:6::10) by DM6PR17MB2396.namprd17.prod.outlook.com (2603:10b6:5:71::32) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4909.17; Mon, 24 Jan 2022 12:59:39 +0000 Received: from DM6PR17MB3113.namprd17.prod.outlook.com ([fe80::f544:fbc7:cdfb:3017]) by DM6PR17MB3113.namprd17.prod.outlook.com ([fe80::f544:fbc7:cdfb:3017%5]) with mapi id 15.20.4909.017; Mon, 24 Jan 2022 12:59:39 +0000 Message-ID: <76d0cb881222ca2b875aa4d13dd49b58a766171d.camel@labware.com> Subject: Re: [PATCH 0/5] create GDB/MI commands using python From: Jan Vrany To: Andrew Burgess Cc: gdb-patches@sourceware.org Date: Mon, 24 Jan 2022 12:59:34 +0000 In-Reply-To: <20220121152208.GK622389@redhat.com> References: <20220117124425.2658516-1-jan.vrany@labware.com> <20220118135541.GG622389@redhat.com> <20220121152208.GK622389@redhat.com> Content-Type: text/plain; charset="UTF-8" User-Agent: Evolution 3.38.3-1 Content-Transfer-Encoding: 8bit X-ClientProxiedBy: LO4P123CA0360.GBRP123.PROD.OUTLOOK.COM (2603:10a6:600:18d::23) To DM6PR17MB3113.namprd17.prod.outlook.com (2603:10b6:5:6::10) MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: 519eddda-db71-4048-17ef-08d9df396149 X-MS-TrafficTypeDiagnostic: DM6PR17MB2396:EE_ X-Microsoft-Antispam-PRVS: X-MS-Oob-TLC-OOBClassifiers: OLM:9508; X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: 5bTbCFu+H1I4dUBSNMYzih5I5+7/83e1DIyFmvFL3yCE4a4b0rnBPfhDzCc/XYyymfB97+S4dfxEPTI0CAmgJs/Lj2ojzbjWsMRjCri5fQkpm1FTPYO86I4rZsjsUB/icXgrtsa4gn5UpVTlodilO0xdgyE2sm0b8H2nvV4aSeHwcukc4ktVVnO1cOCmiEHiquHmiF2IwbdcnlUom7kgv1rwnyu64zkB6CjGFcJUUG5s0xbt6/YKUbaOyY1CHoTVvg6MpoOuDfW0VS0TuBPPQeSqJtdBrK6M7N547gdCXVEOkBoGpBSN7spMjBBqvytWucFkYtP93RnI62eh8YlLyyoWgDjULaDL0xas0UcaJ1CSl20+hha5Fzhqk0vys0Xd0OB/RRgTSmo7i65kPxPEUAO2vmDSRTnui9VKfXDKkl/TzUx1W5WFFC8CgxuI5Wjiv8yVuvn3dOylywmNPrcIouySXp4CVZaxbpaCaU655XsqIsJ1S4poWZKufjcZnEvYhCo5vc9Mc0It0hcm/8c8Ij4fd9Amu6EsotnZ+1m+KmXwqb9jhVftQ0PLB01VbueGB0F81Hne4MVe92KEjt8QXUMnx7DqDmkkphsBADFLT9yzRHoMRFBHxLjjUB67uZ/1KyP79er2IUNbR7VmFOlMnMd2MY9kwnvcy5kQHCDPo4BGM0ouaubIcN3g8CfanucWmFhNC3tC4jniIHl6oyCKEjWRgz+91U2DEXB97f42lPmjlLrc5cCKZLcyH2f23XUBBimgSQTA/skicdjC9nDtkA== X-Forefront-Antispam-Report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:DM6PR17MB3113.namprd17.prod.outlook.com; PTR:; CAT:NONE; SFS:(4636009)(366004)(86362001)(316002)(6512007)(6486002)(4326008)(8936002)(38350700002)(66476007)(66556008)(66946007)(186003)(6916009)(8676002)(36756003)(44832011)(83380400001)(2906002)(2616005)(52116002)(26005)(5660300002)(508600001)(6506007)(84970400001)(38100700002)(966005)(6666004); DIR:OUT; SFP:1101; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?czhmcWxRQytsVE94OGdOTWM5MHIvSlpmWlM1dzZhSjkwZGNOZ3Q2MDJXRnFq?= =?utf-8?B?aElSSGJmbWhGemFkWjFOajVEOXQwcVQ4MlY5TFAveGNXVW4xT3JYMG1ldFhV?= =?utf-8?B?TnVNUm9MVUZBbXU3N1RENWJZKzBPQXJ5V0kwQThvcDdJL1NOUFFMdExkdTBH?= =?utf-8?B?QmxGTEIxQkdHek1nZzZrSCtUM0JtazBYaHJjamdJOXZPYUhqQzBKZTh5akZL?= =?utf-8?B?OVhOZmlncTBrVmpyeU5YL0FPellDaC9kN21JVnZibldCcGZHaUkzT0RtOG5T?= =?utf-8?B?MFNESXRrMHY4ZnREZjVNQXlOWlhJZFVWZUlpZWdRUHJ0Z1V2K0o4T1dya1RJ?= =?utf-8?B?aE9rTi94VzZ1QjlBTk5VMzJ1eHl0S0hVQ2FraVRSR1MwMjZpOFZuajNnWHAr?= =?utf-8?B?WVIwSHBIWXhqMjBPeGF0SGQ4MkxhS1RYWWYwVHdTL1lCWHgyTExzM0JBOXdK?= =?utf-8?B?WEVFTnFydXdCa1FxOWtMZitYczlUWm5ETE9ldGJxQ1dRTThkakVzSkNmMm8x?= =?utf-8?B?WktIY3NaR0FDR2EvMHJFMGtBNWJneVdrMjdCNGtxdVFwcUNQbTlCWkRlK2s3?= =?utf-8?B?NGVvUWMzdGIvL0hoWlFadTVGMC9pdUV0K2N3d1Q3bk1IVGpUMEprTFdXOVVG?= =?utf-8?B?UjNyZW4vNlpSd0tNTUI5YWtWMHZLSkVIenZnQldpWkMvWk9oOTVXcExpcnlJ?= =?utf-8?B?cy9mTHNOZDZDT3gwbCtiNWVWaVhHNllYSmJvRy9oN1hsTkpianJJMUllM3dt?= =?utf-8?B?WkFNWEEwRUsraS9wNXYyMEJybEdaR2RDYS9PQXdFOEZiSEJ2dDNaVWtBWG56?= =?utf-8?B?VDlhUEJ5dGpqcUxxOHMydjl5ci85MkEvODZPSDQyN0lOMjQ5ZEVHczVBSWR5?= =?utf-8?B?MWU4N2FCYmVhYmMzcGo0TGJmcENLVk01aWZDOUVvMFdOTW9hb1ZqQTY5L3NH?= =?utf-8?B?c1VsRmJkWWFTTUxhSG9aV0RDVGc5OStGQmM4b090VUtWQ2lsZ2U5SEFwb0Ja?= =?utf-8?B?eTZHaFpKS2x6ci9ldTVoRGtGOC8rdGhrYjdBRkhGNGEzNFlDUVpEbnljMjRY?= =?utf-8?B?UTk4UmQ5Qm9DL2JQWkxzY2Jjd2JVL2xvWGtCRGhCZmlCNVREOUtxZUlxZy9E?= =?utf-8?B?YklCQ0EyWFVHL3h0ZWRnS1NESGN1MDFiOHlueWVuK0g2VFowblVmZTJXOWEw?= =?utf-8?B?S2plbVZjczZDUUtDYTlPd0xDbW82ZUk0QU5kWGJ2VDg3WTRUTHVYNFdjaTA5?= =?utf-8?B?T05UeWRGaFJwWDZFTlpIZXRjQWV6eHc4bXk3UUxWMm4rcC8xdTdKMFMzRWtP?= =?utf-8?B?MWlxRjNNL1VHV0QzNDl1RFh2U1FsRy9FeGl1c3M5cFd4ZWdWUkRyUFpJT0xJ?= =?utf-8?B?aTRFUTBoVXZhWkFQTmxDelF5VWVuMTlvOFRCbS8rT1NXVzl5QlRDR1JabTJL?= =?utf-8?B?Q2NvaGtQUW02SDdvdjZYdjF4U21FQTdjQVJhZ0hWQTczcTYyeXZFWk5qRGlR?= =?utf-8?B?OGJTMUFzTVhrY1Bjd1puOXduOEpyNXl4RmJTZXA2Yi9lRitGTDd4bzZGSlJS?= =?utf-8?B?UGlqakoycVQxeFdPb2ovR0RyOW9Dd2FlSnkveWpxUEJqNGRuMGFqV1hVOGgv?= =?utf-8?B?L1Q4Q29tSTVHNWJKeklJalNia0hkWjRidjU5dnJYcTBCUGZ2c3ZQVFd3U2Vy?= =?utf-8?B?bE9GSDRoYVFvakRueXVYWUpXZzNPTjA1Sm93bmxpcHprN3QvRk0wcEVqWTNz?= =?utf-8?B?RHIyRHNPZzg1MTMyK0lyakdVWVFsVUFXSmFJQXo3TkNaQ01nMVdxV2phT0pW?= =?utf-8?B?Yktqd1p1VkZaRENMTDllTEVWY0thcERFTjlxK1VHTytiOWxSakhtUGN4NG53?= =?utf-8?B?WU9oNjUyZjl6cUwzM1ZiRnZNbCtRNWpRTWRpTHNqNGpZamhMdE9oWUppTzE4?= =?utf-8?B?dmZONTlabE53VGdzdmo2WmRpT1BWQ282VEFuNEN4ck9nRlB4QWZ3R2JvMW82?= =?utf-8?B?bXNZNkl2YmsrbGRHTEE2Q21MdjQyZjBKTXR2MEgrNnRrZlp6TVovcmx0WVQr?= =?utf-8?B?cSsxcGtCUXU1dmdTNUJZTnhaaWppU204ZHBKMEFFVzB1YjM3bzl5K0Y2ZkVZ?= =?utf-8?B?WEFBVmh3cktEdDNyaHF6dmNwcTB1TUVLVEhQeitUOEdBb3ZNNEtSSkpCb0lx?= =?utf-8?Q?t5J446btwe4iI0xx6qrFk0g=3D?= X-OriginatorOrg: labware.com X-MS-Exchange-CrossTenant-Network-Message-Id: 519eddda-db71-4048-17ef-08d9df396149 X-MS-Exchange-CrossTenant-AuthSource: DM6PR17MB3113.namprd17.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 24 Jan 2022 12:59:39.2008 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: b5db0322-1aa0-4c0a-859c-ad0f96966f4c X-MS-Exchange-CrossTenant-MailboxType: HOSTED X-MS-Exchange-CrossTenant-UserPrincipalName: 5u/usgIi6e7iHbAe4cURokxfK50uL4DShCYG5t0BR9qHlCzjbgmTxjJScG6+mmAhzLe+YwP5y7qBsoOS0o7SdA== X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR17MB2396 X-Spam-Status: No, score=-4.4 required=5.0 tests=BAYES_00, BODY_8BITS, DKIM_SIGNED, DKIM_VALID, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, SPF_HELO_PASS, SPF_PASS, TXREP 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: Mon, 24 Jan 2022 12:59:45 -0000 On Fri, 2022-01-21 at 15:22 +0000, Andrew Burgess wrote: > * Jan Vrany via Gdb-patches [2022-01-18 15:13:01 +0000]: > > > On Tue, 2022-01-18 at 13:55 +0000, Andrew Burgess wrote: > > > * Jan Vrany via Gdb-patches [2022-01-17 12:44:20 +0000]: > > > > > > > This is a restart of an earlier attempts to allow custom > > > > GDB/MI commands written in Python. > > > > > > Thanks for continuing to work on this feature. > > > > > > I too had been looking at getting the remaining patches from your > > > series upstream, and I'd like to discuss some of the differences > > > between the approaches we took. > > > > Great, thanks a lot! > > > > > > > > At the end of this mail you'll find my current work-in-progress > > > patch, it definitely needs the docs and NEWS entries adding, as well > > > as a few extra tests. However, functionality wise I think its mostly > > > there. > > > > > > My patch includes two sets of tests, gdb.python/py-mi-cmd.exp and > > > gdb.python/py-mi-cmd-orig.exp. The former is based on your tests, but > > > with some tweaks based on changes I made. The latter set is your > > > tests taken from this m/l thread. > > > > > > When running the gdb.python/py-mi-cmd-orig.exp tests there should be > > > just 2 failures, both related to the same feature which is not present > > > in my version, that is, the ability of a command to redefine itself, > > > like this: > > > > > >       class my_command(gdb.MICommand): > > >         def invoke(self, args): > > >           my_command("-blah") > > >           return None > > > > > >        my_command("-blah") > > > > > > this works with your version, but not with mine, this is because I'm > > > using python's own reference counting to track when a command can be > > > redefined or not, and, when you are within a commands invoke method > > > the reference count of the containing object is incremented, and this > > > prevents gdb from deleting the command. > > > > > > My question then, is how important is this feature, and what use case > > > do you see for this? Or, was support for this just a happy side > > > effect of the implementation approach you chose? > > > > Initially I did not think of it and it was - IIRC - pointed out in > > some review. I thought to be a corner case, but it turned out to be > > (potentiality very useful feature. > > > > While developing custom commands, pretty printers and so on, it is useful > > to be able to "reload" Python code into running GDB without need to restart  > > it. To support that, I created a "pr" command which reloads all custom python > > code (well, tries to at least), see > > > > https://urldefense.proofpoint.com/v2/url?u=https-3A__swing.fit.cvut.cz_hg_jv-2Dvdb_file_tip_python_vdb_cli.py-23l20&d=DwIFAw&c=sPZ6DeHLiehUHQWKIrsNwWp3t7snrE-az24ztT0w7Jc&r=WpFFGgYa98Yp-c29WHTCwU1wAGFBvszA6a4RzgpMSqc&m=aunQh9rOF7oPv3b6WEJbNxADRSv4-8Dy2BMsx38ToFU&s=jnwDUKw3EN3ruyHZoFfwtAJykINwXr92bHBvZHlvn1k&e= > > > > So ultimately, this custom "pr" command's invoke() causes redefinition > > of the "pr" command itself. I have not done it yet, but having  > > menu item/icon in (my) GDB frontend using something like -python-reload` > > seems desirable from UX POV. > > Thanks, that's not an unreasonable use case. I've tweaked things so > that this case is now supported. > > I've gone through that patch and ported over your NEWS and doc > changes, with some updates based on Eli's feedback, as well as some > new content to cover some of the changes in my patch. > > For the testing, I have, for now, kept gdb.python/py-mi-cmd-orig.exp, > which is your test file taken from your patch series. I added some > 'setup_kfail' markers to 7 tests that now fail due to changes in error > strings, hopefully, this will allow you to review what changed. > However, except for the differences in error string, everything your > original series supported is now supported by this patch. > > From a user's point of view, I think the only differences with your > patch are: > >  1. New .name attribute, that returns the name of the command as a >     string, here's an example session: > >     (gdb) python >     >class MyCommand(gdb.MICommand): >     > def __init__(self): >     > super(MyCommand, self).__init__("-my-command") >     > def invoke(self, args): >     > return None >     > >     >end >     (gdb) python cmd = MyCommand() >     (gdb) python print(cmd.name) >     -my-command >     (gdb) > >  2. New .installed attribute, that allows a command to be installed or >     removed from the mi command table, here's an example session: > >     (gdb) python >     >class MyCommand(gdb.MICommand): >     > def __init__(self, name, message): >     > self._message = message >     > super(MyCommand, self).__init__(name) >     > >     > def invoke(self, args): >     > return self._message >     > >     >end >     (gdb) python cmd1 = MyCommand("-my-command", "cmd1") >     (gdb) python print(cmd1.installed) >     True >     (gdb) interpreter-exec mi "-my-command" >     ^done,result="cmd1" >     (gdb) python cmd2 = MyCommand("-my-command", "cmd2") >     (gdb) python print(cmd2.installed) >     True >     (gdb) python print(cmd1.installed) >     False >     (gdb) interpreter-exec mi "-my-command" >     ^done,result="cmd2" >     (gdb) python cmd1.installed = True >     (gdb) python print(cmd2.installed) >     False >     (gdb) python print(cmd1.installed) >     True >     (gdb) interpreter-exec mi "-my-command" >     ^done,result="cmd1" >     (gdb) python cmd1.installed = False >     (gdb) interpreter-exec mi "-my-command" >     ^error,msg="Undefined MI command: my-command",code="undefined-command" >     (gdb) > >  3. The top level result name can be changed from 'result' to anything >     the user wants, here's an example session: > >     (gdb) python >     >class MyCommand(gdb.MICommand): >     > def __init__(self): >     > super(MyCommand, self).__init__("-my-command", "greeting") >     > def invoke(self, args): >     > return "Hello World" >     > >     >end >     (gdb) python cmd = MyCommand() >     (gdb) interpreter-exec mi "-my-command" >     ^done,greeting="Hello World" >     (gdb) > > I'd love to hear your feedback on this iteration. I'm personally happy with this iteration. I like your improvements  to the Python API! I also tested it briefly with my Python code &  frontend just to see and it worked just fine. Perhaps, I'd rename mi_commands module variable to _mi_commands to  convey the fact it is "internal" to the implementation and should not be touched (?). Thanks! Jan