unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#23453: 25.0.93; dired-do-find-regexp does not work on Windows even after installing GNU find and grep
@ 2016-05-04 19:39 Kaushal Modi
  2016-05-04 19:49 ` Eli Zaretskii
  0 siblings, 1 reply; 14+ messages in thread
From: Kaushal Modi @ 2016-05-04 19:39 UTC (permalink / raw)
  To: 23453, dgutov, eliz

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

--text follows this line--

Hi,

This is on 64-bit Windows 7 Enterprise version.

It did not have the GNU find and grep by default. So I downloaded the
following ports:

(1)
https://sourceforge.net/projects/ezwinports/files/findutils-4.2.30-5-w32-bin.zip/download
(2)
https://sourceforge.net/projects/ezwinports/files/grep-2.10-w32-bin.zip/download

extracted them and added the resultant bin directory
( C:\Users\kmodi\Dropbox\Portable Software\ezwinports\bin ) to the user
PATH environment variable.

The problem is that the user PATH env var comes at a lower order of
precendence than the system value of the PATH env var. So first the admin
controlled directories come in PATH and then the user added directories
come.

So the newly added "C:\Users\kmodi\Dropbox\Portable
Software\ezwinports\bin" comes at the very end when I do C-h v exec-path
and (getenv "PATH").

So the find.exe found by emacs is the one in C:\Windows\System32\find.exe

By I used the brute-force method below just so that the correct find.exe
and grep.exe are found by emacs:

(setq exec-path '("C:/Users/kmodi/Dropbox/Portable Software/ezwinports/bin"
                  "c:/ProgramData/Oracle/Java/javapath"
                  "C:/Program Files (x86)/NVIDIA Corporation/PhysX/Common"
                  "C:/Windows/system32"
                  "C:/Windows"
                  "C:/Windows/System32/Wbem"
                  "C:/Windows/System32/WindowsPowerShell/v1.0/"
                  "C:/Program Files/WIDCOMM/Bluetooth Software/"
                  "C:/Program Files/WIDCOMM/Bluetooth Software/syswow64"
                  "C:/Program Files/Intel/WiFi/bin/"
                  "C:/Program Files/Common Files/Intel/WirelessCommon/"
                  "C:/Program Files (x86)/QuickTime/QTSystem/"
                  "C:/Program Files (x86)/Common Files/Roxio
Shared/DLLShared/"
                  "C:/Program Files (x86)/Common Files/Roxio
Shared/OEM/DLLShared/"
                  "C:/Program Files (x86)/Common Files/Roxio
Shared/OEM/DLLShared/"
                  "C:/Program Files (x86)/Common Files/Roxio
Shared/OEM/12.0/DLLShared/"
                  "C:/Program Files (x86)/Roxio/OEM/AudioCore/"
                  "C:/Users/kmodi/.yari/bin"
                  "C:/Python27"
                  "C:/Program Files/MiKTeX 2.9/miktex/bin/x64/"
                  "C:/Program Files/MATLAB/R2013a/runtime/win64"
                  "C:/Program Files/MATLAB/R2013a/bin"
                  "C:/Program Files (x86)/Intel/OpenCL SDK/2.0/bin/x86"
                  "C:/Program Files (x86)/Intel/OpenCL SDK/2.0/bin/x64"
                  "C:/Users/k"
                  "C:/Program Files/Git/cmd"
                  "C:/Users/kmodi/.yari/bin"
                  "C:/Users/kmodi/.yari/ruby-1.9.3-p194-i386-mingw32/bin"
                  "C:/Users/kmodi/AppData/Local/bin"
                  "."
                  "c:/Users/kmodi/Dropbox/Portable
Software/emacs/libexec/emacs/25.0.93/x86_64-w64-mingw32"))

(setenv "PATH" "C:\\Users\\kmodi\\Dropbox\\Portable
Software\\ezwinports\\bin;C:\\ProgramData\\Oracle\\Java\\javapath;C:\\Program
Files (x86)\\NVIDIA
Corporation\\PhysX\\Common;C:\\Windows\\system32;C:\\Windows;C:\\Windows\\System32\\Wbem;C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\;C:\\Program
Files\\WIDCOMM\\Bluetooth Software\\;C:\\Program Files\\WIDCOMM\\Bluetooth
Software\\syswow64;C:\\Program Files\\Intel\\WiFi\\bin\\;C:\\Program
Files\\Common Files\\Intel\\WirelessCommon\\;C:\\Program Files
(x86)\\QuickTime\\QTSystem\\;C:\\Program Files (x86)\\Common Files\\Roxio
Shared\\DLLShared\\;C:\\Program Files (x86)\\Common Files\\Roxio
Shared\\OEM\\DLLShared\\;C:\\Program Files (x86)\\Common Files\\Roxio
Shared\\OEM\\DLLShared\\;C:\\Program Files (x86)\\Common Files\\Roxio
Shared\\OEM\\12.0\\DLLShared\\;C:\\Program Files
(x86)\\Roxio\\OEM\\AudioCore\\;C:\\Users\\kmodi\\.yari\\bin;C:\\Python27;C:\\Program
Files\\MiKTeX 2.9\\miktex\\bin\\x64\\;C:\\Program
Files\\MATLAB\\R2013a\\runtime\\win64;C:\\Program
Files\\MATLAB\\R2013a\\bin;C:\\Program Files (x86)\\Intel\\OpenCL
SDK\\2.0\\bin\\x86;C:\\Program Files (x86)\\Intel\\OpenCL
SDK\\2.0\\bin\\x64;C:\\Users\\k;C:\\Program
Files\\Git\\cmd;C:\\Users\\kmodi\\.yari\\bin;C:\\Users\\kmodi\\.yari\\ruby-1.9.3-p194-i386-mingw32\\bin;C:\\Users\\kmodi\\AppData\\Local\\bin;.;")

Now I get

"c:/Users/kmodi/Dropbox/Portable Software/ezwinports/bin/find.exe"

when I do (executable-find "find"), and

"c:/Users/kmodi/Dropbox/Portable Software/ezwinports/bin/grep.exe"

when I do (executable-find "grep").

But even now, the A command (dired-do-find-regexp) in dired does not work.

Here's how I verified it to not work:

(1) I downloaded http://git.savannah.gnu.org/cgit/emacs.git/plain/etc/NEWS as
NEWS.txt to a folder in Windows.
(2) In emacs -Q with the above fixes to exec-path and PATH env var, with
find.exe and grep.exe being the correct one, I use C-x d to open dired in
the folder containing NEWS.exe
(3) I mark NEWS.txt using `m' key
(4) Then I do `A' and search for "Emacs"

All I get is "No matches for: Emacs"
(whereas the same steps to search "Emacs" in NEWS.txt work as expected in
RHEL)

- So, what could have gone wrong in this?
- What debug info can I provide? The find.exe and grep.exe look correct. Do
I need to install anything else from ezwinports?

(Also I shouldn't have needed to shuffle the exec-path and PATH as above to
point to the correct find and grep executables.)

In GNU Emacs 25.0.93.2 (x86_64-w64-mingw32)
 of 2016-04-23 built on ..
Windowing system distributor 'Microsoft Corp.', version 6.1.7601
Configured using:
 'configure --prefix=/tmp/emacs --without-imagemagick
 --enable-checking=yes,glyphs --enable-check-lisp-object-type
 'CFLAGS=-Og -gdwarf-4 -g3''

Configured features:
XPM JPEG TIFF GIF PNG RSVG SOUND DBUS NOTIFY ACL GNUTLS LIBXML2 ZLIB
TOOLKIT_SCROLL_BARS

Important settings:
  value of $LANG: ENU
  locale-coding-system: cp1252

Major mode: Messages

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-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
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  buffer-read-only: t
  line-number-mode: t
  transient-mark-mode: t

Recent messages:
user-error: Minibuffer window is not active
Mark set [5 times]
"C:\\Users\\kmodi\\Dropbox\\Portable
Software\\ezwinports\\bin;C:\\ProgramData\\Oracle\\Java\\javapath;C:\\Program
Files (x86)\\NVIDIA
Corporation\\PhysX\\Common;C:\\Windows\\system32;C:\\Windows;C:\\Windows\\System32\\Wbem;C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\;C:\\Program
Files\\WIDCOMM\\Bluetooth Software\\;C:\\Program Files\\WIDCOMM\\Bluetooth
Software\\syswow64;C:\\Program Files\\Intel\\WiFi\\bin\\;C:\\Program
Files\\Common Files\\Intel\\WirelessCommon\\;C:\\Program Files
(x86)\\QuickTime\\QTSystem\\;C:\\Program Files (x86)\\Common Files\\Roxio
Shared\\DLLShared\\;C:\\Program Files (x86)\\Common Files\\Roxio
Shared\\OEM\\DLLShared\\;C:\\Program Files (x86)\\Common Files\\Roxio
Shared\\OEM\\DLLShared\\;C:\\Program Files (x86)\\Common Files\\Roxio
Shared\\OEM\\12.0\\DLLShared\\;C:\\Program Files
(x86)\\Roxio\\OEM\\AudioCore\\;C:\\Users\\kmodi\\.yari\\bin;C:\\Python27;C:\\Program
Files\\MiKTeX 2.9\\miktex\\bin\\x64\\;C:\\Program
Files\\MATLAB\\R2013a\\runtime\\win64;C:\\Program
Files\\MATLAB\\R2013a\\bin;C:\\Program Files (x86)\\Intel\\OpenCL
SDK\\2.0\\bin\\x86;C:\\Program Files (x86)\\Intel\\OpenCL
SDK\\2.0\\bin\\x64;C:\\Users\\k;C:\\Program
Files\\Git\\cmd;C:\\Users\\kmodi\\.yari\\bin;C:\\Users\\kmodi\\.yari\\ruby-1.9.3-p194-i386-mingw32\\bin;C:\\Users\\kmodi\\AppData\\Local\\bin;.;"
[2 times]
user-error: No matches for: icons
Quit
user-error: No matches for: Emacs
user-error: Beginning of history; no preceding item
"c:/Users/kmodi/Dropbox/Portable Software/ezwinports/bin/find.exe"
GNU Emacs 25.0.93.2 (x86_64-w64-mingw32) of 2016-04-23
Making completion list...

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message format-spec rfc822 mml mml-sec
password-cache epg epg-config gnus-util mm-decode mm-bodies mm-encode
mail-parse rfc2231 mailabbrev gmm-utils mailheader sendmail rfc2047
rfc2045 ietf-drums mm-util mail-prsvr mail-utils thingatpt help-fns
find-dired semantic/fw mode-local find-func xref cl-seq project eieio
byte-opt bytecomp byte-compile cconv eieio-core cl-macs gv cl-extra
help-mode easymenu grep compile comint ansi-color ring misearch
multi-isearch dired-aux dired edmacro kmacro cl-loaddefs pcase cl-lib
time-date mule-util tooltip eldoc electric uniquify ediff-hook vc-hooks
lisp-float-type mwheel dos-w32 ls-lisp disp-table w32-win w32-vars
term/common-win tool-bar dnd fontset image regexp-opt fringe
tabulated-list newcomment elisp-mode lisp-mode prog-mode register page
menu-bar rfn-eshadow timer select scroll-bar mouse jit-lock font-lock
syntax facemenu font-core frame 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 charscript case-table epa-hook jka-cmpr-hook help
simple abbrev minibuffer cl-preloaded nadvice loaddefs button faces
cus-face macroexp files text-properties overlay sha1 md5 base64 format
env code-pages mule custom widget hashtable-print-readable backquote
w32notify dbusbind w32 multi-tty make-network-process emacs)

Memory information:
((conses 16 115954 17326)
 (symbols 56 22177 0)
 (miscs 48 87 139)
 (strings 32 22933 5432)
 (string-bytes 1 705086)
 (vectors 16 15394)
 (vector-slots 8 475031 4303)
 (floats 8 230 198)
 (intervals 56 1290 331)
 (buffers 976 15))

-- 

-- 
Kaushal Modi

[-- Attachment #2: Type: text/html, Size: 12749 bytes --]

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

* bug#23453: 25.0.93; dired-do-find-regexp does not work on Windows even after installing GNU find and grep
  2016-05-04 19:39 bug#23453: 25.0.93; dired-do-find-regexp does not work on Windows even after installing GNU find and grep Kaushal Modi
@ 2016-05-04 19:49 ` Eli Zaretskii
  2016-05-04 19:56   ` Kaushal Modi
  0 siblings, 1 reply; 14+ messages in thread
From: Eli Zaretskii @ 2016-05-04 19:49 UTC (permalink / raw)
  To: Kaushal Modi; +Cc: 23453, dgutov

> From: Kaushal Modi <kaushal.modi@gmail.com>
> Date: Wed, 04 May 2016 19:39:45 +0000
> 
> The problem is that the user PATH env var comes at a lower order of precendence than the system value of
> the PATH env var. So first the admin controlled directories come in PATH and then the user added directories
> come.

You need to change the system Path, not the user Path.

Alternatively, rename the MS find.exe to something else.

> By I used the brute-force method below just so that the correct find.exe and grep.exe are found by emacs:
> 
> (setq exec-path '("C:/Users/kmodi/Dropbox/Portable Software/ezwinports/bin"
> "c:/ProgramData/Oracle/Java/javapath"

No, don't do that, it won't work (as you have found out).  Emacs
sometimes invokes commands through the shell, which doesn't know about
exec-path.  This way lies madness.  You should have your PATH and the
corresponding Emacs variables in sync.

(This is all basic Windows setup, nothing related to Emacs, btw.)





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

* bug#23453: 25.0.93; dired-do-find-regexp does not work on Windows even after installing GNU find and grep
  2016-05-04 19:49 ` Eli Zaretskii
