unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#23952: 25.0.95; Regression: vc (git) over TRAMP doesn't work anymore
@ 2016-07-12  5:38 Dima Kogan
  2016-07-12  7:43 ` Michael Albinus
  0 siblings, 1 reply; 10+ messages in thread
From: Dima Kogan @ 2016-07-12  5:38 UTC (permalink / raw)
  To: 23952

Hi. I'm running a very recent emacs built from git (658daf9). In the
last two weeks or so, remote VC mode stopped working. Recipe:

1. emacs -Q
2. C-x C-f /127.0.0.1:emacs (or any other remote path that has a git
   repo)
3. C-x C-v D (or any other VC-mode function)

Instead of seeing VC mode do its thing it now throws an error:

  env: ‘GIT_DIR’: No such file or directory





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

* bug#23952: 25.0.95; Regression: vc (git) over TRAMP doesn't work anymore
  2016-07-12  5:38 bug#23952: 25.0.95; Regression: vc (git) over TRAMP doesn't work anymore Dima Kogan
@ 2016-07-12  7:43 ` Michael Albinus
  2016-07-12  7:49   ` Eli Zaretskii
  2016-07-12  7:52   ` Andreas Schwab
  0 siblings, 2 replies; 10+ messages in thread
From: Michael Albinus @ 2016-07-12  7:43 UTC (permalink / raw)
  To: Dima Kogan; +Cc: 23952

Dima Kogan <dima@secretsauce.net> writes:

> Hi.

Hi,

> I'm running a very recent emacs built from git (658daf9). In the
> last two weeks or so, remote VC mode stopped working. Recipe:
>
> 1. emacs -Q
> 2. C-x C-f /127.0.0.1:emacs (or any other remote path that has a git
>    repo)
> 3. C-x C-v D (or any other VC-mode function)
>
> Instead of seeing VC mode do its thing it now throws an error:
>
>   env: ‘GIT_DIR’: No such file or directory

This is due to Bug#23769, adding "GIT_DIR" to `process-environment'.
Tramp handles properly entries like "key=value" and "key=", entries like
"key" are not handled properly yet.

The following patch towards the emacs-25 branch fixes this:

