unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#64088: [PATCH] Make project-root for vc projects return an absolute path
@ 2023-06-15 20:48 Spencer Baugh
  2023-06-16  5:52 ` Eli Zaretskii
  2023-06-17  2:47 ` Dmitry Gutov
  0 siblings, 2 replies; 13+ messages in thread
From: Spencer Baugh @ 2023-06-15 20:48 UTC (permalink / raw)
  To: 64088

[-- Attachment #1: Type: text/plain, Size: 959 bytes --]

Tags: patch


Before this, if a vc project was in the user's home directory,
project-root would be "~/some/path".  This violates the spec of
project-root, which requires an absolute path.  One concrete bug
caused by this is that (project-forget-project "~/some/path") would
work but (project-forget-project "/home/user/some/path") would not.

I ran into this while trying to use file-notify to automatically
discover and forget projects.  file-notify returns truly absolute paths,
so I ran into the aforementioned concrete bug.

In GNU Emacs 29.0.90 (build 8, x86_64-pc-linux-gnu, X toolkit, cairo
 version 1.15.12, Xaw scroll bars) of 2023-06-04 built on
 igm-qws-u22796a
Repository revision: 11d76029db5f0d9e016f247aac24dd430b729c2a
Repository branch: emacs-29
Windowing system distributor 'The X.Org Foundation', version 11.0.12011000
System Description: CentOS Linux 7 (Core)

Configured using:
 'configure --with-x-toolkit=lucid --with-gif=ifavailable'


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Make-project-root-for-vc-projects-return-an-absolute.patch --]
[-- Type: text/patch, Size: 1356 bytes --]

From 2c3ed85f857759458130a7b707d9b6f4d2c1ef20 Mon Sep 17 00:00:00 2001
From: Spencer Baugh <sbaugh@janestreet.com>
Date: Thu, 15 Jun 2023 16:45:48 -0400
Subject: [PATCH] Make project-root for vc projects return an absolute path

Before this, if a vc project was in the user's home directory,
project-root would be "~/some/path".  This violates the spec of
project-root, which requires an absolute path.  One concrete bug
caused by this is that (project-forget-project "~/some/path") would
work but (project-forget-project "/home/user/some/path") would not.

* lisp/progmodes/project.el (project-try-vc): Expand root directory
before returning project.
---
 lisp/progmodes/project.el | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
