Un-setting HGRCPATH in vc-hg.el problematic

Un-setting HGRCPATH in vc-hg.el problematic

[Sascha Wilde]

Hi*,

in vc-hg-state and vc-hg-working-revision the hg binary is run with
HGRCPATH set to an empty string in its environment.  What is the
rational of doing so?  The comment states "Avoid localization of
messages so we can parse the output." but to my best knowledge the hgrc
files are not used to set the localization...

The problem with this is, that in general the hgrc files are used by
users/administrators to make configurations they actually want to use --
disabling this configurations unconditionally will be a source of
endless surprise and misbehavior.

I stumbled about it, when I suddenly noticed, that I got "Not trusting
file /foo/bar/stuff/.hg/hgrc from untrusted user bob, group users
Not trusting file /foo/bar/stuff/.hg/hgrc from untrusted user bob, group users
156" instead of the revision number in my mode line and was no longer
able to check in the altered file with C-v C-v.

The user bob is configured as trusted in our hg setup so this should not
happen -- but with HGRCPATH='' our setup is simply ignored -- that's
bad.

I'd suggest removing the HGRCPATH='' from the environment.  If there
are really configuration options that should be disabled for good
reasons in this functions, I'd suggest using the --config option of hg
to set them specifically.

cheers
sascha
-- 
Sascha Wilde

"Unix was the first OS where you could carry the media and system
documentation around in a briefcase. This was fixed in BSD4.2."

Re: Un-setting HGRCPATH in vc-hg.el problematic

[Dan Nicolaescu]

Sascha Wilde <wilde@sha-bang.de> writes:

> Hi*,
>
> in vc-hg-state and vc-hg-working-revision the hg binary is run with
> HGRCPATH set to an empty string in its environment.  What is the
> rational of doing so?  The comment states "Avoid localization of
> messages so we can parse the output." but to my best knowledge the hgrc
> files are not used to set the localization...
>
> The problem with this is, that in general the hgrc files are used by
> users/administrators to make configurations they actually want to use --
> disabling this configurations unconditionally will be a source of
> endless surprise and misbehavior.
>
> I stumbled about it, when I suddenly noticed, that I got "Not trusting
> file /foo/bar/stuff/.hg/hgrc from untrusted user bob, group users
> Not trusting file /foo/bar/stuff/.hg/hgrc from untrusted user bob, group users
> 156" instead of the revision number in my mode line and was no longer
> able to check in the altered file with C-v C-v.
>
> The user bob is configured as trusted in our hg setup so this should not
> happen -- but with HGRCPATH='' our setup is simply ignored -- that's
> bad.

This message is probably better sent to bug-gnu-emacs@gnu.org

> I'd suggest removing the HGRCPATH='' from the environment.  If there
> are really configuration options that should be disabled for good
> reasons in this functions, I'd suggest using the --config option of hg
> to set them specifically.

If you can suggest exactly what --config options to use, it would be
very easy to make that change.

Re: Un-setting HGRCPATH in vc-hg.el problematic

[Sascha Wilde]

Dan Nicolaescu <dann@gnu.org> wrote:
> Sascha Wilde <wilde@sha-bang.de> writes:
>> HGRCPATH set to an empty string in its environment.  What is the
>> rational of doing so?  The comment states "Avoid localization of
>> messages so we can parse the output." but to my best knowledge the hgrc
>> files are not used to set the localization...
[...]
> This message is probably better sent to bug-gnu-emacs@gnu.org

As I intended to discuss the rational behind the code and find a more
correct solution I thought the developer list would fit...

>> I'd suggest removing the HGRCPATH='' from the environment.  If there
>> are really configuration options that should be disabled for good
>> reasons in this functions, I'd suggest using the --config option of hg
>> to set them specifically.
>
> If you can suggest exactly what --config options to use, it would be
> very easy to make that change.

I can't make a suggestion like this as long as I don't know what
the intention of setting HGRCPATH='' was in the first place.  As stated
above the comment is misleading as hgrc is not used for any kind of
localization (I wrote "to my best knowledge" but I double checked with
an core Mercurial developer in the meantime).

So at the moment it looks as if the HGRCPATH='' can be simply removed!

