GNU bug report logs -
#23769
25.0.95; Mode Line breakage in vc-git
Previous Next
To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 23769 in the body.
You can then email your comments to 23769 AT debbugs.gnu.org in the normal way.
Toggle the display of automated, internal messages from the tracker.
Report forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#23769; Package
emacs.
(Tue, 14 Jun 2016 11:17:01 GMT)
Full text and
rfc822 format available.
Acknowledgement sent
to
phillip.lord <at> russet.org.uk (Phillip Lord):
New bug report received and forwarded. Copy sent to
bug-gnu-emacs <at> gnu.org.
(Tue, 14 Jun 2016 11:17:02 GMT)
Full text and
rfc822 format available.
Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):
I am seeing a recurrent problem with vc, on Emacs-25. I am running emacs
via cask, which is running a test set.
The error looks like this:
vc-call-backend(Git mode-line-string "/home/phillord/emacs/lentic/de
vc-mode-line("/home/phillord/emacs/lentic/dev-resources/chunk-commen
vc-refresh-state()
run-hooks(find-file-hook)
after-find-file(nil t)
find-file-noselect-1(#<buffer chunk-comment.clj> "~/emacs/lentic/dev
find-file-noselect("/home/phillord/emacs/lentic/dev-resources/chunk-
(setq this (find-file-noselect filename))
(set-buffer (setq this (find-file-noselect filename)))
(save-current-buffer (set-buffer (setq this (find-file-noselect file
Initially, I only saw it when using Emacs in my git commit-hook -- the
same "make" command in the shell caused no problems. However, it is
freely reproducible by launching with "GIT_DIR=.git make -k" which
suggests this is why it is failing during the git commit.
I've tried "instrumenting" (i.e. putting lots of logging) into vc.
The immediate cause of the error appears to be in vc-git--call when it
runs the git command "symbolic-ref".
vc-git--call: about to apply: (git nil (t nil) nil symbolic-ref (HEAD))
vc-git--call: return 128
If GIT_DIR is not set this has a zero return value.
I haven't worked out yet, how to find what the error is (128 is "any
other error").
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#23769; Package
emacs.
(Tue, 14 Jun 2016 16:30:02 GMT)
Full text and
rfc822 format available.
Message #8 received at 23769 <at> debbugs.gnu.org (full text, mbox):
AFAIK Emacs doesn't support GIT_DIR, see eg https://debbugs.gnu.org/5344 .
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#23769; Package
emacs.
(Tue, 14 Jun 2016 17:04:01 GMT)
Full text and
rfc822 format available.
Message #11 received at 23769 <at> debbugs.gnu.org (full text, mbox):
Glenn Morris <rgm <at> gnu.org> writes:
> AFAIK Emacs doesn't support GIT_DIR, see eg https://debbugs.gnu.org/5344 .
Glenn
Thanks for the pointer.
The curious thing in this case is that GIT_DIR is not saying anything at
all, other than the default. Emacs should not error under such
circumstances. In my specific case, even a silent failure would be
acceptable (cause I am using Emacs in batch -- I can't even see the mode
line!).
There's a patch attached that Bug report which never seems to have been
applied.
Phil
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#23769; Package
emacs.
(Tue, 14 Jun 2016 21:31:01 GMT)
Full text and
rfc822 format available.
Message #14 received at 23769 <at> debbugs.gnu.org (full text, mbox):
Glenn Morris <rgm <at> gnu.org> writes:
> AFAIK Emacs doesn't support GIT_DIR, see eg https://debbugs.gnu.org/5344 .
I think I have tracked the cause of the problem.
It happens if a test opens a file in a subdirectory of the git root.
When the file is opened Emacs checks to see whether the file is
versioned. Somewhere down the line Emacs calls vc-git-working-revision,
which eventually results in invocation of `git symbolic-ref HEAD`.
All is good.
Unless, GIT_DIR is set to ".git". Now, `git symbolic-ref HEAD` fails in
subdirectories because GIT_DIR is still set to .git, when the .git
directory is in the root. git symbolic-ref fails with 128 "Fatal: Not a
git repository".
Unfortunately, this is exactly the situation on a pre-commit hook. Git
changes working directory to the root of the git repository, and then
sets GIT_DIR to be .git.
Potential solutions:
1) Ignore return values from git (fail silently)
2) Unset GIT_DIR (but I don't know why it is set by git, and I assume it
is for some reason)
3) Run all vc-git commands from the root dir, and use fully qualified
names.
Will think further.
Phil
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#23769; Package
emacs.
(Tue, 14 Jun 2016 21:53:02 GMT)
Full text and
rfc822 format available.
Message #17 received at 23769 <at> debbugs.gnu.org (full text, mbox):
On 06/14/2016 08:03 PM, Phillip Lord wrote:
>> AFAIK Emacs doesn't support GIT_DIR, see eg https://debbugs.gnu.org/5344 .
> There's a patch attached that Bug report which never seems to have been
> applied.
Have you tried it? Does it fix your problem?
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#23769; Package
emacs.
(Tue, 14 Jun 2016 22:22:01 GMT)
Full text and
rfc822 format available.
Message #20 received at 23769 <at> debbugs.gnu.org (full text, mbox):
On Tue, June 14, 2016 10:52 pm, Dmitry Gutov wrote:
> On 06/14/2016 08:03 PM, Phillip Lord wrote:
>
>
>>> AFAIK Emacs doesn't support GIT_DIR, see eg
>>> https://debbugs.gnu.org/5344 .
>>>
>
>> There's a patch attached that Bug report which never seems to have been
>> applied.
>
> Have you tried it? Does it fix your problem?
Don't think it will, no. That patch is worth considering anyway,
irrespective of this bug.
Think I have a solution to this one though -- will try tomorrow.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#23769; Package
emacs.
(Wed, 15 Jun 2016 20:49:02 GMT)
Full text and
rfc822 format available.
Message #23 received at 23769 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Dmitry Gutov <dgutov <at> yandex.ru> writes:
> On 06/14/2016 08:03 PM, Phillip Lord wrote:
>
>>> AFAIK Emacs doesn't support GIT_DIR, see eg https://debbugs.gnu.org/5344 .
>
>> There's a patch attached that Bug report which never seems to have been
>> applied.
>
> Have you tried it? Does it fix your problem?
The following patch addresses this bug.
John/Eli would you want this for Emacs-25.
I've noticed from instrumenting vc-git that it's not just the
symbolic-ref command that returns 128, but several others. So, it might
be that the best long term solution would be to make change vc-git--call
to set the default directory to the root, which would make GIT_DIR=.git
always correct.
At the moment, though, I've noticed some git commands are called with
simple file names, and some full, so this would probably require more
changes. No sense in making this sort of change for Emacs-25 now.
[0001-Fix-error-from-vc-git-when-GIT_DIR-is-set.patch (text/x-diff, inline)]
From 0419734fd39f8cd67472c0294295ef1508e39d85 Mon Sep 17 00:00:00 2001
From: Phillip Lord <phillip.lord <at> russet.org.uk>
Date: Wed, 15 Jun 2016 17:36:42 +0100
Subject: [PATCH] Fix error from vc-git when GIT_DIR is set
* lisp/vc/vc-git.el (vc-git-working-revision): Set default-directory to
repository root to prevent an error when GIT_DIR is relative to it.
* test/automated/vc-git-tests.el: New file.
Addresses Bug#23769
---
lisp/vc/vc-git.el | 8 ++++--
test/automated/vc-git-tests.el | 60 ++++++++++++++++++++++++++++++++++++++++++
2 files changed, 66 insertions(+), 2 deletions(-)
create mode 100644 test/automated/vc-git-tests.el
diff --git a/lisp/vc/vc-git.el b/lisp/vc/vc-git.el
index f35c84d..ea26a7d 100644
--- a/lisp/vc/vc-git.el
+++ b/lisp/vc/vc-git.el
@@ -260,9 +260,13 @@ vc-git-state
(vc-git--state-code diff-letter)))
(if (vc-git--empty-db-p) 'added 'up-to-date))))
-(defun vc-git-working-revision (_file)
+(defun vc-git-working-revision (file)
"Git-specific version of `vc-working-revision'."
- (let (process-file-side-effects)
+ ;; Call this in the root directory, or Emacs may error when setting
+ ;; the modeline when called from a git pre-commit hook, which sets
+ ;; GIT_DIR. (Bug #23769)
+ (let ((default-directory (vc-git-root file))
+ (process-file-side-effects))
(vc-git--rev-parse "HEAD")))
(defun vc-git--symbolic-ref (file)
diff --git a/test/automated/vc-git-tests.el b/test/automated/vc-git-tests.el
new file mode 100644
index 0000000..8001bcb
--- /dev/null
+++ b/test/automated/vc-git-tests.el
@@ -0,0 +1,60 @@
+;;; vc-git-tests.el --- tests for vc/vc-git.el
+
+;; Copyright (C) 2016 Free Software Foundation, Inc.
+
+;; This file is part of GNU Emacs.
+
+;; GNU Emacs is free software: you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+
+;; GNU Emacs is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+;; GNU General Public License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with GNU Emacs. If not, see <http://www.gnu.org/licenses/>.
+
+;;; Commentary:
+
+;;; Code:
+
+(require 'ert)
+(require 'vc-git)
+
+;; This will break when merged from the emacs-25 branch to master,
+;; because of the change of directory structure. The actual file is
+;; not important at all, so long as it is checked into git.
+(defvar vc-git-tests-file
+ (concat
+ (expand-file-name
+ "data/xref/"
+ (file-name-directory (or load-file-name (buffer-file-name))))
+ "file1.txt"))
+
+
+(ert-deftest open-this-file ()
+ "Test for Bug #23769."
+ (should
+ (let ((git-dir (getenv "GIT_DIR")))
+ (unwind-protect
+ (progn
+ (setenv "GIT_DIR" nil)
+ (find-file vc-git-tests-file))
+ (when (get-file-buffer vc-git-tests-file)
+ (setenv "GIT_DIR" git-dir)
+ (kill-buffer (get-file-buffer vc-git-tests-file))))))
+ (should
+ (let ((git-dir (getenv "GIT_DIR")))
+ (unwind-protect
+ (progn
+ (setenv "GIT_DIR" ".git")
+ (find-file vc-git-tests-file))
+ (when (get-file-buffer vc-git-tests-file)
+ (setenv "GIT_DIR" git-dir)
+ (kill-buffer (get-file-buffer vc-git-tests-file)))))))
+
+
+;;; vc-git-tests.el ends here
--
2.8.4
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#23769; Package
emacs.
(Wed, 15 Jun 2016 21:03:02 GMT)
Full text and
rfc822 format available.
Message #26 received at 23769 <at> debbugs.gnu.org (full text, mbox):
On 06/15/2016 11:48 PM, Phillip Lord wrote:
> I've noticed from instrumenting vc-git that it's not just the
> symbolic-ref command that returns 128, but several others. So, it might
> be that the best long term solution would be to make change vc-git--call
> to set the default directory to the root, which would make GIT_DIR=.git
> always correct.
The patch looks wrong. Why does the problem script set GIT_DIR to '.git'?
In all examples I've found, this variable is set to an absolute value.
In general, its purpose, it seems, is to point to the '.git' directory
when it's named otherwise and/or is situated somewhere outside of the
current directory tree.
The latter situation will break vc-git-root. As such, the submitted
patch is only likely to work in the tautological case you've descried.
And it will add some performance penalty to each call, because
vc-git-root, though usually fast, is not free.
> At the moment, though, I've noticed some git commands are called with
> simple file names, and some full, so this would probably require more
> changes.
Indeed.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#23769; Package
emacs.
(Wed, 15 Jun 2016 21:57:02 GMT)
Full text and
rfc822 format available.
Message #29 received at 23769 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
>>>>> Phillip Lord <phillip.lord <at> russet.org.uk> writes:
> The following patch addresses this bug.
> John/Eli would you want this for Emacs-25.
This can wait until 25.2, since no one has had a chance to use this in the
pretests.
--
John Wiegley GPG fingerprint = 4710 CF98 AF9B 327B B80F
http://newartisans.com 60E1 46C4 BD1A 7AC1 4BA2
[signature.asc (application/pgp-signature, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#23769; Package
emacs.
(Wed, 15 Jun 2016 22:11:01 GMT)
Full text and
rfc822 format available.
Message #32 received at 23769 <at> debbugs.gnu.org (full text, mbox):
Dmitry Gutov <dgutov <at> yandex.ru> writes:
> On 06/15/2016 11:48 PM, Phillip Lord wrote:
>
>> I've noticed from instrumenting vc-git that it's not just the
>> symbolic-ref command that returns 128, but several others. So, it might
>> be that the best long term solution would be to make change vc-git--call
>> to set the default directory to the root, which would make GIT_DIR=.git
>> always correct.
>
> The patch looks wrong. Why does the problem script set GIT_DIR to '.git'?
Try it on a pre-commit hook. CWD is set to the root, and GIT_DIR is set
to .git. Other people have found this:
http://longair.net/blog/2011/04/09/missing-git-hooks-documentation/
AFAICT, it's not actually documented by git. Or it is but the
documentation is incomprehensible (hey, it's git, it could happen!).
> In all examples I've found, this variable is set to an absolute value. In
> general, its purpose, it seems, is to point to the '.git' directory when it's
> named otherwise and/or is situated somewhere outside of the current directory
> tree.
>
> The latter situation will break vc-git-root.
That situation does indeed happen. For example, I normally checkout
emacs into worktrees where you get this when commiting on emacs-25
branch which is a worktree off master.
/home/phillord/src/git/emacs-git/master/.git/worktrees/emacs-25
But in this case, I think vc-git-root will still work.
(defun vc-git-root (file)
(or (vc-file-getprop file 'git-root)
(vc-file-setprop file 'git-root (vc-find-root file ".git"))))
We still find ".git" because it is a *file* (not a directory) at the top
level of a worktree.
> As such, the submitted patch is only likely to work in the
> tautological case you've descried.
It's definately a risk. Git does many things, and people use it in many
ways.
> And it will add some performance penalty to each call, because
> vc-git-root, though usually fast, is not free.
Not free, but it is a constant time look up after the first.
There is a simpler option. I am trying to solve the root cause of the
problem but, as you say, that might be fraught. Since the problem only
seems to cause an error with vc-git-mode-line-string, we could just
discard the error from vc-git-working-revision in this case.
Failing that, now I know what the problem is, at least I have a
workaround (unset GIT_DIR in the pre-commit hook).
Phil
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#23769; Package
emacs.
(Wed, 15 Jun 2016 22:29:01 GMT)
Full text and
rfc822 format available.
Message #35 received at 23769 <at> debbugs.gnu.org (full text, mbox):
On Wed, Jun 15, 2016 at 6:09 PM, Phillip Lord
<phillip.lord <at> russet.org.uk> wrote:
>
> Failing that, now I know what the problem is, at least I have a
> workaround (unset GIT_DIR in the pre-commit hook).
Apparently this "_the_ right way to write hooks that touch the files
in the work tree."
http://permalink.gmane.org/gmane.comp.version-control.git/136276
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#23769; Package
emacs.
(Wed, 15 Jun 2016 23:22:02 GMT)
Full text and
rfc822 format available.
Message #38 received at 23769 <at> debbugs.gnu.org (full text, mbox):
On 06/16/2016 01:27 AM, Noam Postavsky wrote:
> Apparently this "_the_ right way to write hooks that touch the files
> in the work tree."
>
> http://permalink.gmane.org/gmane.comp.version-control.git/136276
Is it the same behavior we're talking about?
The thread says that GIT_DIR is set to '.', not to '.git'. I doubt the
former setting would mesh well with binding default-directory to
repository's root like the patch proposes.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#23769; Package
emacs.
(Wed, 15 Jun 2016 23:27:01 GMT)
Full text and
rfc822 format available.
Message #41 received at 23769 <at> debbugs.gnu.org (full text, mbox):
On 06/16/2016 01:09 AM, Phillip Lord wrote:
> Try it on a pre-commit hook. CWD is set to the root, and GIT_DIR is set
> to .git. Other people have found this:
>
> http://longair.net/blog/2011/04/09/missing-git-hooks-documentation/
OK, so hooks set it themselves.
> AFAICT, it's not actually documented by git. Or it is but the
> documentation is incomprehensible (hey, it's git, it could happen!).
Yup.
> That situation does indeed happen. For example, I normally checkout
> emacs into worktrees where you get this when commiting on emacs-25
> branch which is a worktree off master.
Does git-worktree use GIT_DIR in some way?
> There is a simpler option. I am trying to solve the root cause of the
> problem but, as you say, that might be fraught. Since the problem only
> seems to cause an error with vc-git-mode-line-string, we could just
> discard the error from vc-git-working-revision in this case.
That sounds like a last-resort option.
> Failing that, now I know what the problem is, at least I have a
> workaround (unset GIT_DIR in the pre-commit hook).
We could also unset GIT_DIR locally inside vc-git--call. At least when
it's set to a known value such as '.git'.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#23769; Package
emacs.
(Thu, 16 Jun 2016 07:13:02 GMT)
Full text and
rfc822 format available.
Message #44 received at 23769 <at> debbugs.gnu.org (full text, mbox):
Dmitry Gutov <dgutov <at> yandex.ru> writes:
> On 06/16/2016 01:27 AM, Noam Postavsky wrote:
>
>> Apparently this "_the_ right way to write hooks that touch the files
>> in the work tree."
>>
>> http://permalink.gmane.org/gmane.comp.version-control.git/136276
>
> Is it the same behavior we're talking about?
>
> The thread says that GIT_DIR is set to '.', not to '.git'. I doubt the former
> setting would mesh well with binding default-directory to repository's root
> like the patch proposes.
According to this:
http://longair.net/blog/2011/04/09/missing-git-hooks-documentation/
that's for the post-recieve hook. In this case CWD is the git directory.
The logic of this is (I guess) that it works also in a bare repo.
pre-commit can only ever run in a non-bare repo since you cannot commit
if you don't have a worktree.
I suspect emacs will not be affected by this because if anyone did
run emacs in post-recieve they would only touch files *inside* the git
repo which Emacs would identify as not version-controlled.
But, again, it illustrates the point that GIT_DIR can (and is often) set
to a relative directory.
Phil
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#23769; Package
emacs.
(Thu, 16 Jun 2016 07:42:02 GMT)
Full text and
rfc822 format available.
Message #47 received at 23769 <at> debbugs.gnu.org (full text, mbox):
Dmitry Gutov <dgutov <at> yandex.ru> writes:
> On 06/16/2016 01:09 AM, Phillip Lord wrote:
>> That situation does indeed happen. For example, I normally checkout
>> emacs into worktrees where you get this when commiting on emacs-25
>> branch which is a worktree off master.
>
> Does git-worktree use GIT_DIR in some way?
Well, it uses the directory that GIT_DIR points to (so
master/.git/worktrees/emacs-25 contains COMMIT_MSG, HEAD, index and so
on).
Whether is uses GIT_DIR, who knows. I pressume that all git commands
obey GIT_DIR.
>> There is a simpler option. I am trying to solve the root cause of the
>> problem but, as you say, that might be fraught. Since the problem only
>> seems to cause an error with vc-git-mode-line-string, we could just
>> discard the error from vc-git-working-revision in this case.
>
> That sounds like a last-resort option.
It's certainly true that it would nice to fix it elsewhere, but I am
unconvinced that setting the mode-line should ever result in an error as
a normal part of it's operation.
>> Failing that, now I know what the problem is, at least I have a
>> workaround (unset GIT_DIR in the pre-commit hook).
>
> We could also unset GIT_DIR locally inside vc-git--call. At least when
> it's set to a known value such as '.git'.
Yep; I guess, we know exactly what vc-git--call does with git, and if it
never depends on GIT_DIR that should work. Although setting and
unsetting GIT_DIR seems a bit of a pain.
Personally, I prefer my first option of calling git consistently --
always with CWD equal to root, and always with file paths relative to
the root. This way, GIT_DIR should not matter.
I presume the vc.el facade allows this, but if so, as this is going onto
master now, there is time.
Phil
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#23769; Package
emacs.
(Thu, 16 Jun 2016 07:46:02 GMT)
Full text and
rfc822 format available.
Message #50 received at 23769 <at> debbugs.gnu.org (full text, mbox):
Noam Postavsky <npostavs <at> users.sourceforge.net> writes:
> On Wed, Jun 15, 2016 at 6:09 PM, Phillip Lord
> <phillip.lord <at> russet.org.uk> wrote:
>>
>> Failing that, now I know what the problem is, at least I have a
>> workaround (unset GIT_DIR in the pre-commit hook).
>
> Apparently this "_the_ right way to write hooks that touch the files
> in the work tree."
>
> http://permalink.gmane.org/gmane.comp.version-control.git/136276
I agree that this will work. But it worth reading the justification --
"in earlier versions of git" -- so that will be git 1.6 latest, as the
post was in 2010.
So, I don't think that Emacs should *require* this to be the case. git
doesn't appear to any more.
Phil
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#23769; Package
emacs.
(Thu, 16 Jun 2016 11:26:01 GMT)
Full text and
rfc822 format available.
Message #53 received at 23769 <at> debbugs.gnu.org (full text, mbox):
On 06/16/2016 10:41 AM, Phillip Lord wrote:
> Well, it uses the directory that GIT_DIR points to (so
> master/.git/worktrees/emacs-25 contains COMMIT_MSG, HEAD, index and so
> on).
>
> Whether is uses GIT_DIR, who knows. I pressume that all git commands
> obey GIT_DIR.
Well, maybe it's used internally. It's of no concern to us here.
> It's certainly true that it would nice to fix it elsewhere, but I am
> unconvinced that setting the mode-line should ever result in an error as
> a normal part of it's operation.
It shouldn't. But it should let the user know that something's going
wrong, so they can file a bug report.
> Yep; I guess, we know exactly what vc-git--call does with git, and if it
> never depends on GIT_DIR that should work. Although setting and
> unsetting GIT_DIR seems a bit of a pain.
How come? It'll be about as short as your current patch, if not shorter.
> Personally, I prefer my first option of calling git consistently --
> always with CWD equal to root, and always with file paths relative to
> the root. This way, GIT_DIR should not matter.
Let's not be hasty about it. If new arguments arise in favor of this, we
can do it, but so far unsetting GIT_DIR seems like the cheapest option.
Like this:
diff --git a/lisp/vc/vc-git.el b/lisp/vc/vc-git.el
index f35c84d..4e495a5 100644
--- a/lisp/vc/vc-git.el
+++ b/lisp/vc/vc-git.el
@@ -1450,7 +1450,9 @@ vc-git--call
(or coding-system-for-read vc-git-log-output-coding-system))
(coding-system-for-write
(or coding-system-for-write vc-git-commits-coding-system))
- (process-environment (cons "PAGER=" process-environment)))
+ (process-environment (cons
+ "GIT_DIR="
+ (cons "PAGER=" process-environment))))
(apply 'process-file vc-git-program nil buffer nil command args)))
(defun vc-git--out-ok (command &rest args)
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#23769; Package
emacs.
(Thu, 16 Jun 2016 16:08:02 GMT)
Full text and
rfc822 format available.
Message #56 received at 23769 <at> debbugs.gnu.org (full text, mbox):
On 06/16/2016 02:25 PM, Dmitry Gutov wrote:
> If new arguments arise in favor of this, we
> can do it, but so far unsetting GIT_DIR seems like the cheapest option.
> Like this:
Actually, this is better. The previous one broke vc-git. :)
diff --git a/lisp/vc/vc-git.el b/lisp/vc/vc-git.el
index f35c84d..2b827a3 100644
--- a/lisp/vc/vc-git.el
+++ b/lisp/vc/vc-git.el
@@ -1451,6 +1451,7 @@ vc-git--call
(coding-system-for-write
(or coding-system-for-write vc-git-commits-coding-system))
(process-environment (cons "PAGER=" process-environment)))
+ (setenv "GIT_DIR" nil)
(apply 'process-file vc-git-program nil buffer nil command args)))
(defun vc-git--out-ok (command &rest args)
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#23769; Package
emacs.
(Thu, 16 Jun 2016 16:56:02 GMT)
Full text and
rfc822 format available.
Message #59 received at 23769 <at> debbugs.gnu.org (full text, mbox):
On Thu, Jun 16, 2016 at 12:06 PM, Dmitry Gutov <dgutov <at> yandex.ru> wrote:
> Actually, this is better. The previous one broke vc-git. :)
>
> diff --git a/lisp/vc/vc-git.el b/lisp/vc/vc-git.el
> index f35c84d..2b827a3 100644
> --- a/lisp/vc/vc-git.el
> +++ b/lisp/vc/vc-git.el
> @@ -1451,6 +1451,7 @@ vc-git--call
> (coding-system-for-write
> (or coding-system-for-write vc-git-commits-coding-system))
> (process-environment (cons "PAGER=" process-environment)))
> + (setenv "GIT_DIR" nil)
>
I think you want (cons "GIT_DIR" ...) [without "="], using setenv can
modify the process-environment list by side-effect.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#23769; Package
emacs.
(Thu, 16 Jun 2016 17:00:02 GMT)
Full text and
rfc822 format available.
Message #62 received at 23769 <at> debbugs.gnu.org (full text, mbox):
On 06/16/2016 07:54 PM, Noam Postavsky wrote:
> I think you want (cons "GIT_DIR" ...) [without "="], using setenv can
> modify the process-environment list by side-effect.
You're probably right.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#23769; Package
emacs.
(Thu, 16 Jun 2016 21:55:02 GMT)
Full text and
rfc822 format available.
Message #65 received at 23769 <at> debbugs.gnu.org (full text, mbox):
Dmitry Gutov <dgutov <at> yandex.ru> writes:
> On 06/16/2016 02:25 PM, Dmitry Gutov wrote:
>> If new arguments arise in favor of this, we
>> can do it, but so far unsetting GIT_DIR seems like the cheapest option.
>> Like this:
>
> Actually, this is better. The previous one broke vc-git. :)
>
> diff --git a/lisp/vc/vc-git.el b/lisp/vc/vc-git.el
> index f35c84d..2b827a3 100644
> --- a/lisp/vc/vc-git.el
> +++ b/lisp/vc/vc-git.el
> @@ -1451,6 +1451,7 @@ vc-git--call
> (coding-system-for-write
> (or coding-system-for-write vc-git-commits-coding-system))
> (process-environment (cons "PAGER=" process-environment)))
> + (setenv "GIT_DIR" nil)
> (apply 'process-file vc-git-program nil buffer nil command args)))
>
> (defun vc-git--out-ok (command &rest args)
This just sets GIT_DIR nil globally right? It does appear to work, but
is it not a bit blunt.
Phil
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#23769; Package
emacs.
(Thu, 16 Jun 2016 21:55:02 GMT)
Full text and
rfc822 format available.
Message #68 received at 23769 <at> debbugs.gnu.org (full text, mbox):
Noam Postavsky <npostavs <at> users.sourceforge.net> writes:
> On Thu, Jun 16, 2016 at 12:06 PM, Dmitry Gutov <dgutov <at> yandex.ru> wrote:
>> Actually, this is better. The previous one broke vc-git. :)
>>
>> diff --git a/lisp/vc/vc-git.el b/lisp/vc/vc-git.el
>> index f35c84d..2b827a3 100644
>> --- a/lisp/vc/vc-git.el
>> +++ b/lisp/vc/vc-git.el
>> @@ -1451,6 +1451,7 @@ vc-git--call
>> (coding-system-for-write
>> (or coding-system-for-write vc-git-commits-coding-system))
>> (process-environment (cons "PAGER=" process-environment)))
>> + (setenv "GIT_DIR" nil)
>>
>
> I think you want (cons "GIT_DIR" ...) [without "="], using setenv can
> modify the process-environment list by side-effect.
Confused. Can you right a full patch?
Phil
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#23769; Package
emacs.
(Thu, 16 Jun 2016 22:05:02 GMT)
Full text and
rfc822 format available.
Message #71 received at 23769 <at> debbugs.gnu.org (full text, mbox):
On 06/17/2016 12:47 AM, Phillip Lord wrote:
> This just sets GIT_DIR nil globally right? It does appear to work, but
> is it not a bit blunt.
It would be a fine solution because we have a local binding for
process-environment just a little bit above. But setenv changes it
destructively, as Noam pointed out, so back to refining the other patch.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#23769; Package
emacs.
(Thu, 16 Jun 2016 22:14:02 GMT)
Full text and
rfc822 format available.
Message #74 received at 23769 <at> debbugs.gnu.org (full text, mbox):
On 06/17/2016 12:47 AM, Phillip Lord wrote:
> Confused. Can you right a full patch?
Here's a version:
diff --git a/lisp/vc/vc-git.el b/lisp/vc/vc-git.el
index f35c84d..74004d9 100644
--- a/lisp/vc/vc-git.el
+++ b/lisp/vc/vc-git.el
@@ -1451,6 +1451,7 @@ vc-git--call
(coding-system-for-write
(or coding-system-for-write vc-git-commits-coding-system))
(process-environment (cons "PAGER=" process-environment)))
+ (push "GIT_DIR" process-environment)
(apply 'process-file vc-git-program nil buffer nil command args)))
(defun vc-git--out-ok (command &rest args)
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#23769; Package
emacs.
(Thu, 16 Jun 2016 22:25:02 GMT)
Full text and
rfc822 format available.
Message #77 received at 23769 <at> debbugs.gnu.org (full text, mbox):
Dmitry Gutov <dgutov <at> yandex.ru> writes:
> On 06/17/2016 12:47 AM, Phillip Lord wrote:
>
>> Confused. Can you right a full patch?
>
> Here's a version:
>
> diff --git a/lisp/vc/vc-git.el b/lisp/vc/vc-git.el
> index f35c84d..74004d9 100644
> --- a/lisp/vc/vc-git.el
> +++ b/lisp/vc/vc-git.el
> @@ -1451,6 +1451,7 @@ vc-git--call
> (coding-system-for-write
> (or coding-system-for-write vc-git-commits-coding-system))
> (process-environment (cons "PAGER=" process-environment)))
> + (push "GIT_DIR" process-environment)
> (apply 'process-file vc-git-program nil buffer nil command args)))
>
> (defun vc-git--out-ok (command &rest args)
In my hands this needs to be "GIT_DIR=", at least for my tests to pass!
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#23769; Package
emacs.
(Thu, 16 Jun 2016 22:32:02 GMT)
Full text and
rfc822 format available.
Message #80 received at 23769 <at> debbugs.gnu.org (full text, mbox):
On 06/17/2016 01:23 AM, Phillip Lord wrote:
> In my hands this needs to be "GIT_DIR=", at least for my tests to pass!
"GIT_DIR=" turns the value into an empty string instead of nil. That
breaks vc-git here in normal usage.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#23769; Package
emacs.
(Thu, 16 Jun 2016 22:43:02 GMT)
Full text and
rfc822 format available.
Message #83 received at 23769 <at> debbugs.gnu.org (full text, mbox):
Dmitry Gutov <dgutov <at> yandex.ru> writes:
> On 06/17/2016 01:23 AM, Phillip Lord wrote:
>
>> In my hands this needs to be "GIT_DIR=", at least for my tests to pass!
>
> "GIT_DIR=" turns the value into an empty string instead of nil. That breaks
> vc-git here in normal usage.
A problem as "GIT_DIR" on it's own, still breaks with my originally
reported error (at least when running my ert test I sent with my patch).
Are you not getting this failure also?
Phil
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#23769; Package
emacs.
(Fri, 17 Jun 2016 02:42:01 GMT)
Full text and
rfc822 format available.
Message #86 received at 23769 <at> debbugs.gnu.org (full text, mbox):
On 06/17/2016 01:42 AM, Phillip Lord wrote:
> A problem as "GIT_DIR" on it's own, still breaks with my originally
> reported error (at least when running my ert test I sent with my patch).
>
> Are you not getting this failure also?
Sorry, I was only testing that the addition doesn't break anything in
the normal case, otherwise relying on process-environment's docstring.
There was a problem with the patch that not all vc-git code goes through
vc-git--call (in fact, most don't), but even patching vc-git-command in
a similar fashion doesn't change the behavior if Emacs was started with
GIT_DIR=.git.
Could this feature be actually broken? Or how does Git ignore the
modification?
"To use ‘process-environment’ to
remove an environment variable, include only its name in the list,
without "=VALUE"."
diff --git a/lisp/vc/vc-git.el b/lisp/vc/vc-git.el
index f35c84d..a544a2e 100644
--- a/lisp/vc/vc-git.el
+++ b/lisp/vc/vc-git.el
@@ -1423,7 +1423,9 @@ vc-git-command
(let ((coding-system-for-read
(or coding-system-for-read vc-git-log-output-coding-system))
(coding-system-for-write
- (or coding-system-for-write vc-git-commits-coding-system)))
+ (or coding-system-for-write vc-git-commits-coding-system))
+ (process-environment process-environment))
+ (push "GIT_DIR" process-environment)
(apply 'vc-do-command (or buffer "*vc*") okstatus vc-git-program
;; http://debbugs.gnu.org/16897
(unless (and (not (cdr-safe file-or-list))
@@ -1451,6 +1453,7 @@ vc-git--call
(coding-system-for-write
(or coding-system-for-write vc-git-commits-coding-system))
(process-environment (cons "PAGER=" process-environment)))
+ (push "GIT_DIR" process-environment)
(apply 'process-file vc-git-program nil buffer nil command args)))
(defun vc-git--out-ok (command &rest args)
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#23769; Package
emacs.
(Fri, 17 Jun 2016 03:38:02 GMT)
Full text and
rfc822 format available.
Message #89 received at 23769 <at> debbugs.gnu.org (full text, mbox):
On Thu, Jun 16, 2016 at 10:41 PM, Dmitry Gutov <dgutov <at> yandex.ru> wrote:
> Could this feature be actually broken? Or how does Git ignore the
> modification?
>
> "To use ‘process-environment’ to
> remove an environment variable, include only its name in the list,
> without "=VALUE"."
Turns out this feature is broken, see
http://debbugs.gnu.org/cgi/bugreport.cgi?bug=23779
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#23769; Package
emacs.
(Fri, 17 Jun 2016 06:55:01 GMT)
Full text and
rfc822 format available.
Message #92 received at 23769 <at> debbugs.gnu.org (full text, mbox):
> From: Noam Postavsky <npostavs <at> users.sourceforge.net>
> Date: Thu, 16 Jun 2016 23:37:06 -0400
> Cc: 23769 <at> debbugs.gnu.org, Phillip Lord <phillip.lord <at> russet.org.uk>
>
> On Thu, Jun 16, 2016 at 10:41 PM, Dmitry Gutov <dgutov <at> yandex.ru> wrote:
> > Could this feature be actually broken? Or how does Git ignore the
> > modification?
> >
> > "To use ‘process-environment’ to
> > remove an environment variable, include only its name in the list,
> > without "=VALUE"."
>
> Turns out this feature is broken, see
> http://debbugs.gnu.org/cgi/bugreport.cgi?bug=23779
(setenv "FOO") works for me.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#23769; Package
emacs.
(Fri, 17 Jun 2016 12:07:01 GMT)
Full text and
rfc822 format available.
Message #95 received at 23769 <at> debbugs.gnu.org (full text, mbox):
On 06/17/2016 09:54 AM, Eli Zaretskii wrote:
> (setenv "FOO") works for me.
setenv works destructively. To use it and remain good citizens, we'd
have to `(copy-sequence process-environment)` every time we call a Git
command.
Maybe that's not huge in the grand scheme of things, but it certainly
looks wasteful.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#23769; Package
emacs.
(Fri, 17 Jun 2016 13:22:01 GMT)
Full text and
rfc822 format available.
Message #98 received at 23769 <at> debbugs.gnu.org (full text, mbox):
> Cc: 23769 <at> debbugs.gnu.org, phillip.lord <at> russet.org.uk
> From: Dmitry Gutov <dgutov <at> yandex.ru>
> Date: Fri, 17 Jun 2016 15:06:10 +0300
>
> On 06/17/2016 09:54 AM, Eli Zaretskii wrote:
>
> > (setenv "FOO") works for me.
>
> setenv works destructively. To use it and remain good citizens, we'd
> have to `(copy-sequence process-environment)` every time we call a Git
> command.
>
> Maybe that's not huge in the grand scheme of things, but it certainly
> looks wasteful.
If you want to manually remove the variable before adding it with no
value, that will also work. Otherwise, I see no magic that would do
anything beyond the expected effect on any list.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#23769; Package
emacs.
(Fri, 17 Jun 2016 13:55:01 GMT)
Full text and
rfc822 format available.
Message #101 received at 23769 <at> debbugs.gnu.org (full text, mbox):
On 06/17/2016 04:21 PM, Eli Zaretskii wrote:
> If you want to manually remove the variable before adding it with no
> value, that will also work.
That would also be destructive.
> Otherwise, I see no magic that would do
> anything beyond the expected effect on any list.
process-environment just needs to be fixed.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#23769; Package
emacs.
(Fri, 17 Jun 2016 15:11:01 GMT)
Full text and
rfc822 format available.
Message #104 received at 23769 <at> debbugs.gnu.org (full text, mbox):
Dmitry Gutov <dgutov <at> yandex.ru> writes:
> On 06/17/2016 09:54 AM, Eli Zaretskii wrote:
>
>> (setenv "FOO") works for me.
>
> setenv works destructively. To use it and remain good citizens, we'd have to
> `(copy-sequence process-environment)` every time we call a Git command.
>
> Maybe that's not huge in the grand scheme of things, but it certainly looks
> wasteful.
I used this in my tests....
(let ((foo (getenv "FOO")))
(unwind-protect
(progn
(setenv "FOO")
(apply 'git-program args))
(setenv "FOO" foo)))
It's why I that setting and unsetting GIT_DIR seemed a bit of a pain
earlier. It should work though.
Phil
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#23769; Package
emacs.
(Tue, 21 Jun 2016 16:47:01 GMT)
Full text and
rfc822 format available.
Message #107 received at 23769 <at> debbugs.gnu.org (full text, mbox):
Hi Philip,
Could you please pull the latest changes from emacs-25, rebuild, and
check that applying the patch from
http://debbugs.gnu.org/cgi/bugreport.cgi?bug=23769#86 fixes this problem?
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#23769; Package
emacs.
(Tue, 21 Jun 2016 20:46:02 GMT)
Full text and
rfc822 format available.
Message #110 received at 23769 <at> debbugs.gnu.org (full text, mbox):
Dmitry Gutov <dgutov <at> yandex.ru> writes:
> Hi Philip,
It's "Phillip" actually, but "Phil" is easier.
>
> Could you please pull the latest changes from emacs-25, rebuild, and
> check that applying the patch from
> http://debbugs.gnu.org/cgi/bugreport.cgi?bug=23769#86 fixes this
> problem?
Yes, this appears to work on the emacs-25 head.
Are you going to add the test case that I sent in also? If not, I'll
just add it to master (I don't think it will merge cleanly, so this
might be the best solution anyway).
Phil
Reply sent
to
Dmitry Gutov <dgutov <at> yandex.ru>:
You have taken responsibility.
(Tue, 21 Jun 2016 23:08:01 GMT)
Full text and
rfc822 format available.
Notification sent
to
phillip.lord <at> russet.org.uk (Phillip Lord):
bug acknowledged by developer.
(Tue, 21 Jun 2016 23:08:02 GMT)
Full text and
rfc822 format available.
Message #115 received at 23769-done <at> debbugs.gnu.org (full text, mbox):
On 06/21/2016 11:45 PM, Phillip Lord wrote:
> It's "Phillip" actually, but "Phil" is easier.
Oh. Sorry, Phillip.
>> Could you please pull the latest changes from emacs-25, rebuild, and
>> check that applying the patch from
>> http://debbugs.gnu.org/cgi/bugreport.cgi?bug=23769#86 fixes this
>> problem?
>
> Yes, this appears to work on the emacs-25 head.
Thanks. Installed, with Eli's blessing.
> Are you going to add the test case that I sent in also? If not, I'll
> just add it to master (I don't think it will merge cleanly, so this
> might be the best solution anyway).
Yes, please go ahead with it on master.
bug archived.
Request was from
Debbugs Internal Request <help-debbugs <at> gnu.org>
to
internal_control <at> debbugs.gnu.org.
(Wed, 20 Jul 2016 11:24:05 GMT)
Full text and
rfc822 format available.
This bug report was last modified 8 years and 336 days ago.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.