bug#21457: 25.0.50; vc-test-git03-working-revision test fails

bug#21457: 25.0.50; vc-test-git03-working-revision test fails

[Eli Zaretskii]

This started lately, probably within the last couple of weeks.

The error information in the log is:

  Test vc-test-git03-working-revision backtrace:
    vc-git-working-revision("c:/DOCUME~1/Zaretzky/LOCALS~1/Temp/vc-test4
    apply(vc-git-working-revision "c:/DOCUME~1/Zaretzky/LOCALS~1/Temp/vc
    vc-call-backend(Git working-revision "c:/DOCUME~1/Zaretzky/LOCALS~1/
    vc-working-revision("c:/DOCUME~1/Zaretzky/LOCALS~1/Temp/vc-test4572X
    vc-test--working-revision(Git)
    (lambda nil (let ((fn-90 (function ert-test-passed-p)) (args-91 (lis
    ert--run-test-internal([cl-struct-ert--test-execution-info [cl-struc
    ert-run-test([cl-struct-ert-test vc-test-git03-working-revision "Che
    ert-run-or-rerun-test([cl-struct-ert--stats t [[cl-struct-ert-test v
    ert-run-tests(t #[385 "\306^B\307\"\203G^@\211\211G\310U\203^T^@\211@\20
    ert-run-tests-batch(nil)
    ert-run-tests-batch-and-exit()
    command-line-1(("-L" ";." "-l" "ert" "-l" "vc-tests.elc" "-f" "ert-r
    command-line()
    normal-top-level()
  Test vc-test-git03-working-revision condition:
      (args-out-of-range #<killed buffer> 1 41)  <<<<<<<<<<<<<<<<<<<<<
     FAILED  14/25  vc-test-git03-working-revision

What does the marked line mean?  What kind of error/problem should I
look for?




In GNU Emacs 25.0.50.411 (i686-pc-mingw32)
 of 2015-09-11
Repository revision: 8aa0386420f9d982b99568f27a5953dfc737640e
Windowing system distributor 'Microsoft Corp.', version 5.1.2600
Configured using:
 'configure --prefix=/d/usr --enable-checking=yes,glyphs --with-wide-int
 'CFLAGS=-gdwarf-4 -g3 -O0''

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

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

Major mode: Lisp Interaction

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
  line-number-mode: t
  transient-mark-mode: t

Recent messages:
For information about GNU Emacs and the GNU system, type C-h C-a.

Load-path shadows:
None found.

Features:
(shadow sort gnus-util mail-extr emacsbug message dired format-spec
rfc822 mml mml-sec mm-decode mm-bodies mm-encode mail-parse rfc2231
mailabbrev gmm-utils mailheader sendmail rfc2047 rfc2045 ietf-drums
mm-util help-fns help-mode easymenu cl-loaddefs pcase cl-lib mail-prsvr
mail-utils 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 w32 multi-tty
make-network-process emacs)

Memory information:
((conses 16 84952 6041)
 (symbols 56 19937 0)
 (miscs 48 37 86)
 (strings 16 15963 5898)
 (string-bytes 1 392849)
 (vectors 16 11218)
 (vector-slots 8 403214 4311)
 (floats 8 125 31)
 (intervals 40 260 63)
 (buffers 856 11))

bug#21457: 25.0.50; vc-test-git03-working-revision test fails

[Glenn Morris]

Eli Zaretskii wrote:

>       (args-out-of-range #<killed buffer> 1 41)  <<<<<<<<<<<<<<<<<<<<<

I guess this comes from vc-git--rev-parse.
So I guess "git rev-parse WHATEVER" returns fewer than 40 characters for you.
You could try looking it more details at what vc-git--rev-parse is doing.

Perhaps it should use (min (point-max) (+ (point-min) 40)).

bug#21457: 25.0.50; vc-test-git03-working-revision test fails

[Glenn Morris]

Glenn Morris wrote:

> Perhaps it should use (min (point-max) (+ (point-min) 40)).

Scratch that, better to test for an actual hash, "[0-9a-f]\\{40\\}".

bug#21457: 25.0.50; vc-test-git03-working-revision test fails

[Eli Zaretskii]

> From: Glenn Morris <rgm@gnu.org>
> Cc: 21457@debbugs.gnu.org
> Date: Fri, 11 Sep 2015 16:34:45 -0400
> 
> Glenn Morris wrote:
> 
> > Perhaps it should use (min (point-max) (+ (point-min) 40)).
> 
> Scratch that, better to test for an actual hash, "[0-9a-f]\\{40\\}".

But it doesn't return a hash in a repo that was just initted, it
returns "HEAD".  Try this:

  $ mkdir ttt
  $ cd ttt
  $ git init
  Initialized empty Git repository in D:/gnu/git/ttt/.git/
  $ git rev-parse HEAD
  HEAD
  fatal: ambiguous argument 'HEAD': unknown revision or path not in the working tree.
  Use '--' to separate paths from revisions, like this:
  'git <command> [<revision>...] -- [<file>...]'
  $ echo $?
  128

IOW, it returns "HEAD" and exits with an error status.  Maybe the
problem started when I upgraded Git to 2.5.1 lately.

The patches below make this work for me.  The first one should
probably be applied regardless, as without it vc-git--rev-parse is not
safe to call in some borderline cases.

diff --git a/lisp/vc/vc-git.el b/lisp/vc/vc-git.el
index 8a0f554..af2b39f 100644
--- a/lisp/vc/vc-git.el
+++ b/lisp/vc/vc-git.el
@@ -1137,7 +1137,9 @@ (defun vc-git--rev-parse (rev)
   (with-temp-buffer
     (and
      (vc-git--out-ok "rev-parse" rev)
-     (buffer-substring-no-properties (point-min) (+ (point-min) 40)))))
+     (buffer-substring-no-properties (point-min)
+                                     (min (1- (point-max))
+                                          (+ (point-min) 40))))))
 
 (defun vc-git-next-revision (file rev)
   "Git-specific version of `vc-next-revision'."
diff --git a/test/automated/vc-tests.el b/test/automated/vc-tests.el
index a7242e9..3476846 100644
--- a/test/automated/vc-tests.el
+++ b/test/automated/vc-tests.el
@@ -367,12 +367,15 @@ (defun vc-test--working-revision (backend)
 
 	  ;; nil: CVS Mtn RCS SCCS
 	  ;; "0": Bzr Hg SRC SVN
-	  ;; "master": Git
+	  ;; "master" or "HEAD": Git
+          (message "%s: %s"
+                   backend
+                   (vc-working-revision default-directory backend))
 	  (should (eq (vc-working-revision default-directory)
 		      (vc-working-revision default-directory backend)))
 	  (should
 	   (member
-	    (vc-working-revision default-directory) '(nil "0" "master")))
+	    (vc-working-revision default-directory) '(nil "0" "master" "HEAD")))
 
 	  (let ((tmp-name (expand-file-name "foo" default-directory)))
 	    ;; Check initial working revision, should be nil until
@@ -380,22 +383,22 @@ (defun vc-test--working-revision (backend)
 
 	    ;; nil: CVS Mtn RCS SCCS SVN
 	    ;; "0": Bzr Hg SRC
-	    ;; "master": Git
+	    ;; "master" or "HEAD: Git
 	    (should (eq (vc-working-revision tmp-name)
 			(vc-working-revision tmp-name backend)))
 	    (should
-	     (member (vc-working-revision tmp-name) '(nil "0" "master")))
+	     (member (vc-working-revision tmp-name) '(nil "0" "master" "HEAD")))
 
 	    ;; Write a new file.  Check working revision.
 	    (write-region "foo" nil tmp-name nil 'nomessage)
 
 	    ;; nil: CVS Mtn RCS SCCS SVN
 	    ;; "0": Bzr Hg SRC
-	    ;; "master": Git
+	    ;; "master" or "HEAD": Git
 	    (should (eq (vc-working-revision tmp-name)
 			(vc-working-revision tmp-name backend)))
 	    (should
-	     (member (vc-working-revision tmp-name) '(nil "0" "master")))
+	     (member (vc-working-revision tmp-name) '(nil "0" "master" "HEAD")))
 
 	    ;; Register a file.  Check working revision.
 	    (vc-register
@@ -403,11 +406,11 @@ (defun vc-test--working-revision (backend)
 
 	    ;; nil: Mtn RCS SCCS
 	    ;; "0": Bzr CVS Hg SRC SVN
-	    ;; "master": Git
+	    ;; "master" or "HEAD": Git
 	    (should (eq (vc-working-revision tmp-name)
 			(vc-working-revision tmp-name backend)))
 	    (should
-	     (member (vc-working-revision tmp-name) '(nil "0" "master")))
+	     (member (vc-working-revision tmp-name) '(nil "0" "master" "HEAD")))
 
 	    ;; Unregister the file.  Check working revision.
 	    (condition-case nil
@@ -416,13 +419,13 @@ (defun vc-test--working-revision (backend)
 
 		  ;; nil: RCS
 		  ;; "0": Bzr Hg
-		  ;; "master": Git
+		  ;; "master" or "HEAD": Git
 		  ;; unsupported: CVS Mtn SCCS SRC SVN
 		  (should (eq (vc-working-revision tmp-name)
 			      (vc-working-revision tmp-name backend)))
 		  (should
 		   (member
-		    (vc-working-revision tmp-name) '(nil "0" "master"))))
+		    (vc-working-revision tmp-name) '(nil "0" "master" "HEAD"))))
 	      (vc-not-supported t))))
 
       ;; Save exit.

bug#21457: 25.0.50; vc-test-git03-working-revision test fails

[Andreas Schwab]

Eli Zaretskii <eliz@gnu.org> writes:

> The patches below make this work for me.  The first one should
> probably be applied regardless, as without it vc-git--rev-parse is not
> safe to call in some borderline cases.
>
> diff --git a/lisp/vc/vc-git.el b/lisp/vc/vc-git.el
> index 8a0f554..af2b39f 100644
> --- a/lisp/vc/vc-git.el
> +++ b/lisp/vc/vc-git.el
> @@ -1137,7 +1137,9 @@ (defun vc-git--rev-parse (rev)
>    (with-temp-buffer
>      (and
>       (vc-git--out-ok "rev-parse" rev)
> -     (buffer-substring-no-properties (point-min) (+ (point-min) 40)))))
> +     (buffer-substring-no-properties (point-min)
> +                                     (min (1- (point-max))
> +                                          (+ (point-min) 40))))))

Why does vc-git--out-ok return true when git rev-parse failed?

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

bug#21457: 25.0.50; vc-test-git03-working-revision test fails

[Eli Zaretskii]

> From: Andreas Schwab <schwab@linux-m68k.org>
> Cc: Glenn Morris <rgm@gnu.org>,  21457@debbugs.gnu.org
> Date: Sat, 12 Sep 2015 11:42:59 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > The patches below make this work for me.  The first one should
> > probably be applied regardless, as without it vc-git--rev-parse is not
> > safe to call in some borderline cases.
> >
> > diff --git a/lisp/vc/vc-git.el b/lisp/vc/vc-git.el
> > index 8a0f554..af2b39f 100644
> > --- a/lisp/vc/vc-git.el
> > +++ b/lisp/vc/vc-git.el
> > @@ -1137,7 +1137,9 @@ (defun vc-git--rev-parse (rev)
> >    (with-temp-buffer
> >      (and
> >       (vc-git--out-ok "rev-parse" rev)
> > -     (buffer-substring-no-properties (point-min) (+ (point-min) 40)))))
> > +     (buffer-substring-no-properties (point-min)
> > +                                     (min (1- (point-max))
> > +                                          (+ (point-min) 40))))))
> 
> Why does vc-git--out-ok return true when git rev-parse failed?

Good question, I will look into it soon.

Do you know whether "HEAD" is indeed what the latest Git should return
in this case, though?  The original test expected "master".  Was this
some change in Git?  I cannot find anything about this in Google, but
that's a small wonder as including "HEAD" in the query brings in a lot
of useless clutter.

bug#21457: 25.0.50; vc-test-git03-working-revision test fails

[Eli Zaretskii]

> Date: Sat, 12 Sep 2015 18:03:38 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: 21457@debbugs.gnu.org
> 
> > > diff --git a/lisp/vc/vc-git.el b/lisp/vc/vc-git.el
> > > index 8a0f554..af2b39f 100644
> > > --- a/lisp/vc/vc-git.el
> > > +++ b/lisp/vc/vc-git.el
> > > @@ -1137,7 +1137,9 @@ (defun vc-git--rev-parse (rev)
> > >    (with-temp-buffer
> > >      (and
> > >       (vc-git--out-ok "rev-parse" rev)
> > > -     (buffer-substring-no-properties (point-min) (+ (point-min) 40)))))
> > > +     (buffer-substring-no-properties (point-min)
> > > +                                     (min (1- (point-max))
> > > +                                          (+ (point-min) 40))))))
> > 
> > Why does vc-git--out-ok return true when git rev-parse failed?
> 
> Good question

Answer: that's my local 'git' script, if I invoke the Git binary
directly, the result is 128.

But I still don't understand how this

	  ;; Create empty repository.  Check working revision of
	  ;; repository, should be nil.
	  (make-directory default-directory)
	  (vc-test--create-repo-function backend)

	  ;; nil: CVS Mtn RCS SCCS
	  ;; "0": Bzr Hg SRC SVN
	  ;; "master" or "HEAD": Git
	  (should (eq (vc-working-revision default-directory)
		      (vc-working-revision default-directory backend)))

was supposed to work, since "git rev-parse HEAD" fails in an empty
repository and returns just "HEAD", not a 40-digit hash or "master".

What am I missing?

bug#21457: 25.0.50; vc-test-git03-working-revision test fails

[Eli Zaretskii]

> Date: Sat, 12 Sep 2015 18:36:41 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: 21457@debbugs.gnu.org
> 
> But I still don't understand how this
> 
> 	  ;; Create empty repository.  Check working revision of
> 	  ;; repository, should be nil.
> 	  (make-directory default-directory)
> 	  (vc-test--create-repo-function backend)
> 
> 	  ;; nil: CVS Mtn RCS SCCS
> 	  ;; "0": Bzr Hg SRC SVN
> 	  ;; "master" or "HEAD": Git
> 	  (should (eq (vc-working-revision default-directory)
> 		      (vc-working-revision default-directory backend)))
> 
> was supposed to work, since "git rev-parse HEAD" fails in an empty
> repository and returns just "HEAD", not a 40-digit hash or "master".

Ah, okay, I get it: vc-working-revision was supposed to fail and
return nil in both cases.  Which makes this test not a very useful
one.  But then how was this supposed to work?

	    ;; Register a file.  Check working revision.
	    (vc-register
	     (list backend (list (file-name-nondirectory tmp-name))))

	    ;; nil: Mtn RCS SCCS
	    ;; "0": Bzr CVS Hg SRC SVN
	    ;; "master" or "HEAD": Git
	    (should (eq (vc-working-revision tmp-name)
			(vc-working-revision tmp-name backend)))
	    (should
	     (member (vc-working-revision tmp-name) '(nil "0" "master")))

bug#21457: 25.0.50; vc-test-git03-working-revision test fails

[Andreas Schwab]

Eli Zaretskii <eliz@gnu.org> writes:

> Do you know whether "HEAD" is indeed what the latest Git should return
> in this case, though?

Unrecognized arguments are printed unchanged.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

bug#21457: 25.0.50; vc-test-git03-working-revision test fails

[Eli Zaretskii]

> From: Andreas Schwab <schwab@linux-m68k.org>
> Cc: 21457@debbugs.gnu.org
> Date: Sat, 12 Sep 2015 18:54:29 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Do you know whether "HEAD" is indeed what the latest Git should return
> > in this case, though?
> 
> Unrecognized arguments are printed unchanged.

OK, I fixed my script, and vc-tests.el now passes without any changes.

Sorry for wasting everybody's time.