Re: master 370b216f086: New variable 'project-files-relative-names'

Re: master 370b216f086: New variable 'project-files-relative-names'

[Michael Albinus]

Dmitry Gutov <dgutov@yandex.ru> writes:

Hi Dmitry,

>     * test/lisp/progmodes/project-tests.el (project-find-regexp):
>     New test.

This fails in batch mode:

--8<---------------cut here---------------start------------->8---
# make -C test project-tests
Test project-find-regexp backtrace:
  write-region(nil nil "~/.emacs.d/projects" nil silent)
  project--write-project-list()
  project--remember-dir("/home/albinus/src/emacs/test/lisp/progmodes/p
  project-remember-project((transient . "/home/albinus/src/emacs/test/
  project-current(t)
  project-find-regexp("etc")
  (let* ((matches nil) (xref-search-program 'grep) (xref-show-xrefs-fu
  (let* ((directory (let* ((testfile (and t "/home/albinus/src/emacs/t
  #f(lambda () [cl-struct-project-tests--trivial-tags t] (let* ((fn-13
  #f(compiled-function () #<bytecode -0xab8edc4a3389ce5>)()
  handler-bind-1(#f(compiled-function () #<bytecode -0xab8edc4a3389ce5
  ert--run-test-internal(#s(ert--test-execution-info :test #s(ert-test
  ert-run-test(#s(ert-test :name project-find-regexp :documentation "C
  ert-run-or-rerun-test(#s(ert--stats :selector (not ...) :tests [...
  ert-run-tests((not (tag :unstable)) #f(compiled-function (event-type
  ert-run-tests-batch((not (tag :unstable)))
  ert-run-tests-batch-and-exit((not (tag :unstable)))
  eval((ert-run-tests-batch-and-exit '(not (tag :unstable))) t)
  command-line-1(("-L" ":." "-l" "ert" "--eval" "(setq treesit-extra-l
  command-line()
  normal-top-level()
Test project-find-regexp condition:
    (file-missing "Opening output file" "No such file or directory"
		  "/nonexistent/.emacs.d/projects")
   FAILED  1/8  project-find-regexp (0.007078 sec) at lisp/progmodes/project-tests.el:166
--8<---------------cut here---------------end--------------->8---

Best regards, Michael.

Re: master 370b216f086: New variable 'project-files-relative-names'

[Eli Zaretskii]

> From: Michael Albinus <michael.albinus@gmx.de>
> Cc: Dmitry Gutov <dmitry@gutov.dev>
> Date: Sun, 05 May 2024 09:53:10 +0200
> 
> Dmitry Gutov <dgutov@yandex.ru> writes:
> 
> Hi Dmitry,
> 
> >     * test/lisp/progmodes/project-tests.el (project-find-regexp):
> >     New test.
> 
> This fails in batch mode:
> 
> --8<---------------cut here---------------start------------->8---
> # make -C test project-tests
> Test project-find-regexp backtrace:

This test passes here, FWIW (but
project-vc-supports-project-in-different-dir fails).

Re: master 370b216f086: New variable 'project-files-relative-names'

[Eli Zaretskii]

> Date: Sun, 05 May 2024 12:14:26 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: emacs-devel@gnu.org, dmitry@gutov.dev
> 
> > >     * test/lisp/progmodes/project-tests.el (project-find-regexp):
> > >     New test.
> > 
> > This fails in batch mode:
> > 
> > --8<---------------cut here---------------start------------->8---
> > # make -C test project-tests
> > Test project-find-regexp backtrace:
> 
> This test passes here, FWIW (but
> project-vc-supports-project-in-different-dir fails).

After some debugging, the reason seems to be the use of that
":(exclude,glob,top)%s" thingie in project--vc-list-files.  Which
version of Git introduced it?  When I use it here, I get an empty list
of files.

Aren't there alternatives for these exclusion patterns that older
versions of Git support?

Re: master 370b216f086: New variable 'project-files-relative-names'

[Dmitry Gutov]

On 05/05/2024 17:19, Eli Zaretskii wrote:
> After some debugging, the reason seems to be the use of that
> ":(exclude,glob,top)%s" thingie in project--vc-list-files.  Which
> version of Git introduced it?

I don't know the exact version, but git pathspecs are mentioned in this 
2014 SO question and in this 2015 article:

https://stackoverflow.com/questions/27711924/whats-a-pathspec-in-the-git-command
https://kgrz.io/git-intro-to-pathspec.html

That seems old enough to me?

And the use of this feature was added to project.el in 2020, FWIW.

Re: master 370b216f086: New variable 'project-files-relative-names'

[Eli Zaretskii]

> Date: Sun, 5 May 2024 18:46:30 +0300
> Cc: michael.albinus@gmx.de, emacs-devel@gnu.org
> From: Dmitry Gutov <dmitry@gutov.dev>
> 
> On 05/05/2024 17:19, Eli Zaretskii wrote:
> > After some debugging, the reason seems to be the use of that
> > ":(exclude,glob,top)%s" thingie in project--vc-list-files.  Which
> > version of Git introduced it?
> 
> I don't know the exact version, but git pathspecs are mentioned in this 
> 2014 SO question and in this 2015 article:
> 
> https://stackoverflow.com/questions/27711924/whats-a-pathspec-in-the-git-command
> https://kgrz.io/git-intro-to-pathspec.html
> 
> That seems old enough to me?
> 
> And the use of this feature was added to project.el in 2020, FWIW.

Maybe it's some other problem with it, because these pathspecs are
documented in my Git's man pages, but the commands I tried came up
with an empty list on my system, but with a non-empty list on another
one, where a newer Git is installed.  Could you perhaps show an
example of such a command line that I could try here?

Regardless, top of the backtrace from the failure looks like this:

  Test project-vc-supports-project-in-different-dir backtrace:
    string-match("\0" nil 0)
    split-string(nil "\0" t)
    project--vc-list-files("d:/gnu/git/emacs/trunk/test/lisp/progmodes/p

IOW, it tries to split-string nil.

Re: master 370b216f086: New variable 'project-files-relative-names'

[Dmitry Gutov]

On 05/05/2024 19:21, Eli Zaretskii wrote:
>> Date: Sun, 5 May 2024 18:46:30 +0300
>> Cc: michael.albinus@gmx.de, emacs-devel@gnu.org
>> From: Dmitry Gutov <dmitry@gutov.dev>
>>
>> On 05/05/2024 17:19, Eli Zaretskii wrote:
>>> After some debugging, the reason seems to be the use of that
>>> ":(exclude,glob,top)%s" thingie in project--vc-list-files.  Which
>>> version of Git introduced it?
>>
>> I don't know the exact version, but git pathspecs are mentioned in this
>> 2014 SO question and in this 2015 article:
>>
>> https://stackoverflow.com/questions/27711924/whats-a-pathspec-in-the-git-command
>> https://kgrz.io/git-intro-to-pathspec.html
>>
>> That seems old enough to me?
>>
>> And the use of this feature was added to project.el in 2020, FWIW.
> 
> Maybe it's some other problem with it, because these pathspecs are
> documented in my Git's man pages, but the commands I tried came up
> with an empty list on my system, but with a non-empty list on another
> one, where a newer Git is installed.  Could you perhaps show an
> example of such a command line that I could try here?

BTW, the pathspecs are only used when the EXTRA-IGNORES argument is 
non-nil. Meaning project-vc-ignores is set, either globally or through 
dir-locals. Perhaps you have a value in there that confuses Git?

Here's what the command looks like if I just set project-vc-ignores to 
'("./test"):

   git --no-pager ls-files -z -c --exclude-standard -o -- 
:(exclude,glob,top)test .

> Regardless, top of the backtrace from the failure looks like this:
> 
>    Test project-vc-supports-project-in-different-dir backtrace:
>      string-match("\0" nil 0)
>      split-string(nil "\0" t)
>      project--vc-list-files("d:/gnu/git/emacs/trunk/test/lisp/progmodes/p
> 
> IOW, it tries to split-string nil.

Yeah, the error was somewhat cryptic. I've now pushed a patch below that 
should print the VCS command used when an error happens, see if it makes 
things more clear.

diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
index a3181340411..52fe4df9080 100644
--- a/lisp/progmodes/project.el
+++ b/lisp/progmodes/project.el
@@ -648,7 +648,7 @@ project-files
         (list (project-root project)))))

  (declare-function vc-git--program-version "vc-git")
-(declare-function vc-git--run-command-string "vc-git")
+(declare-function vc-git-command "vc-git")
  (declare-function vc-hg-command "vc-hg")

  (defun project--vc-list-files (dir backend extra-ignores)
@@ -701,7 +701,8 @@ project--vc-list-files
                              file
                            (concat default-directory file))))
                      (split-string
-                     (apply #'vc-git--run-command-string nil "ls-files" 
args)
+                     (with-output-to-string
+                       (apply #'vc-git-command standard-output 0 nil 
"ls-files" args))
                       "\0" t))))
         (when (project--vc-merge-submodules-p default-directory)
           ;; Unfortunately, 'ls-files --recurse-submodules' conflicts 
with '-o'.

Re: master 370b216f086: New variable 'project-files-relative-names'

[Eli Zaretskii]

> Date: Sun, 5 May 2024 20:28:56 +0300
> Cc: michael.albinus@gmx.de, emacs-devel@gnu.org
> From: Dmitry Gutov <dmitry@gutov.dev>
> 
> > Maybe it's some other problem with it, because these pathspecs are
> > documented in my Git's man pages, but the commands I tried came up
> > with an empty list on my system, but with a non-empty list on another
> > one, where a newer Git is installed.  Could you perhaps show an
> > example of such a command line that I could try here?
> 
> BTW, the pathspecs are only used when the EXTRA-IGNORES argument is 
> non-nil. Meaning project-vc-ignores is set, either globally or through 
> dir-locals. Perhaps you have a value in there that confuses Git?

I'm only running the test, so EXTRA-IGNORES are as the
project-vc-supports-project-in-different-dir test sets it.

> 
> Here's what the command looks like if I just set project-vc-ignores to 
> '("./test"):
> 
>    git --no-pager ls-files -z -c --exclude-standard -o -- :(exclude,glob,top)test .

This fails here with:

  bash: syntax error near unexpected token `('

and I need to quote ":(exclude,glob,top)test" to get a list of files.

> > Regardless, top of the backtrace from the failure looks like this:
> > 
> >    Test project-vc-supports-project-in-different-dir backtrace:
> >      string-match("\0" nil 0)
> >      split-string(nil "\0" t)
> >      project--vc-list-files("d:/gnu/git/emacs/trunk/test/lisp/progmodes/p
> > 
> > IOW, it tries to split-string nil.
> 
> Yeah, the error was somewhat cryptic. I've now pushed a patch below that 
> should print the VCS command used when an error happens, see if it makes 
> things more clear.

Thanks, with this patch I get

  Test project-vc-supports-project-in-different-dir condition:
      (error
       "Failed (status 128): git --no-pager ls-files -z -c --exclude-standard -o -- :(exclude,glob,top)**/etc .")

And if I quote the ":(exclude,glob,top)**/etc" pathspec in
project-vc-supports-project-in-different-dir, I get the nil value
again:

  Test project-vc-supports-project-in-different-dir condition:
      (ert-test-failed
       ((should (equal '(".dir-locals.el" "foo") (mapcar ... ...))) :form
	(equal (".dir-locals.el" "foo") nil) :value nil :explanation
	(different-types (".dir-locals.el" "foo") nil)))

This could be Windows-specific: Git is invoked via a script, which
might not behave correctly with quoting.

Re: master 370b216f086: New variable 'project-files-relative-names'

[Dmitry Gutov]

On 05/05/2024 20:43, Eli Zaretskii wrote:
>> Date: Sun, 5 May 2024 20:28:56 +0300
>> Cc: michael.albinus@gmx.de, emacs-devel@gnu.org
>> From: Dmitry Gutov <dmitry@gutov.dev>
>>
>>> Maybe it's some other problem with it, because these pathspecs are
>>> documented in my Git's man pages, but the commands I tried came up
>>> with an empty list on my system, but with a non-empty list on another
>>> one, where a newer Git is installed.  Could you perhaps show an
>>> example of such a command line that I could try here?
>>
>> BTW, the pathspecs are only used when the EXTRA-IGNORES argument is
>> non-nil. Meaning project-vc-ignores is set, either globally or through
>> dir-locals. Perhaps you have a value in there that confuses Git?
> 
> I'm only running the test, so EXTRA-IGNORES are as the
> project-vc-supports-project-in-different-dir test sets it.

Okay. So they *are* present, and you haven't seen the problem until now 
because the feature is optional.

>> Here's what the command looks like if I just set project-vc-ignores to
>> '("./test"):
>>
>>     git --no-pager ls-files -z -c --exclude-standard -o -- :(exclude,glob,top)test .
> 
> This fails here with:
> 
>    bash: syntax error near unexpected token `('
> 
> and I need to quote ":(exclude,glob,top)test" to get a list of files.

I see.

But quoting a command argument when invoking it from a shell doesn't 
necessarily mean that you also have to quote it the same way when going 
through 'process-file' or 'call-process' -- in the shell you have to 
cover for the shell's syntax, which shouldn't affect what the commands 
accept as arguments (most of the time).

>>> Regardless, top of the backtrace from the failure looks like this:
>>>
>>>     Test project-vc-supports-project-in-different-dir backtrace:
>>>       string-match("\0" nil 0)
>>>       split-string(nil "\0" t)
>>>       project--vc-list-files("d:/gnu/git/emacs/trunk/test/lisp/progmodes/p
>>>
>>> IOW, it tries to split-string nil.
>>
>> Yeah, the error was somewhat cryptic. I've now pushed a patch below that
>> should print the VCS command used when an error happens, see if it makes
>> things more clear.
> 
> Thanks, with this patch I get
> 
>    Test project-vc-supports-project-in-different-dir condition:
>        (error
>         "Failed (status 128): git --no-pager ls-files -z -c --exclude-standard -o -- :(exclude,glob,top)**/etc .")
> 
> And if I quote the ":(exclude,glob,top)**/etc" pathspec in
> project-vc-supports-project-in-different-dir, I get the nil value
> again:

Not quite: it's a different nil value. Previously you got nil as the 
return value of the function vc-git--run-command-string, which basically 
meant that the command had finished with an error.

Now the backtrace below indicates that the function calls have all 
finished fine, it's just that project-files returns nil. Meaning, the 
command returned empty output -- but with exit status 0. Seems worth 
investigating some more: whether the command was called in the right 
directory, and what it outputs when you call the same command in the 
right directory.

>    Test project-vc-supports-project-in-different-dir condition:
>        (ert-test-failed
>         ((should (equal '(".dir-locals.el" "foo") (mapcar ... ...))) :form
> 	(equal (".dir-locals.el" "foo") nil) :value nil :explanation
> 	(different-types (".dir-locals.el" "foo") nil)))
> 
> This could be Windows-specific: Git is invoked via a script, which
> might not behave correctly with quoting.

It could be that the wrapper script does not support quoting of the 
parens in the arguments correctly. I'm not sure how to handle that 
situation best, but the first step would probably be to try to fix that 
bug in the wrapper locally.

If the bug is indeed in there, an Emacs-internal workaround would likely 
make things worse whenever the fix for the wrapper makes it upstream to 
the official packaging.

Re: master 370b216f086: New variable 'project-files-relative-names'

[Dmitry Gutov]

Hi Michael,

On 05/05/2024 10:53, Michael Albinus wrote:

>>      * test/lisp/progmodes/project-tests.el (project-find-regexp):
>>      New test.
> This fails in batch mode:
> 
> --8<---------------cut here---------------start------------->8---
> # make -C test project-tests
> Test project-find-regexp backtrace:
>    write-region(nil nil "~/.emacs.d/projects" nil silent)
>    project--write-project-list()

Thank you for the report, should now be fixed.

Re: master 370b216f086: New variable 'project-files-relative-names'

[Michael Albinus]

Dmitry Gutov <dmitry@gutov.dev> writes:

> Hi Michael,

Hi Dmitry,

>> # make -C test project-tests
>> Test project-find-regexp backtrace:
>>    write-region(nil nil "~/.emacs.d/projects" nil silent)
>>    project--write-project-list()
>
> Thank you for the report, should now be fixed.

Confirmed.