--8<---------------cut here---------------start------------->8---
*** /home/albinus/src/emacs-25/lisp/net/tramp-sh.el.~f981b3136742a8597674dd4915afeb1b220a0464~	2016-07-12 09:41:08.663542129 +0200
--- /home/albinus/src/emacs-25/lisp/net/tramp-sh.el	2016-07-12 09:41:03.843470688 +0200
***************
*** 3034,3040 ****
  		 (cons prompt (nreverse (copy-sequence process-environment)))
  		 env)
  	      (or (member elt (default-toplevel-value 'process-environment))
! 		  (setq env (cons elt env)))))
  	   (command
  	    (when (stringp program)
  	      (format "cd %s && exec %s env %s %s"
--- 3034,3043 ----
  		 (cons prompt (nreverse (copy-sequence process-environment)))
  		 env)
  	      (or (member elt (default-toplevel-value 'process-environment))
!                   (setq env
!                         (if (string-match "=" elt)
!                             (append env `(,elt))
!                           (append `("-u" ,elt) env))))))
  	   (command
  	    (when (stringp program)
  	      (format "cd %s && exec %s env %s %s"
***************
*** 3135,3141 ****
        (setq env
  	    (dolist (elt (nreverse (copy-sequence process-environment)) env)
  	      (or (member elt (default-toplevel-value 'process-environment))
! 		  (setq env (cons elt env)))))
        (when env
  	(setq command
  	      (format
--- 3138,3147 ----
        (setq env
  	    (dolist (elt (nreverse (copy-sequence process-environment)) env)
  	      (or (member elt (default-toplevel-value 'process-environment))
!                   (setq env
!                         (if (string-match "=" elt)
!                             (append env `(,elt))
!                           (append `("-u" ,elt) env))))))
        (when env
  	(setq command
  	      (format
--8<---------------cut here---------------end--------------->8---

Should this be pushed to the emacs-25 branch, or to master?

Best regards, Michael.





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

* bug#23952: 25.0.95; Regression: vc (git) over TRAMP doesn't work anymore
  2016-07-12  7:43 ` Michael Albinus
@ 2016-07-12  7:49   ` Eli Zaretskii
  2016-07-12  7:52   ` Andreas Schwab
  1 sibling, 0 replies; 10+ messages in thread
From: Eli Zaretskii @ 2016-07-12  7:49 UTC (permalink / raw)
  To: Michael Albinus; +Cc: 23952, dima

> From: Michael Albinus <michael.albinus@gmx.de>
> Date: Tue, 12 Jul 2016 09:43:58 +0200
> Cc: 23952@debbugs.gnu.org
> 
> Should this be pushed to the emacs-25 branch, or to master?

emacs-25, please.

Thanks.





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

* bug#23952: 25.0.95; Regression: vc (git) over TRAMP doesn't work anymore
  2016-07-12  7:43 ` Michael Albinus
  2016-07-12  7:49   ` Eli Zaretskii
@ 2016-07-12  7:52   ` Andreas Schwab
  2016-07-12 14:53     ` Michael Albinus
  1 sibling, 1 reply; 10+ messages in thread
From: Andreas Schwab @ 2016-07-12  7:52 UTC (permalink / raw)
  To: Michael Albinus; +Cc: 23952, Dima Kogan

Michael Albinus <michael.albinus@gmx.de> writes:

> !                   (setq env
> !                         (if (string-match "=" elt)
> !                             (append env `(,elt))
> !                           (append `("-u" ,elt) env))))))

env -u is a GNU extension.

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."





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

* bug#23952: 25.0.95; Regression: vc (git) over TRAMP doesn't work anymore
  2016-07-12  7:52   ` Andreas Schwab
@ 2016-07-12 14:53     ` Michael Albinus
  2016-07-12 15:07       ` Eli Zaretskii
  0 siblings, 1 reply; 10+ messages in thread
From: Michael Albinus @ 2016-07-12 14:53 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: 23952, Dima Kogan

Andreas Schwab <schwab@suse.de> writes:

> env -u is a GNU extension.

Indeed. The following patch should care for non-gnu remote systems:

--8<---------------cut here---------------start------------->8---
*** /home/albinus/src/emacs-25/lisp/net/tramp-sh.el.~f981b3136742a8597674dd4915afeb1b220a0464~	2016-07-12 16:47:21.714648048 +0200
--- /home/albinus/src/emacs-25/lisp/net/tramp-sh.el	2016-07-12 16:36:03.505265390 +0200
***************
*** 3027,3044 ****
  			   tramp-initial-end-of-output))
  	   ;; We use as environment the difference to toplevel
  	   ;; `process-environment'.
! 	   env
! 	   (env
! 	    (dolist
! 		(elt
! 		 (cons prompt (nreverse (copy-sequence process-environment)))
! 		 env)
! 	      (or (member elt (default-toplevel-value 'process-environment))
! 		  (setq env (cons elt env)))))
  	   (command
  	    (when (stringp program)
! 	      (format "cd %s && exec %s env %s %s"
  		      (tramp-shell-quote-argument localname)
  		      (if heredoc (format "<<'%s'" tramp-end-of-heredoc) "")
  		      (mapconcat 'tramp-shell-quote-argument env " ")
  		      (if heredoc
--- 3027,3049 ----
  			   tramp-initial-end-of-output))
  	   ;; We use as environment the difference to toplevel
  	   ;; `process-environment'.
! 	   env uenv
! 	   (env (dolist (elt (cons prompt process-environment) env)
!                   (or (member elt (default-toplevel-value 'process-environment))
!                       (if (string-match "=" elt)
!                           (setq env (append env `(,elt)))
!                         (if (tramp-get-env-with-u-option v)
!                             (setq env (append `("-u" ,elt) env))
!                           (setq uenv (cons elt uenv)))))))
  	   (command
  	    (when (stringp program)
! 	      (format "cd %s && %s exec %s env %s %s"
  		      (tramp-shell-quote-argument localname)
+                       (if uenv
+                           (format
+                            "unset %s &&"
+                            (mapconcat 'tramp-shell-quote-argument uenv " "))
+                         "")
  		      (if heredoc (format "<<'%s'" tramp-end-of-heredoc) "")
  		      (mapconcat 'tramp-shell-quote-argument env " ")
  		      (if heredoc
***************
*** 3127,3146 ****
      (error "Implementation does not handle immediate return"))
  
    (with-parsed-tramp-file-name default-directory nil
!     (let (command env input tmpinput stderr tmpstderr outbuf ret)
        ;; Compute command.
        (setq command (mapconcat 'tramp-shell-quote-argument
  			       (cons program args) " "))
        ;; We use as environment the difference to toplevel `process-environment'.
!       (setq env
! 	    (dolist (elt (nreverse (copy-sequence process-environment)) env)
! 	      (or (member elt (default-toplevel-value 'process-environment))
! 		  (setq env (cons elt env)))))
        (when env
  	(setq command
  	      (format
  	       "env %s %s"
  	       (mapconcat 'tramp-shell-quote-argument env " ") command)))
        ;; Determine input.
        (if (null infile)
  	  (setq input "/dev/null")
--- 3132,3159 ----
      (error "Implementation does not handle immediate return"))
  
    (with-parsed-tramp-file-name default-directory nil
!     (let (command env uenv input tmpinput stderr tmpstderr outbuf ret)
        ;; Compute command.
        (setq command (mapconcat 'tramp-shell-quote-argument
  			       (cons program args) " "))
        ;; We use as environment the difference to toplevel `process-environment'.
!       (dolist (elt process-environment)
!         (or (member elt (default-toplevel-value 'process-environment))
!             (if (string-match "=" elt)
!                 (setq env (append env `(,elt)))
!               (if (tramp-get-env-with-u-option v)
!                   (setq env (append `("-u" ,elt) env))
!                 (setq uenv (cons elt uenv))))))
        (when env
  	(setq command
  	      (format
  	       "env %s %s"
  	       (mapconcat 'tramp-shell-quote-argument env " ") command)))
+       (when uenv
+         (setq command
+               (format
+                "unset %s && %s"
+                (mapconcat 'tramp-shell-quote-argument uenv " ") command)))
        ;; Determine input.
        (if (null infile)
  	  (setq input "/dev/null")
***************
*** 5695,5700 ****
--- 5708,5720 ----
         ((and (equal id-format 'string) (not (stringp res))) "UNKNOWN")
         (t res)))))
  
+ (defun tramp-get-env-with-u-option (vec)
+   (with-tramp-connection-property vec "env-u-option"
+     (tramp-message vec 5 "Checking, whether `env -u' works")
+     ;; Option "-u" is a GNU extension.
+     (tramp-send-command-and-check
+      vec "env FOO=foo env -u FOO 2>/dev/null | grep -qv FOO" t)))
+ 
  ;; Some predefined connection properties.
  (defun tramp-get-inline-compress (vec prop size)
    "Return the compress command related to PROP.
--8<---------------cut here---------------end--------------->8---

Should this still go to the emacs-25 branch?

> Andreas.

Best regards, Michael.





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

* bug#23952: 25.0.95; Regression: vc (git) over TRAMP doesn't work anymore
  2016-07-12 14:53     ` Michael Albinus
@ 2016-07-12 15:07       ` Eli Zaretskii
  2016-07-12 15:37         ` Michael Albinus
  0 siblings, 1 reply; 10+ messages in thread
From: Eli Zaretskii @ 2016-07-12 15:07 UTC (permalink / raw)
  To: Michael Albinus; +Cc: 23952, schwab, dima

> From: Michael Albinus <michael.albinus@gmx.de>
> Date: Tue, 12 Jul 2016 16:53:15 +0200
> Cc: 23952@debbugs.gnu.org, Dima Kogan <dima@secretsauce.net>
> 
> Should this still go to the emacs-25 branch?

Do you consider it safe enough for the release branch?





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

* bug#23952: 25.0.95; Regression: vc (git) over TRAMP doesn't work anymore
  2016-07-12 15:07       ` Eli Zaretskii
@ 2016-07-12 15:37         ` Michael Albinus
  2016-07-12 16:08           ` Dima Kogan
  0 siblings, 1 reply; 10+ messages in thread
From: Michael Albinus @ 2016-07-12 15:37 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 23952, schwab, dima

Eli Zaretskii <eliz@gnu.org> writes:

>> Should this still go to the emacs-25 branch?
>
> Do you consider it safe enough for the release branch?

Well, I've tested it here in several variants, for GNU "env -u", and for
the non-GNU "env" variant. In both cases, `vc-dir' works again for a
remote git-controlled directory. So I regard it as safe, yes.

Maybe Dima could recheck the patch?

I would also add some few tests to tramp-tests.el. Tomorrow.

Best regards, Michael.





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

* bug#23952: 25.0.95; Regression: vc (git) over TRAMP doesn't work anymore
  2016-07-12 15:37         ` Michael Albinus
@ 2016-07-12 16:08           ` Dima Kogan
  2016-07-12 17:47             ` Eli Zaretskii
  0 siblings, 1 reply; 10+ messages in thread
From: Dima Kogan @ 2016-07-12 16:08 UTC (permalink / raw)
  To: Michael Albinus; +Cc: 23952, schwab

Michael Albinus <michael.albinus@gmx.de> writes:

> Eli Zaretskii <eliz@gnu.org> writes:
>
>>> Should this still go to the emacs-25 branch?
>>
>> Do you consider it safe enough for the release branch?
>
> Well, I've tested it here in several variants, for GNU "env -u", and for
> the non-GNU "env" variant. In both cases, `vc-dir' works again for a
> remote git-controlled directory. So I regard it as safe, yes.
>
> Maybe Dima could recheck the patch?

I just tested it on my machine (fairly vanilla Debian/sid install) and
it fixes the issue. Thank you, Michael.

dima





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

* bug#23952: 25.0.95; Regression: vc (git) over TRAMP doesn't work anymore
  2016-07-12 16:08           ` Dima Kogan
@ 2016-07-12 17:47             ` Eli Zaretskii
  2016-07-12 18:15               ` Michael Albinus
  0 siblings, 1 reply; 10+ messages in thread
From: Eli Zaretskii @ 2016-07-12 17:47 UTC (permalink / raw)
  To: Dima Kogan; +Cc: 23952, schwab, michael.albinus

> From: Dima Kogan <lists@dima.secretsauce.net>
> Cc: Eli Zaretskii <eliz@gnu.org>, schwab@suse.de, 23952@debbugs.gnu.org
> Date: Tue, 12 Jul 2016 09:08:43 -0700
> 
> Michael Albinus <michael.albinus@gmx.de> writes:
> 
> > Eli Zaretskii <eliz@gnu.org> writes:
> >
> >>> Should this still go to the emacs-25 branch?
> >>
> >> Do you consider it safe enough for the release branch?
> >
> > Well, I've tested it here in several variants, for GNU "env -u", and for
> > the non-GNU "env" variant. In both cases, `vc-dir' works again for a
> > remote git-controlled directory. So I regard it as safe, yes.
> >
> > Maybe Dima could recheck the patch?
> 
> I just tested it on my machine (fairly vanilla Debian/sid install) and
> it fixes the issue. Thank you, Michael.

OK, thanks.  Let's push to emacs-25, then.





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

* bug#23952: 25.0.95; Regression: vc (git) over TRAMP doesn't work anymore
  2016-07-12 17:47             ` Eli Zaretskii
@ 2016-07-12 18:15               ` Michael Albinus
  0 siblings, 0 replies; 10+ messages in thread
From: Michael Albinus @ 2016-07-12 18:15 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Dima Kogan, schwab, 23952-done

Eli Zaretskii <eliz@gnu.org> writes:

>> > Maybe Dima could recheck the patch?
>> 
>> I just tested it on my machine (fairly vanilla Debian/sid install) and
>> it fixes the issue. Thank you, Michael.
>
> OK, thanks.  Let's push to emacs-25, then.

Done, closing the bug.

"make -C test/automated tramp-tests" passes also all tests, at least
there is no obvious regression. As promised, I'll add another test for
environment variables, tomorrow.

Best regards, Michael.





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

end of thread, other threads:[~2016-07-12 18:15 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-07-12  5:38 bug#23952: 25.0.95; Regression: vc (git) over TRAMP doesn't work anymore Dima Kogan
2016-07-12  7:43 ` Michael Albinus
2016-07-12  7:49   ` Eli Zaretskii
2016-07-12  7:52   ` Andreas Schwab
2016-07-12 14:53     ` Michael Albinus
2016-07-12 15:07       ` Eli Zaretskii
2016-07-12 15:37         ` Michael Albinus
2016-07-12 16:08           ` Dima Kogan
2016-07-12 17:47             ` Eli Zaretskii
2016-07-12 18:15               ` Michael Albinus

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