From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Dmitry Gutov Newsgroups: gmane.emacs.bugs Subject: bug#73320: [PATCH] project--vc-list-files: use Git's sparse-index Date: Thu, 19 Sep 2024 12:44:40 +0300 Message-ID: <96ddaf6a-ffdc-4c1f-9141-ee14b41ebd11@gutov.dev> References: <73758f39-1e18-471a-9dfb-0ceade12dacf@gutov.dev> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="32024"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Mozilla Thunderbird Cc: 73320@debbugs.gnu.org To: Sean Allred Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Thu Sep 19 11:46:03 2024 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1srDjd-00088M-Ek for geb-bug-gnu-emacs@m.gmane-mx.org; Thu, 19 Sep 2024 11:46:02 +0200 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1srDjV-0006aJ-49; Thu, 19 Sep 2024 05:45:53 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1srDjO-0006Zl-9o for bug-gnu-emacs@gnu.org; Thu, 19 Sep 2024 05:45:49 -0400 Original-Received: from debbugs.gnu.org ([2001:470:142:5::43]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1srDjN-0008Nk-Vr for bug-gnu-emacs@gnu.org; Thu, 19 Sep 2024 05:45:46 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=debbugs.gnu.org; s=debbugs-gnu-org; h=In-Reply-To:From:References:MIME-Version:Date:To:Subject; bh=18b+FWGOZQoVzXqYIvFmJxUFDNbpunjTD31kWGnI3vk=; b=fi80ql7qM63KEn3jNFyHmb68fbW7FgFw44JRojchec/EYTGIUn6YzIO0sFX60rpW7WxinFyIoL0QktJnZxf0u3LGE0OaTp3Xya75UIxgMQvOEQAKztL6jD1wJ1w0WELrYFvJ9YMdno2stzIhXN1sddxDtNcCgCiSBp2Cq8s3zZPmfu1Tr94DXGBN+83h3yM2DZwpbUB7y8pFfQG8jd1qaNTYg6kULiOYSlaNSVhVh5MJBYVlWj1GbHwoQjklWO2Q4TYizs31ApjTMZq+cG+EGznFm9/xBYyEFa/0iX16IiQ6f/Zrq0YR8znde95/PkZf56pthLE7iS/uOeolqF5OfA==; Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1srDje-0003YY-8E for bug-gnu-emacs@gnu.org; Thu, 19 Sep 2024 05:46:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Dmitry Gutov Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Thu, 19 Sep 2024 09:46:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 73320 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch Original-Received: via spool by 73320-submit@debbugs.gnu.org id=B73320.172673911113098 (code B ref 73320); Thu, 19 Sep 2024 09:46:02 +0000 Original-Received: (at 73320) by debbugs.gnu.org; 19 Sep 2024 09:45:11 +0000 Original-Received: from localhost ([127.0.0.1]:59818 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1srDip-0003PC-DB for submit@debbugs.gnu.org; Thu, 19 Sep 2024 05:45:11 -0400 Original-Received: from fhigh5-smtp.messagingengine.com ([103.168.172.156]:34421) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1srDik-0003OL-PT for 73320@debbugs.gnu.org; Thu, 19 Sep 2024 05:45:10 -0400 Original-Received: from phl-compute-01.internal (phl-compute-01.phl.internal [10.202.2.41]) by mailfhigh.phl.internal (Postfix) with ESMTP id DD30E1140247; Thu, 19 Sep 2024 05:44:43 -0400 (EDT) Original-Received: from phl-mailfrontend-01 ([10.202.2.162]) by phl-compute-01.internal (MEProxy); Thu, 19 Sep 2024 05:44:43 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gutov.dev; h=cc :cc:content-transfer-encoding:content-type:content-type:date :date:from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:subject:subject:to:to; s=fm3; t=1726739083; x=1726825483; bh=18b+FWGOZQoVzXqYIvFmJxUFDNbpunjTD31kWGnI3vk=; b= GfICfI68oI+I71834CoIQh1HH6mvVpyccbNFkH80w0P67tGOH/YZcFBucABaqLdz 5mTWBzjbe2GQ/Xq76Ps4xWagAtHStIl/NNTiMbYhVu+aJ9x/YZtJRY6TZfn14A1z Bd7el1hol0DDIyPZpWVVWMzZBna2qcUILF+ZzwxN+Bf0cwuIubaK/T7oRhzrWpHZ UeZhkJ3iBF57AufoMweouBET8/VMfisIK1vwWNiR0CbPOeJrGHg9TkrvWT0SZ4HN PB3+l1RmQHrr0m03Aqj/7h3n6pFpV9W0A9RWvhR34QKwJctc0fa6SJ/Kf33yALii GrWnXqYRtgm24tUGShNbiA== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-transfer-encoding :content-type:content-type:date:date:feedback-id:feedback-id :from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:subject:subject:to:to:x-me-proxy:x-me-proxy :x-me-sender:x-me-sender:x-sasl-enc; s=fm2; t=1726739083; x= 1726825483; bh=18b+FWGOZQoVzXqYIvFmJxUFDNbpunjTD31kWGnI3vk=; b=J pJf0i9dw2f2u0w88rnFXbEZibRcgAANq6CQmHHR/zw6Cixg1CTAqV82Ns10xWkB1 Yc59GGZ0E08uolslwPNQe3fz6Lz+RQugjm7K3MnmKhWl7XFth88ud9v332xVRhcs TE+boC0kp17ruU0K+kEpaDCD3NZRm+hMk4+Lfe6D8wx8k/w/UXfEF/zlMYSOGZVh AVW2glUfSl3RE5xXvWqoH0KdHDVzzKbb2x5LSuXRfXWhjsfAE2zuR05P53BGxc9S ne3DWeq1z/sx/KyyzbZjCrSRWyS13cCzrhBFLt2IuhxoyLZYirNSkjOMvwDWYRne qDKxJpFErSh7J+/x1+79A== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeeftddrudeluddgudelucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdggtfgfnhhsuhgsshgtrhhisggvpdfu rfetoffkrfgpnffqhgenuceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnh htshculddquddttddmnecujfgurhepkfffgggfuffvvehfhfgjtgfgsehtjeertddtvdej necuhfhrohhmpeffmhhithhrhicuifhuthhovhcuoegumhhithhrhiesghhuthhovhdrug gvvheqnecuggftrfgrthhtvghrnhepteduleejgeehtefgheegjeekueehvdevieekueef tddvtdevfefhvdevgedujeehnecuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpe hmrghilhhfrhhomhepughmihhtrhihsehguhhtohhvrdguvghvpdhnsggprhgtphhtthho pedvpdhmohguvgepshhmthhpohhuthdprhgtphhtthhopegrlhhlrhgvugdrshgvrghnse hgmhgrihhlrdgtohhmpdhrtghpthhtohepjeeffedvtdesuggvsggsuhhgshdrghhnuhdr ohhrgh X-ME-Proxy: Feedback-ID: i07de48aa:Fastmail Original-Received: by mail.messagingengine.com (Postfix) with ESMTPA; Thu, 19 Sep 2024 05:44:42 -0400 (EDT) Content-Language: en-US In-Reply-To: X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Original-Sender: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Xref: news.gmane.io gmane.emacs.bugs:292039 Archived-At: On 19/09/2024 07:25, Sean Allred wrote: >> diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el >> index b29d5ed5404..a2e3f3f52e6 100644 >> --- a/lisp/progmodes/project.el >> +++ b/lisp/progmodes/project.el >> @@ -663,7 +663,7 @@ project--vc-list-files >> (pcase backend >> (`Git >> (let* ((default-directory (expand-file-name >> (file-name-as-directory dir))) >> - (args '("-z")) >> + (args '("-z" "--sparse")) >> (vc-git-use-literal-pathspecs nil) >> (include-untracked (project--value-in-dir >> 'project-vc-include-untracked >> @@ -703,7 +703,8 @@ project--vc-list-files >> (delq nil >> (mapcar >> (lambda (file) >> - (unless (member file submodules) >> + (unless (or (member file submodules) >> + (eq ?/ (aref file (1- (length file))))) >> (if project-files-relative-names >> file >> (concat default-directory file)))) > > Works fine for me :-) Though I've added an additional version check > inlined below. Great, thanks! >>> Incidentally looking at the version check within `project-files`, it's >>> worthwhile to point out that `--sparse` is likely /not/ compatible with >>> ancient versions of Git. [...] >> >> [...] >> >> We can call vc-git--program-version the same way it's used in >> vc-git-state. Which version should we make the minimum? > > The `--sparse` option was introduced in 2.35. The following seems to > work well for me: > > diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el > index b29d5ed5404..873bc92729d 100644 > --- a/lisp/progmodes/project.el > +++ b/lisp/progmodes/project.el > @@ -663,7 +663,8 @@ project--vc-list-files > (pcase backend > (`Git > (let* ((default-directory (expand-file-name (file-name-as-directory dir))) > - (args '("-z")) > + (args `("-z" ,@(when (version<= "2.35" (vc-git--program-version)) > + '("--sparse")))) > (vc-git-use-literal-pathspecs nil) > (include-untracked (project--value-in-dir > 'project-vc-include-untracked > @@ -703,7 +704,8 @@ project--vc-list-files > (delq nil > (mapcar > (lambda (file) > - (unless (member file submodules) > + (unless (or (member file submodules) > + (eq ?/ (aref file (1- (length file))))) > (if project-files-relative-names > file > (concat default-directory file)))) Like Eli suggested, we can use directory-name-p instead of the second condition. > Since we're getting a bit busy with our conditions, though, it might be > better to start using `cond`: > > diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el > index 873bc92729d..b42415154e3 100644 > --- a/lisp/progmodes/project.el > +++ b/lisp/progmodes/project.el > @@ -704,11 +704,11 @@ project--vc-list-files > (delq nil > (mapcar > (lambda (file) > - (unless (or (member file submodules) > - (eq ?/ (aref file (1- (length file))))) > - (if project-files-relative-names > - file > - (concat default-directory file)))) > + (cond > + ((member file submodules) nil) > + ((eq ?/ (aref file (1- (length file)))) nil) > + (project-files-relative-names file) > + (t (concat default-directory file)))) > (split-string > (with-output-to-string > (apply #'vc-git-command standard-output 0 nil "ls-files" args)) > > This seems to help readability -- at least to me. There's probably also > a nominal performance benefit since `cond` is a special form. I think I'd rather keep the 'unless' for now: it groups the first two cases a bit more clearly. > I've pushed this as branch `sa/sparse-index-2` to my repository. (This > is in addition to the `sa/sparse-index` branch, which contains the > `file-exists-p` check mentioned below plus what might be, I take it, an > ultimately unneeded opt-out parameter in `project-files`.) Yeah, it doesn't seem to be useful to return files that don't exist in the current work directory - no idea what a caller would do with them. > It's worth noting that actually performing a `file-exists-p` check here > would have the added benefit of handling the awkward state between Git > 2.25 (where sparse-checkout was introduced) and 2.35 (where git-ls-files > learned --sparse) where ls-files could still report things that _look_ > like files but are not present. This would be fixed by just replacing > the (eq ..) form with (not (file-exists-p file)). 'file-exists-p' does I/O, so it would make processing an order of a magnitude slower. Here's a benchmark you can try: (benchmark-run 1 (project-files (project-current)))