unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#55386: 29.0.50; check-declare-directory doesn't work on Windows
@ 2022-05-12 15:22 Yasuhiro Kimura
  2022-05-12 15:57 ` Lars Ingebrigtsen
  0 siblings, 1 reply; 19+ messages in thread
From: Yasuhiro Kimura @ 2022-05-12 15:22 UTC (permalink / raw)
  To: 55386

[How to reproduce the problem]

1. cd C:/Users/yasu/Temp
2. git clone https://git.savannah.gnu.org/git/emacs.git
3. Start Emacs with 'emacs -Q'
4. Type '(check-declare-directory "C:/Users/yasu/Temp/emacs/lisp")'
   and C-j.

[Expected result]

Retern value is inserted to *scratch* buffer.

[What really happens]

*Backtrace* buffer is displayed with following backtrace information.

----------------------------------------------------------------------
Debugger entered--Lisp error: (file-error "Opening input file" "Invalid argument" "c:/Users/yasu/ファイルが見つかりません - \"^[ \11]*(declare-funct...")
  insert-file-contents("ファイルが見つかりません - \"^[ \11]*(declare-function\"")
  check-declare-scan("ファイルが見つかりません - \"^[ \11]*(declare-function\"")
  check-declare-files("ファイルが見つかりません - \"^[ \11]*(declare-function\"")
  apply(check-declare-files "ファイルが見つかりません - \"^[ \11]*(declare-function\"")
  check-declare-directory("C:/Users/yasu/Temp/emacs/lisp")
  (progn (check-declare-directory "C:/Users/yasu/Temp/emacs/lisp"))
  eval((progn (check-declare-directory "C:/Users/yasu/Temp/emacs/lisp")) t)
  elisp--eval-last-sexp(t)
  eval-last-sexp(t)
  eval-print-last-sexp(nil)
  funcall-interactively(eval-print-last-sexp nil)
  call-interactively(eval-print-last-sexp nil nil)
  command-execute(eval-print-last-sexp)
----------------------------------------------------------------------

I guess check-declare-directory don't handle drive letter of Windows
properly.


In GNU Emacs 29.0.50 (build 1, x86_64-w64-mingw32)
 of 2022-05-12 built on HALF
Repository revision: c8d7a27438b294e20ca0f8f6f1dd74d4a273dc96
Repository branch: master
Windowing system distributor 'Microsoft Corp.', version 10.0.22000
System Description: Microsoft Windows 10 Enterprise (v10.0.2009.22000.675)

Configured using:
 'configure --prefix=/c/Emacs --without-dbus'

Configured features:
ACL GIF GMP GNUTLS HARFBUZZ JPEG LIBXML2 MODULES NOTIFY W32NOTIFY
PDUMPER PNG RSVG SOUND SQLITE3 THREADS TIFF TOOLKIT_SCROLL_BARS WEBP
XPM ZLIB

Important settings:
  value of $LANG: JPN
  locale-coding-system: cp932

Major mode: Fundamental

Minor modes in effect:
  global-git-commit-mode: t
  magit-auto-revert-mode: t
  shell-dirtrack-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  show-paren-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  blink-cursor-mode: t
  buffer-read-only: t
  line-number-mode: t
  indent-tabs-mode: t
  transient-mark-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t

Load-path shadows:
c:/Emacs/share/emacs/site-lisp/transient hides c:/Emacs/share/emacs/29.0.50/lisp/transient
c:/Emacs/share/emacs/site-lisp/flim/sasl hides c:/Emacs/share/emacs/29.0.50/lisp/net/sasl

Features:
(shadow pp mew-varsx mew-win32 mew-w3m w3m doc-view jka-compr
image-mode exif timezone w3m-hist w3m-fb bookmark-w3m w3m-ems wid-edit
w3m-favicon w3m-image tab-line w3m-proc w3m-util mew-auth mew-config
mew-imap2 mew-imap mew-nntp2 mew-nntp mew-pop mew-smtp mew-ssl mew-ssh
mew-net mew-highlight mew-sort mew-fib mew-ext mew-refile mew-demo
mew-attach mew-draft mew-message mew-thread mew-virtual mew-summary4
mew-summary3 mew-summary2 mew-summary mew-search mew-pick mew-passwd
mew-scan mew-syntax mew-bq mew-smime mew-pgp mew-header mew-exec
mew-mark mew-mime mew-edit mew-decode mew-encode mew-cache mew-minibuf
mew-complete mew-addrbook mew-local mew-vars3 mew-vars2 mew-vars
mew-env mew-lang-jp mew-mule3 mew-mule mew-gemacs mew-key mew-func
mew-blvs mew-const mew emacsbug magit-version yaml-mode mime-setup
mail-mime-setup semi-setup semi-def alist path-util apel-ver product
rng-nxml rng-valid rng-loc rng-uri rng-parse nxml-parse rng-match
rng-dt rng-util rng-pttrn nxml-ns nxml-mode nxml-outln nxml-rap
sgml-mode facemenu dom nxml-util nxml-enc xmltok forge-list
forge-commands forge-semi forge-bitbucket buck forge-gogs gogs
forge-gitea gtea forge-gitlab glab forge-github ghub-graphql treepy
gsexp ghub let-alist gnutls forge-notify forge-revnote forge-pullreq
forge-issue forge-topic yaml pcase parse-time iso8601 bug-reference
forge-post markdown-mode color noutline outline forge-repo forge
forge-core forge-db closql emacsql-sqlite advice emacsql
emacsql-compiler url-http url-auth url-gw nsm magit-submodule
magit-obsolete magit-blame magit-stash magit-reflog magit-bisect
magit-push magit-pull magit-fetch magit-clone magit-remote
magit-commit magit-sequence magit-notes magit-worktree magit-tag
magit-merge magit-branch magit-reset magit-files magit-refs
magit-status magit package browse-url url url-proxy url-privacy
url-expand url-methods url-history url-cookie url-domsuf url-util
url-handlers url-parse auth-source url-vars magit-repos magit-apply
magit-wip magit-log which-func imenu magit-diff smerge-mode diff
diff-mode easy-mmode git-commit log-edit message sendmail mailcap
yank-media rmc puny dired dired-loaddefs rfc822 mml mml-sec
password-cache epa derived epg rfc6068 epg-config gnus-util
text-property-search time-date mm-decode mm-bodies mm-encode
mail-parse rfc2231 rfc2047 rfc2045 mm-util ietf-drums mail-prsvr
mailabbrev mail-utils gmm-utils mailheader pcvs-util add-log
magit-core magit-autorevert autorevert filenotify magit-margin
magit-transient magit-process with-editor shell pcomplete comint ring
server ansi-color magit-mode transient cl-extra edmacro kmacro
help-mode magit-git magit-base magit-section cl-seq format-spec crm
eieio eieio-core cl-macs eieio-loaddefs dash compat-27 compat-26
compat json map seq gv subr-x byte-opt bytecomp byte-compile cconv
gitignore-mode gitconfig-mode conf-mode rx gitattributes-mode
thingatpt cl-loaddefs cl-lib cp5022x japan-util iso-transl tooltip
eldoc paren electric uniquify ediff-hook vc-hooks lisp-float-type
elisp-mode mwheel dos-w32 ls-lisp disp-table term/w32-win w32-win
w32-vars term/common-win tool-bar dnd fontset image regexp-opt fringe
tabulated-list replace newcomment text-mode lisp-mode prog-mode
register page tab-bar menu-bar rfn-eshadow isearch easymenu timer
select scroll-bar mouse jit-lock font-lock syntax font-core
term/tty-colors frame minibuffer nadvice simple cl-generic cham
georgian utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao
korean japanese eucjp-ms cp51932 hebrew greek romanian slovak czech
european ethiopic indian cyrillic chinese composite emoji-zwj
charscript charprop case-table epa-hook jka-cmpr-hook help abbrev
obarray oclosure cl-preloaded button loaddefs faces cus-face macroexp
files window text-properties overlay sha1 md5 base64 format env
code-pages mule custom widget keymap hashtable-print-readable
backquote threads w32notify w32 multi-tty make-network-process emacs)

Memory information:
((conses 16 262013 11023)
 (symbols 48 26306 16)
 (strings 32 81908 2908)
 (string-bytes 1 2429996)
 (vectors 16 50644)
 (vector-slots 8 737227 23350)
 (floats 8 254 159)
 (intervals 56 758 228)
 (buffers 992 13))

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

* bug#55386: 29.0.50; check-declare-directory doesn't work on Windows
  2022-05-12 15:22 bug#55386: 29.0.50; check-declare-directory doesn't work on Windows Yasuhiro Kimura
@ 2022-05-12 15:57 ` Lars Ingebrigtsen
  2022-05-12 16:43   ` Eli Zaretskii
  2022-05-12 17:35   ` Yasuhiro Kimura
  0 siblings, 2 replies; 19+ messages in thread
