From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 58506 invoked by alias); 6 Sep 2017 18:18:00 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 58494 invoked by uid 89); 6 Sep 2017 18:17:59 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=worrying, tackling X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 06 Sep 2017 18:17:54 +0000 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 24A8D285B2; Wed, 6 Sep 2017 18:17:53 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 24A8D285B2 Authentication-Results: ext-mx06.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx06.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=sergiodj@redhat.com Received: from localhost (unused-10-15-17-193.yyz.redhat.com [10.15.17.193]) by smtp.corp.redhat.com (Postfix) with ESMTPS id DD0E25D6A0; Wed, 6 Sep 2017 18:17:52 +0000 (UTC) From: Sergio Durigan Junior To: Pedro Alves Cc: GDB Patches , Eli Zaretskii Subject: Re: [PATCH/RFC] Implement the ability to set the current working directory in GDBserver References: <20170830043811.776-1-sergiodj@redhat.com> <79779c39-8f54-c5da-5450-e67a35294e08@redhat.com> <87shg1yr78.fsf@redhat.com> <745cb8c5-f203-e16d-3e36-81005d82a42f@redhat.com> Date: Wed, 06 Sep 2017 18:18:00 -0000 In-Reply-To: <745cb8c5-f203-e16d-3e36-81005d82a42f@redhat.com> (Pedro Alves's message of "Wed, 6 Sep 2017 15:20:45 +0100") Message-ID: <8760cvzo6n.fsf@redhat.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.2 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-IsSubscribed: yes X-SW-Source: 2017-09/txt/msg00160.txt.bz2 On Wednesday, September 06 2017, Pedro Alves wrote: > On 09/05/2017 06:45 PM, Sergio Durigan Junior wrote: >> On Thursday, August 31 2017, Pedro Alves wrote: >> >> As for your questions. I looked at the code to find places where the >> "current_directory" variable was being used. This is the variable that >> ultimately gets changed when "cd" is used. >> >> Aside from impacting the inferior's cwd, current_directory is also used >> on the ".gdb_history" machinery. >> >> tmpenv = getenv ("GDBHISTFILE"); >> if (tmpenv) >> history_filename = xstrdup (tmpenv); >> else if (!history_filename) >> { >> /* We include the current directory so that if the user changes >> directories the file written will be the same as the one >> that was read. */ >> #ifdef __MSDOS__ >> /* No leading dots in file names are allowed on MSDOS. */ >> history_filename = concat (current_directory, "/_gdb_history", >> (char *)NULL); >> #else >> history_filename = concat (current_directory, "/.gdb_history", >> (char *)NULL); >> #endif >> } >> read_history (history_filename); >> >> As John Baldwin also mentioned, 'cd' has an effect when loading GDB >> scripts. And probably has an effect when loading other stuff. >> >> Since gdbserver doesn't really support loading scripts and also doesn't >> use .gdb_history, I don't think they are relevant in this case. > > Well, that's where I disagree -- I think we need to take a step back > and look at the wider picture. > > For example, these settings are per-inferior: > > (gdb) set environment > (gdb) set args > > E.g.,: > > (gdb) inferior 1 > (gdb) show args > "foo" > (gdb) inferior 2 > (gdb) show args > "bar" > (gdb) inferior 1 > (gdb) show args > "foo" > > This allows preparing multiple independent inferior > environments, before starting all inferiors. > > From that perspective, the inferior's current working directory > looks to me exactly the same kind of variable as "args" or > "environment". So from that angle, it'd seem to make sense to > make the current working directory that "cd" affects be a > per-inferior setting. However, that may conflict with the use > cases that expect "cd" to affect where GDB loads scripts > from [a host setting], which seems unrelated to the inferior's > current working directory [which is a target setting and may > resolve to a path in a different machine]. > > To address that, it'd seem natural to add a "set cwd" command > (to go with set args/environment) that would set up a per-inferior > current working directory, and leave "cd" for gdb's own current working > directory, which affects other things like loading scripts. This makes sense to me. I confess I hadn't thought about how the "cd" command is used for other things inside GDB; I was just tackling the problem of the inferior's cwd, as is obvious from my patch. Now I understand why you said my approach was not the best. And I agree: it works when you consider the inferior's cwd only, but it can break other use cases that we obviously want to maintain. > With that approach, if "show cwd" is empty, then the inferior > would inherit gdb's current directory, so it'd end up being > a backward compatible extension. Right, that makes sense. And perhaps we could print a (temporary) warning when "cd" is used, saying that the "right" way to change the inferior's cwd is to use "set cwd". But these are just implementation details. > Making the setting be per-inferior instead of adding a > remote-specific "set remote directory" still addresses > local/remote parity, because the interface/feature ends up > working the same way independently of native vs remote target. Right. > Consider this a strawman proposal, to get the discussion going. > I'm not exactly sure it's the best interface either. OK. This proposal is better than my RFC, and I have the impression that it is the right way forward, even if we have to make adjustments. What I can do right now is prepare a patch to simply implement the new command, without worrying about the integration with gdbserver. > I also wonder whether "set sysroot" should affect this setting > in some way. Maybe. Maybe not. I haven't thought about that. I want to say that "set sysroot" and "set cwd" are two different (though correlated) commands, but I'm not 100% sure. IIUC "set sysroot" is used only for calculating the absolute paths of shared libraries, and mostly in the context of a remote debugging. Currently, it doesn't seem that "set sysroot" affects anything related to the "cd" command, which makes sense to me. But I may be missing some context here. In any case, as a first step of this task I think it makes sense to go ahead and decouple the concept of setting an inferior's cwd from the "cd" command, creating the "set cwd" command, as you proposed. WDYT? -- Sergio GPG key ID: 237A 54B1 0287 28BF 00EF 31F4 D0EB 7628 65FC 5E36 Please send encrypted e-mail if possible http://sergiodj.net/