@ 2016-05-04 19:56   ` Kaushal Modi
  2016-05-04 20:24     ` Kaushal Modi
  2016-05-05  2:37     ` Eli Zaretskii
  0 siblings, 2 replies; 14+ messages in thread
From: Kaushal Modi @ 2016-05-04 19:56 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 23453, dgutov

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

>
> You need to change the system Path, not the user Path.
>
> Alternatively, rename the MS find.exe to something else.
>

I do not have admin access (work computer). Wouldn't (setenv "PATH" "..")
be the same?

If I rename the MS find.exe (which I cannot without admin aceess), will it
not affect any other program using it?


> > By I used the brute-force method below just so that the correct find.exe
> and grep.exe are found by emacs:
> >
> > (setq exec-path '("C:/Users/kmodi/Dropbox/Portable
> Software/ezwinports/bin"
> > "c:/ProgramData/Oracle/Java/javapath"
>
> No, don't do that, it won't work (as you have found out).  Emacs
> sometimes invokes commands through the shell, which doesn't know about
> exec-path.  This way lies madness.  You should have your PATH and the
> corresponding Emacs variables in sync.
>
> (This is all basic Windows setup, nothing related to Emacs, btw.)
>

As I mentioned, I also set the PATH using (setenv "PATH "..."). Isn't
executable-find returning the correct find.exe and grep.exe a proof that
the right executable is being found.

I have had this in my config and it works fine (for a different executable
chrome.exe):

    (let ((chrome-path "C:/Program Files (x86)/Google/Chrome/Application/"))
      (setq exec-path (append exec-path `(,chrome-path)))
      (setq browse-url-generic-program (executable-find "chrome")))