From: Lars Ingebrigtsen @ 2022-05-12 15:57 UTC (permalink / raw)
  To: Yasuhiro Kimura; +Cc: 55386

Yasuhiro Kimura <yasu@utahime.org> writes:

> [How to reproduce the problem]
>
> 1. cd C:/Users/yasu/Temp
> 2. git clone https://git.savannah.gnu.org/git/emacs.git
> 3. Start Emacs with 'emacs -Q'
> 4. Type '(check-declare-directory "C:/Users/yasu/Temp/emacs/lisp")'
>    and C-j.

[...]

> Debugger entered--Lisp error: (file-error "Opening input file" "Invalid argument" "c:/Users/yasu/ファイルが見つかりません - \"^[ \11]*(declare-funct...")
>   insert-file-contents("ファイルが見つかりません - \"^[ \11]*(declare-function\"")
>   check-declare-scan("ファイルが見つかりません - \"^[ \11]*(declare-function\"")
>   check-declare-files("ファイルが見つかりません - \"^[ \11]*(declare-function\"")
>   apply(check-declare-files "ファイルが見つかりません - \"^[ \11]*(declare-function\"")

check-declare-directory is just a wrapper around `find', and I'm surprised
that this even vaguely works on Windows:

(defun check-declare-directory (root)
[...]
  (let ((files (process-lines-ignore-status
                find-program root
                "-name" "*.el"
                "-exec" grep-program
                "-l" "^[ \t]*(declare-function" "{}" "+")))

If you run this "manually", what does it output?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no





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

* bug#55386: 29.0.50; check-declare-directory doesn't work on Windows
  2022-05-12 15:57 ` Lars Ingebrigtsen
