GNU bug report logs - #66115
[PATCH] Ensure that directory is expanded in package-vc-checkout

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 66115 in the body.
You can then email your comments to 66115 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Joseph Turner <joseph <at> breatheoutbreathe.in>
To: bug-gnu-emacs <at> gnu.org
Subject: [PATCH] Ensure that directory is expanded in package-vc-checkout
Date: Wed, 20 Sep 2023 00:14:02 -0700

[Message part 1 (text/plain, inline)]

Tags: patch

Tags: patch

This patch fixes a serious bug which caused package-vc-checkout to fail
entirely when its second argument `directory' was not expanded.

[0001-Ensure-that-directory-is-expanded-in-package-vc-chec.patch (text/patch, attachment)]

Message #8 received at 66115 <at> debbugs.gnu.org (full text, mbox):

From: Philip Kaludercic <philipk <at> posteo.net>
To: Joseph Turner <joseph <at> breatheoutbreathe.in>
Cc: 66115 <at> debbugs.gnu.org
Subject: Re: bug#66115: [PATCH] Ensure that directory is expanded in
 package-vc-checkout
Date: Fri, 22 Sep 2023 08:17:46 +0000

[Message part 1 (text/plain, inline)]

Joseph Turner <joseph <at> breatheoutbreathe.in> writes:

> Tags: patch
>
> Tags: patch
>
> This patch fixes a serious bug which caused package-vc-checkout to fail
> entirely when its second argument `directory' was not expanded.
>
> From 75a6ec74ff5e31edc7592b68e7ad9d1ec26f515c Mon Sep 17 00:00:00 2001
> From: Joseph Turner <joseph <at> breatheoutbreathe.in>
> Date: Wed, 20 Sep 2023 00:12:10 -0700
> Subject: [PATCH] Ensure that directory is expanded in package-vc-checkout
>
> * lisp/emacs-lisp/package-vc.el (package-vc-checkout)
> ---
>  lisp/emacs-lisp/package-vc.el | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/lisp/emacs-lisp/package-vc.el b/lisp/emacs-lisp/package-vc.el
> index 29b540d86b8..47d8c996ff0 100644
> --- a/lisp/emacs-lisp/package-vc.el
> +++ b/lisp/emacs-lisp/package-vc.el
> @@ -834,6 +834,7 @@ for the last released version of the package."
>               (setf dir subdir)
>             (user-error "Directory not empty: %S" (expand-file-name dir)))))
>       (list desc dir (and current-prefix-arg :last-release))))
> +  (setf directory (expand-file-name directory))
>    (package-vc--archives-initialize)
>    (let ((pkg-spec (or (package-vc--desc->spec pkg-desc)
>                        (and-let* ((extras (package-desc-extras pkg-desc))

DIRECTORY is passed on to `package-vc--clone' and in turn `vc-clone'.
Wouldn't it be more robust to ensure that `vc-clone' can handle
unexpanded directory names, eg. like this:

[Message part 2 (text/plain, inline)]

diff --git a/lisp/vc/vc.el b/lisp/vc/vc.el
index 2f4b028bb4a..7f334397a5e 100644
--- a/lisp/vc/vc.el
+++ b/lisp/vc/vc.el
@@ -3676,8 +3676,7 @@ vc-clone
 checkout.  If BACKEND is nil, iterate through every known backend
 in `vc-handled-backends' until one succeeds.  If REV is non-nil,
 it indicates a specific revision to check out."
-  (unless directory
-    (setq directory default-directory))
+  (setq directory (expand-file-name (or directory default-directory)))
   (if backend
       (progn
         (unless (memq backend vc-handled-backends)

Message #11 received at 66115 <at> debbugs.gnu.org (full text, mbox):

From: Joseph Turner <joseph <at> breatheoutbreathe.in>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: 66115 <at> debbugs.gnu.org
Subject: Re: bug#66115: [PATCH] Ensure that directory is expanded in
 package-vc-checkout
Date: Fri, 22 Sep 2023 01:33:45 -0700

Philip Kaludercic <philipk <at> posteo.net> writes:

> Joseph Turner <joseph <at> breatheoutbreathe.in> writes:
>
>> Tags: patch
>>
>> Tags: patch
>>
>> This patch fixes a serious bug which caused package-vc-checkout to fail
>> entirely when its second argument `directory' was not expanded.
>>
>> From 75a6ec74ff5e31edc7592b68e7ad9d1ec26f515c Mon Sep 17 00:00:00 2001
>> From: Joseph Turner <joseph <at> breatheoutbreathe.in>
>> Date: Wed, 20 Sep 2023 00:12:10 -0700
>> Subject: [PATCH] Ensure that directory is expanded in package-vc-checkout
>>
>> * lisp/emacs-lisp/package-vc.el (package-vc-checkout)
>> ---
>>  lisp/emacs-lisp/package-vc.el | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/lisp/emacs-lisp/package-vc.el b/lisp/emacs-lisp/package-vc.el
>> index 29b540d86b8..47d8c996ff0 100644
>> --- a/lisp/emacs-lisp/package-vc.el
>> +++ b/lisp/emacs-lisp/package-vc.el
>> @@ -834,6 +834,7 @@ for the last released version of the package."
>>               (setf dir subdir)
>>             (user-error "Directory not empty: %S" (expand-file-name dir)))))
>>       (list desc dir (and current-prefix-arg :last-release))))
>> +  (setf directory (expand-file-name directory))
>>    (package-vc--archives-initialize)
>>    (let ((pkg-spec (or (package-vc--desc->spec pkg-desc)
>>                        (and-let* ((extras (package-desc-extras pkg-desc))
>
> DIRECTORY is passed on to `package-vc--clone' and in turn `vc-clone'.
> Wouldn't it be more robust to ensure that `vc-clone' can handle
> unexpanded directory names, eg. like this:
>
> diff --git a/lisp/vc/vc.el b/lisp/vc/vc.el
> index 2f4b028bb4a..7f334397a5e 100644
> --- a/lisp/vc/vc.el
> +++ b/lisp/vc/vc.el
> @@ -3676,8 +3676,7 @@ vc-clone
>  checkout.  If BACKEND is nil, iterate through every known backend
>  in `vc-handled-backends' until one succeeds.  If REV is non-nil,
>  it indicates a specific revision to check out."
> -  (unless directory
> -    (setq directory default-directory))
> +  (setq directory (expand-file-name (or directory default-directory)))
>    (if backend
>        (progn
>          (unless (memq backend vc-handled-backends)

Yes, certainly. Your solution is better.

Message #16 received at 66115-done <at> debbugs.gnu.org (full text, mbox):

From: Philip Kaludercic <philipk <at> posteo.net>
To: Joseph Turner <joseph <at> breatheoutbreathe.in>
Cc: 66115-done <at> debbugs.gnu.org
Subject: Re: bug#66115: [PATCH] Ensure that directory is expanded in
 package-vc-checkout
Date: Fri, 22 Sep 2023 10:51:13 +0000

Joseph Turner <joseph <at> breatheoutbreathe.in> writes:

> Philip Kaludercic <philipk <at> posteo.net> writes:
>
>> Joseph Turner <joseph <at> breatheoutbreathe.in> writes:
>>
>>> Tags: patch
>>>
>>> Tags: patch
>>>
>>> This patch fixes a serious bug which caused package-vc-checkout to fail
>>> entirely when its second argument `directory' was not expanded.
>>>
>>> From 75a6ec74ff5e31edc7592b68e7ad9d1ec26f515c Mon Sep 17 00:00:00 2001
>>> From: Joseph Turner <joseph <at> breatheoutbreathe.in>
>>> Date: Wed, 20 Sep 2023 00:12:10 -0700
>>> Subject: [PATCH] Ensure that directory is expanded in package-vc-checkout
>>>
>>> * lisp/emacs-lisp/package-vc.el (package-vc-checkout)
>>> ---
>>>  lisp/emacs-lisp/package-vc.el | 1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff --git a/lisp/emacs-lisp/package-vc.el b/lisp/emacs-lisp/package-vc.el
>>> index 29b540d86b8..47d8c996ff0 100644
>>> --- a/lisp/emacs-lisp/package-vc.el
>>> +++ b/lisp/emacs-lisp/package-vc.el
>>> @@ -834,6 +834,7 @@ for the last released version of the package."
>>>               (setf dir subdir)
>>>             (user-error "Directory not empty: %S" (expand-file-name dir)))))
>>>       (list desc dir (and current-prefix-arg :last-release))))
>>> +  (setf directory (expand-file-name directory))
>>>    (package-vc--archives-initialize)
>>>    (let ((pkg-spec (or (package-vc--desc->spec pkg-desc)
>>>                        (and-let* ((extras (package-desc-extras pkg-desc))
>>
>> DIRECTORY is passed on to `package-vc--clone' and in turn `vc-clone'.
>> Wouldn't it be more robust to ensure that `vc-clone' can handle
>> unexpanded directory names, eg. like this:
>>
>> diff --git a/lisp/vc/vc.el b/lisp/vc/vc.el
>> index 2f4b028bb4a..7f334397a5e 100644
>> --- a/lisp/vc/vc.el
>> +++ b/lisp/vc/vc.el
>> @@ -3676,8 +3676,7 @@ vc-clone
>>  checkout.  If BACKEND is nil, iterate through every known backend
>>  in `vc-handled-backends' until one succeeds.  If REV is non-nil,
>>  it indicates a specific revision to check out."
>> -  (unless directory
>> -    (setq directory default-directory))
>> +  (setq directory (expand-file-name (or directory default-directory)))
>>    (if backend
>>        (progn
>>          (unless (memq backend vc-handled-backends)
>
> Yes, certainly. Your solution is better.

OK, closing the issue.

Message #19 received at 66115-done <at> debbugs.gnu.org (full text, mbox):

From: Joseph Turner <joseph <at> breatheoutbreathe.in>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: 66115-done <at> debbugs.gnu.org
Subject: Re: bug#66115: [PATCH] Ensure that directory is expanded in
 package-vc-checkout
Date: Sat, 23 Sep 2023 22:34:24 -0700

It appears that the fix was applied to master. Would it make sense to
apply it to emacs-29?

Message #22 received at 66115 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Joseph Turner <joseph <at> breatheoutbreathe.in>
Cc: 66115 <at> debbugs.gnu.org, philipk <at> posteo.net
Subject: Re: bug#66115: [PATCH] Ensure that directory is expanded in
 package-vc-checkout
Date: Sun, 24 Sep 2023 09:54:38 +0300

> Cc: 66115-done <at> debbugs.gnu.org
> Date: Sat, 23 Sep 2023 22:34:24 -0700
> From:  Joseph Turner via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs <at> gnu.org>
> 
> It appears that the fix was applied to master. Would it make sense to
> apply it to emacs-29?

The installed change affects much more than the original issue, so I'd
prefer for it to stay on master.

I'm okay with installing the original patch on emacs-29, but be sure
to say "do not merge to master" in the commit log message if you do.

Message #31 received at 66115 <at> debbugs.gnu.org (full text, mbox):

From: Philip Kaludercic <philipk <at> posteo.net>
To: Joseph Turner <joseph <at> breatheoutbreathe.in>
Cc: 66115 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#66115: [PATCH] Ensure that directory is expanded in
 package-vc-checkout
Date: Thu, 16 Nov 2023 06:44:07 +0000

Joseph Turner <joseph <at> breatheoutbreathe.in> writes:

> Eli Zaretskii <eliz <at> gnu.org> writes:
>
>>> Cc: 66115-done <at> debbugs.gnu.org
>>> Date: Sat, 23 Sep 2023 22:34:24 -0700
>>> From:  Joseph Turner via "Bug reports for GNU Emacs,
>>>  the Swiss army knife of text editors" <bug-gnu-emacs <at> gnu.org>
>>>
>>> It appears that the fix was applied to master. Would it make sense to
>>> apply it to emacs-29?
>>
>> The installed change affects much more than the original issue, so I'd
>> prefer for it to stay on master.
>>
>> I'm okay with installing the original patch on emacs-29, but be sure
>> to say "do not merge to master" in the commit log message if you do.
>
> Philip, shall we merge the original patch on emacs-29 with "do not merge
> to master"?

I don't object.

> Thank you!!
>
> Joseph

Message #34 received at 66115 <at> debbugs.gnu.org (full text, mbox):

From: Joseph Turner <joseph <at> breatheoutbreathe.in>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: 66115 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#66115: [PATCH] Ensure that directory is expanded in
 package-vc-checkout
Date: Sun, 19 Nov 2023 14:16:39 -0800

[Message part 1 (text/plain, inline)]

Philip Kaludercic <philipk <at> posteo.net> writes:

> Joseph Turner <joseph <at> breatheoutbreathe.in> writes:
>
>> Eli Zaretskii <eliz <at> gnu.org> writes:
>>
>>>> Cc: 66115-done <at> debbugs.gnu.org
>>>> Date: Sat, 23 Sep 2023 22:34:24 -0700
>>>> From:  Joseph Turner via "Bug reports for GNU Emacs,
>>>>  the Swiss army knife of text editors" <bug-gnu-emacs <at> gnu.org>
>>>>
>>>> It appears that the fix was applied to master. Would it make sense to
>>>> apply it to emacs-29?
>>>
>>> The installed change affects much more than the original issue, so I'd
>>> prefer for it to stay on master.
>>>
>>> I'm okay with installing the original patch on emacs-29, but be sure
>>> to say "do not merge to master" in the commit log message if you do.
>>
>> Philip, shall we merge the original patch on emacs-29 with "do not merge
>> to master"?
>
> I don't object.

Would you be willing to merge the attached patch?

Thank you!!

Joseph

[0001-Ensure-that-directory-is-expanded-in-package-vc-chec.patch (text/x-diff, attachment)]

Message #37 received at 66115 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Joseph Turner <joseph <at> breatheoutbreathe.in>
Cc: 66115 <at> debbugs.gnu.org, philipk <at> posteo.net
Subject: Re: bug#66115: [PATCH] Ensure that directory is expanded in
 package-vc-checkout
Date: Sat, 25 Nov 2023 12:17:04 +0200

> From: Joseph Turner <joseph <at> breatheoutbreathe.in>
> Cc: Eli Zaretskii <eliz <at> gnu.org>, 66115 <at> debbugs.gnu.org
> Date: Sun, 19 Nov 2023 14:16:39 -0800
> 
> Philip Kaludercic <philipk <at> posteo.net> writes:
> 
> > Joseph Turner <joseph <at> breatheoutbreathe.in> writes:
> >
> >> Eli Zaretskii <eliz <at> gnu.org> writes:
> >>
> >>>> Cc: 66115-done <at> debbugs.gnu.org
> >>>> Date: Sat, 23 Sep 2023 22:34:24 -0700
> >>>> From:  Joseph Turner via "Bug reports for GNU Emacs,
> >>>>  the Swiss army knife of text editors" <bug-gnu-emacs <at> gnu.org>
> >>>>
> >>>> It appears that the fix was applied to master. Would it make sense to
> >>>> apply it to emacs-29?
> >>>
> >>> The installed change affects much more than the original issue, so I'd
> >>> prefer for it to stay on master.
> >>>
> >>> I'm okay with installing the original patch on emacs-29, but be sure
> >>> to say "do not merge to master" in the commit log message if you do.
> >>
> >> Philip, shall we merge the original patch on emacs-29 with "do not merge
> >> to master"?
> >
> > I don't object.
> 
> Would you be willing to merge the attached patch?

I tried, but it doesn't apply to emacs-29.  Would you please submit a
change relative to the emacs-29 branch?

TIA

Message #40 received at 66115 <at> debbugs.gnu.org (full text, mbox):

From: Joseph Turner <joseph <at> breatheoutbreathe.in>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 66115 <at> debbugs.gnu.org, philipk <at> posteo.net
Subject: Re: bug#66115: [PATCH] Ensure that directory is expanded in
 package-vc-checkout
Date: Sat, 25 Nov 2023 14:59:46 -0800

[Message part 1 (text/plain, inline)]

Eli Zaretskii <eliz <at> gnu.org> writes:

> From: Joseph Turner <joseph <at> breatheoutbreathe.in>
>> Would you be willing to merge the attached patch?
>
> I tried, but it doesn't apply to emacs-29.  Would you please submit a
> change relative to the emacs-29 branch?

Sorry about that!  Here you go!

Thanks,

Joseph

[0001-Ensure-that-directory-is-expanded-in-package-vc-chec.patch (text/x-diff, attachment)]

Message #45 received at 66115-done <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Joseph Turner <joseph <at> breatheoutbreathe.in>
Cc: 66115-done <at> debbugs.gnu.org, philipk <at> posteo.net
Subject: Re: bug#66115: [PATCH] Ensure that directory is expanded in
 package-vc-checkout
Date: Sun, 26 Nov 2023 12:41:18 +0200

> From: Joseph Turner <joseph <at> breatheoutbreathe.in>
> Cc: philipk <at> posteo.net, 66115 <at> debbugs.gnu.org
> Date: Sat, 25 Nov 2023 14:59:46 -0800
> 
> Eli Zaretskii <eliz <at> gnu.org> writes:
> 
> > From: Joseph Turner <joseph <at> breatheoutbreathe.in>
> >> Would you be willing to merge the attached patch?
> >
> > I tried, but it doesn't apply to emacs-29.  Would you please submit a
> > change relative to the emacs-29 branch?
> 
> Sorry about that!  Here you go!

Thanks, installed on the emacs-29 branch, and closing the bug.

Message #48 received at 66115-done <at> debbugs.gnu.org (full text, mbox):

From: Joseph Turner <joseph <at> breatheoutbreathe.in>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 66115-done <at> debbugs.gnu.org, philipk <at> posteo.net
Subject: Re: bug#66115: [PATCH] Ensure that directory is expanded in
 package-vc-checkout
Date: Sun, 26 Nov 2023 12:39:34 -0800

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Joseph Turner <joseph <at> breatheoutbreathe.in>
>> Cc: philipk <at> posteo.net, 66115 <at> debbugs.gnu.org
>> Date: Sat, 25 Nov 2023 14:59:46 -0800
>>
>> Eli Zaretskii <eliz <at> gnu.org> writes:
>>
>> > From: Joseph Turner <joseph <at> breatheoutbreathe.in>
>> >> Would you be willing to merge the attached patch?
>> >
>> > I tried, but it doesn't apply to emacs-29.  Would you please submit a
>> > change relative to the emacs-29 branch?
>>
>> Sorry about that!  Here you go!
>
> Thanks, installed on the emacs-29 branch, and closing the bug.

Thank you, Eli!  I made a mistake - I forgot to add "Do not merge into
master" in the commit message.  Do we need to add a commit on the master
branch which reverts this one?

Joseph

Message #51 received at 66115-done <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Joseph Turner <joseph <at> breatheoutbreathe.in>
Cc: 66115-done <at> debbugs.gnu.org, philipk <at> posteo.net
Subject: Re: bug#66115: [PATCH] Ensure that directory is expanded in
 package-vc-checkout
Date: Mon, 27 Nov 2023 14:01:19 +0200

> From: Joseph Turner <joseph <at> breatheoutbreathe.in>
> Cc: philipk <at> posteo.net, 66115-done <at> debbugs.gnu.org
> Date: Sun, 26 Nov 2023 12:39:34 -0800
> 
> > Thanks, installed on the emacs-29 branch, and closing the bug.
> 
> Thank you, Eli!  I made a mistake - I forgot to add "Do not merge into
> master" in the commit message.  Do we need to add a commit on the master
> branch which reverts this one?

Yes, after the next merge from emacs-29 to master.

Message #54 received at 66115-done <at> debbugs.gnu.org (full text, mbox):

From: Joseph Turner <joseph <at> breatheoutbreathe.in>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 66115-done <at> debbugs.gnu.org, philipk <at> posteo.net
Subject: Re: bug#66115: [PATCH] Ensure that directory is expanded in
 package-vc-checkout
Date: Fri, 08 Dec 2023 02:25:19 -0800

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Joseph Turner <joseph <at> breatheoutbreathe.in>
>> Cc: philipk <at> posteo.net, 66115-done <at> debbugs.gnu.org
>> Date: Sun, 26 Nov 2023 12:39:34 -0800
>>
>> > Thanks, installed on the emacs-29 branch, and closing the bug.
>>
>> Thank you, Eli!  I made a mistake - I forgot to add "Do not merge into
>> master" in the commit message.  Do we need to add a commit on the master
>> branch which reverts this one?
>
> Yes, after the next merge from emacs-29 to master.

I see that you've done this already.  Thank you!

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.