-- 

-- 
Kaushal Modi

[-- Attachment #2: Type: text/html, Size: 2078 bytes --]

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

* bug#23453: 25.0.93; dired-do-find-regexp does not work on Windows even after installing GNU find and grep
  2016-05-04 19:56   ` Kaushal Modi
@ 2016-05-04 20:24     ` Kaushal Modi
  2016-05-04 21:14       ` Kaushal Modi
  2016-05-05  2:37     ` Eli Zaretskii
  1 sibling, 1 reply; 14+ messages in thread
From: Kaushal Modi @ 2016-05-04 20:24 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 23453, dgutov

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

Oh, as it turns out, the bug is there but not related to not finding
find/grep.

It IS actually finding the find.exe and grep.exe correctly using the
exec-path and/or (setenv "PATH" ..) hacks.

The problem was that the file was in a path with spaces in it:

C:\Users\kmodi\Desktop\_keep this folder empty\NEWS.txt

When I try using `A' in dired with above file marked and search for
"Emacs", I get "No matches for: Emacs". Note the spaces in the directory
name: "_keep this folder empty"

But when I copied NEWS.txt to C:\Users\kmodi\Desktop\NEWS.txt and then did
the same "Emacs" search using `A' in dired, it worked!

I can recreate this bug in RHEL too!

(1) mkdir -p /tmp/some\ dir
(2) Download http://git.savannah.gnu.org/cgit/emacs.git/plain/etc/NEWS as
/tmp/some\ dir/NEWS.txt
(3) In emacs -Q, use C-x d to open dired in /tmp/some\ dir/
(4) Mark NEWS.txt using `m' key
(5) Then I do `A' and search for "Emacs" (or probably anything else too)
(6) You should get "No matches for: Emacs"

Can you please rename the bug title to "25.0.93; dired-do-find-regexp does
not work when path contains spaces"?

Blocking bug?
-- 

-- 
Kaushal Modi

[-- Attachment #2: Type: text/html, Size: 2301 bytes --]

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

* bug#23453: 25.0.93; dired-do-find-regexp does not work on Windows even after installing GNU find and grep
  2016-05-04 20:24     ` Kaushal Modi
@ 2016-05-04 21:14       ` Kaushal Modi
  2016-05-04 21:39         ` Kaushal Modi
  0 siblings, 1 reply; 14+ messages in thread
From: Kaushal Modi @ 2016-05-04 21:14 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 23453, dgutov

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

I have verified the below patch to solve this issue:
Can you please review/fix/commit as needed?

From a20c0f42dbbecb356b286875b684f01fd25c6ebb Mon Sep 17 00:00:00 2001
From: Kaushal Modi <kaushal.modi@gmail.com>
Date: Wed, 4 May 2016 17:06:44 -0400
Subject: [PATCH] Shell escape arguments for find cmd used by xref

* lisp/progmodes/xref.el (xref-collect-matches): When the dir has
  characters like spaces (e.g. /tmp/some dir/), those need to be escaped
  before passing it as an argument to the shell command like `find'.
  The escaping is done using `shell-quote-argument' (bug#23453).
---
 lisp/progmodes/xref.el | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lisp/progmodes/xref.el b/lisp/progmodes/xref.el
index 62cef23..ff87bc3 100644
--- a/lisp/progmodes/xref.el
+++ b/lisp/progmodes/xref.el
@@ -890,7 +890,7 @@ xref-collect-matches
          (grep-highlight-matches nil)
          (command (xref--rgrep-command (xref--regexp-to-extended regexp)
                                        files
-                                       (expand-file-name dir)
+                                       (shell-quote-argument
(expand-file-name dir))
                                        ignores))
          (buf (get-buffer-create " *xref-grep*"))
          (grep-re (caar grep-regexp-alist))
-- 
2.6.0.rc0.24.gec371ff




On Wed, May 4, 2016 at 4:24 PM Kaushal Modi <kaushal.modi@gmail.com> wrote:

> Oh, as it turns out, the bug is there but not related to not finding
> find/grep.
>
> It IS actually finding the find.exe and grep.exe correctly using the
> exec-path and/or (setenv "PATH" ..) hacks.
>
> The problem was that the file was in a path with spaces in it:
>
> C:\Users\kmodi\Desktop\_keep this folder empty\NEWS.txt
>
> When I try using `A' in dired with above file marked and search for
> "Emacs", I get "No matches for: Emacs". Note the spaces in the directory
> name: "_keep this folder empty"
>
> But when I copied NEWS.txt to C:\Users\kmodi\Desktop\NEWS.txt and then
> did the same "Emacs" search using `A' in dired, it worked!
>
> I can recreate this bug in RHEL too!
>
> (1) mkdir -p /tmp/some\ dir
> (2) Download http://git.savannah.gnu.org/cgit/emacs.git/plain/etc/NEWS as
> /tmp/some\ dir/NEWS.txt
> (3) In emacs -Q, use C-x d to open dired in /tmp/some\ dir/
> (4) Mark NEWS.txt using `m' key
> (5) Then I do `A' and search for "Emacs" (or probably anything else too)
> (6) You should get "No matches for: Emacs"
>
> Can you please rename the bug title to "25.0.93; dired-do-find-regexp
> does not work when path contains spaces"?
>
> Blocking bug?
> --
>
> --
> Kaushal Modi
>
-- 

-- 
Kaushal Modi

[-- Attachment #2: Type: text/html, Size: 4572 bytes --]

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

* bug#23453: 25.0.93; dired-do-find-regexp does not work on Windows even after installing GNU find and grep
  2016-05-04 21:14       ` Kaushal Modi
@ 2016-05-04 21:39         ` Kaushal Modi
  2016-05-04 22:15           ` Dmitry Gutov
  0 siblings, 1 reply; 14+ messages in thread
From: Kaushal Modi @ 2016-05-04 21:39 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 23453, dgutov

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

Here's an alternative patch which does the same thing:

From 788e97faa859798b67ec7b0b7facd843fae85c5f Mon Sep 17 00:00:00 2001
From: Kaushal Modi <kaushal.modi@gmail.com>
Date: Wed, 4 May 2016 17:37:42 -0400
Subject: [PATCH] Shell escape arguments for find cmd used by xref

* lisp/progmodes/xref.el (xref--rgrep-command): When the dir has
  characters like spaces (e.g. /tmp/some dir/), those need to be escaped
  before passing it as an argument to the shell command like `find'.
  The escaping is done using `shell-quote-argument' (bug#23453).
---
 lisp/progmodes/xref.el | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lisp/progmodes/xref.el b/lisp/progmodes/xref.el
index 62cef23..1764e88 100644
--- a/lisp/progmodes/xref.el
+++ b/lisp/progmodes/xref.el
@@ -921,8 +921,8 @@ xref--rgrep-command
             (concat " -o " find-name-arg " "))
            " "
            (shell-quote-argument ")"))
