From mboxrd@z Thu Jan 1 00:00:00 1970 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on inbox.vuxu.org X-Spam-Level: X-Spam-Status: No, score=-3.3 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, MAILING_LIST_MULTI,RCVD_IN_DNSWL_MED autolearn=ham autolearn_force=no version=3.4.4 Received: (qmail 8623 invoked from network); 11 Nov 2022 11:50:13 -0000 Received: from zero.zsh.org (2a02:898:31:0:48:4558:7a:7368) by inbox.vuxu.org with ESMTPUTF8; 11 Nov 2022 11:50:13 -0000 ARC-Seal: i=1; cv=none; a=rsa-sha256; d=zsh.org; s=rsa-20210803; t=1668167413; b=EvXnuVww4wnx42dBsr/fw/0U4rbeGYMpFKjtHge6xi6NkcS7oDl8DnFKmB9u9NkPkb0QtlY0xI sbxNx48IM++LOOWdpOqRPw0xuxNI03Le30QKRM55PS6ngtZXtIMXhbu7gwn4Pkqkz13IKo5zaO nV/PIDopBxS3AJpD4JIFyUiJrFblZFfmIIyGimLfkGVJC4CF3uXwkM7mafESZBfVLG8BkpU2s4 k8nFcSGNIv7F9DhMb8yvo0cs7mGlvzIV9Xgv2YVvt1iUfzgyjdyRNdgD4vdTvkPy4hswdFAeZE aLP+DHWcpFrsXzE/3wf103mTtf7NIPEsAugT4FvfZjCcJQ==; ARC-Authentication-Results: i=1; zsh.org; iprev=pass (out2-smtp.messagingengine.com) smtp.remote-ip=66.111.4.26; dkim=pass header.d=daniel.shahaf.name header.s=fm3 header.a=rsa-sha256; dkim=pass header.d=messagingengine.com header.s=fm1 header.a=rsa-sha256; dmarc=none header.from=daniel.shahaf.name; arc=none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed; d=zsh.org; s=rsa-20210803; t=1668167413; bh=Fm2dGgouisz7WMwSyOyeuASaboCdwFopLluox30IlNs=; h=List-Archive:List-Owner:List-Post:List-Unsubscribe:List-Subscribe:List-Help: List-Id:Sender:In-Reply-To:Content-Transfer-Encoding:Content-Type: MIME-Version:References:Message-ID:Subject:Cc:To:From:Date:DKIM-Signature: DKIM-Signature:DKIM-Signature; b=PGFljwIpPKn9/Y1HHkq0yVlhHR9mMKhE7hmHrNcn/H+XR1VHfAlrmXBrwHcccG7Vqv5uiCImpG 1mg7va7sRRJogeXDU89JIuBh1hOGMfE16aQc3f1kuMxvEecqrZPR7rKCc8RfWEvZxwKag/9BB0 6XBP7wKOYlzCMDxzMK0gKG0qRk7z0hiRikQx7jP2aEzrBVB7gdCSNLtZ6nq0kw2BfMPapGtBuc qprFxLb6Zh+18Sv9j3leCFgLw+KpkbcKkeboSQ2x3+pWWy92ZJfoZ2zJYJyiDrY5JR1YRNmLTz H4Sv7G70YvjUFjPxh9ycPTlmn5gmCVCw8Rh3UkJYtKtdCw==; DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=zsh.org; s=rsa-20210803; h=List-Archive:List-Owner:List-Post:List-Unsubscribe: List-Subscribe:List-Help:List-Id:Sender:In-Reply-To:Content-Transfer-Encoding :Content-Type:MIME-Version:References:Message-ID:Subject:Cc:To:From:Date: Reply-To:Content-ID:Content-Description:Resent-Date:Resent-From:Resent-Sender :Resent-To:Resent-Cc:Resent-Message-ID; bh=B/89gIQrgJtx8RFs8bw3LSQcvzZKcz1sDD+Bo1RZ5fA=; b=OTCJMO7fU+BFLiHcacKeGqf+eS nJ2Pdguiu4nE+0opEu1/giG3Ju4JPRbFc1T9R7fHw8rmlV6Vuqo1P6O3hizPJRB/RVlq9gJ43a/dp 60XD7og/Cfqpna35dNR3hMMVCjHW6Kc2PONFxOoEuYkWVABh9PFpQUlYcb6ogi8FBjSCCdCJiGzpM cnQKEjAPB0MsMU1m1UDLLO5+7vt/rfOXjkTJylkJgKDzsk7xoU7hA8VrhnjoiYvGwl+NtAqJpvzsZ K7mWuEqcLmmN81twa5bhIUTlIu4Q54xeR4iaOSoCv/LMm3pmZ5zapV0c+vdQXSHti0XWk9y6lNFQI IXMEU4Ug==; Received: by zero.zsh.org with local id 1otSY0-000IWE-HC; Fri, 11 Nov 2022 11:50:12 +0000 Authentication-Results: zsh.org; iprev=pass (out2-smtp.messagingengine.com) smtp.remote-ip=66.111.4.26; dkim=pass header.d=daniel.shahaf.name header.s=fm3 header.a=rsa-sha256; dkim=pass header.d=messagingengine.com header.s=fm1 header.a=rsa-sha256; dmarc=none header.from=daniel.shahaf.name; arc=none Received: from out2-smtp.messagingengine.com ([66.111.4.26]:51227) by zero.zsh.org with esmtps (TLS1.3:TLS_AES_256_GCM_SHA384:256) id 1otSXO-000IAH-Pe; Fri, 11 Nov 2022 11:49:36 +0000 Received: from compute4.internal (compute4.nyi.internal [10.202.2.44]) by mailout.nyi.internal (Postfix) with ESMTP id 60ED65C026B; Fri, 11 Nov 2022 06:49:33 -0500 (EST) Received: from mailfrontend2 ([10.202.2.163]) by compute4.internal (MEProxy); Fri, 11 Nov 2022 06:49:33 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= daniel.shahaf.name; h=cc:cc:content-transfer-encoding :content-type:date:date:from:from:in-reply-to:in-reply-to :message-id:mime-version:references:reply-to:sender:subject :subject:to:to; s=fm3; t=1668167373; x=1668253773; bh=B/89gIQrgJ tx8RFs8bw3LSQcvzZKcz1sDD+Bo1RZ5fA=; b=a3fBdHwi8fC6mZm5//SHV75Z9i HTSFx2S6QmfGcMppW5MedAMGof3fK3rcNjHKxbMXdIU8zyL975Vnbp6vfkIliCOC eRs+i4f7iWJr0x3UUOIh71lkbPsGLcytNgxElZB0HEX46tW99dvwa+yVQxcXsrUM NcwBI21mwt/4nNFqstaeevMHWH0wsU410+J3gxG9YYtBzbGrCXwfJhYC/qgDm2r1 vePMBuQq4SCl/PaNbrAW1C3+s5Pg3x+idaI2SaXr5O6N9fXVIlBZH2NqnGq5tvEp 1LA82iWmsFWhSx54HLL3bwSVp2iKkJTAiNAFt9Vfu2sI5rb8Rybd1iKnglkg== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-transfer-encoding :content-type:date:date:feedback-id:feedback-id:from:from :in-reply-to:in-reply-to:message-id:mime-version:references :reply-to:sender:subject:subject:to:to:x-me-proxy:x-me-proxy :x-me-sender:x-me-sender:x-sasl-enc; s=fm1; t=1668167373; x= 1668253773; bh=B/89gIQrgJtx8RFs8bw3LSQcvzZKcz1sDD+Bo1RZ5fA=; b=J 0KYUnBeesBwSDCLlnIPAgY26Cx1SbafzowqVzizvMbtHp/8Ybp0JaIru7BE4jkIF nIfNzXI7+uMk88ffbjR5W6Yl9ip97vN0ALdHSYGteVLIpvFgFxQmL4o5KRs0xSBY RMvm37EaQUgOZ6c+nY1evfCeXtyTA9WVpF1zJzLKdhysPReYBZIkcY6f4SqN3oYz VmiffXtFQFjIrUuyLJfcf8dDkpJnwlRe1908fD3Wo+xm8EHoKEyd1EBm8ozd+SHm y3vM4qt3nLFrnhHpDoKQ50ER3GsYhs3Bp6ab3GGJQigEaUbC++Qe8eBVBMQl3RbM ipT+jg4HCxLL1smOgSO/g== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvgedrfeeigdefvdcutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu uegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenuc fjughrpeffhffvvefukfhfgggtugfgjggfsehtkedttddtreejnecuhfhrohhmpeffrghn ihgvlhcuufhhrghhrghfuceougdrshesuggrnhhivghlrdhshhgrhhgrfhdrnhgrmhgvqe enucggtffrrghtthgvrhhnpeelteetiedvvdfhfeefuddvleeufeehgeefieelteegleeu ieejgfefjedvffffkeenucffohhmrghinheprhgvphholhhoghihrdhorhhgnecuvehluh hsthgvrhfuihiivgeptdenucfrrghrrghmpehmrghilhhfrhhomhepugdrshesuggrnhhi vghlrdhshhgrhhgrfhdrnhgrmhgv X-ME-Proxy: Feedback-ID: i425e4195:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Fri, 11 Nov 2022 06:49:32 -0500 (EST) Received: by tarpaulin.shahaf.local2 (Postfix, from userid 1000) id 4N7xnh6zK3z3YH; Fri, 11 Nov 2022 11:49:28 +0000 (UTC) Date: Fri, 11 Nov 2022 11:49:28 +0000 From: Daniel Shahaf To: Peter Grayson Cc: zsh-workers@zsh.org Subject: Re: [PATCH] Remove StGit patch detection from vcs_info Message-ID: <20221111114928.GF27622@tarpaulin.shahaf.local2> References: <20221101030429.38029-1-pete@jpgrayson.net> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20221101030429.38029-1-pete@jpgrayson.net> User-Agent: Mutt/1.10.1 (2018-07-13) X-Seq: 50940 Archived-At: X-Loop: zsh-workers@zsh.org Errors-To: zsh-workers-owner@zsh.org Precedence: list Precedence: bulk Sender: zsh-workers-request@zsh.org X-no-archive: yes List-Id: List-Help: , List-Subscribe: , List-Unsubscribe: , List-Post: List-Owner: List-Archive: Sorry for my long RTT on this. Peter Grayson wrote on Mon, Oct 31, 2022 at 23:04:30 -0400: > The vcs_info patch detection code attempted to interrogate StGit patch > stack state by inspecting .git/patches/applied and > .git/patches/unapplied. As of StGit 1.0, StGit stack and patch state is > no longer maintained via files in the .git/ directory, but is instead > captured in the repo's object database, accessible via the > refs/stacks/ reference. > > Thus zsh's approach for interrogating StGit patch state is long > obsoleted. > *nod* > This patch excises the code that attempts to interrogate StGit stack > state. It would alternatively be possible to repair this code to > interrogate the StGit stack state in a different manner, but: > Hang on. This isn't a binary decision, "remove the code or update it to support StGit 2.x". This is actually two independent decisions: 1. Whether to keep or remove the StGit 0.x support code. 2. Whether patches adding StGit 2.x support code would be considered. > - It is not clear that StGit is a sufficiently popular tool to warrant > direct inclusion in zsh. [re #1]: The code is already written (i.e., a sunk cost). The ongoing costs for vcs_info users who don't use StGit, for vcs_info users who use stg≥1.x, and for vcs_info maintainers are minimal. Conversely, /removing/ the code might break some users' proverbial toaster heating — e.g., users who self-compile latest zsh on LTS distros, and users on current distros that still ship stgit 0.x . We can document (in comments, for users, or both) that the version of StGit vcs_info supports is EOL. [re #2]: - I expect implementing StGit 2.x support would be a nearly one-time cost with long-time returns (especially as StGit 3.x isn't on the horizon). Low popularity over a long time is still a fair justification. - Adding StGit support to vcs_info might cause some zsh users to discover and begin to use StGit. (That's not a reason for including StGit support; that's just a cost/benefit analysis of implementing StGit 2.x support. The number of people who use both vcs_info and StGit relates to the denominator.) - StGit support can be written in such a way that the effect on zsh users who don't use StGit would be minimal. - Maintainers who advise against their own tools are generally exactly those we'd like to work with :) > - Interrogating the StGit stack needs to be done using StGit's > proscribed interface, the stg executable, and not by interrogating ("prescribed"?) > either the .git filesystem nor by inspecting the git object database > with the git executable. [re #1]: Considering there are unlikely to be any further stg-0.x releases, and that newer stg's internals are different, the layering violation in VCS_INFO_get_data_git — though admittedly not ideal — seems unlikely to cause any breakage down the road. [Famous last words, yes.] [re #2]: Fair enough. VCS_INFO_get_data_git does tend to poke around in .git dirs directly — partly for performance, partly because when the code was written there mightn't have been APIs for what we needed — but agreed that new code should use APIs if possible. > - StGit versions prior to 2.0 are implemented in Python and thus have an > unacceptable runtime overhead to be included in vcs_info (on the order > of hundreds of milliseconds). [re both stg-0.x and stg-1.x]: And why would that be unacceptable? A few hundreds of milliseconds — let's round up and say a microsecond — is short enough that "Press on an empty prompt and get a new one instantly" would still be the user's experience, wouldn't it? > - StGit 2.0 is implemented in Rust is considerably faster (a few tens of > milliseconds to interrogate stack state), but any non-zero overhead to > vcs_info still seems like too much given the value of this patch > information and the relative non-popularity of StGit. [re #2]: If the overhead is more than we consider acceptable, couldn't this be addressed by implementing StGit 2.x support but making it off by default (require an opt-in)? Compare the «check-for-staged-changes» and «get-unapplied» styles, and contrast «use-simple» and «enable». For reference, the stg-0.x support code performs one stat(2) for people who don't use StGit, v. three stat(2)s and two cat(1)s for those who do, and seemed to work well enough when I last tested it (on a toy example). > - Just removing the code is the lowest-risk approach for the zsh code > base. [re #1]: I would hesitate to remove that code. It supports a now-EOL version, true, but it may still have users, and induces little cost for other users and for vcs_info maintainers. WDYT? [re #2]: It sounds like StGit 2.x support can be implemented at the cost of one fork(2) for those who don't use StGit and under a microsecond for those who do. That doesn't sound like a deal breaker at all. Furthermore, will StGit 2.x users want to show in the prompt information about the patch stack? If so, I'd say that's an argument in favour of including StGit 2.x support in vcs_info, since vcs_info is designed to allow this. (Also, code — say, applied-string hooks — can be reused across stg(1) and other kinds of patch series.) Your points — cost/benefit, forward compatibility, performancy, etc. — are valid and, should a patch implementing support for stg-2.x be (UK)tabled, would be good to keep in mind during review. I'd welcome your review of any such particular patch, too, should you have the time and inclination. However, I don't see any obvious roadblock to implementing StGit 2.x support in zsh. Thanks for the willingness (elsethread) to relicence, and Congrats on the 2.0 release :) Cheers, Daniel > Signed-off-by: Peter Grayson > --- > Functions/VCS_Info/Backends/VCS_INFO_get_data_git | 11 ++--------- > 1 file changed, 2 insertions(+), 9 deletions(-) > > diff --git a/Functions/VCS_Info/Backends/VCS_INFO_get_data_git b/Functions/VCS_Info/Backends/VCS_INFO_get_data_git > index e45eebc8e..a3f4dbdf0 100644 > --- a/Functions/VCS_Info/Backends/VCS_INFO_get_data_git > +++ b/Functions/VCS_Info/Backends/VCS_INFO_get_data_git > @@ -184,15 +184,8 @@ fi > VCS_INFO_adjust > VCS_INFO_git_getaction ${gitdir} > > -local patchdir=${gitdir}/patches/${gitbranch} > -if [[ -d $patchdir ]] && [[ -f $patchdir/applied ]] \ > - && [[ -f $patchdir/unapplied ]] > -then > - # stgit > - git_patches_applied=(${(f)"$(< "${patchdir}/applied")"}) > - git_patches_unapplied=(${(f)"$(< "${patchdir}/unapplied")"}) > - VCS_INFO_git_handle_patches > -elif [[ -d "${gitdir}/rebase-merge" ]]; then > +local patchdir > +if [[ -d "${gitdir}/rebase-merge" ]]; then > # 'git rebase -i' > patchdir="${gitdir}/rebase-merge" > local p > -- > 2.38.1 > >