@ 2022-05-12 16:43   ` Eli Zaretskii
  2022-05-13 12:21     ` Lars Ingebrigtsen
  2022-05-12 17:35   ` Yasuhiro Kimura
  1 sibling, 1 reply; 19+ messages in thread
From: Eli Zaretskii @ 2022-05-12 16:43 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: yasu, 55386

> Cc: 55386@debbugs.gnu.org
> From: Lars Ingebrigtsen <larsi@gnus.org>
> Date: Thu, 12 May 2022 17:57:01 +0200
> 
> > Debugger entered--Lisp error: (file-error "Opening input file" "Invalid argument" "c:/Users/yasu/ファイルが見つかりません - \"^[ \11]*(declare-funct...")
> >   insert-file-contents("ファイルが見つかりません - \"^[ \11]*(declare-function\"")
> >   check-declare-scan("ファイルが見つかりません - \"^[ \11]*(declare-function\"")
> >   check-declare-files("ファイルが見つかりません - \"^[ \11]*(declare-function\"")
> >   apply(check-declare-files "ファイルが見つかりません - \"^[ \11]*(declare-function\"")
> 
> check-declare-directory is just a wrapper around `find', and I'm surprised
> that this even vaguely works on Windows:

Why are you surprised?

> (defun check-declare-directory (root)
> [...]
>   (let ((files (process-lines-ignore-status
>                 find-program root
>                 "-name" "*.el"
>                 "-exec" grep-program
>                 "-l" "^[ \t]*(declare-function" "{}" "+")))
> 
> If you run this "manually", what does it output?

Here, it produces a long list of *.el files.

The string "ファイルが見つかりません" translates from Japanese as
"File not found".  So I suspect the OP doesn't have a port of GNU Find
on Path before the Windows program of the same name (which does
something completely different), or maybe the version of Find or Grep
the OP has don't support non-ASCII characters encoded in the OP's
locale's codepage.

Basically, that command tries to tell us that some (or all) of the
files were not found.

I wonder whether it could be a good idea to replace the find/grep
command by something that traverses the files in Lisp, like
dired-do-search or somesuch?  This would resolve any problems with
file names and incompatible versions of Find and Grep.





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

* bug#55386: 29.0.50; check-declare-directory doesn't work on Windows
  2022-05-12 15:57 ` Lars Ingebrigtsen
  2022-05-12 16:43   ` Eli Zaretskii
@ 2022-05-12 17:35   ` Yasuhiro Kimura
  2022-05-13 12:25     ` Lars Ingebrigtsen
  1 sibling, 1 reply; 19+ messages in thread
From: Yasuhiro Kimura @ 2022-05-12 17:35 UTC (permalink / raw)
  To: larsi; +Cc: 55386

From: Lars Ingebrigtsen <larsi@gnus.org>
Subject: Re: bug#55386: 29.0.50; check-declare-directory doesn't work on Windows
Date: Thu, 12 May 2022 17:57:01 +0200

> check-declare-directory is just a wrapper around `find', and I'm surprised
> that this even vaguely works on Windows:
> 
> (defun check-declare-directory (root)
> [...]
>   (let ((files (process-lines-ignore-status
>                 find-program root
>                 "-name" "*.el"
>                 "-exec" grep-program
>                 "-l" "^[ \t]*(declare-function" "{}" "+")))
> 
> If you run this "manually", what does it output?
> 
> -- 
> (domestic pets only, the antidote for overdose, milk.)
>    bloggy blog: http://lars.ingebrigtsen.no

Thanks for reply. And sorry, it seems I misunderstood the problem. At
first I faced the error of check-declare-directory when I tried to
build and install some 3rd party elisp applications. Recently they
start to use the function at their build process and it causes build
error as following.

(MINGW64)yasu@half[1018]% make lisp
make[1]: Entering directory '/c/Users/yasu/Work/Emacs/with-editor/lisp'
Compiling with-editor.el
 Creating with-editor-autoloads.el
 Checking function declarations

Error: file-error ("Opening input file" "Invalid argument" "c:/usr/bin/find: paths must precede expression: `with-editor.el'")
  mapbacktrace(#f(compiled-function (evald func args flags) #<bytecode -0x10a1f18ad911994>))
  debug-early-backtrace()
  debug-early(error (file-error "Opening input file" "Invalid argument" "c:/usr/bin/find: paths must precede expression: `with-editor.el'"))
  insert-file-contents("/usr/bin/find: paths must precede expression: `with-editor.el'")
  check-declare-scan("/usr/bin/find: paths must precede expression: `with-editor.el'")
  check-declare-files("/usr/bin/find: paths must precede expression: `with-editor.el'" "/usr/bin/find: possible unquoted pattern after predicate `-name'?")
  apply(check-declare-files ("/usr/bin/find: paths must precede expression: `with-editor.el'" "/usr/bin/find: possible unquoted pattern after predicate `-name'?"))
  check-declare-directory("~/Work/Emacs/with-editor/lisp/")
  eval((check-declare-directory default-directory) t)
  command-line-1(("--eval" "(setq with-editor-emacsclient-executable nil)" "-L" "../../compat" "-L" "../../vterm" "-L" "." "--eval" "(check-declare-directory default-directory)"))
  command-line()
  normal-top-level()
Opening input file: Invalid argument, c:/usr/bin/find: paths must precede expression: `with-editor.el'
make[1]: *** [Makefile:14: check-declare] Error 127
make[1]: Leaving directory '/c/Users/yasu/Work/Emacs/with-editor/lisp'
make: *** [Makefile:25: lisp] Error 2
(MINGW64)yasu@half[1019]%  

In this case I executed make.exe from shell of MSYS2. So find.exe and
grep.exe of MSYS2 are used.

But when I directly evaluated check-declare-directory from *scratch*
buffer, I executed emacs from command prompt of Windows. And in this
case there is neither find.exe nor grep.exe in my PATH directories. So
check-declare-directory fails differently. And I wrongly reported it
as the bug of Emacs. Actually if I execute emacs from shell of MSYS2,
then steps of original bug report mail results in expected behavior.

I don't know why above build error happens. But probably it is bug of
the elisp applications in question.  So I'll report it to the author
of them.

Best Regards.

---
Yasuhiro KIMURA





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

* bug#55386: 29.0.50; check-declare-directory doesn't work on Windows
  2022-05-12 16:43   ` Eli Zaretskii
@ 2022-05-13 12:21     ` Lars Ingebrigtsen
  2022-05-13 12:35       ` Eli Zaretskii
  0 siblings, 1 reply; 19+ messages in thread
From: Lars Ingebrigtsen @ 2022-05-13 12:21 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: yasu, 55386

Eli Zaretskii <eliz@gnu.org> writes:

> Why are you surprised?

Because:

> So I suspect the OP doesn't have a port of GNU Find
> on Path before the Windows program of the same name (which does
> something completely different), or maybe the version of Find or Grep
> the OP has don't support non-ASCII characters encoded in the OP's
> locale's codepage.

There's always stuff like this in Windows-related bug reports.  😀

> I wonder whether it could be a good idea to replace the find/grep
> command by something that traverses the files in Lisp, like
> dired-do-search or somesuch?  This would resolve any problems with
> file names and incompatible versions of Find and Grep.

It would be massively slower, though.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no





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

* bug#55386: 29.0.50; check-declare-directory doesn't work on Windows
  2022-05-12 17:35   ` Yasuhiro Kimura
@ 2022-05-13 12:25     ` Lars Ingebrigtsen
  2022-05-13 12:31       ` Eli Zaretskii
  0 siblings, 1 reply; 19+ messages in thread
From: Lars Ingebrigtsen @ 2022-05-13 12:25 UTC (permalink / raw)
  To: Yasuhiro Kimura; +Cc: Eli Zaretskii, 55386

Yasuhiro Kimura <yasu@utahime.org> writes:

>   insert-file-contents("/usr/bin/find: paths must precede expression: `with-editor.el'")
>   check-declare-scan("/usr/bin/find: paths must precede expression: `with-editor.el'")
>   check-declare-files("/usr/bin/find: paths must precede expression: `with-editor.el'" "/usr/bin/find: possible unquoted pattern after predicate `-name'?")
>   apply(check-declare-files ("/usr/bin/find: paths must precede expression: `with-editor.el'" "/usr/bin/find: possible unquoted pattern after predicate `-name'?"))
>   check-declare-directory("~/Work/Emacs/with-editor/lisp/")

[...]

> But when I directly evaluated check-declare-directory from *scratch*
> buffer, I executed emacs from command prompt of Windows. And in this
> case there is neither find.exe nor grep.exe in my PATH directories. So
> check-declare-directory fails differently. And I wrongly reported it
> as the bug of Emacs. Actually if I execute emacs from shell of MSYS2,
> then steps of original bug report mail results in expected behavior.
>
> I don't know why above build error happens. But probably it is bug of
> the elisp applications in question.  So I'll report it to the author
> of them.

There does seem to be an issue in `check-declare-directory' in Emacs
under Windows, though -- find is complaining about syntax errors in the
"find" command we're constructing.

Eli, does `check-declare-directory' work for you on some Lisp directory
on Windows?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no





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

* bug#55386: 29.0.50; check-declare-directory doesn't work on Windows
  2022-05-13 12:25     ` Lars Ingebrigtsen
@ 2022-05-13 12:31       ` Eli Zaretskii
  0 siblings, 0 replies; 19+ messages in thread
From: Eli Zaretskii @ 2022-05-13 12:31 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: yasu, 55386

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: 55386@debbugs.gnu.org, Eli Zaretskii <eliz@gnu.org>
> Date: Fri, 13 May 2022 14:25:10 +0200
> 
> There does seem to be an issue in `check-declare-directory' in Emacs
> under Windows, though -- find is complaining about syntax errors in the
> "find" command we're constructing.
> 
> Eli, does `check-declare-directory' work for you on some Lisp directory
> on Windows?

Yes, it does.

Yasuhiro Kimura, can you tell where you got your ports of Find and
Grep?





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

* bug#55386: 29.0.50; check-declare-directory doesn't work on Windows
  2022-05-13 12:21     ` Lars Ingebrigtsen
@ 2022-05-13 12:35       ` Eli Zaretskii
  2022-05-13 15:46         ` Lars Ingebrigtsen
  0 siblings, 1 reply; 19+ messages in thread
From: Eli Zaretskii @ 2022-05-13 12:35 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: yasu, 55386

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: yasu@utahime.org,  55386@debbugs.gnu.org
> Date: Fri, 13 May 2022 14:21:01 +0200
> 
> > Why are you surprised?
> 
> Because:
> 
> > So I suspect the OP doesn't have a port of GNU Find
> > on Path before the Windows program of the same name (which does
> > something completely different), or maybe the version of Find or Grep
> > the OP has don't support non-ASCII characters encoded in the OP's
> > locale's codepage.
> 
> There's always stuff like this in Windows-related bug reports.  😀

There's a difference between expecting problems and being surprised it
can work at all.

> > I wonder whether it could be a good idea to replace the find/grep
> > command by something that traverses the files in Lisp, like
> > dired-do-search or somesuch?  This would resolve any problems with
> > file names and incompatible versions of Find and Grep.
> 
> It would be massively slower, though.

I'm not sure.  How much time does it take for the find/grep command to
finish working on our lisp/ directory on your system?

And this command is not really time-critical anyway.

In any case, we could use the Lisp path only on Windows, since having
a slower command is better than having a broken command.





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

* bug#55386: 29.0.50; check-declare-directory doesn't work on Windows
  2022-05-13 12:35       ` Eli Zaretskii
@ 2022-05-13 15:46         ` Lars Ingebrigtsen
  2022-05-14  9:09           ` Eli Zaretskii
  0 siblings, 1 reply; 19+ messages in thread
From: Lars Ingebrigtsen @ 2022-05-13 15:46 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: yasu, 55386

Eli Zaretskii <eliz@gnu.org> writes:

> I'm not sure.  How much time does it take for the find/grep command to
> finish working on our lisp/ directory on your system?

Let's see...

(benchmark-run (check-declare-directory "~/src/emacs/trunk/lisp/"))

11 seconds.  Perhaps a pure-Lisp solution wouldn't be that much slower,
anyway?

(Hm...  it finds over a 100 in-tree declarations that it says are
malformed/wrong...  Perhaps somebody should have a look at that.)

> And this command is not really time-critical anyway.

That's true.

> In any case, we could use the Lisp path only on Windows, since having
> a slower command is better than having a broken command.

If we have a Lisp solution, I think I'd prefer to use that on all
platforms.  Easier to debug when there only one code path, for one.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no





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

* bug#55386: 29.0.50; check-declare-directory doesn't work on Windows
  2022-05-13 15:46         ` Lars Ingebrigtsen
@ 2022-05-14  9:09           ` Eli Zaretskii
  2022-05-14 11:40             ` Lars Ingebrigtsen
  2022-05-14 13:27             ` Eli Zaretskii
  0 siblings, 2 replies; 19+ messages in thread
From: Eli Zaretskii @ 2022-05-14  9:09 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: yasu, 55386

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: yasu@utahime.org,  55386@debbugs.gnu.org
> Date: Fri, 13 May 2022 17:46:50 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > I'm not sure.  How much time does it take for the find/grep command to
> > finish working on our lisp/ directory on your system?
> 
> Let's see...
> 
> (benchmark-run (check-declare-directory "~/src/emacs/trunk/lisp/"))
> 
> 11 seconds.  Perhaps a pure-Lisp solution wouldn't be that much slower,
> anyway?
> 
> (Hm...  it finds over a 100 in-tree declarations that it says are
> malformed/wrong...  Perhaps somebody should have a look at that.)

Yes, please.

> > And this command is not really time-critical anyway.
> 
> That's true.
> 
> > In any case, we could use the Lisp path only on Windows, since having
> > a slower command is better than having a broken command.
> 
> If we have a Lisp solution, I think I'd prefer to use that on all
> platforms.  Easier to debug when there only one code path, for one.

How about the below?  It's 3% to 9% slower than the find/grep version
(because it examines more files, I think), but much simpler (IMNSHO),
and works on any platform without any caveats.

diff --git a/lisp/emacs-lisp/check-declare.el b/lisp/emacs-lisp/check-declare.el
index b3c9651..83187ac 100644
--- a/lisp/emacs-lisp/check-declare.el
+++ b/lisp/emacs-lisp/check-declare.el
@@ -319,11 +319,7 @@ check-declare-directory
   (setq root (directory-file-name (file-relative-name root)))
   (or (file-directory-p root)
       (error "Directory `%s' not found" root))
-  (let ((files (process-lines-ignore-status
-                find-program root
-                "-name" "*.el"
-                "-exec" grep-program
-                "-l" "^[ \t]*(declare-function" "{}" "+")))
+  (let ((files (directory-files-recursively root "\\.el\\'")))
     (when files
       (apply #'check-declare-files files))))
 





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

* bug#55386: 29.0.50; check-declare-directory doesn't work on Windows
  2022-05-14  9:09           ` Eli Zaretskii
@ 2022-05-14 11:40             ` Lars Ingebrigtsen
  2022-05-14 12:05               ` Eli Zaretskii
  2022-05-14 13:27             ` Eli Zaretskii
  1 sibling, 1 reply; 19+ messages in thread
From: Lars Ingebrigtsen @ 2022-05-14 11:40 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: yasu, 55386

Eli Zaretskii <eliz@gnu.org> writes:

> How about the below?  It's 3% to 9% slower than the find/grep version
> (because it examines more files, I think), but much simpler (IMNSHO),
> and works on any platform without any caveats.

[...]

> -  (let ((files (process-lines-ignore-status
> -                find-program root
> -                "-name" "*.el"
> -                "-exec" grep-program
> -                "-l" "^[ \t]*(declare-function" "{}" "+")))
> +  (let ((files (directory-files-recursively root "\\.el\\'")))
>      (when files
>        (apply #'check-declare-files files))))

I'm surprised that it's just 9% slower -- there's 2K files in the Emacs
tree, and only one a quarter of them have a declare-function.  Is
process-lines-ignore-status really slow or something?

But the change looks fine to me in any case.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no





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

* bug#55386: 29.0.50; check-declare-directory doesn't work on Windows
  2022-05-14 11:40             ` Lars Ingebrigtsen
@ 2022-05-14 12:05               ` Eli Zaretskii
  2022-05-14 15:36                 ` Lars Ingebrigtsen
  0 siblings, 1 reply; 19+ messages in thread
From: Eli Zaretskii @ 2022-05-14 12:05 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: yasu, 55386

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: yasu@utahime.org,  55386@debbugs.gnu.org
> Date: Sat, 14 May 2022 13:40:11 +0200
> 
> > -  (let ((files (process-lines-ignore-status
> > -                find-program root
> > -                "-name" "*.el"
> > -                "-exec" grep-program
> > -                "-l" "^[ \t]*(declare-function" "{}" "+")))
> > +  (let ((files (directory-files-recursively root "\\.el\\'")))
> >      (when files
> >        (apply #'check-declare-files files))))
> 
> I'm surprised that it's just 9% slower -- there's 2K files in the Emacs
> tree, and only one a quarter of them have a declare-function.  Is
> process-lines-ignore-status really slow or something?

I don't know if it's really slow, but it runs Grep on each file, and
that slows down the command it launches.  directory-files-recursively
is much faster, but then check-declare-files has more files to check.

I think the slowdown depends on the system and on the build.  I only
tested in unoptimized builds, and the GNU/Linux system to which I have
access is a relatively slow VM.  So maybe you should time this
yourself before we decide whether the slow-down is acceptable?

Or what kind of slow-down can we endure before we feel uneasy about
the change?

> But the change looks fine to me in any case.

I can install now if you are okay with the above.





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

* bug#55386: 29.0.50; check-declare-directory doesn't work on Windows
  2022-05-14  9:09           ` Eli Zaretskii
  2022-05-14 11:40             ` Lars Ingebrigtsen
@ 2022-05-14 13:27             ` Eli Zaretskii
  2022-05-14 13:39               ` Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2022-05-14 15:37               ` Lars Ingebrigtsen
  1 sibling, 2 replies; 19+ messages in thread
From: Eli Zaretskii @ 2022-05-14 13:27 UTC (permalink / raw)
  To: larsi, Po Lu; +Cc: yasu, 55386

> Cc: yasu@utahime.org, 55386@debbugs.gnu.org
> Date: Sat, 14 May 2022 12:09:56 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> 
> > (Hm...  it finds over a 100 in-tree declarations that it says are
> > malformed/wrong...  Perhaps somebody should have a look at that.)
> 
> Yes, please.

I've now fixed many of those.  The ones left are:

  . those from Org -- should be fixed in Org repository
  . references to packages not in core (like BBDB and W3M)
  . stuff that _looks_ like functions, but isn't: compiler-macros,
    constructors, games we play with setf etc. -- this is where
    check-declare "Needs Work"(TM) to be smarter

There are two warnings regarding PGTK which I didn't know what to do
about:

  lisp/frame.el:1996:Warning (check-declare): said ‘pgtk-frame-list-z-order’ was
      defined in src/pgtkfns.c: function not found

  lisp/term/pgtk-win.el:48:Warning (check-declare): said ‘pgtk-hide-emacs’ was
      defined in src/pgtkfns.c: function not found

Those functions indeed don't exist, AFAICT, but they _are_ called from
Lisp.  Po Lu, can you please DTRT there?





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

* bug#55386: 29.0.50; check-declare-directory doesn't work on Windows
  2022-05-14 13:27             ` Eli Zaretskii
@ 2022-05-14 13:39               ` Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2022-05-14 13:44                 ` Eli Zaretskii
  2022-05-14 15:37               ` Lars Ingebrigtsen
  1 sibling, 1 reply; 19+ messages in thread
From: Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2022-05-14 13:39 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: yasu, larsi, 55386

Eli Zaretskii <eliz@gnu.org> writes:

>   lisp/frame.el:1996:Warning (check-declare): said ‘pgtk-frame-list-z-order’ was
>       defined in src/pgtkfns.c: function not found
>
>   lisp/term/pgtk-win.el:48:Warning (check-declare): said ‘pgtk-hide-emacs’ was
>       defined in src/pgtkfns.c: function not found
>
> Those functions indeed don't exist, AFAICT, but they _are_ called from
> Lisp.  Po Lu, can you please DTRT there?

Those are copy-paste errors from when the Lisp part of the PGTK port was
first written by blindly copying code from the NS port that doesn't
really apply.  (The original author of the PGTK port is probably the
only person who knows why that happened.)  I will delete them.

Thanks.





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

* bug#55386: 29.0.50; check-declare-directory doesn't work on Windows
  2022-05-14 13:39               ` Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2022-05-14 13:44                 ` Eli Zaretskii
  0 siblings, 0 replies; 19+ messages in thread
From: Eli Zaretskii @ 2022-05-14 13:44 UTC (permalink / raw)
  To: Po Lu; +Cc: yasu, larsi, 55386

> From: Po Lu <luangruo@yahoo.com>
> Cc: larsi@gnus.org,  yasu@utahime.org,  55386@debbugs.gnu.org
> Date: Sat, 14 May 2022 21:39:49 +0800
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >   lisp/frame.el:1996:Warning (check-declare): said ‘pgtk-frame-list-z-order’ was
> >       defined in src/pgtkfns.c: function not found
> >
> >   lisp/term/pgtk-win.el:48:Warning (check-declare): said ‘pgtk-hide-emacs’ was
> >       defined in src/pgtkfns.c: function not found
> >
> > Those functions indeed don't exist, AFAICT, but they _are_ called from
> > Lisp.  Po Lu, can you please DTRT there?
> 
> Those are copy-paste errors from when the Lisp part of the PGTK port was
> first written by blindly copying code from the NS port that doesn't
> really apply.  (The original author of the PGTK port is probably the
> only person who knows why that happened.)  I will delete them.

That's what I thought, but couldn't be sure.  Thanks.





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

* bug#55386: 29.0.50; check-declare-directory doesn't work on Windows
  2022-05-14 12:05               ` Eli Zaretskii
@ 2022-05-14 15:36                 ` Lars Ingebrigtsen
  2022-05-14 16:09                   ` Eli Zaretskii
  0 siblings, 1 reply; 19+ messages in thread
From: Lars Ingebrigtsen @ 2022-05-14 15:36 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: yasu, 55386

Eli Zaretskii <eliz@gnu.org> writes:

>> But the change looks fine to me in any case.
>
> I can install now if you are okay with the above.

Sure, go ahead.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no





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

* bug#55386: 29.0.50; check-declare-directory doesn't work on Windows
  2022-05-14 13:27             ` Eli Zaretskii
  2022-05-14 13:39               ` Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2022-05-14 15:37               ` Lars Ingebrigtsen
  2022-05-14 16:34                 ` Eli Zaretskii
  1 sibling, 1 reply; 19+ messages in thread
From: Lars Ingebrigtsen @ 2022-05-14 15:37 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Po Lu, yasu, 55386

Eli Zaretskii <eliz@gnu.org> writes:

>   . stuff that _looks_ like functions, but isn't: compiler-macros,
>     constructors, games we play with setf etc. -- this is where
>     check-declare "Needs Work"(TM) to be smarter

Perhaps open a new bug report for that?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no





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

* bug#55386: 29.0.50; check-declare-directory doesn't work on Windows
  2022-05-14 15:36                 ` Lars Ingebrigtsen
@ 2022-05-14 16:09                   ` Eli Zaretskii
  0 siblings, 0 replies; 19+ messages in thread
From: Eli Zaretskii @ 2022-05-14 16:09 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: yasu, 55386

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: yasu@utahime.org,  55386@debbugs.gnu.org
> Date: Sat, 14 May 2022 17:36:23 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> But the change looks fine to me in any case.
> >
> > I can install now if you are okay with the above.
> 
> Sure, go ahead.

Installed.





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

* bug#55386: 29.0.50; check-declare-directory doesn't work on Windows
  2022-05-14 15:37               ` Lars Ingebrigtsen
@ 2022-05-14 16:34                 ` Eli Zaretskii
  0 siblings, 0 replies; 19+ messages in thread
From: Eli Zaretskii @ 2022-05-14 16:34 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: luangruo, yasu, 55386

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: Po Lu <luangruo@yahoo.com>,  yasu@utahime.org,  55386@debbugs.gnu.org
> Date: Sat, 14 May 2022 17:37:32 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >   . stuff that _looks_ like functions, but isn't: compiler-macros,
> >     constructors, games we play with setf etc. -- this is where
> >     check-declare "Needs Work"(TM) to be smarter
> 
> Perhaps open a new bug report for that?

Done.





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

end of thread, other threads:[~2022-05-14 16:34 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-12 15:22 bug#55386: 29.0.50; check-declare-directory doesn't work on Windows Yasuhiro Kimura
2022-05-12 15:57 ` Lars Ingebrigtsen
2022-05-12 16:43   ` Eli Zaretskii
2022-05-13 12:21     ` Lars Ingebrigtsen
2022-05-13 12:35       ` Eli Zaretskii
2022-05-13 15:46         ` Lars Ingebrigtsen
2022-05-14  9:09           ` Eli Zaretskii
2022-05-14 11:40             ` Lars Ingebrigtsen
2022-05-14 12:05               ` Eli Zaretskii
2022-05-14 15:36                 ` Lars Ingebrigtsen
2022-05-14 16:09                   ` Eli Zaretskii
2022-05-14 13:27             ` Eli Zaretskii
2022-05-14 13:39               ` Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors
2022-05-14 13:44                 ` Eli Zaretskii
2022-05-14 15:37               ` Lars Ingebrigtsen
2022-05-14 16:34                 ` Eli Zaretskii
2022-05-12 17:35   ` Yasuhiro Kimura
2022-05-13 12:25     ` Lars Ingebrigtsen
2022-05-13 12:31       ` Eli Zaretskii

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).