-   dir
-   (xref--find-ignores-arguments ignores dir)))
+   (shell-quote-argument dir)
+   (xref--find-ignores-arguments ignores (shell-quote-argument dir))))

 (defun xref--find-ignores-arguments (ignores dir)
   "Convert IGNORES and DIR to a list of arguments for 'find'.
-- 
2.6.0.rc0.24.gec371ff

-- 

-- 
Kaushal Modi

[-- Attachment #2: Type: text/html, Size: 1813 bytes --]

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

* bug#23453: 25.0.93; dired-do-find-regexp does not work on Windows even after installing GNU find and grep
  2016-05-04 21:39         ` Kaushal Modi
@ 2016-05-04 22:15           ` Dmitry Gutov
  2016-05-04 22:30             ` Kaushal Modi
  2016-05-05  2:44             ` Eli Zaretskii
  0 siblings, 2 replies; 14+ messages in thread
From: Dmitry Gutov @ 2016-05-04 22:15 UTC (permalink / raw)
  To: Kaushal Modi, Eli Zaretskii; +Cc: 23453

Oops, thanks for catching this.

On 05/05/2016 12:39 AM, Kaushal Modi wrote:

> +   (shell-quote-argument dir)

This looks good.

> +   (xref--find-ignores-arguments ignores (shell-quote-argument dir))))

