* bug#64242: [PATCH] Fix VC package build when there is no docs dir
@ 2023-06-23 5:45 Daniel Semyonov via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-06-23 6:02 ` Eli Zaretskii
0 siblings, 1 reply; 10+ messages in thread
From: Daniel Semyonov via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2023-06-23 5:45 UTC (permalink / raw)
To: 64242
[-- Attachment #1: Type: text/plain, Size: 684 bytes --]
Tags: patch
This fixes a small mistake introduced recently in
'package-vc--build-documentation' which causes it to error out when its
'file' argument isn't a directory.
In GNU Emacs 30.0.50 (build 4, x86_64-pc-linux-gnu, GTK+ Version
3.24.38, cairo version 1.16.0) of 2023-06-21 built on coldharbour
Repository revision: 6085ee8139cc3d815a5028babb4daf438df9d06b
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12101008
System Description: Void Linux
Configured using:
'configure -C --prefix=/opt/Emacs --with-x --with-x-toolkit=gtk3
--without-gsettings --without-dbus --with-xinput2 --with-small-ja-dic
--with-native-compilation'
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fix-VC-package-build-when-there-is-no-docs-dir.patch --]
[-- Type: text/patch, Size: 1184 bytes --]
From b21139b7d900d6c3bd38b2790758d2c1b631ada3 Mon Sep 17 00:00:00 2001
From: Daniel Semyonov <daniel@dsemy.com>
Date: Fri, 23 Jun 2023 08:40:57 +0300
Subject: [PATCH] Fix VC package build when there is no docs dir
* lisp/emacs-lisp/package-vc.el (package-vc--build-documentation): Set
'docs-directory' to 'default-directory' if 'file' isn't a directory.
---
lisp/emacs-lisp/package-vc.el | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/lisp/emacs-lisp/package-vc.el b/lisp/emacs-lisp/package-vc.el
index f34cfb3120b..b87bcbc738e 100644
--- a/lisp/emacs-lisp/package-vc.el
+++ b/lisp/emacs-lisp/package-vc.el
@@ -386,7 +386,8 @@ package-vc--build-documentation
otherwise it's assumed to be an Info file."
(let* ((pkg-name (package-desc-name pkg-desc))
(default-directory (package-desc-dir pkg-desc))
- (docs-directory (expand-file-name (file-name-directory file)))
+ (docs-directory (if (file-name-directory file)
+ (expand-file-name file) default-directory))
(output (expand-file-name (format "%s.info" pkg-name)))
clean-up)
(when (string-match-p "\\.org\\'" file)
--
2.41.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* bug#64242: [PATCH] Fix VC package build when there is no docs dir
2023-06-23 5:45 bug#64242: [PATCH] Fix VC package build when there is no docs dir Daniel Semyonov via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2023-06-23 6:02 ` Eli Zaretskii
2023-06-23 6:32 ` Daniel Semyonov via Bug reports for GNU Emacs, the Swiss army knife of text editors
0 siblings, 1 reply; 10+ messages in thread
From: Eli Zaretskii @ 2023-06-23 6:02 UTC (permalink / raw)
To: Daniel Semyonov; +Cc: 64242
> Date: Fri, 23 Jun 2023 08:45:03 +0300
> From: Daniel Semyonov via "Bug reports for GNU Emacs,
> the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
>
> This fixes a small mistake introduced recently in
> 'package-vc--build-documentation' which causes it to error out when its
> 'file' argument isn't a directory.
What do you mean by "isn't a directory"? FILE is supposed to be a
documentation file, not a directory, see the doc string of this
function. Do you mean that FILE is a relative file name so that
file-name-directory returns nil for it? If so, I think the correct
fix would be to reverse the order:
(docs-directory (file-name-directory (expand-file-name file)))
Can you show a recipe to reproduce this problem, preferably starting
from "emacs -Q"?
And finally, this issue exists on the emacs-29 release branch as well,
doesn't it?
Thanks.
^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#64242: [PATCH] Fix VC package build when there is no docs dir
2023-06-23 6:02 ` Eli Zaretskii
@ 2023-06-23 6:32 ` Daniel Semyonov via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-06-23 7:10 ` Eli Zaretskii
0 siblings, 1 reply; 10+ messages in thread
From: Daniel Semyonov via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2023-06-23 6:32 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 64242
[-- Attachment #1: Type: text/plain, Size: 1224 bytes --]
>>>>> Eli Zaretskii writes:
> What do you mean by "isn't a directory"? FILE is supposed to be a
> documentation file, not a directory, see the doc string of this
> function. Do you mean that FILE is a relative file name so that
> file-name-directory returns nil for it? If so, I think the
> correct fix would be to reverse the order:
> (docs-directory (file-name-directory (expand-file-name file)))
Yeah, you're completely right, this was a misunderstanding on my part;
my fix just happened to also work so I made some incorrect assumptions.
After some further testing, this seems to happen for packages for which
the doc file is in the base directory of the package (which causes
'file-name-directory' to return nil as the doc file name is relative to
the package dir).
I attached an updated patch.
> Can you show a recipe to reproduce this problem, preferably
> starting from "emacs -Q"?
1. $ emacs -Q
2. M-x package-vc-install RET eat RET
3. A few seconds later,
"package-vc--unpack-1: Wrong type argument: stringp, nil"
> And finally, this issue exists on the emacs-29 release branch as
> well, doesn't it?
I haven't tested it, but I don't see a reason it wont.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: patch --]
[-- Type: text/x-patch, Size: 1111 bytes --]
From 0160171b1663bf349af7ffe6de0b2ea6b4b326a7 Mon Sep 17 00:00:00 2001
From: Daniel Semyonov <daniel@dsemy.com>
Date: Fri, 23 Jun 2023 08:40:57 +0300
Subject: [PATCH] Fix VC package build when doc file isn't in a subdir
* lisp/emacs-lisp/package-vc.el (package-vc--build-documentation):
Expand 'file' before attempting to get its directory.
---
lisp/emacs-lisp/package-vc.el | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lisp/emacs-lisp/package-vc.el b/lisp/emacs-lisp/package-vc.el
index f34cfb3120b..db8b41aee6a 100644
--- a/lisp/emacs-lisp/package-vc.el
+++ b/lisp/emacs-lisp/package-vc.el
@@ -386,7 +386,7 @@ package-vc--build-documentation
otherwise it's assumed to be an Info file."
(let* ((pkg-name (package-desc-name pkg-desc))
(default-directory (package-desc-dir pkg-desc))
- (docs-directory (expand-file-name (file-name-directory file)))
+ (docs-directory (file-name-directory (expand-file-name file)))
(output (expand-file-name (format "%s.info" pkg-name)))
clean-up)
(when (string-match-p "\\.org\\'" file)
--
2.41.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* bug#64242: [PATCH] Fix VC package build when there is no docs dir
2023-06-23 6:32 ` Daniel Semyonov via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2023-06-23 7:10 ` Eli Zaretskii
2023-06-23 7:35 ` Philip Kaludercic
0 siblings, 1 reply; 10+ messages in thread
From: Eli Zaretskii @ 2023-06-23 7:10 UTC (permalink / raw)
To: Daniel Semyonov, Philip Kaludercic; +Cc: 64242
> From: Daniel Semyonov <daniel@dsemy.com>
> Cc: 64242@debbugs.gnu.org
> Date: Fri, 23 Jun 2023 09:32:01 +0300
>
> >>>>> Eli Zaretskii writes:
>
> > What do you mean by "isn't a directory"? FILE is supposed to be a
> > documentation file, not a directory, see the doc string of this
> > function. Do you mean that FILE is a relative file name so that
> > file-name-directory returns nil for it? If so, I think the
> > correct fix would be to reverse the order:
>
> > (docs-directory (file-name-directory (expand-file-name file)))
>
> Yeah, you're completely right, this was a misunderstanding on my part;
> my fix just happened to also work so I made some incorrect assumptions.
>
> After some further testing, this seems to happen for packages for which
> the doc file is in the base directory of the package (which causes
> 'file-name-directory' to return nil as the doc file name is relative to
> the package dir).
>
> I attached an updated patch.
>
> > Can you show a recipe to reproduce this problem, preferably
> > starting from "emacs -Q"?
>
> 1. $ emacs -Q
> 2. M-x package-vc-install RET eat RET
> 3. A few seconds later,
> "package-vc--unpack-1: Wrong type argument: stringp, nil"
>
> > And finally, this issue exists on the emacs-29 release branch as
> > well, doesn't it?
>
> I haven't tested it, but I don't see a reason it wont.
Philip, any comments or objections to installing this on the emacs-29
branch?
Thanks.
^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#64242: [PATCH] Fix VC package build when there is no docs dir
2023-06-23 7:10 ` Eli Zaretskii
@ 2023-06-23 7:35 ` Philip Kaludercic
2023-06-24 10:24 ` Philip Kaludercic
0 siblings, 1 reply; 10+ messages in thread
From: Philip Kaludercic @ 2023-06-23 7:35 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 64242, Daniel Semyonov
Eli Zaretskii <eliz@gnu.org> writes:
>> From: Daniel Semyonov <daniel@dsemy.com>
>> Cc: 64242@debbugs.gnu.org
>> Date: Fri, 23 Jun 2023 09:32:01 +0300
>>
>> >>>>> Eli Zaretskii writes:
>>
>> > What do you mean by "isn't a directory"? FILE is supposed to be a
>> > documentation file, not a directory, see the doc string of this
>> > function. Do you mean that FILE is a relative file name so that
>> > file-name-directory returns nil for it? If so, I think the
>> > correct fix would be to reverse the order:
>>
>> > (docs-directory (file-name-directory (expand-file-name file)))
>>
>> Yeah, you're completely right, this was a misunderstanding on my part;
>> my fix just happened to also work so I made some incorrect assumptions.
>>
>> After some further testing, this seems to happen for packages for which
>> the doc file is in the base directory of the package (which causes
>> 'file-name-directory' to return nil as the doc file name is relative to
>> the package dir).
>>
>> I attached an updated patch.
>>
>> > Can you show a recipe to reproduce this problem, preferably
>> > starting from "emacs -Q"?
>>
>> 1. $ emacs -Q
>> 2. M-x package-vc-install RET eat RET
>> 3. A few seconds later,
>> "package-vc--unpack-1: Wrong type argument: stringp, nil"
>>
>> > And finally, this issue exists on the emacs-29 release branch as
>> > well, doesn't it?
>>
>> I haven't tested it, but I don't see a reason it wont.
>
> Philip, any comments or objections to installing this on the emacs-29
> branch?
I have none. There were just come complains last time, because we made
the change too soon, so I believe the consensus was to wait for a bit
longer and then it was just forgotten.
> Thanks.
^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#64242: [PATCH] Fix VC package build when there is no docs dir
2023-06-23 7:35 ` Philip Kaludercic
@ 2023-06-24 10:24 ` Philip Kaludercic
2023-06-24 10:43 ` Eli Zaretskii
0 siblings, 1 reply; 10+ messages in thread
From: Philip Kaludercic @ 2023-06-24 10:24 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 64242, Daniel Semyonov
Philip Kaludercic <philipk@posteo.net> writes:
> Eli Zaretskii <eliz@gnu.org> writes:
>
>>> From: Daniel Semyonov <daniel@dsemy.com>
>>> Cc: 64242@debbugs.gnu.org
>>> Date: Fri, 23 Jun 2023 09:32:01 +0300
>>>
>>> >>>>> Eli Zaretskii writes:
>>>
>>> > What do you mean by "isn't a directory"? FILE is supposed to be a
>>> > documentation file, not a directory, see the doc string of this
>>> > function. Do you mean that FILE is a relative file name so that
>>> > file-name-directory returns nil for it? If so, I think the
>>> > correct fix would be to reverse the order:
>>>
>>> > (docs-directory (file-name-directory (expand-file-name file)))
>>>
>>> Yeah, you're completely right, this was a misunderstanding on my part;
>>> my fix just happened to also work so I made some incorrect assumptions.
>>>
>>> After some further testing, this seems to happen for packages for which
>>> the doc file is in the base directory of the package (which causes
>>> 'file-name-directory' to return nil as the doc file name is relative to
>>> the package dir).
>>>
>>> I attached an updated patch.
>>>
>>> > Can you show a recipe to reproduce this problem, preferably
>>> > starting from "emacs -Q"?
>>>
>>> 1. $ emacs -Q
>>> 2. M-x package-vc-install RET eat RET
>>> 3. A few seconds later,
>>> "package-vc--unpack-1: Wrong type argument: stringp, nil"
>>>
>>> > And finally, this issue exists on the emacs-29 release branch as
>>> > well, doesn't it?
>>>
>>> I haven't tested it, but I don't see a reason it wont.
>>
>> Philip, any comments or objections to installing this on the emacs-29
>> branch?
>
> I have none. There were just come complains last time, because we made
> the change too soon, so I believe the consensus was to wait for a bit
> longer and then it was just forgotten.
Oops, I misread this thread and was under the assumption you were
responding to a different message. Sorry about that! Shouldn't write
messages when I am in a hurry.
Regarding Daniel's patch, I don't believe we need `file-name-directory'
at all, the variable `docs-directory' is just passed to makeinfo -I, and to
my knowledge it should make a difference if the argument ends with a
directory delimiter or not.
--
Philip Kaludercic
^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#64242: [PATCH] Fix VC package build when there is no docs dir
2023-06-24 10:24 ` Philip Kaludercic
@ 2023-06-24 10:43 ` Eli Zaretskii
2023-06-24 15:34 ` Philip Kaludercic
0 siblings, 1 reply; 10+ messages in thread
From: Eli Zaretskii @ 2023-06-24 10:43 UTC (permalink / raw)
To: Philip Kaludercic; +Cc: 64242, daniel
> From: Philip Kaludercic <philipk@posteo.net>
> Cc: 64242@debbugs.gnu.org, Daniel Semyonov <daniel@dsemy.com>
> Date: Sat, 24 Jun 2023 10:24:47 +0000
>
> Regarding Daniel's patch, I don't believe we need `file-name-directory'
> at all, the variable `docs-directory' is just passed to makeinfo -I, and to
> my knowledge it should make a difference if the argument ends with a
> directory delimiter or not.
Are you thinking about directory-file-name instead of
file-name-directory? The latter returns the parent directory of its
argument, it doesn't affect the trailing slash. The FILE argument
passed to package-vc--build-documentation is a file, not a directory,
so the file-name-directory call seems justified if we want to pass the
directory to makeinfo.
Or what am I missing?
^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#64242: [PATCH] Fix VC package build when there is no docs dir
2023-06-24 10:43 ` Eli Zaretskii
@ 2023-06-24 15:34 ` Philip Kaludercic
2023-06-24 17:07 ` Eli Zaretskii
0 siblings, 1 reply; 10+ messages in thread
From: Philip Kaludercic @ 2023-06-24 15:34 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 64242, daniel
Eli Zaretskii <eliz@gnu.org> writes:
>> From: Philip Kaludercic <philipk@posteo.net>
>> Cc: 64242@debbugs.gnu.org, Daniel Semyonov <daniel@dsemy.com>
>> Date: Sat, 24 Jun 2023 10:24:47 +0000
>>
>> Regarding Daniel's patch, I don't believe we need `file-name-directory'
>> at all, the variable `docs-directory' is just passed to makeinfo -I, and to
>> my knowledge it should make a difference if the argument ends with a
>> directory delimiter or not.
>
> Are you thinking about directory-file-name instead of
> file-name-directory?
Eh, yes of course I didn't try the patch and expected that to fail.
> The latter returns the parent directory of its
> argument, it doesn't affect the trailing slash. The FILE argument
> passed to package-vc--build-documentation is a file, not a directory,
> so the file-name-directory call seems justified if we want to pass the
> directory to makeinfo.
Right, my bad.
> Or what am I missing?
No, it is all my fault. I'll try the patch our and if it all works I'll
push it to emacs-29. Ok?
--
Philip Kaludercic
^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#64242: [PATCH] Fix VC package build when there is no docs dir
2023-06-24 15:34 ` Philip Kaludercic
@ 2023-06-24 17:07 ` Eli Zaretskii
2023-06-25 21:41 ` Philip Kaludercic
0 siblings, 1 reply; 10+ messages in thread
From: Eli Zaretskii @ 2023-06-24 17:07 UTC (permalink / raw)
To: Philip Kaludercic; +Cc: 64242, daniel
> From: Philip Kaludercic <philipk@posteo.net>
> Cc: 64242@debbugs.gnu.org, daniel@dsemy.com
> Date: Sat, 24 Jun 2023 15:34:40 +0000
>
> I'll try the patch our and if it all works I'll push it to emacs-29.
> Ok?
Yes, please.
^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#64242: [PATCH] Fix VC package build when there is no docs dir
2023-06-24 17:07 ` Eli Zaretskii
@ 2023-06-25 21:41 ` Philip Kaludercic
0 siblings, 0 replies; 10+ messages in thread
From: Philip Kaludercic @ 2023-06-25 21:41 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 64242-done, daniel
Eli Zaretskii <eliz@gnu.org> writes:
>> From: Philip Kaludercic <philipk@posteo.net>
>> Cc: 64242@debbugs.gnu.org, daniel@dsemy.com
>> Date: Sat, 24 Jun 2023 15:34:40 +0000
>>
>> I'll try the patch our and if it all works I'll push it to emacs-29.
>> Ok?
>
> Yes, please.
OK, I have tested it and pushed the patch. Thanks Daniel!
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2023-06-25 21:41 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-06-23 5:45 bug#64242: [PATCH] Fix VC package build when there is no docs dir Daniel Semyonov via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-06-23 6:02 ` Eli Zaretskii
2023-06-23 6:32 ` Daniel Semyonov via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-06-23 7:10 ` Eli Zaretskii
2023-06-23 7:35 ` Philip Kaludercic
2023-06-24 10:24 ` Philip Kaludercic
2023-06-24 10:43 ` Eli Zaretskii
2023-06-24 15:34 ` Philip Kaludercic
2023-06-24 17:07 ` Eli Zaretskii
2023-06-25 21:41 ` Philip Kaludercic
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.