index 2cffc1c0669..72cdb94bce4 100644
--- a/lisp/progmodes/project.el
+++ b/lisp/progmodes/project.el
@@ -689,7 +689,7 @@ project-try-vc
           (let* ((parent (file-name-directory (directory-file-name root))))
             (setq root (vc-call-backend 'Git 'root parent))))
         (when root
-          (setq project (list 'vc backend root))
+          (setq project (list 'vc backend (expand-file-name root)))
           ;; FIXME: Cache for a shorter time.
           (vc-file-setprop dir 'project-vc project)
           project))))
-- 
2.39.3


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* bug#64088: [PATCH] Make project-root for vc projects return an absolute path
  2023-06-15 20:48 bug#64088: [PATCH] Make project-root for vc projects return an absolute path Spencer Baugh
@ 2023-06-16  5:52 ` Eli Zaretskii
  2023-06-17  2:43   ` Dmitry Gutov
  2023-06-17  2:47 ` Dmitry Gutov
  1 sibling, 1 reply; 13+ messages in thread
From: Eli Zaretskii @ 2023-06-16  5:52 UTC (permalink / raw)
  To: Spencer Baugh; +Cc: 64088

> From: Spencer Baugh <sbaugh@janestreet.com>
> Date: Thu, 15 Jun 2023 16:48:05 -0400
> 
> Before this, if a vc project was in the user's home directory,
> project-root would be "~/some/path".  This violates the spec of
> project-root, which requires an absolute path.

File names like "~/some/file" are considered absolute in Emacs:

  (file-name-absolute-p "~/foo") => t

> One concrete bug
> caused by this is that (project-forget-project "~/some/path") would
> work but (project-forget-project "/home/user/some/path") would not.

That problem is simple to solve where it happens, no?  That is,
project-forget-project should try looking not just for the literal
root, but also for its expanded and abbreviated forms, and that's all.

> I ran into this while trying to use file-notify to automatically
> discover and forget projects.  file-notify returns truly absolute paths,
> so I ran into the aforementioned concrete bug.

And please do not use "path" when you mean file names.

> >From 2c3ed85f857759458130a7b707d9b6f4d2c1ef20 Mon Sep 17 00:00:00 2001
> From: Spencer Baugh <sbaugh@janestreet.com>
> Date: Thu, 15 Jun 2023 16:45:48 -0400
> Subject: [PATCH] Make project-root for vc projects return an absolute path
> 
> Before this, if a vc project was in the user's home directory,
> project-root would be "~/some/path".  This violates the spec of
> project-root, which requires an absolute path.  One concrete bug
> caused by this is that (project-forget-project "~/some/path") would
> work but (project-forget-project "/home/user/some/path") would not.
> 
> * lisp/progmodes/project.el (project-try-vc): Expand root directory
> before returning project.
> ---
>  lisp/progmodes/project.el | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
> index 2cffc1c0669..72cdb94bce4 100644
> --- a/lisp/progmodes/project.el
> +++ b/lisp/progmodes/project.el
> @@ -689,7 +689,7 @@ project-try-vc
>            (let* ((parent (file-name-directory (directory-file-name root))))
>              (setq root (vc-call-backend 'Git 'root parent))))
>          (when root
> -          (setq project (list 'vc backend root))
> +          (setq project (list 'vc backend (expand-file-name root)))
>            ;; FIXME: Cache for a shorter time.
>            (vc-file-setprop dir 'project-vc project)
>            project))))

I think this fixes a problem in the wrong place.  It should be fixed
where it happens, i.e. in project--remove-from-project-list.

Thanks.





^ permalink raw reply	[flat|nested] 13+ messages in thread

* bug#64088: [PATCH] Make project-root for vc projects return an absolute path
  2023-06-16  5:52 ` Eli Zaretskii
@ 2023-06-17  2:43   ` Dmitry Gutov
  2023-06-17  6:25     ` Eli Zaretskii
  0 siblings, 1 reply; 13+ messages in thread
From: Dmitry Gutov @ 2023-06-17  2:43 UTC (permalink / raw)
  To: Eli Zaretskii, Spencer Baugh; +Cc: 64088

On 16/06/2023 08:52, Eli Zaretskii wrote:
>> One concrete bug
>> caused by this is that (project-forget-project "~/some/path") would
>> work but (project-forget-project "/home/user/some/path") would not.
> That problem is simple to solve where it happens, no?  That is,
> project-forget-project should try looking not just for the literal
> root, but also for its expanded and abbreviated forms, and that's all.

Alternatively, we could force-convert it to one or the other form when 
saving a new value to the list. Then we'd only need to convert a 
searches value to one of the forms.






^ permalink raw reply	[flat|nested] 13+ messages in thread

* bug#64088: [PATCH] Make project-root for vc projects return an absolute path
  2023-06-15 20:48 bug#64088: [PATCH] Make project-root for vc projects return an absolute path Spencer Baugh
  2023-06-16  5:52 ` Eli Zaretskii
@ 2023-06-17  2:47 ` Dmitry Gutov
  2023-06-27 20:01   ` Spencer Baugh
  1 sibling, 1 reply; 13+ messages in thread
From: Dmitry Gutov @ 2023-06-17  2:47 UTC (permalink / raw)
  To: Spencer Baugh, 64088

On 15/06/2023 23:48, Spencer Baugh wrote:
> -          (setq project (list 'vc backend root))
> +          (setq project (list 'vc backend (expand-file-name root)))

Spec violated or not (like Eli said, abbreviated names are still 
absolute), we should probably try to fix this in another place, so that 
other backends (whatever external ones exist) don't have to worry about 
the distinction.

As you can see, in project-buffers, for example, we call expand-file-name.





^ permalink raw reply	[flat|nested] 13+ messages in thread

* bug#64088: [PATCH] Make project-root for vc projects return an absolute path
  2023-06-17  2:43   ` Dmitry Gutov
@ 2023-06-17  6:25     ` Eli Zaretskii
  2023-06-18  0:43       ` Dmitry Gutov
  0 siblings, 1 reply; 13+ messages in thread
From: Eli Zaretskii @ 2023-06-17  6:25 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: sbaugh, 64088

> Date: Sat, 17 Jun 2023 05:43:58 +0300
> Cc: 64088@debbugs.gnu.org
> From: Dmitry Gutov <dmitry@gutov.dev>
> 
> On 16/06/2023 08:52, Eli Zaretskii wrote:
> >> One concrete bug
> >> caused by this is that (project-forget-project "~/some/path") would
> >> work but (project-forget-project "/home/user/some/path") would not.
> > That problem is simple to solve where it happens, no?  That is,
> > project-forget-project should try looking not just for the literal
> > root, but also for its expanded and abbreviated forms, and that's all.
> 
> Alternatively, we could force-convert it to one or the other form when 
> saving a new value to the list. Then we'd only need to convert a 
> searches value to one of the forms.

How do we know such conversion will not cause other similar problems?

IME, low-level functions should not make any assumptions about what
higher-level code will need.  On the contrary, it's higher-level code
which knows what it needs that should do the conversions it needs.





^ permalink raw reply	[flat|nested] 13+ messages in thread

* bug#64088: [PATCH] Make project-root for vc projects return an absolute path
  2023-06-17  6:25     ` Eli Zaretskii
@ 2023-06-18  0:43       ` Dmitry Gutov
  0 siblings, 0 replies; 13+ messages in thread
From: Dmitry Gutov @ 2023-06-18  0:43 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: sbaugh, 64088

On 17/06/2023 09:25, Eli Zaretskii wrote:
> IME, low-level functions should not make any assumptions about what
> higher-level code will need.  On the contrary, it's higher-level code
> which knows what it needs that should do the conversions it needs.

That's the idea: project-root is always a callee, not a caller, so it's 
"low-level", in your terminology.

The conversion (or normalization) would happen on the higher level: when 
the projects are added to the list, or searched.





^ permalink raw reply	[flat|nested] 13+ messages in thread

* bug#64088: [PATCH] Make project-root for vc projects return an absolute path
  2023-06-17  2:47 ` Dmitry Gutov
@ 2023-06-27 20:01   ` Spencer Baugh
  2023-07-10  2:17     ` Dmitry Gutov
  0 siblings, 1 reply; 13+ messages in thread
From: Spencer Baugh @ 2023-06-27 20:01 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: 64088

Dmitry Gutov <dmitry@gutov.dev> writes:
> On 15/06/2023 23:48, Spencer Baugh wrote:
>> -          (setq project (list 'vc backend root))
>> +          (setq project (list 'vc backend (expand-file-name root)))
>
> Spec violated or not (like Eli said, abbreviated names are still
> absolute), we should probably try to fix this in another place, so
> that other backends (whatever external ones exist) don't have to worry
> about the distinction.
>
> As you can see, in project-buffers, for example, we call expand-file-name.

OK, how about this?

diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
index ddb033d50f9..b365e195415 100644
--- a/lisp/progmodes/project.el
+++ b/lisp/progmodes/project.el
@@ -1575,7 +1575,10 @@ project-list-file
 
 (defvar project--list 'unset
   "List structure containing root directories of known projects.
-With some possible metadata (to be decided).")
+With some possible metadata (to be decided).
+
+Entries should be canonicalized with `expand-file-name' before
+being added.")
 
 (defun project--read-project-list ()
   "Initialize `project--list' using contents of `project-list-file'."
@@ -1613,7 +1616,7 @@ project-remember-project
 Save the result in `project-list-file' if the list of projects
 has changed, and NO-WRITE is nil."
   (project--ensure-read-project-list)
-  (let ((dir (project-root pr)))
+  (let ((dir (expand-file-name (project-root pr))))
     (unless (equal (caar project--list) dir)
       (dolist (ent project--list)
         (when (equal dir (car ent))
@@ -1629,7 +1632,7 @@ project--remove-from-project-list
 from the list using REPORT-MESSAGE, which is a format string
 passed to `message' as its first argument."
   (project--ensure-read-project-list)
-  (when-let ((ent (assoc project-root project--list)))
+  (when-let ((ent (assoc (expand-file-name project-root) project--list)))
     (setq project--list (delq ent project--list))
     (message report-message project-root)
     (project--write-project-list)))





^ permalink raw reply related	[flat|nested] 13+ messages in thread

* bug#64088: [PATCH] Make project-root for vc projects return an absolute path
  2023-06-27 20:01   ` Spencer Baugh
@ 2023-07-10  2:17     ` Dmitry Gutov
  2023-07-10 23:45       ` Spencer Baugh
  0 siblings, 1 reply; 13+ messages in thread
From: Dmitry Gutov @ 2023-07-10  2:17 UTC (permalink / raw)
  To: Spencer Baugh; +Cc: 64088

On 27/06/2023 23:01, Spencer Baugh wrote:
> OK, how about this?

Maybe we should go in the other direction? And call abbreviate-file-name 
on them?

Because otherwise in project-prompt-project-dir we'll always show the 
expanded directory names, taking up extra space and usually repeating 
the full name of the user's home directory for no good reason.





^ permalink raw reply	[flat|nested] 13+ messages in thread

* bug#64088: [PATCH] Make project-root for vc projects return an absolute path
  2023-07-10  2:17     ` Dmitry Gutov
@ 2023-07-10 23:45       ` Spencer Baugh
  2023-08-12  0:45         ` Dmitry Gutov
  0 siblings, 1 reply; 13+ messages in thread
From: Spencer Baugh @ 2023-07-10 23:45 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: 64088

Dmitry Gutov <dmitry@gutov.dev> writes:
> On 27/06/2023 23:01, Spencer Baugh wrote:
>> OK, how about this?
>
> Maybe we should go in the other direction? And call
> abbreviate-file-name on them?
>
> Because otherwise in project-prompt-project-dir we'll always show the
> expanded directory names, taking up extra space and usually repeating
> the full name of the user's home directory for no good reason.

That seems reasonable if we let-bind directory-abbrev-alist to nil
around it; otherwise we'll be dependent on the user's configuration and
we might add a directory in one way, and then they add a new value to
directory-abbrev-alist and we add it again a second way.

Although maybe that's fine?  And probably users of
directory-abbrev-alist would like to have those abbreviations show up in
project-prompt-project-dir.

Also we could always call abbreviate-file-name at
project-prompt-project-dir time.





^ permalink raw reply	[flat|nested] 13+ messages in thread

* bug#64088: [PATCH] Make project-root for vc projects return an absolute path
  2023-07-10 23:45       ` Spencer Baugh
@ 2023-08-12  0:45         ` Dmitry Gutov
  2023-08-19 12:31           ` sbaugh
  0 siblings, 1 reply; 13+ messages in thread
From: Dmitry Gutov @ 2023-08-12  0:45 UTC (permalink / raw)
  To: Spencer Baugh; +Cc: 64088

On 11/07/2023 02:45, Spencer Baugh wrote:
> Dmitry Gutov <dmitry@gutov.dev> writes:
>> On 27/06/2023 23:01, Spencer Baugh wrote:
>>> OK, how about this?
>>
>> Maybe we should go in the other direction? And call
>> abbreviate-file-name on them?
>>
>> Because otherwise in project-prompt-project-dir we'll always show the
>> expanded directory names, taking up extra space and usually repeating
>> the full name of the user's home directory for no good reason.
> 
> That seems reasonable if we let-bind directory-abbrev-alist to nil
> around it; otherwise we'll be dependent on the user's configuration and
> we might add a directory in one way, and then they add a new value to
> directory-abbrev-alist and we add it again a second way.

That's also fair.

> Although maybe that's fine?  And probably users of
> directory-abbrev-alist would like to have those abbreviations show up in
> project-prompt-project-dir.

They probably would. But indeed if the list was saved with one 
configuration, and read with another, it could lead to a mistake.

> Also we could always call abbreviate-file-name at
> project-prompt-project-dir time.

Meaning it will be called N times (for the number of projects) every 
time the list is displayed, instead of just once, when the project is 
saved/visited. But maybe it's fine too? How slow could that be?

It's probably not too significant problem either way, no matter which 
direction we choose. And we could revert later.

What do you think? Feel free to submit a new patch (note that the one 
you submitted previously would need the abbreviate-file-name calls added 
to it, before the list is displayed).





^ permalink raw reply	[flat|nested] 13+ messages in thread

* bug#64088: [PATCH] Make project-root for vc projects return an absolute path
  2023-08-12  0:45         ` Dmitry Gutov
@ 2023-08-19 12:31           ` sbaugh
  2023-08-19 12:57             ` sbaugh
  0 siblings, 1 reply; 13+ messages in thread
From: sbaugh @ 2023-08-19 12:31 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: Spencer Baugh, 64088

[-- Attachment #1: Type: text/plain, Size: 2106 bytes --]

Dmitry Gutov <dmitry@gutov.dev> writes:
> On 11/07/2023 02:45, Spencer Baugh wrote:
>> Dmitry Gutov <dmitry@gutov.dev> writes:
>>> On 27/06/2023 23:01, Spencer Baugh wrote:
>>>> OK, how about this?
>>>
>>> Maybe we should go in the other direction? And call
>>> abbreviate-file-name on them?
>>>
>>> Because otherwise in project-prompt-project-dir we'll always show the
>>> expanded directory names, taking up extra space and usually repeating
>>> the full name of the user's home directory for no good reason.
>> That seems reasonable if we let-bind directory-abbrev-alist to nil
>> around it; otherwise we'll be dependent on the user's configuration and
>> we might add a directory in one way, and then they add a new value to
>> directory-abbrev-alist and we add it again a second way.
>
> That's also fair.
>
>> Although maybe that's fine?  And probably users of
>> directory-abbrev-alist would like to have those abbreviations show up in
>> project-prompt-project-dir.
>
> They probably would. But indeed if the list was saved with one
> configuration, and read with another, it could lead to a mistake.
>
>> Also we could always call abbreviate-file-name at
>> project-prompt-project-dir time.
>
> Meaning it will be called N times (for the number of projects) every
> time the list is displayed, instead of just once, when the project is
> saved/visited. But maybe it's fine too? How slow could that be?

I was sad about this but I think I've got the solution now: We just
maintain project--list in abbreviated form and project-list-file in
expanded form.

I think that solves all the problems: the persistent file is independent
of user configuration, so if the user changes their abbreviations
they'll get new ones the next time the file is read (presumably the next
time they run Emacs).  But project--list is abbreviated, so
project-prompt-project-dir gets abbreviated dirs.

It does mean that if they change their directory-abbrev-alist while
Emacs is running they could get duplicate entries, but that already
could happen before this patch, and it's not a big problem IMO.

See patch below.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Expand-project-file-names-before-storing-them.patch --]
[-- Type: text/x-patch, Size: 2790 bytes --]

From 193a105b40b3a002f51e1f59dd2a3d48443e11fe Mon Sep 17 00:00:00 2001
From: Spencer Baugh <sbaugh@catern.com>
Date: Sat, 19 Aug 2023 08:24:45 -0400
Subject: [PATCH] Expand project file names before storing them

Before, whatever project-root returned, we stored as the root
directory of the project in project-list and project-list-file.  This
could lead to duplicate entries or bad behavior if projects were
accessed by different file names, e.g. both /home/user/src/emacs and
~/src/emacs.

Now project-list-file contains only expanded paths and project--list
contains only abbreviated paths.  We abbreviate filenames before
setting project--list, and expand filenames before writing to
project-list-file.

* lisp/progmodes/project.el (project--write-project-list): Call
expand-file-name.
(project--read-project-list, project-remember-project)
(project--remove-from-project-list): Call abbreviate-file-name.
---
 lisp/progmodes/project.el | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
index 11fa93fb70d..e99a9061275 100644
--- a/lisp/progmodes/project.el
+++ b/lisp/progmodes/project.el
@@ -1586,7 +1586,7 @@ project--read-project-list
           (when (file-exists-p filename)
             (with-temp-buffer
               (insert-file-contents filename)
-              (read (current-buffer)))))
+              (mapcar #'abbreviate-file-name (read (current-buffer))))))
     (unless (seq-every-p
              (lambda (elt) (stringp (car-safe elt)))
              project--list)
@@ -1606,7 +1606,7 @@ project--write-project-list
       (insert ";;; -*- lisp-data -*-\n")
       (let ((print-length nil)
             (print-level nil))
-        (pp project--list (current-buffer)))
+        (pp (mapcar #'expand-file-name project--list) (current-buffer)))
       (write-region nil nil filename nil 'silent))))
 
 ;;;###autoload
@@ -1615,7 +1615,7 @@ project-remember-project
 Save the result in `project-list-file' if the list of projects
 has changed, and NO-WRITE is nil."
   (project--ensure-read-project-list)
-  (let ((dir (project-root pr)))
+  (let ((dir (abbreviate-file-name (project-root pr))))
     (unless (equal (caar project--list) dir)
       (dolist (ent project--list)
         (when (equal dir (car ent))
@@ -1631,7 +1631,7 @@ project--remove-from-project-list
 from the list using REPORT-MESSAGE, which is a format string
 passed to `message' as its first argument."
   (project--ensure-read-project-list)
-  (when-let ((ent (assoc project-root project--list)))
+  (when-let ((ent (assoc (abbreviate-file-name project-root) project--list)))
     (setq project--list (delq ent project--list))
     (message report-message project-root)
     (project--write-project-list)))
-- 
2.41.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* bug#64088: [PATCH] Make project-root for vc projects return an absolute path
  2023-08-19 12:31           ` sbaugh
@ 2023-08-19 12:57             ` sbaugh
  2023-08-23  0:24               ` Dmitry Gutov
  0 siblings, 1 reply; 13+ messages in thread
From: sbaugh @ 2023-08-19 12:57 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: Spencer Baugh, 64088

[-- Attachment #1: Type: text/plain, Size: 2350 bytes --]

sbaugh@catern.com writes:
> Dmitry Gutov <dmitry@gutov.dev> writes:
>> On 11/07/2023 02:45, Spencer Baugh wrote:
>>> Dmitry Gutov <dmitry@gutov.dev> writes:
>>>> On 27/06/2023 23:01, Spencer Baugh wrote:
>>>>> OK, how about this?
>>>>
>>>> Maybe we should go in the other direction? And call
>>>> abbreviate-file-name on them?
>>>>
>>>> Because otherwise in project-prompt-project-dir we'll always show the
>>>> expanded directory names, taking up extra space and usually repeating
>>>> the full name of the user's home directory for no good reason.
>>> That seems reasonable if we let-bind directory-abbrev-alist to nil
>>> around it; otherwise we'll be dependent on the user's configuration and
>>> we might add a directory in one way, and then they add a new value to
>>> directory-abbrev-alist and we add it again a second way.
>>
>> That's also fair.
>>
>>> Although maybe that's fine?  And probably users of
>>> directory-abbrev-alist would like to have those abbreviations show up in
>>> project-prompt-project-dir.
>>
>> They probably would. But indeed if the list was saved with one
>> configuration, and read with another, it could lead to a mistake.
>>
>>> Also we could always call abbreviate-file-name at
>>> project-prompt-project-dir time.
>>
>> Meaning it will be called N times (for the number of projects) every
>> time the list is displayed, instead of just once, when the project is
>> saved/visited. But maybe it's fine too? How slow could that be?
>
> I was sad about this but I think I've got the solution now: We just
> maintain project--list in abbreviated form and project-list-file in
> expanded form.
>
> I think that solves all the problems: the persistent file is independent
> of user configuration, so if the user changes their abbreviations
> they'll get new ones the next time the file is read (presumably the next
> time they run Emacs).  But project--list is abbreviated, so
> project-prompt-project-dir gets abbreviated dirs.
>
> It does mean that if they change their directory-abbrev-alist while
> Emacs is running they could get duplicate entries, but that already
> could happen before this patch, and it's not a big problem IMO.
>
> See patch below.

Oops, this patch needed a few tweaks.  Especially, skipping the
expansion and abbreviation for remote files, to avoid making unnecesary
remote connections.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Expand-project-file-names-before-storing-them.patch --]
[-- Type: text/x-patch, Size: 3355 bytes --]

From 9523b4dea1234083e65b141a281c1108f52b046b Mon Sep 17 00:00:00 2001
From: Spencer Baugh <sbaugh@catern.com>
Date: Sat, 19 Aug 2023 08:24:45 -0400
Subject: [PATCH] Expand project file names before storing them

Before, whatever project-root returned, we stored as the root
directory of the project in project-list and project-list-file.  This
could lead to duplicate entries or bad behavior if projects were
accessed by different file names, e.g. both /home/user/src/emacs and
~/src/emacs.

Now project-list-file contains only expanded paths and project--list
contains only abbreviated paths.  We abbreviate filenames before
setting project--list, and expand filenames before writing to
project-list-file.  We only do this for local files, though, to avoid
making remote connections; the situation will still be bad for remote
projects, but at least this is an improvement.

* lisp/progmodes/project.el (project--write-project-list): Call
expand-file-name.
(project--read-project-list, project-remember-project)
(project--remove-from-project-list): Call abbreviate-file-name.
---
 lisp/progmodes/project.el | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
index 11fa93fb70d..21d5df8f0cc 100644
--- a/lisp/progmodes/project.el
+++ b/lisp/progmodes/project.el
@@ -1586,7 +1586,12 @@ project--read-project-list
           (when (file-exists-p filename)
             (with-temp-buffer
               (insert-file-contents filename)
-              (read (current-buffer)))))
+              (mapcar
+               (lambda (elem)
+                 (let ((name (car elem)))
+                   (list (if (file-remote-p name) name
+                           (abbreviate-file-name name)))))
+               (read (current-buffer))))))
     (unless (seq-every-p
              (lambda (elt) (stringp (car-safe elt)))
              project--list)
@@ -1606,7 +1611,12 @@ project--write-project-list
       (insert ";;; -*- lisp-data -*-\n")
       (let ((print-length nil)
             (print-level nil))
-        (pp project--list (current-buffer)))
+        (pp (mapcar (lambda (elem)
+                      (let ((name (car elem)))
+                        (list (if (file-remote-p name) name
+                                (expand-file-name name)))))
+                    project--list)
+            (current-buffer)))
       (write-region nil nil filename nil 'silent))))
 
 ;;;###autoload
@@ -1615,7 +1625,7 @@ project-remember-project
 Save the result in `project-list-file' if the list of projects
 has changed, and NO-WRITE is nil."
   (project--ensure-read-project-list)
-  (let ((dir (project-root pr)))
+  (let ((dir (abbreviate-file-name (project-root pr))))
     (unless (equal (caar project--list) dir)
       (dolist (ent project--list)
         (when (equal dir (car ent))
@@ -1631,7 +1641,7 @@ project--remove-from-project-list
 from the list using REPORT-MESSAGE, which is a format string
 passed to `message' as its first argument."
   (project--ensure-read-project-list)
-  (when-let ((ent (assoc project-root project--list)))
+  (when-let ((ent (assoc (abbreviate-file-name project-root) project--list)))
     (setq project--list (delq ent project--list))
     (message report-message project-root)
     (project--write-project-list)))
-- 
2.41.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* bug#64088: [PATCH] Make project-root for vc projects return an absolute path
  2023-08-19 12:57             ` sbaugh
@ 2023-08-23  0:24               ` Dmitry Gutov
  0 siblings, 0 replies; 13+ messages in thread
From: Dmitry Gutov @ 2023-08-23  0:24 UTC (permalink / raw)
  To: sbaugh; +Cc: Spencer Baugh, 64088-done

On 19/08/2023 15:57, sbaugh@catern.com wrote:
>>> Meaning it will be called N times (for the number of projects) every
>>> time the list is displayed, instead of just once, when the project is
>>> saved/visited. But maybe it's fine too? How slow could that be?
>> I was sad about this but I think I've got the solution now: We just
>> maintain project--list in abbreviated form and project-list-file in
>> expanded form.

Here's something else I thought of: project--write-project-list is 
called every time a different project is returned by (project-current t) 
-- i.e. when MAYBE-PROMPT=t, and when the found dir is not at the top of 
the project history already.

So under certain conditions (slow CPU/spinning media/long project 
history) it might slow down (project-current t) calls. In my testing 
it's totally fine, though, and there are possible mitigations (limit the 
history and/or move saving to kill-hook). Just something to keep in mind.

>> I think that solves all the problems: the persistent file is independent
>> of user configuration, so if the user changes their abbreviations
>> they'll get new ones the next time the file is read (presumably the next
>> time they run Emacs).  But project--list is abbreviated, so
>> project-prompt-project-dir gets abbreviated dirs.
>>
>> It does mean that if they change their directory-abbrev-alist while
>> Emacs is running they could get duplicate entries, but that already
>> could happen before this patch, and it's not a big problem IMO.
>>
>> See patch below.
> Oops, this patch needed a few tweaks.  Especially, skipping the
> expansion and abbreviation for remote files, to avoid making unnecesary
> remote connections.

Thanks! Pushed to master, and finally closing ;-)





^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2023-08-23  0:24 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-15 20:48 bug#64088: [PATCH] Make project-root for vc projects return an absolute path Spencer Baugh
2023-06-16  5:52 ` Eli Zaretskii
2023-06-17  2:43   ` Dmitry Gutov
2023-06-17  6:25     ` Eli Zaretskii
2023-06-18  0:43       ` Dmitry Gutov
2023-06-17  2:47 ` Dmitry Gutov
2023-06-27 20:01   ` Spencer Baugh
2023-07-10  2:17     ` Dmitry Gutov
2023-07-10 23:45       ` Spencer Baugh
2023-08-12  0:45         ` Dmitry Gutov
2023-08-19 12:31           ` sbaugh
2023-08-19 12:57             ` sbaugh
2023-08-23  0:24               ` Dmitry Gutov

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).