I'm not sure about this line, on the other hand. Is it actually 
required? If yes, this quoting should be performed inside 
xref--find-ignores-arguments, I think.

P.S. The last few messages were addressed to bug-gnu-emacs@gnu.org 
instead of this bug's address. Sending this to the right address now.





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

* bug#23453: 25.0.93; dired-do-find-regexp does not work on Windows even after installing GNU find and grep
  2016-05-04 22:15           ` Dmitry Gutov
@ 2016-05-04 22:30             ` Kaushal Modi
  2016-05-05  1:18               ` Dmitry Gutov
  2016-05-05  2:44             ` Eli Zaretskii
  1 sibling, 1 reply; 14+ messages in thread
From: Kaushal Modi @ 2016-05-04 22:30 UTC (permalink / raw)
  To: Dmitry Gutov, Eli Zaretskii; +Cc: 23453

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

>
> > +   (shell-quote-argument dir)
>
> This looks good.
>

Thanks.


> > +   (xref--find-ignores-arguments ignores (shell-quote-argument dir))))
>
> I'm not sure about this line, on the other hand. Is it actually
> required?


My guess would be yes based on just C-h f xref--find-ignores-arguments.

Convert IGNORES and DIR to a list of arguments for ’find’.
IGNORES is a list of glob patterns.  DIR is an absolute
directory, used as the root of the ignore globs.

