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 3629 invoked from network); 12 Nov 2022 14:47:36 -0000 Received: from zero.zsh.org (2a02:898:31:0:48:4558:7a:7368) by inbox.vuxu.org with ESMTPUTF8; 12 Nov 2022 14:47:36 -0000 ARC-Seal: i=1; cv=none; a=rsa-sha256; d=zsh.org; s=rsa-20210803; t=1668264456; b=Se212zOJFW1j5jsF8cXb8a92cHhX0Qk7wSMsJUFud0pVWWOjavkzrlD+FmNFDyiRmqMPUUCNC7 +FC/vEuPX9xj7TvNmG90iAnkt54enQAgBQnlJwwXIUGCjI3wNzV3Q2GmlwqYYFBVl99yKU+1A1 thzn8ff3FI+n3kvp8+bc41fyXUZ8/LMnX32aVZBXOjF6gkiSdxb2dn+m200r9+xchi4/KSwR3T 8bdfcQqE2es9cwE/n+sEd6/v81BW4Cmyg9okitwPo4uNE+WgkDWVxE3FTVdw6Ym3+c+Y0NOWWf Kpb1y4sfj1UWVBK6TTnD/d3aPcn56E+vzZ+U+h7mIOw3ng==; ARC-Authentication-Results: i=1; zsh.org; iprev=pass (out5-smtp.messagingengine.com) smtp.remote-ip=66.111.4.29; dkim=pass header.d=jpgrayson.net header.s=fm3 header.a=rsa-sha256; dkim=pass header.d=messagingengine.com header.s=fm1 header.a=rsa-sha256; dmarc=none header.from=jpgrayson.net; arc=none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed; d=zsh.org; s=rsa-20210803; t=1668264456; bh=xQnmSvQIwdm45Q2gFVNblF+vzf98AofrRhJfN6vc2m8=; h=List-Archive:List-Owner:List-Post:List-Unsubscribe:List-Subscribe:List-Help: List-Id:Sender:Content-Transfer-Encoding:Content-Type:Subject:Cc:To:From: Date:References:In-Reply-To:Message-ID:MIME-Version:DKIM-Signature: DKIM-Signature:DKIM-Signature; b=IAIOOnr/deDA8QcVLtom4gB0MyZxLjZRsoYMX9YZft3VOzRzBV/XHg0StnmSHdm6IuC3+JScfA V5a6bOnLp5WSEe6okdYvZgzaB5JGW5tupffQrg3hfDZ0Jh0m0FJZ/KiU6bQwyMfepJqu+Ah2Ic iLXywu1vGotOEq2XLB6WFS6Mb0v2929/k6Dr9s0+Gtsyf56uS7Exp2SHBZLfYOzQ/ogCj3Ki9c WF1tEzWEx7onTTMDj+aYIPCnPY3BTOm2KrYI92s5Wu7zJntIRmhD0HUgtX7EiUbfxBrzFqANJI Ik8pEYOReTSIps2V6rmBrkNCn8TGTQGnSX1X0sSsvtypxQ==; 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:Content-Transfer-Encoding: Content-Type:Subject:Cc:To:From:Date:References:In-Reply-To:Message-Id: Mime-Version:Reply-To:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID; bh=90t5PISWEGq0hqBoZwpvFrOJuqPfaVqNVF67UbYUp3A=; b=guB1ddXhIBmTKBzu9lsQV71HFx zBUlqj/UTj7vpLmw5O3FDku/vr8hGUeZVTDM/Lr/HOJlx/yrmslX74pS95iXZN99W66aaNSqAedIk ZYSK0FsoBSU0QFAOt/WXiKjh3lIDyffCEzcF/avyHhj0LKWrCvhQSD7QN919tvFv4e7i2KatNPsQo HVPqRaIyHqYfJ59EIJwJ42khukajHl9E9+xB9/kIOTPgEtt51Rv7QCOfpHyi2kxi5fa9C6Rr8rgIA v6MboEByZoGz7kFuEcPCxX8xPQ81cYexQypMyOZO2RPW9JbS3M7Y2kMw1tzLOZEeZyUYM3eD8eYiJ zn+y6cYw==; Received: by zero.zsh.org with local id 1otrnE-000Is7-Co; Sat, 12 Nov 2022 14:47:36 +0000 Authentication-Results: zsh.org; iprev=pass (out5-smtp.messagingengine.com) smtp.remote-ip=66.111.4.29; dkim=pass header.d=jpgrayson.net header.s=fm3 header.a=rsa-sha256; dkim=pass header.d=messagingengine.com header.s=fm1 header.a=rsa-sha256; dmarc=none header.from=jpgrayson.net; arc=none Received: from out5-smtp.messagingengine.com ([66.111.4.29]:33635) by zero.zsh.org with esmtps (TLS1.3:TLS_AES_256_GCM_SHA384:256) id 1otrmb-000IW8-N4; Sat, 12 Nov 2022 14:46:59 +0000 Received: from compute4.internal (compute4.nyi.internal [10.202.2.44]) by mailout.nyi.internal (Postfix) with ESMTP id EF8A05C007F; Sat, 12 Nov 2022 09:46:55 -0500 (EST) Received: from imap48 ([10.202.2.98]) by compute4.internal (MEProxy); Sat, 12 Nov 2022 09:46:55 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=jpgrayson.net; 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=1668264415; x= 1668350815; bh=90t5PISWEGq0hqBoZwpvFrOJuqPfaVqNVF67UbYUp3A=; b=V j9x7k6nRNWmDmHT/4/5gfsDNRLMc328zGg3CP8nG/vH6VdEyDDrtwu5ANJSkqIeA XQxN7kF0jp0SPDLyBFnPPyfphNBe+SROGEUUKj7ipaDp0uFTW2iptZ1KOSf+xlzY 1s317IRkHqyqSgE1KPaRU2SAlVrRV6WPPs6yH9sY1fFhZXcf2xOCGyd1oGeZnke1 nwbBT5BChSfEbTQl9+tKB+ccVOqAeBJgwaFLS6hAlVm/nw5vri5MFN+n2de1fju+ nmdaztxjrNQvFXHlwDWVYZnICRSihQoCsh5UxygfTP8l7zU5Oss+3saYKy0ywm2e SxzohjFABlj5awZu0KxhQ== 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=1668264415; x= 1668350815; bh=90t5PISWEGq0hqBoZwpvFrOJuqPfaVqNVF67UbYUp3A=; b=k 2r8mZJ0wAYUErTRM6VzoBpdSB/+AMKgw3Aa3ESvp8YHUkGXWTe0ZmYVGmcIXVGAk 1kI5Fym80Wxnwo9lCOoy9RfrnlDyEOMSBj2ZTVPwZr/4FH5u1eI98qtrwxg4qFeC ddCOKzE6ni9OVwKZwkNKvkla4tCPUlSbeMIDdXmFKlCeE9pqykYRPfdDNIj3+BmA voHplhc6vWkWnFdH+kcvziMLCGM22PX4lxT18+UhMGsBucUNsEY6sGBBglmnsF+N BYiHuplbxUBZmobs4rAvlHNgdoHMSAiJCZK5ZKs8WdEdIcZwC0nJfDo+z2TO7k06 Iw/GB94ed1ECKhtQtfA8w== X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvgedrfeekgdeikecutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu uegrihhlohhuthemuceftddtnecunecujfgurhepofgfggfkjghffffhvfevufgtgfesth hqredtreerjeenucfhrhhomhepfdfrvghtvghrucfirhgrhihsohhnfdcuoehpvghtvges jhhpghhrrgihshhonhdrnhgvtheqnecuggftrfgrthhtvghrnhepkeehieffgfejtdekie eutefhudffgfetvdekvdelveejleffveejuefhtdeikeehnecuffhomhgrihhnpehrvghp ohhlohhghidrohhrghenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrih hlfhhrohhmpehpvghtvgesjhhpghhrrgihshhonhdrnhgvth X-ME-Proxy: Feedback-ID: iefe944c0:Fastmail Received: by mailuser.nyi.internal (Postfix, from userid 501) id A737931A0063; Sat, 12 Nov 2022 09:46:55 -0500 (EST) X-Mailer: MessagingEngine.com Webmail Interface User-Agent: Cyrus-JMAP/3.7.0-alpha0-1115-g8b801eadce-fm-20221102.001-g8b801ead Mime-Version: 1.0 Message-Id: <603fd1b9-1b11-4729-99cb-19e1c4ef8b37@app.fastmail.com> In-Reply-To: <20221111114928.GF27622@tarpaulin.shahaf.local2> References: <20221101030429.38029-1-pete@jpgrayson.net> <20221111114928.GF27622@tarpaulin.shahaf.local2> Date: Sat, 12 Nov 2022 09:46:02 -0500 From: "Peter Grayson" To: "Daniel Shahaf" Cc: zsh-workers@zsh.org Subject: Re: [PATCH] Remove StGit patch detection from vcs_info Content-Type: text/plain;charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Seq: 50941 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: On Fri, Nov 11, 2022, at 6:49 AM, Daniel Shahaf wrote: > Sorry for my long RTT on this. Thank you for this detailed response. > 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. >>=20 >> Thus zsh's approach for interrogating StGit patch state is long >> obsoleted.=20 > > *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=C2=A02.x". This is actually two independent decisions: > > 1. Whether to keep or remove the StGit=C2=A00.x support code. > > 2. Whether patches adding StGit=C2=A02.x support code would be conside= red. Or option 3, which would be to support all versions of StGit by using stg command lines that are compatible with all versions of StGit going back to 2008. >> - 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 u= se > stg=E2=89=A51.x, and for vcs_info maintainers are minimal. Conversely, > /removing/ the code might break some users' proverbial toaster heating= =C2=A0=E2=80=94 > e.g., users who self-compile latest zsh on LTS distros, and users on > current distros that still ship stgit=C2=A00.x=20 > . > 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 justifica= tion. > > - 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=C2=A02.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=C2=A0=E2=80=94 though admittedly no= t ideal=C2=A0=E2=80=94 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=C2=A0=E2=80=94 partly for performance, partly becau= se when the > code was written there mightn't have been APIs for what we needed=C2=A0= =E2=80=94 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 or= der >> of hundreds of milliseconds). > > [re both stg-0.x and stg-1.x]: > > And why would that be unacceptable? A few hundreds of milliseconds=C2= =A0=E2=80=94 > let's round up and say a microsecond=C2=A0=E2=80=94 is short enough th= at "Press > on an empty prompt and get a new one instantly" would still be > the user's experience, wouldn't it? Rounding up a few hundred milliseconds gets into the *seconds* domain, n= ot microseconds. That said, I think StGit can be supported in zsh such that only users wi= th StGit <2.0 installed would possibly be exposed to undue overhead. >> - 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=C2=A02.x support but making it= off by > default (require an opt-in)? Compare the =C2=ABcheck-for-staged-chang= es=C2=BB and > =C2=ABget-unapplied=C2=BB styles, and contrast =C2=ABuse-simple=C2=BB = and =C2=ABenable=C2=BB. > > 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 d= o, > 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? You've convinced me that retaining StGit support in zsh's vcs_info is the way to go. And I've convinced myself that supporting all versions of StG= it is straightforward. So let's do that. > [re #2]: It sounds like StGit=C2=A02.x support can be implemented at t= he cost > of one fork(2) for those who don't use StGit and under a microsecond f= or > those who do. That doesn't sound like a deal breaker at all. Running `stg series` with StGit 2.0 takes about 12ms in my environment. StGit 1.5 it is about 32ms. Not a microsecond, but perhaps acceptable nonetheless. > Furthermore, will StGit 2.x users want to show in the prompt informati= on > about the patch stack? If so, I'd say that's an argument in favour of > including StGit=C2=A02.x support in vcs_info, since vcs_info is design= ed to > allow this. (Also, code=C2=A0=E2=80=94 say, applied-string hooks=C2=A0= =E2=80=94 can be reused > across stg(1) and other kinds of patch series.) I need to admit that I have been a long-time user of prezto and its sorin prompt which uses its own async vcs_info alternative (git-info), and so I have not been a user of vcs_info. This discussion prompted me to try out vcs_info, including its support for applied/unapplied patches. And I've patched it to support StGit 2.0 as well as older StGit versions by using stg commands as discussed. Its working great and having patch series info in my prompt is nice. > Your points=C2=A0=E2=80=94 cost/benefit, forward compatibility, perfor= mancy, etc.=C2=A0=E2=80=94 > 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=C2=A02.x support in zsh. Patch to update StGit support will be forthcoming. > Thanks for the willingness (elsethread) to relicence, and Congrats on > the 2.0 release :) Thank you! > Cheers, > > Daniel > >> Signed-off-by: Peter Grayson >> --- >> Functions/VCS_Info/Backends/VCS_INFO_get_data_git | 11 ++--------- >> 1 file changed, 2 insertions(+), 9 deletions(-) >>=20 >> diff --git a/Functions/VCS_Info/Backends/VCS_INFO_get_data_git b/Func= tions/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} >> =20 >> -local patchdir=3D${gitdir}/patches/${gitbranch} >> -if [[ -d $patchdir ]] && [[ -f $patchdir/applied ]] \ >> - && [[ -f $patchdir/unapplied ]] >> -then >> - # stgit >> - git_patches_applied=3D(${(f)"$(< "${patchdir}/applied")"}) >> - git_patches_unapplied=3D(${(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=3D"${gitdir}/rebase-merge" >> local p >> --=20 >> 2.38.1 >>=20 >>