* bug#23779: 25.0.95; consing "SHELLVAR" onto process-environment doesn't remove it from subprocess env
@ 2016-06-17 3:33 Noam Postavsky
2016-06-17 7:11 ` Eli Zaretskii
0 siblings, 1 reply; 25+ messages in thread
From: Noam Postavsky @ 2016-06-17 3:33 UTC (permalink / raw
To: 23779
Start emacs -Q, evaluate the following lisp code (I wrote the return
values after ;=>)
(defun check-env-var (var)
(catch 'ret
(dolist (var=val (process-lines "env"))
(when (string-prefix-p var var=val)
(throw 'ret var=val)))))
(check-env-var "SHELL");=>"SHELL=/bin/bash"
(let ((process-environment (copy-sequence process-environment)))
(setenv "SHELL" nil)
(check-env-var "SHELL"));=>nil
(let ((process-environment (cons "SHELL" process-environment)))
(check-env-var "SHELL"));=>"SHELL=/bin/bash"
(let ((process-environment (cons "SHELL=" process-environment)))
(check-env-var "SHELL"));=>"SHELL="
As you can see from the 3rd expression, contrary to its docstring,
consing the env variable "SHELL" onto process-environment has no
effect at all.
process-environment is a variable defined in ‘C source code’.
Its value is
[...]
Documentation:
List of overridden environment variables for subprocesses to inherit.
Each element should be a string of the form ENVVARNAME=VALUE.
Entries in this list take precedence to those in the frame-local
environments. Therefore, let-binding ‘process-environment’ is an easy
way to temporarily change the value of an environment variable,
irrespective of where it comes from. To use ‘process-environment’ to
remove an environment variable, include only its name in the list,
without "=VALUE".
^ permalink raw reply [flat|nested] 25+ messages in thread
* bug#23779: 25.0.95; consing "SHELLVAR" onto process-environment doesn't remove it from subprocess env
2016-06-17 3:33 bug#23779: 25.0.95; consing "SHELLVAR" onto process-environment doesn't remove it from subprocess env Noam Postavsky
@ 2016-06-17 7:11 ` Eli Zaretskii
2016-06-17 12:17 ` Dmitry Gutov
0 siblings, 1 reply; 25+ messages in thread
From: Eli Zaretskii @ 2016-06-17 7:11 UTC (permalink / raw
To: Noam Postavsky; +Cc: 23779
> From: Noam Postavsky <npostavs@users.sourceforge.net>
> Date: Thu, 16 Jun 2016 23:33:02 -0400
>
> (check-env-var "SHELL");=>"SHELL=/bin/bash"
> (let ((process-environment (copy-sequence process-environment)))
> (setenv "SHELL" nil)
> (check-env-var "SHELL"));=>nil
> (let ((process-environment (cons "SHELL" process-environment)))
> (check-env-var "SHELL"));=>"SHELL=/bin/bash"
> (let ((process-environment (cons "SHELL=" process-environment)))
> (check-env-var "SHELL"));=>"SHELL="
>
> As you can see from the 3rd expression, contrary to its docstring,
> consing the env variable "SHELL" onto process-environment has no
> effect at all.
>
> process-environment is a variable defined in ‘C source code’.
> Its value is
> [...]
> Documentation:
> List of overridden environment variables for subprocesses to inherit.
> Each element should be a string of the form ENVVARNAME=VALUE.
>
> Entries in this list take precedence to those in the frame-local
> environments. Therefore, let-binding ‘process-environment’ is an easy
> way to temporarily change the value of an environment variable,
> irrespective of where it comes from. To use ‘process-environment’ to
> remove an environment variable, include only its name in the list,
> without "=VALUE".
Where does it say that you can use 'cons' or 'push', and only them, to
the effect of removing the variable from the environment passed to
child processes? process-environment is just a simple list, so these
two functions just _add_ another member to the list, they don't remove
the old member.
My reading of the last sentence you cite is that you must manually
remove the old member "SHELL=whatever", and _then_ add a member that
has no value.
Am I missing something?
^ permalink raw reply [flat|nested] 25+ messages in thread
* bug#23779: 25.0.95; consing "SHELLVAR" onto process-environment doesn't remove it from subprocess env
2016-06-17 7:11 ` Eli Zaretskii
@ 2016-06-17 12:17 ` Dmitry Gutov
2016-06-17 14:01 ` Eli Zaretskii
0 siblings, 1 reply; 25+ messages in thread
From: Dmitry Gutov @ 2016-06-17 12:17 UTC (permalink / raw
To: Eli Zaretskii, Noam Postavsky; +Cc: 23779
On 06/17/2016 10:11 AM, Eli Zaretskii wrote:
> Where does it say that you can use 'cons' or 'push', and only them, to
> the effect of removing the variable from the environment passed to
> child processes?
That works with other Emacs features, such as auto-mode-alist.
You can also override the values in process-environment using a cons
(which strongly suggests the semantics of "first element wins"). Just
not "remove" them, currently.
> process-environment is just a simple list, so these
> two functions just _add_ another member to the list, they don't remove
> the old member.
What would be the point of ever using the "only its name" form if you
have to scrub process-environment of all other mentions of this variable?
> My reading of the last sentence you cite is that you must manually
> remove the old member "SHELL=whatever", and _then_ add a member that
> has no value.
If I've removed all mentions of "GIT_DIR" in there, it's already
removed, and the subprocesses shouldn't see it. Why add the new member then?
^ permalink raw reply [flat|nested] 25+ messages in thread
* bug#23779: 25.0.95; consing "SHELLVAR" onto process-environment doesn't remove it from subprocess env
2016-06-17 12:17 ` Dmitry Gutov
@ 2016-06-17 14:01 ` Eli Zaretskii
2016-06-17 14:12 ` Dmitry Gutov
0 siblings, 1 reply; 25+ messages in thread
From: Eli Zaretskii @ 2016-06-17 14:01 UTC (permalink / raw
To: Dmitry Gutov; +Cc: 23779, npostavs
> Cc: 23779@debbugs.gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Fri, 17 Jun 2016 15:17:38 +0300
>
> On 06/17/2016 10:11 AM, Eli Zaretskii wrote:
>
> > Where does it say that you can use 'cons' or 'push', and only them, to
> > the effect of removing the variable from the environment passed to
> > child processes?
>
> That works with other Emacs features, such as auto-mode-alist.
There should be code to make it work, it won't work by itself.
(auto-mode-alist is different, because it's used entirely in Lisp. By
contrast, here we must construct the C-level environment array we pass
to child programs so as to remove the variable from it.)
> You can also override the values in process-environment using a cons
> (which strongly suggests the semantics of "first element wins"). Just
> not "remove" them, currently.
Looks like this never worked as intended. Does the patch below fix
this?
diff --git a/src/callproc.c b/src/callproc.c
index db602f5..2fb5b1d 100644
--- a/src/callproc.c
+++ b/src/callproc.c
@@ -1099,7 +1099,7 @@ add_env (char **env, char **new_env, char *string)
char *p = *ep, *q = string;
while (ok)
{
- if (*q != *p)
+ if (*p && *q != *p)
break;
if (*q == 0)
/* The string is a lone variable name; keep it for now, we
^ permalink raw reply related [flat|nested] 25+ messages in thread
* bug#23779: 25.0.95; consing "SHELLVAR" onto process-environment doesn't remove it from subprocess env
2016-06-17 14:01 ` Eli Zaretskii
@ 2016-06-17 14:12 ` Dmitry Gutov
2016-06-17 14:19 ` Eli Zaretskii
0 siblings, 1 reply; 25+ messages in thread
From: Dmitry Gutov @ 2016-06-17 14:12 UTC (permalink / raw
To: Eli Zaretskii; +Cc: 23779, npostavs
On 06/17/2016 05:01 PM, Eli Zaretskii wrote:
> There should be code to make it work, it won't work by itself.
No argument there.
> Looks like this never worked as intended. Does the patch below fix
> this?
Looks like it does. Please push at your convenience.
I wonder if we should make setenv work non-destructively now.
^ permalink raw reply [flat|nested] 25+ messages in thread
* bug#23779: 25.0.95; consing "SHELLVAR" onto process-environment doesn't remove it from subprocess env
2016-06-17 14:12 ` Dmitry Gutov
@ 2016-06-17 14:19 ` Eli Zaretskii
2016-06-17 14:47 ` Dmitry Gutov
2016-06-19 2:27 ` Paul Eggert
0 siblings, 2 replies; 25+ messages in thread
From: Eli Zaretskii @ 2016-06-17 14:19 UTC (permalink / raw
To: Dmitry Gutov, Paul Eggert, Andreas Schwab; +Cc: 23779, npostavs
> Cc: npostavs@users.sourceforge.net, 23779@debbugs.gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Fri, 17 Jun 2016 17:12:52 +0300
>
> On 06/17/2016 05:01 PM, Eli Zaretskii wrote:
>
> > Looks like this never worked as intended. Does the patch below fix
> > this?
>
> Looks like it does. Please push at your convenience.
I'd like the patch to be eyeballed by a few more people. Paul,
Andreas, do you see any problems with it? If not, I'd like to push it
to the emacs-25 branch.
> I wonder if we should make setenv work non-destructively now.
Why should we do that? We have initial-environment if we need the
original value.
^ permalink raw reply [flat|nested] 25+ messages in thread
* bug#23779: 25.0.95; consing "SHELLVAR" onto process-environment doesn't remove it from subprocess env
2016-06-17 14:19 ` Eli Zaretskii
@ 2016-06-17 14:47 ` Dmitry Gutov
2016-06-17 16:52 ` Andreas Schwab
` (2 more replies)
2016-06-19 2:27 ` Paul Eggert
1 sibling, 3 replies; 25+ messages in thread
From: Dmitry Gutov @ 2016-06-17 14:47 UTC (permalink / raw
To: Eli Zaretskii, Paul Eggert, Andreas Schwab; +Cc: 23779, npostavs
On 06/17/2016 05:19 PM, Eli Zaretskii wrote:
>> I wonder if we should make setenv work non-destructively now.
>
> Why should we do that? We have initial-environment if we need the
> original value.
Normally, we only want to change the environment for the duration of a
command. So, what are the downsides?
One the plus side:
- setenv-internal will become simpler.
- We won't have to cons manually anymore. The code will become a bit
nicer, like:
diff --git a/lisp/vc/vc-git.el b/lisp/vc/vc-git.el
index f35c84d..5315e0a 100644
--- a/lisp/vc/vc-git.el
+++ b/lisp/vc/vc-git.el
@@ -1450,7 +1450,8 @@ 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 process-environment))
+ (setenv "PAGER")
(apply 'process-file vc-git-program nil buffer nil command args)))
(defun vc-git--out-ok (command &rest args)
^ permalink raw reply related [flat|nested] 25+ messages in thread
* bug#23779: 25.0.95; consing "SHELLVAR" onto process-environment doesn't remove it from subprocess env
2016-06-17 14:47 ` Dmitry Gutov
@ 2016-06-17 16:52 ` Andreas Schwab
2016-06-17 16:55 ` Dmitry Gutov
2016-06-17 17:06 ` Eli Zaretskii
2016-06-18 1:36 ` Noam Postavsky
2 siblings, 1 reply; 25+ messages in thread
From: Andreas Schwab @ 2016-06-17 16:52 UTC (permalink / raw
To: Dmitry Gutov; +Cc: npostavs, 23779, Paul Eggert
Dmitry Gutov <dgutov@yandex.ru> writes:
> diff --git a/lisp/vc/vc-git.el b/lisp/vc/vc-git.el
> index f35c84d..5315e0a 100644
> --- a/lisp/vc/vc-git.el
> +++ b/lisp/vc/vc-git.el
> @@ -1450,7 +1450,8 @@ 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 process-environment))
> + (setenv "PAGER")
> (apply 'process-file vc-git-program nil buffer nil command args)))
Is that in any way different from git --no-pager?
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."
^ permalink raw reply [flat|nested] 25+ messages in thread
* bug#23779: 25.0.95; consing "SHELLVAR" onto process-environment doesn't remove it from subprocess env
2016-06-17 16:52 ` Andreas Schwab
@ 2016-06-17 16:55 ` Dmitry Gutov
0 siblings, 0 replies; 25+ messages in thread
From: Dmitry Gutov @ 2016-06-17 16:55 UTC (permalink / raw
To: Andreas Schwab; +Cc: npostavs, 23779, Paul Eggert
On 06/17/2016 07:52 PM, Andreas Schwab wrote:
> Dmitry Gutov <dgutov@yandex.ru> writes:
>> (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 process-environment))
>> + (setenv "PAGER")
>> (apply 'process-file vc-git-program nil buffer nil command args)))
>
> Is that in any way different from git --no-pager?
The effect is likely the same. But that is beside the point: the example
is not about PAGER but about setenv.
^ permalink raw reply [flat|nested] 25+ messages in thread
* bug#23779: 25.0.95; consing "SHELLVAR" onto process-environment doesn't remove it from subprocess env
2016-06-17 14:47 ` Dmitry Gutov
2016-06-17 16:52 ` Andreas Schwab
@ 2016-06-17 17:06 ` Eli Zaretskii
2016-06-17 19:01 ` Dmitry Gutov
2016-06-18 1:36 ` Noam Postavsky
2 siblings, 1 reply; 25+ messages in thread
From: Eli Zaretskii @ 2016-06-17 17:06 UTC (permalink / raw
To: Dmitry Gutov; +Cc: npostavs, 23779, schwab, eggert
> Cc: npostavs@users.sourceforge.net, 23779@debbugs.gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Fri, 17 Jun 2016 17:47:49 +0300
>
> On 06/17/2016 05:19 PM, Eli Zaretskii wrote:
>
> >> I wonder if we should make setenv work non-destructively now.
> >
> > Why should we do that? We have initial-environment if we need the
> > original value.
>
> Normally, we only want to change the environment for the duration of a
> command.
Now that 'push' works, why do we need setenv for that?
> So, what are the downsides?
That there's no way of changing the environment permanently?
> - We won't have to cons manually anymore. The code will become a bit
> nicer, like:
>
> diff --git a/lisp/vc/vc-git.el b/lisp/vc/vc-git.el
> index f35c84d..5315e0a 100644
> --- a/lisp/vc/vc-git.el
> +++ b/lisp/vc/vc-git.el
> @@ -1450,7 +1450,8 @@ 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 process-environment))
> + (setenv "PAGER")
I'm not sure I see why that is nicer.
^ permalink raw reply [flat|nested] 25+ messages in thread
* bug#23779: 25.0.95; consing "SHELLVAR" onto process-environment doesn't remove it from subprocess env
2016-06-17 17:06 ` Eli Zaretskii
@ 2016-06-17 19:01 ` Dmitry Gutov
2016-06-17 20:10 ` Eli Zaretskii
0 siblings, 1 reply; 25+ messages in thread
From: Dmitry Gutov @ 2016-06-17 19:01 UTC (permalink / raw
To: Eli Zaretskii; +Cc: npostavs, 23779, schwab, eggert
On 06/17/2016 08:06 PM, Eli Zaretskii wrote:
> Now that 'push' works, why do we need setenv for that?
So that the user doesn't have to (push "VAR=value"), or (push "VAR="),
or know the difference between the latter and (push "VAR").
setenv is a better abstraction. In fact, if you use it, you don't even
have to know the format of process-environment.
>> So, what are the downsides?
>
> That there's no way of changing the environment permanently?
The environment is changes as a result. It just doesn't modify the
original list, and, as such, contains duplicate values for the changed
variables.
> I'm not sure I see why that is nicer.
Do you know what value "PAGER=" assigns to the variable PAGER?
^ permalink raw reply [flat|nested] 25+ messages in thread
* bug#23779: 25.0.95; consing "SHELLVAR" onto process-environment doesn't remove it from subprocess env
2016-06-17 19:01 ` Dmitry Gutov
@ 2016-06-17 20:10 ` Eli Zaretskii
2016-06-17 21:26 ` Dmitry Gutov
0 siblings, 1 reply; 25+ messages in thread
From: Eli Zaretskii @ 2016-06-17 20:10 UTC (permalink / raw
To: Dmitry Gutov; +Cc: npostavs, 23779, schwab, eggert
> Cc: eggert@cs.ucla.edu, schwab@linux-m68k.org,
> npostavs@users.sourceforge.net, 23779@debbugs.gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Fri, 17 Jun 2016 22:01:19 +0300
>
> On 06/17/2016 08:06 PM, Eli Zaretskii wrote:
>
> > Now that 'push' works, why do we need setenv for that?
>
> So that the user doesn't have to (push "VAR=value"), or (push "VAR="),
> or know the difference between the latter and (push "VAR").
>
> setenv is a better abstraction. In fact, if you use it, you don't even
> have to know the format of process-environment.
We will have to agree to disagree on this one.
> >> So, what are the downsides?
> >
> > That there's no way of changing the environment permanently?
>
> The environment is changes as a result. It just doesn't modify the
> original list, and, as such, contains duplicate values for the changed
> variables.
What do I do if I do want to change the original one?
> > I'm not sure I see why that is nicer.
>
> Do you know what value "PAGER=" assigns to the variable PAGER?
Not sure what that means, or why do you ask that.
^ permalink raw reply [flat|nested] 25+ messages in thread
* bug#23779: 25.0.95; consing "SHELLVAR" onto process-environment doesn't remove it from subprocess env
2016-06-17 20:10 ` Eli Zaretskii
@ 2016-06-17 21:26 ` Dmitry Gutov
2016-06-18 7:51 ` Eli Zaretskii
0 siblings, 1 reply; 25+ messages in thread
From: Dmitry Gutov @ 2016-06-17 21:26 UTC (permalink / raw
To: Eli Zaretskii; +Cc: 23779, eggert, schwab, npostavs
On 06/17/2016 11:10 PM, Eli Zaretskii wrote:
> What do I do if I do want to change the original one?
That's what I was asking: are there scenarios where someone would want
to do that? If they're very exceptional, if might be fine if the caller
has to manipulate the process-environment list on a lower level themselves.
> Not sure what that means, or why do you ask that.
Do you know which value
(setq process-environment (cons "PAGER=" ...))
assigns to the environment variable PAGER?
^ permalink raw reply [flat|nested] 25+ messages in thread
* bug#23779: 25.0.95; consing "SHELLVAR" onto process-environment doesn't remove it from subprocess env
2016-06-17 14:47 ` Dmitry Gutov
2016-06-17 16:52 ` Andreas Schwab
2016-06-17 17:06 ` Eli Zaretskii
@ 2016-06-18 1:36 ` Noam Postavsky
2016-06-18 1:44 ` Dmitry Gutov
2 siblings, 1 reply; 25+ messages in thread
From: Noam Postavsky @ 2016-06-18 1:36 UTC (permalink / raw
To: Dmitry Gutov; +Cc: 23779, Andreas Schwab, Paul Eggert
On Fri, Jun 17, 2016 at 10:47 AM, Dmitry Gutov <dgutov@yandex.ru> wrote:
> On 06/17/2016 05:19 PM, Eli Zaretskii wrote:
>
>>> I wonder if we should make setenv work non-destructively now.
>>
>>
>> Why should we do that? We have initial-environment if we need the
>> original value.
>
>
> Normally, we only want to change the environment for the duration of a
> command. So, what are the downsides?
process-environment could grow without bound. Only a serious problem
if some code calls setenv many times without let-binding
process-environment which is probably a mistake, but still there could
be code like that.
^ permalink raw reply [flat|nested] 25+ messages in thread
* bug#23779: 25.0.95; consing "SHELLVAR" onto process-environment doesn't remove it from subprocess env
2016-06-18 1:36 ` Noam Postavsky
@ 2016-06-18 1:44 ` Dmitry Gutov
0 siblings, 0 replies; 25+ messages in thread
From: Dmitry Gutov @ 2016-06-18 1:44 UTC (permalink / raw
To: Noam Postavsky; +Cc: 23779, Andreas Schwab, Paul Eggert
On 06/18/2016 04:36 AM, Noam Postavsky wrote:
> process-environment could grow without bound. Only a serious problem
> if some code calls setenv many times without let-binding
> process-environment which is probably a mistake, but still there could
> be code like that.
Yes, it's easy to see an artificial scenario leading to that. But is it
at all probable to see one in practice?
Well, anyway, I believe I've made my case about setenv. It would be
nice, but far less important than getting Eli's patch in.
^ permalink raw reply [flat|nested] 25+ messages in thread
* bug#23779: 25.0.95; consing "SHELLVAR" onto process-environment doesn't remove it from subprocess env
2016-06-17 21:26 ` Dmitry Gutov
@ 2016-06-18 7:51 ` Eli Zaretskii
0 siblings, 0 replies; 25+ messages in thread
From: Eli Zaretskii @ 2016-06-18 7:51 UTC (permalink / raw
To: Dmitry Gutov; +Cc: 23779, eggert, schwab, npostavs
> Cc: npostavs@users.sourceforge.net, 23779@debbugs.gnu.org,
> schwab@linux-m68k.org, eggert@cs.ucla.edu
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Sat, 18 Jun 2016 00:26:28 +0300
>
> On 06/17/2016 11:10 PM, Eli Zaretskii wrote:
>
> > What do I do if I do want to change the original one?
>
> That's what I was asking: are there scenarios where someone would want
> to do that? If they're very exceptional, if might be fine if the caller
> has to manipulate the process-environment list on a lower level themselves.
I don't think we have good means of finding that out. In general, a
use case that was possible (and for many years at that) should remain
possible, unless there's clear and hard evidence that it's no longer
valid, something I very much doubt we have in this case.
> > Not sure what that means, or why do you ask that.
>
> Do you know which value
>
> (setq process-environment (cons "PAGER=" ...))
>
> assigns to the environment variable PAGER?
An empty value, AFAIK.
^ permalink raw reply [flat|nested] 25+ messages in thread
* bug#23779: 25.0.95; consing "SHELLVAR" onto process-environment doesn't remove it from subprocess env
2016-06-17 14:19 ` Eli Zaretskii
2016-06-17 14:47 ` Dmitry Gutov
@ 2016-06-19 2:27 ` Paul Eggert
2016-06-19 15:01 ` Eli Zaretskii
1 sibling, 1 reply; 25+ messages in thread
From: Paul Eggert @ 2016-06-19 2:27 UTC (permalink / raw
To: Eli Zaretskii, Dmitry Gutov, Andreas Schwab; +Cc: 23779, npostavs
[-- Attachment #1: Type: text/plain, Size: 650 bytes --]
On 06/17/2016 04:19 PM, Eli Zaretskii wrote:
>> Looks like it does. Please push at your convenience.
> I'd like the patch to be eyeballed by a few more people. Paul,
> Andreas, do you see any problems with it? If not, I'd like to push it
> to the emacs-25 branch
The patch is correct. The code is tricky so I'm not surprised you wanted
another pair of eyes. The attached patch is a very minor tweak of your
patch that made it a bit easier for me to follow. I resisted the
temptation of cleaning up the surrounding code to make it more readable.
I like the idea of putting this into the emacs-25 branch. (Or your
patch; doesn't really matter.)
[-- Attachment #2: 0001-Consing-V-into-process-environment-should-remove-V.patch --]
[-- Type: text/x-patch, Size: 940 bytes --]
From ed5dabd6bb61613649f619f6f210d070812895d7 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Sun, 19 Jun 2016 03:58:37 +0200
Subject: [PATCH] Consing "V" into process-environment should remove V
Problem reported by Noam Postavsky (Bug#23779).
* src/callproc.c (add_env): Consider new string "FOO=BAR" to match plain
"FOO" in the old proto-environment. This is a slight tweak of the fix
suggested by Eli Zaretskii in http://bugs.gnu.org/23779#14
---
src/callproc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/callproc.c b/src/callproc.c
index db602f5..38f3f92 100644
--- a/src/callproc.c
+++ b/src/callproc.c
@@ -1099,7 +1099,7 @@ add_env (char **env, char **new_env, char *string)
char *p = *ep, *q = string;
while (ok)
{
- if (*q != *p)
+ if (*q != *p && *p)
break;
if (*q == 0)
/* The string is a lone variable name; keep it for now, we
--
2.5.5
^ permalink raw reply related [flat|nested] 25+ messages in thread
* bug#23779: 25.0.95; consing "SHELLVAR" onto process-environment doesn't remove it from subprocess env
2016-06-19 2:27 ` Paul Eggert
@ 2016-06-19 15:01 ` Eli Zaretskii
2016-06-19 22:53 ` Paul Eggert
0 siblings, 1 reply; 25+ messages in thread
From: Eli Zaretskii @ 2016-06-19 15:01 UTC (permalink / raw
To: Paul Eggert; +Cc: 23779, schwab, dgutov, npostavs
> Cc: npostavs@users.sourceforge.net, 23779@debbugs.gnu.org
> From: Paul Eggert <eggert@cs.ucla.edu>
> Date: Sun, 19 Jun 2016 04:27:32 +0200
>
> On 06/17/2016 04:19 PM, Eli Zaretskii wrote:
> >> Looks like it does. Please push at your convenience.
> > I'd like the patch to be eyeballed by a few more people. Paul,
> > Andreas, do you see any problems with it? If not, I'd like to push it
> > to the emacs-25 branch
> The patch is correct.
Thanks for the review. I will push the patch soon to emacs-25.
> The code is tricky so I'm not surprised you wanted another pair of
> eyes.
Indeed.
> The attached patch is a very minor tweak of your
> patch that made it a bit easier for me to follow.
I think I will go with my version, mainly because it will most
probably be short-lived, and so is not worth optimizing.
> I resisted the temptation of cleaning up the surrounding code to
> make it more readable.
Feel free to do that on master; I can mark the emacs-25 fix "not to be
merged".
^ permalink raw reply [flat|nested] 25+ messages in thread
* bug#23779: 25.0.95; consing "SHELLVAR" onto process-environment doesn't remove it from subprocess env
2016-06-19 15:01 ` Eli Zaretskii
@ 2016-06-19 22:53 ` Paul Eggert
2016-06-20 14:21 ` Eli Zaretskii
0 siblings, 1 reply; 25+ messages in thread
From: Paul Eggert @ 2016-06-19 22:53 UTC (permalink / raw
To: Eli Zaretskii; +Cc: 23779, schwab, dgutov, npostavs
On 06/19/2016 05:01 PM, Eli Zaretskii wrote:
>> >I resisted the temptation of cleaning up the surrounding code to
>> >make it more readable.
> Feel free to do that on master; I can mark the emacs-25 fix "not to be
> merged".
Please don't bother. I'll merge emacs-25 to master before fiddling with
this stuff on master, if I ever get around to fiddling. That's my usual
practice for this sort of thing.
^ permalink raw reply [flat|nested] 25+ messages in thread
* bug#23779: 25.0.95; consing "SHELLVAR" onto process-environment doesn't remove it from subprocess env
2016-06-19 22:53 ` Paul Eggert
@ 2016-06-20 14:21 ` Eli Zaretskii
2016-06-21 13:53 ` Dmitry Gutov
0 siblings, 1 reply; 25+ messages in thread
From: Eli Zaretskii @ 2016-06-20 14:21 UTC (permalink / raw
To: Paul Eggert; +Cc: dgutov, schwab, 23779-done, npostavs
> Cc: dgutov@yandex.ru, schwab@linux-m68k.org, npostavs@users.sourceforge.net,
> 23779@debbugs.gnu.org
> From: Paul Eggert <eggert@cs.ucla.edu>
> Date: Mon, 20 Jun 2016 00:53:21 +0200
>
> On 06/19/2016 05:01 PM, Eli Zaretskii wrote:
> >> >I resisted the temptation of cleaning up the surrounding code to
> >> >make it more readable.
> > Feel free to do that on master; I can mark the emacs-25 fix "not to be
> > merged".
>
> Please don't bother.
OK, thanks. Pushed as 5f37572 on the emacs-25 branch, and closing.
^ permalink raw reply [flat|nested] 25+ messages in thread
* bug#23779: 25.0.95; consing "SHELLVAR" onto process-environment doesn't remove it from subprocess env
2016-06-20 14:21 ` Eli Zaretskii
@ 2016-06-21 13:53 ` Dmitry Gutov
2016-06-21 15:21 ` Eli Zaretskii
0 siblings, 1 reply; 25+ messages in thread
From: Dmitry Gutov @ 2016-06-21 13:53 UTC (permalink / raw
To: 23779, eliz, npostavs
On 06/20/2016 05:21 PM, Eli Zaretskii wrote:
> OK, thanks. Pushed as 5f37572 on the emacs-25 branch, and closing.
Can we fix bug#23769 along with it?
The solution using process-environment looks obviously safe to me.
^ permalink raw reply [flat|nested] 25+ messages in thread
* bug#23779: 25.0.95; consing "SHELLVAR" onto process-environment doesn't remove it from subprocess env
2016-06-21 13:53 ` Dmitry Gutov
@ 2016-06-21 15:21 ` Eli Zaretskii
2016-06-21 15:24 ` Dmitry Gutov
0 siblings, 1 reply; 25+ messages in thread
From: Eli Zaretskii @ 2016-06-21 15:21 UTC (permalink / raw
To: Dmitry Gutov; +Cc: 23779, npostavs
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Tue, 21 Jun 2016 16:53:40 +0300
>
> On 06/20/2016 05:21 PM, Eli Zaretskii wrote:
>
> > OK, thanks. Pushed as 5f37572 on the emacs-25 branch, and closing.
>
> Can we fix bug#23769 along with it?
>
> The solution using process-environment looks obviously safe to me.
Please show a patch you have in mind.
^ permalink raw reply [flat|nested] 25+ messages in thread
* bug#23779: 25.0.95; consing "SHELLVAR" onto process-environment doesn't remove it from subprocess env
2016-06-21 15:21 ` Eli Zaretskii
@ 2016-06-21 15:24 ` Dmitry Gutov
2016-06-21 16:12 ` Eli Zaretskii
0 siblings, 1 reply; 25+ messages in thread
From: Dmitry Gutov @ 2016-06-21 15:24 UTC (permalink / raw
To: Eli Zaretskii; +Cc: 23779, npostavs
On 06/21/2016 06:21 PM, Eli Zaretskii wrote:
> Please show a patch you have in mind.
The one in http://debbugs.gnu.org/cgi/bugreport.cgi?bug=23769#86.
^ permalink raw reply [flat|nested] 25+ messages in thread
* bug#23779: 25.0.95; consing "SHELLVAR" onto process-environment doesn't remove it from subprocess env
2016-06-21 15:24 ` Dmitry Gutov
@ 2016-06-21 16:12 ` Eli Zaretskii
2016-06-21 23:05 ` Dmitry Gutov
0 siblings, 1 reply; 25+ messages in thread
From: Eli Zaretskii @ 2016-06-21 16:12 UTC (permalink / raw
To: Dmitry Gutov; +Cc: 23779, npostavs
> Cc: 23779@debbugs.gnu.org, npostavs@users.sourceforge.net
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Tue, 21 Jun 2016 18:24:38 +0300
>
> On 06/21/2016 06:21 PM, Eli Zaretskii wrote:
>
> > Please show a patch you have in mind.
>
> The one in http://debbugs.gnu.org/cgi/bugreport.cgi?bug=23769#86.
Looks OK to me. (For some reason, I was under the impression that it
doesn't necessarily fix the problem.)
^ permalink raw reply [flat|nested] 25+ messages in thread
* bug#23779: 25.0.95; consing "SHELLVAR" onto process-environment doesn't remove it from subprocess env
2016-06-21 16:12 ` Eli Zaretskii
@ 2016-06-21 23:05 ` Dmitry Gutov
0 siblings, 0 replies; 25+ messages in thread
From: Dmitry Gutov @ 2016-06-21 23:05 UTC (permalink / raw
To: Eli Zaretskii; +Cc: 23779, npostavs
On 06/21/2016 07:12 PM, Eli Zaretskii wrote:
>> The one in http://debbugs.gnu.org/cgi/bugreport.cgi?bug=23769#86.
>
> Looks OK to me. (For some reason, I was under the impression that it
> doesn't necessarily fix the problem.)
It did not when it was presented. Hence the filing of this bug.
^ permalink raw reply [flat|nested] 25+ messages in thread
end of thread, other threads:[~2016-06-21 23:05 UTC | newest]
Thread overview: 25+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-06-17 3:33 bug#23779: 25.0.95; consing "SHELLVAR" onto process-environment doesn't remove it from subprocess env Noam Postavsky
2016-06-17 7:11 ` Eli Zaretskii
2016-06-17 12:17 ` Dmitry Gutov
2016-06-17 14:01 ` Eli Zaretskii
2016-06-17 14:12 ` Dmitry Gutov
2016-06-17 14:19 ` Eli Zaretskii
2016-06-17 14:47 ` Dmitry Gutov
2016-06-17 16:52 ` Andreas Schwab
2016-06-17 16:55 ` Dmitry Gutov
2016-06-17 17:06 ` Eli Zaretskii
2016-06-17 19:01 ` Dmitry Gutov
2016-06-17 20:10 ` Eli Zaretskii
2016-06-17 21:26 ` Dmitry Gutov
2016-06-18 7:51 ` Eli Zaretskii
2016-06-18 1:36 ` Noam Postavsky
2016-06-18 1:44 ` Dmitry Gutov
2016-06-19 2:27 ` Paul Eggert
2016-06-19 15:01 ` Eli Zaretskii
2016-06-19 22:53 ` Paul Eggert
2016-06-20 14:21 ` Eli Zaretskii
2016-06-21 13:53 ` Dmitry Gutov
2016-06-21 15:21 ` Eli Zaretskii
2016-06-21 15:24 ` Dmitry Gutov
2016-06-21 16:12 ` Eli Zaretskii
2016-06-21 23:05 ` Dmitry Gutov
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.