If DIR is going to be an argument to `find', it should be shell quoted too,
right? For the example in this bug report though, it does not matter with
or without shell quoting dir there (in arg to
xref--find-ignores-arguments). The second version of patch simply has the
shell-quote-argument propagated into the xref--rgrep-command.


> If yes, this quoting should be performed inside
> xref--find-ignores-arguments, I think.


Or how about this 3rd version of the patch:

From 1f114a74de1d28e06edd9c074774a087c1d19bd5 Mon Sep 17 00:00:00 2001
From: Kaushal Modi <kaushal.modi@gmail.com>
Date: Wed, 4 May 2016 18:25:50 -0400
Subject: [PATCH] Shell escape arguments for find/grep used by xref

* lisp/progmodes/xref.el (xref--rgrep-command): When the dir has
  characters like spaces (e.g. /tmp/some dir/), those need to be escaped
  before passing it as an argument to the shell command like `find'.
  The escaping is done using `shell-quote-argument' (bug#23453).
---
 lisp/progmodes/xref.el | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lisp/progmodes/xref.el b/lisp/progmodes/xref.el
index 62cef23..ccf20c1 100644
--- a/lisp/progmodes/xref.el
+++ b/lisp/progmodes/xref.el
@@ -910,6 +910,7 @@ xref--rgrep-command
   (require 'find-dired)      ; for `find-name-arg'
   (defvar grep-find-template)
   (defvar find-name-arg)