But there might be some other reasons (Bug#5846 seems to indicate that)
besides the localization issue for trying to deactivate .hgrc files...

Maybe Chong Yidong knows some details, as he made the last changes to
the code in question?

cheers
ssacha
-- 
Sascha Wilde
Nota bene: wenn Word für Längeres geeignet wäre, würde es schließlich
nicht Word, sondern Sentence, Page oder Article heißen
 	-- Matthias Mühlich in dctt

Re: Un-setting HGRCPATH in vc-hg.el problematic

[Dan Nicolaescu]

Sascha Wilde <wilde@sha-bang.de> writes:

> Dan Nicolaescu <dann@gnu.org> wrote:
>> Sascha Wilde <wilde@sha-bang.de> writes:
>>> HGRCPATH set to an empty string in its environment.  What is the
>>> rational of doing so?  The comment states "Avoid localization of
>>> messages so we can parse the output." but to my best knowledge the hgrc
>>> files are not used to set the localization...
> [...]
>> This message is probably better sent to bug-gnu-emacs@gnu.org
>
> As I intended to discuss the rational behind the code and find a more
> correct solution I thought the developer list would fit...

Filing a bug will make sure the issue is not forgotten, and it also
implies that more info is needed so that the issue can be
independently reproduced.

>>> I'd suggest removing the HGRCPATH='' from the environment.  If there
>>> are really configuration options that should be disabled for good
>>> reasons in this functions, I'd suggest using the --config option of hg
>>> to set them specifically.
>>
>> If you can suggest exactly what --config options to use, it would be
>> very easy to make that change.
>
> I can't make a suggestion like this as long as I don't know what
> the intention of setting HGRCPATH='' was in the first place.  As stated
> above the comment is misleading as hgrc is not used for any kind of
> localization (I wrote "to my best knowledge" but I double checked with
> an core Mercurial developer in the meantime).

Setting HGRCPATH was the result of some discussion on the list (here or
the bug list).

We want to be able to reliably parse the results of hg log regardless
of the users .hgrc settings.

> So at the moment it looks as if the HGRCPATH='' can be simply removed!

What happens if the user changes the format of hg log in her .hgrc ? 

> But there might be some other reasons (Bug#5846 seems to indicate that)
> besides the localization issue for trying to deactivate .hgrc files...
>
> Maybe Chong Yidong knows some details, as he made the last changes to
> the code in question?
>
> cheers
> ssacha
> -- 
> Sascha Wilde
> Nota bene: wenn Word für Längeres geeignet wäre, würde es schließlich
> nicht Word, sondern Sentence, Page oder Article heißen
>  	-- Matthias Mühlich in dctt

Re: Un-setting HGRCPATH in vc-hg.el problematic

[Chong Yidong]

Sascha Wilde <wilde@sha-bang.de> writes:

> I can't make a suggestion like this as long as I don't know what
> the intention of setting HGRCPATH='' was in the first place.  As stated
> above the comment is misleading as hgrc is not used for any kind of
> localization (I wrote "to my best knowledge" but I double checked with
> an core Mercurial developer in the meantime).
>
> So at the moment it looks as if the HGRCPATH='' can be simply removed!
>
> But there might be some other reasons (Bug#5846 seems to indicate that)
> besides the localization issue for trying to deactivate .hgrc files...
>
> Maybe Chong Yidong knows some details, as he made the last changes to
> the code in question?

See Bug#5960.

Re: Un-setting HGRCPATH in vc-hg.el problematic

[Sascha Wilde]

[-- Attachment #1: Type: text/plain, Size: 1881 bytes --]

Hi *,

sorry for the extreme late reply...

Anyway, I created a patch which hopefully resolves my issues while not
reintroducing any of the old ones.

Dan Nicolaescu <dann@gnu.org> wrote:
> Sascha Wilde <wilde@sha-bang.de> writes:
[...]
> Setting HGRCPATH was the result of some discussion on the list (here or
> the bug list).
>
> We want to be able to reliably parse the results of hg log regardless
> of the users .hgrc settings.

In case my attached patch (re)creates any of the problems discussed back
then I would be grateful for a pointer to the thread in question.

>> So at the moment it looks as if the HGRCPATH='' can be simply removed!
>
> What happens if the user changes the format of hg log in her .hgrc ? 

I see only two possible ways how this could happen: alias definitions
(also redefining the original command name is discouraged) and default
definitions (currently repriced in favor for aliases).

My patch addresses them both...

>> But there might be some other reasons (Bug#5846 seems to indicate that)
>> besides the localization issue for trying to deactivate .hgrc files...

As said before: localization is orthogonal to hgrc.

And the problem described in the second part of Bug#5846 was really due
to the -- meanwhile fixed -- use of an abbreviated hg  command in
vc-hg-working-revision.

So all problems reported in Bug#5846 should stay fixed with my proposed
patch. 

Chong Yidong <cyd@stupidchicken.com> wrote:
> Sascha Wilde <wilde@sha-bang.de> writes:
>> Maybe Chong Yidong knows some details, as he made the last changes to
>> the code in question?
>
> See Bug#5960.

Sorry, but neither does Bug#5960 mention hgrc nor does the related patch
include the hgrc related part.

So here is my patch, I successfully tested it with an "evil" hgrc
redefining 'log' and 'parents'.  It would be great if it could be
committed to get some wider testing...


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: emacs-vc-hg-hgrc.patch --]
[-- Type: text/x-patch, Size: 2184 bytes --]

changeset:   109607:955b39955186
branch:      wilde
tag:         tip
user:        Sascha Wilde <wilde@sha-bang.de>
date:        Sun Aug 01 17:27:24 2010 +0200
files:       lisp/ChangeLog lisp/vc/vc-hg.el
description:
Replaced setting HGRCPATH="" with selective --config options.


diff -r 4af5866a5568 -r 955b39955186 lisp/ChangeLog
--- a/lisp/ChangeLog	Sun Aug 01 06:37:41 2010 +0200
+++ b/lisp/ChangeLog	Sun Aug 01 17:27:24 2010 +0200
@@ -1,3 +1,8 @@
+2010-08-01  Sascha Wilde  <wilde@sha-bang.de>
+
+	* vc/vc-hg.el (vc-hg-state,vc-hg-working-revision): Replaced
+	setting HGRCPATH to "" by some less invasive --config options.
+
 2010-08-01  Chong Yidong  <cyd@stupidchicken.com>
 
 	* emacs-lisp/package.el (package--list-packages): Fix column
diff -r 4af5866a5568 -r 955b39955186 lisp/vc/vc-hg.el
--- a/lisp/vc/vc-hg.el	Sun Aug 01 06:37:41 2010 +0200
+++ b/lisp/vc/vc-hg.el	Sun Aug 01 17:27:24 2010 +0200
@@ -170,10 +170,12 @@
 		      (let ((process-environment
 			     ;; Avoid localization of messages so we
 			     ;; can parse the output.
-			     (append (list "TERM=dumb" "LANGUAGE=C" "HGRCPATH=")
+			     (append (list "TERM=dumb" "LANGUAGE=C")
 				     process-environment)))
 			(process-file
 			 "hg" nil t nil
+			 "--config" "alias.status=status"
+			 "--config" "defaults.status="
 			 "status" "-A" (file-relative-name file)))
                     ;; Some problem happened.  E.g. We can't find an `hg'
                     ;; executable.
@@ -198,7 +200,7 @@
       ((status nil)
        (default-directory (file-name-directory file))
        ;; Avoid localization of messages so we can parse the output.
-       (avoid-local-env (append (list "TERM=dumb" "LANGUAGE=C" "HGRCPATH=")
+       (avoid-local-env (append (list "TERM=dumb" "LANGUAGE=C")
 				     process-environment))
        (out
         (with-output-to-string
@@ -210,6 +212,8 @@
 			;; Ignore all errors.
 			(process-file
 			 "hg" nil t nil
+			 "--config" "alias.parents=parents"
+			 "--config" "defaults.parents="
 			 "parents" "--template" "{rev}" (file-relative-name file)))
                     ;; Some problem happened.  E.g. We can't find an `hg'
                     ;; executable.


[-- Attachment #3: Type: text/plain, Size: 173 bytes --]


cheers
sascha
-- 
Sascha Wilde
Wer HTML postet oder gepostetes HTML quotet oder sich gepostetes oder
gequotetes HTML beschafft, um es in Verkehr zu bringen, wird geplonkt.

Re: Un-setting HGRCPATH in vc-hg.el problematic

[Dan Nicolaescu]

Sascha Wilde <wilde@sha-bang.de> writes:

> Hi *,
>
> sorry for the extreme late reply...
>
> Anyway, I created a patch which hopefully resolves my issues while not
> reintroducing any of the old ones.
>
> Dan Nicolaescu <dann@gnu.org> wrote:
>> Sascha Wilde <wilde@sha-bang.de> writes:
> [...]
>> Setting HGRCPATH was the result of some discussion on the list (here or
>> the bug list).
>>
>> We want to be able to reliably parse the results of hg log regardless
>> of the users .hgrc settings.
>
> In case my attached patch (re)creates any of the problems discussed back
> then I would be grateful for a pointer to the thread in question.
>
>>> So at the moment it looks as if the HGRCPATH='' can be simply removed!
>>
>> What happens if the user changes the format of hg log in her .hgrc ? 
>
> I see only two possible ways how this could happen: alias definitions
> (also redefining the original command name is discouraged) and default
> definitions (currently repriced in favor for aliases).
>
> My patch addresses them both...

> Sorry, but neither does Bug#5960 mention hgrc nor does the related patch
> include the hgrc related part.

It might have been another bug...


> So here is my patch, I successfully tested it with an "evil" hgrc
> redefining 'log' and 'parents'.  

How about testing with an hgrc that changes the defaults for log and parents?

Re: Un-setting HGRCPATH in vc-hg.el problematic

[Sascha Wilde]

Dan Nicolaescu <dann@gnu.org> wrote:
> Sascha Wilde <wilde@sha-bang.de> writes:
[...]
>> So here is my patch, I successfully tested it with an "evil" hgrc
>> redefining 'log' and 'parents'.  
>
> How about testing with an hgrc that changes the defaults for log and parents?

Sorry, I should have been more precise about the tests I made, but I
did test this, too.

As I wrote (and one can see from my patch) both: redefined (alias)
commands and changed defaults for commands are reseted to the expected
defaults by my proposed change.  I successfully tested both cases.

cheers
sascha
-- 
Sascha Wilde  :  "I heard that if you play the Windows CD backward, you
              :  get a satanic message. But that's nothing compared to
              :  when you play it forward: It installs Windows...." 
              :  -- G. R. Gaudreau

Re: Un-setting HGRCPATH in vc-hg.el problematic

[Dan Nicolaescu]

Sascha Wilde <wilde@sha-bang.de> writes:

> Dan Nicolaescu <dann@gnu.org> wrote:
>> Sascha Wilde <wilde@sha-bang.de> writes:
> [...]
>>> So here is my patch, I successfully tested it with an "evil" hgrc
>>> redefining 'log' and 'parents'.  
>>
>> How about testing with an hgrc that changes the defaults for log and parents?
>
> Sorry, I should have been more precise about the tests I made, but I
> did test this, too.
>
> As I wrote (and one can see from my patch) both: redefined (alias)
> commands and changed defaults for commands are reseted to the expected
> defaults by my proposed change.  I successfully tested both cases.

In that case your patch looks ready to be checked in.

Re: Un-setting HGRCPATH in vc-hg.el problematic

[Sascha Wilde]

Dan Nicolaescu <dann@gnu.org> wrote:
> Sascha Wilde <wilde@sha-bang.de> writes:
>> Dan Nicolaescu <dann@gnu.org> wrote:
>>> Sascha Wilde <wilde@sha-bang.de> writes:
>> [...]
>>>> So here is my patch, I successfully tested it with an "evil" hgrc
>>>> redefining 'log' and 'parents'.  
>>>
>>> How about testing with an hgrc that changes the defaults for log and parents?
>>
>> Sorry, I should have been more precise about the tests I made, but I
>> did test this, too.
>>
>> As I wrote (and one can see from my patch) both: redefined (alias)
>> commands and changed defaults for commands are reseted to the expected
>> defaults by my proposed change.  I successfully tested both cases.
>
> In that case your patch looks ready to be checked in.

I'm happy you agree -- I would be grateful if someone with commit rights
could do so...

cheers
sascha
-- 
Sascha Wilde 
"If you were young again, would you start writing TeX again or would
you use Microsoft Word, or another word processor?" - "I hope to die
before I *have* to use Microsoft Word."    --  Prof. Donald E. Knuth

Re: Un-setting HGRCPATH in vc-hg.el problematic

[Sascha Wilde]

Sascha Wilde <wilde@sha-bang.de> wrote:
> Dan Nicolaescu <dann@gnu.org> wrote:
>> Sascha Wilde <wilde@sha-bang.de> writes:
>>> Dan Nicolaescu <dann@gnu.org> wrote:
>>>> Sascha Wilde <wilde@sha-bang.de> writes:
>>> [...]
>>>>> So here is my patch, I successfully tested it with an "evil" hgrc
>>>>> redefining 'log' and 'parents'.  
>>>>
>>>> How about testing with an hgrc that changes the defaults for log and parents?
>>>
>>> Sorry, I should have been more precise about the tests I made, but I
>>> did test this, too.
>>>
>>> As I wrote (and one can see from my patch) both: redefined (alias)
>>> commands and changed defaults for commands are reseted to the expected
>>> defaults by my proposed change.  I successfully tested both cases.
>>
>> In that case your patch looks ready to be checked in.
>
> I'm happy you agree -- I would be grateful if someone with commit rights
> could do so...

ping

sascha
-- 
Sascha Wilde
Nota bene: wenn Word für Längeres geeignet wäre, würde es schließlich
nicht Word, sondern Sentence, Page oder Article heißen
 	-- Matthias Mühlich in dctt

Re: Un-setting HGRCPATH in vc-hg.el problematic

[Stefan Monnier]

> ping

pong (on branch emacs-23)


        Stefan "thanks and sorry for the delay"

Re: Un-setting HGRCPATH in vc-hg.el problematic

[Sascha Wilde]

Stefan Monnier <monnier@iro.umontreal.ca> wrote:
>> ping
>
> pong (on branch emacs-23)

Thank you.

cheers
sascha
-- 
Sascha Wilde : "Ist es nicht schon schlimm genug, dass ICH hier rumtrolle?"
             : (Henning Leise in d.o.c.)