+  (setq dir (shell-quote-argument dir)) ; /some dir/ → /some\ dir/
   (grep-expand-template
    grep-find-template
    regexp
-- 
2.6.0.rc0.24.gec371ff

It feels untidy to shell-quote dir separately; once in the
grep-expand-template form and second time inside
xref--find-ignores-arguments. Also it could get confusing keeping track of
if dir was already shell-quoted by the time it entered
xref--find-ignores-arguments or not.

A comment in that last function also says:

    ;; `shell-quote-argument' quotes the tilde as well.
    (cl-assert (not (string-match-p "\\`~" dir)))

So looks like dir is expected to be shell-quoted before it entered that
function.


-- 

-- 
Kaushal Modi

[-- Attachment #2: Type: text/html, Size: 3705 bytes --]

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

* bug#23453: 25.0.93; dired-do-find-regexp does not work on Windows even after installing GNU find and grep
  2016-05-04 22:30             ` Kaushal Modi
@ 2016-05-05  1:18               ` Dmitry Gutov
  2016-05-05 16:23                 ` Kaushal Modi
  0 siblings, 1 reply; 14+ messages in thread
From: Dmitry Gutov @ 2016-05-05  1:18 UTC (permalink / raw)
  To: Kaushal Modi, Eli Zaretskii; +Cc: 23453

On 05/05/2016 01:30 AM, Kaushal Modi wrote:

> Convert IGNORES and DIR to a list of arguments for ’find’.
> IGNORES is a list of glob patterns.  DIR is an absolute
> directory, used as the root of the ignore globs.
>
> If DIR is going to be an argument to `find', it should be shell quoted
> too, right?

That doesn't mean that it isn't being quoted now, in the function in 
question.

> Or how about this 3rd version of the patch:

It's functionally equivalent to the second one, I believe.

> It feels untidy to shell-quote dir separately; once in the
> grep-expand-template form and second time inside
> xref--find-ignores-arguments. Also it could get confusing keeping track
> of if dir was already shell-quoted by the time it entered
> xref--find-ignores-arguments or not.

shell-quoting is the last thing you do to an argument. Quoting an then 
transforming it rarely makes sense.

> A comment in that last function also says:
>
>     ;; `shell-quote-argument' quotes the tilde as well.
>     (cl-assert (not (string-match-p "\\`~" dir)))
>
> So looks like dir is expected to be shell-quoted before it entered that
> function.

Actually, this comment means the opposite: we would want ~ in the 
command line unquoted, but that won't happen because DIR is quoted 
later. So we require DIR to be non-abbreviated.

Anyway, let's leave that off until you find a definite case where it's a 
problem. I've pushed the other change now in ab3ba91.

Please see if it fixes the problem, and if so, close the bug.





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

* bug#23453: 25.0.93; dired-do-find-regexp does not work on Windows even after installing GNU find and grep
  2016-05-04 19:56   ` Kaushal Modi
  2016-05-04 20:24     ` Kaushal Modi
@ 2016-05-05  2:37     ` Eli Zaretskii
  2016-05-05 16:36       ` Kaushal Modi
  1 sibling, 1 reply; 14+ messages in thread
From: Eli Zaretskii @ 2016-05-05  2:37 UTC (permalink / raw)
  To: Kaushal Modi; +Cc: 23453, dgutov

> From: Kaushal Modi <kaushal.modi@gmail.com>
> Date: Wed, 04 May 2016 19:56:02 +0000
> Cc: bug-gnu-emacs@gnu.org, dgutov@yandex.ru
> 
>  You need to change the system Path, not the user Path.
> 
>  Alternatively, rename the MS find.exe to something else.
> 
> I do not have admin access (work computer). Wouldn't (setenv "PATH" "..") be the same?

No.

> If I rename the MS find.exe (which I cannot without admin aceess), will it not affect any other program using it?

Nothing uses it.

> As I mentioned, I also set the PATH using (setenv "PATH "..."). Isn't executable-find returning the correct
> find.exe and grep.exe a proof that the right executable is being found.

No.  See the source of executable-find.

> I have had this in my config and it works fine (for a different executable chrome.exe):
> 
> (let ((chrome-path "C:/Program Files (x86)/Google/Chrome/Application/"))
> (setq exec-path (append exec-path `(,chrome-path)))
> (setq browse-url-generic-program (executable-find "chrome")))

Bad idea, as I explained in my earlier message.  You need to change
PATH _outside_ of Emacs.






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

* bug#23453: 25.0.93; dired-do-find-regexp does not work on Windows even after installing GNU find and grep
  2016-05-04 22:15           ` Dmitry Gutov
  2016-05-04 22:30             ` Kaushal Modi
@ 2016-05-05  2:44             ` Eli Zaretskii
  1 sibling, 0 replies; 14+ messages in thread
From: Eli Zaretskii @ 2016-05-05  2:44 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: 23453, kaushal.modi

> Cc: 23453@debbugs.gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Thu, 5 May 2016 01:15:37 +0300
> 
> Oops, thanks for catching this.
> 
> On 05/05/2016 12:39 AM, Kaushal Modi wrote:
> 
> > +   (shell-quote-argument dir)
> 
> This looks good.
> 
> > +   (xref--find-ignores-arguments ignores (shell-quote-argument dir))))
> 
> I'm not sure about this line, on the other hand. Is it actually 
> required? If yes, this quoting should be performed inside 
> xref--find-ignores-arguments, I think.

Indeed, it is IMO wrong to use shell-quote-argument anywhere except
immediately before invoking the shell command.  If this is what is
needed to fix the bug, the bug is elsewhere.





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

* bug#23453: 25.0.93; dired-do-find-regexp does not work on Windows even after installing GNU find and grep
  2016-05-05  1:18               ` Dmitry Gutov
@ 2016-05-05 16:23                 ` Kaushal Modi
  0 siblings, 0 replies; 14+ messages in thread
From: Kaushal Modi @ 2016-05-05 16:23 UTC (permalink / raw)
  To: Dmitry Gutov, Eli Zaretskii; +Cc: 23453-done

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

On Wed, May 4, 2016 at 9:18 PM Dmitry Gutov <dgutov@yandex.ru> wrote:

> That doesn't mean that it isn't being quoted now, in the function in
> question.
>

OK


> It's functionally equivalent to the second one, I believe.
>

It is! I just rearranged the position for shell-quote-argument. Actually
all 3 patches were the same, functionally.


> Actually, this comment means the opposite: we would want ~ in the
> command line unquoted, but that won't happen because DIR is quoted
> later. So we require DIR to be non-abbreviated.
>
> Anyway, let's leave that off until you find a definite case where it's a
> problem.


OK


> I've pushed the other change now in ab3ba91.
>
> Please see if it fixes the problem, and if so, close the bug.
>

It does fix the bug, thanks!
-- 

-- 
Kaushal Modi

[-- Attachment #2: Type: text/html, Size: 1599 bytes --]

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

* bug#23453: 25.0.93; dired-do-find-regexp does not work on Windows even after installing GNU find and grep
  2016-05-05  2:37     ` Eli Zaretskii
@ 2016-05-05 16:36       ` Kaushal Modi
  2016-05-05 16:52         ` Eli Zaretskii
  0 siblings, 1 reply; 14+ messages in thread
From: Kaushal Modi @ 2016-05-05 16:36 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 23453@debbugs.gnu.org, dgutov

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

>
> > I do not have admin access (work computer). Wouldn't (setenv "PATH"
> "..") be the same?
>
> No.
>

But the fact that I am able to do so says otherwise. I removed the path
customization from both user and system PATH values (I had got temp admin
rights to update the system PATH in Windows), added the below to my config
and now find/grep work beautifully.

  (let ((ezwinports-path "C:/Users/kmodi/Dropbox/Portable
Software/ezwinports/bin"))
      (setq exec-path (add-to-list 'exec-path ezwinports-path))
      (setenv "PATH" (concat (replace-regexp-in-string
                              "/" "\\\\" ezwinports-path) ";"
                              (getenv "PATH"))))

> If I rename the MS find.exe (which I cannot without admin aceess), will
> it not affect any other program using it?
>
> Nothing uses it.
>

I wasn't aware of that, thanks.

> As I mentioned, I also set the PATH using (setenv "PATH "..."). Isn't
> executable-find returning the correct
> > find.exe and grep.exe a proof that the right executable is being found.
>
> No.  See the source of executable-find.
>

OK, but I am able to reference to the chrome, find, grep binaryies
correctly using only the tweaks to exeec-path and PATH from within emacs. I
do not need to change it in Windows settings.

As executable-find uses exec-path, I think that manipulating just that
suffices for applications like chrome and firefox that are not executed
from shell (unlike find and grep). I am saying this from what I practically
see on my Windows machine.


> > I have had this in my config and it works fine (for a different
> executable chrome.exe):
> >
> > (let ((chrome-path "C:/Program Files (x86)/Google/Chrome/Application/"))
> > (setq exec-path (append exec-path `(,chrome-path)))
> > (setq browse-url-generic-program (executable-find "chrome")))
>
> Bad idea, as I explained in my earlier message.  You need to change
> PATH _outside_ of Emacs.
>

Well, as mentioned earlier, changing the exec-path and doing setenv PATH
from within emacs works just fine.

In summary I have this in my Windows emacs config and it works great
without needing external PATH manipulation.

=====
(let ((firefox-path "C:/Users/kmodi/AppData/Local/Mozilla Firefox/"))
      (setq exec-path (append exec-path `(,firefox-path))))

    (let ((chrome-path "C:/Program Files (x86)/Google/Chrome/Application/"))
      (setq exec-path (append exec-path `(,chrome-path)))
      (setq browse-url-generic-program (executable-find "chrome")))

    (let ((ezwinports-path "C:/Users/kmodi/Dropbox/Portable
Software/ezwinports/bin"))
      (setq exec-path (add-to-list 'exec-path ezwinports-path))
      (setenv "PATH" (concat (replace-regexp-in-string "/" "\\\\"
ezwinports-path)
                             ";" (getenv "PATH"))))
=====
-- 

-- 
Kaushal Modi

[-- Attachment #2: Type: text/html, Size: 4142 bytes --]

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

* bug#23453: 25.0.93; dired-do-find-regexp does not work on Windows even after installing GNU find and grep
  2016-05-05 16:36       ` Kaushal Modi
@ 2016-05-05 16:52         ` Eli Zaretskii
  0 siblings, 0 replies; 14+ messages in thread
From: Eli Zaretskii @ 2016-05-05 16:52 UTC (permalink / raw)
  To: Kaushal Modi; +Cc: 23453, dgutov

> From: Kaushal Modi <kaushal.modi@gmail.com>
> Date: Thu, 05 May 2016 16:36:00 +0000
> Cc: dgutov@yandex.ru, "23453@debbugs.gnu.org" <23453@debbugs.gnu.org>
> 
>  > I have had this in my config and it works fine (for a different executable chrome.exe):
>  >
>  > (let ((chrome-path "C:/Program Files (x86)/Google/Chrome/Application/"))
>  > (setq exec-path (append exec-path `(,chrome-path)))
>  > (setq browse-url-generic-program (executable-find "chrome")))
> 
>  Bad idea, as I explained in my earlier message. You need to change
>  PATH _outside_ of Emacs.
> 
> Well, as mentioned earlier, changing the exec-path and doing setenv PATH from within emacs works just
> fine.

Suit yourself, but my grey hair says this has subtle problems.  You
are welcome to wait for them to happen, of course ;-)  A lesson
learned from your own experience is much more vivid that if you learn
from that of others.





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

end of thread, other threads:[~2016-05-05 16:52 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-05-04 19:39 bug#23453: 25.0.93; dired-do-find-regexp does not work on Windows even after installing GNU find and grep Kaushal Modi
2016-05-04 19:49 ` Eli Zaretskii
2016-05-04 19:56   ` Kaushal Modi
2016-05-04 20:24     ` Kaushal Modi
2016-05-04 21:14       ` Kaushal Modi
2016-05-04 21:39         ` Kaushal Modi
2016-05-04 22:15           ` Dmitry Gutov
2016-05-04 22:30             ` Kaushal Modi
2016-05-05  1:18               ` Dmitry Gutov
2016-05-05 16:23                 ` Kaushal Modi
2016-05-05  2:44             ` Eli Zaretskii
2016-05-05  2:37     ` Eli Zaretskii
2016-05-05 16:36       ` Kaushal Modi
2016-05-05 16:52         ` 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).