[PATCH 0/6] test: generic fixes

[Felipe Contreras]

Hi,

I already sent some of these, but others are new. I'm mainly interested in the
first one, because it's clearly breaking Mercurial because the .hgrc is not
parsed.

FWIW. These are the results with remote-hg:

fixed 3
success 70
failed 0
broken 4
total 77

Felipe Contreras (6):
test: remove empty HGRCPATH
test: fix end of lines
test: trivial whitespace cleanups
test: trivial simplifications
test: turn on debugging only when it can be used
test: fix 'push not create bookmark'

test/test-lib.sh | 16 ++++++++--------
test/test_anonymous_branches.t | 2 +-
test/test_author.t | 2 +-
test/test_bookmarks.t | 2 +-
test/test_notes.t | 2 +-
test/test_pull.t | 2 +-
test/test_push.t | 4 ++--
test/test_push_tags.t | 2 +-
test/test_special_cases.t | 2 +-
9 files changed, 17 insertions(+), 17 deletions(-)

--
1.8.3.rc1.579.g184e698

[Felipe Contreras]

Extensions can't interfere, because we need to enable them in our .hgrc.

Signed-off-by: Felipe Contreras <felipe.c...@gmail.com>
---
test/test-lib.sh | 1 -
1 file changed, 1 deletion(-)

diff --git a/test/test-lib.sh b/test/test-lib.sh
index 856f198..ba21ee9 100644
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -8,7 +8,6 @@ export GIT_USER="$GIT_AUTHOR_NAME <$GIT_AUTHOR_EMAIL>"
export HG_USER="Hg User <hg....@example.com>"
export DEBUG_GITIFYHG=on
export GIT_PAGER=cat
-export HGRCPATH='' # So extensions like pager don't interfere
export NL='
'

--
1.8.3.rc1.579.g184e698

[Felipe Contreras]

Somebody's text editor has been very naughty.

Signed-off-by: Felipe Contreras <felipe.c...@gmail.com>
---

test/test-lib.sh | 2 +-

test/test_anonymous_branches.t | 2 +-
test/test_author.t | 2 +-
test/test_bookmarks.t | 2 +-
test/test_notes.t | 2 +-
test/test_pull.t | 2 +-

test/test_push_tags.t | 2 +-
test/test_special_cases.t | 2 +-

8 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/test/test-lib.sh b/test/test-lib.sh
index ba21ee9..4cb1063 100644
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -106,4 +106,4 @@ assert_git_notes() {
echo $1
test "`git log --pretty="format:%N" --notes='hg' | grep -v '^$'`" = "$1"

-}
\ No newline at end of file
+}
diff --git a/test/test_anonymous_branches.t b/test/test_anonymous_branches.t
index 62f1098..09f23d1 100755
--- a/test/test_anonymous_branches.t
+++ b/test/test_anonymous_branches.t
@@ -82,4 +82,4 @@ test_expect_failure 'pull from anonymous branch' '
cd ..
'

-test_done
\ No newline at end of file
+test_done
diff --git a/test/test_author.t b/test/test_author.t
index 3e7a4cb..7126b2c 100755
--- a/test/test_author.t
+++ b/test/test_author.t
@@ -147,4 +147,4 @@ test_expect_success 'author abuse quotes' '
'


-test_done
\ No newline at end of file
+test_done
diff --git a/test/test_bookmarks.t b/test/test_bookmarks.t
index 9e51b26..0a5df0d 100755
--- a/test/test_bookmarks.t
+++ b/test/test_bookmarks.t
@@ -197,4 +197,4 @@ test_expect_failure 'pull_from_bookmark' '
cd ..
'

-test_done
\ No newline at end of file
+test_done
diff --git a/test/test_notes.t b/test/test_notes.t
index b11f85d..5319a5f 100755
--- a/test/test_notes.t
+++ b/test/test_notes.t
@@ -114,4 +114,4 @@ test_expect_success 'simple push updates after pull' '
cd ..
'

-test_done
\ No newline at end of file
+test_done
diff --git a/test/test_pull.t b/test/test_pull.t
index 355f4d8..06347d5 100755
--- a/test/test_pull.t
+++ b/test/test_pull.t
@@ -116,4 +116,4 @@ test_expect_success 'pull tags' '
cd ..
'

-test_done
\ No newline at end of file
+test_done
diff --git a/test/test_push_tags.t b/test/test_push_tags.t
index cb69913..6ec6833 100755
--- a/test/test_push_tags.t
+++ b/test/test_push_tags.t
@@ -142,4 +142,4 @@ test_expect_success 'push only new tag' '



-test_done
\ No newline at end of file
+test_done
diff --git a/test/test_special_cases.t b/test/test_special_cases.t
index 47e5c2c..72ec040 100755
--- a/test/test_special_cases.t
+++ b/test/test_special_cases.t
@@ -118,4 +118,4 @@ test_expect_success 'symlinks' '
test_done


-here
\ No newline at end of file
+here
--
1.8.3.rc1.579.g184e698

[Felipe Contreras]

Signed-off-by: Felipe Contreras <felipe.c...@gmail.com>
---

test/test-lib.sh | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/test/test-lib.sh b/test/test-lib.sh
index 4cb1063..8c5fbc4 100644
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -44,6 +44,7 @@ make_hg_commit() {
hg add $2 &&
hg commit -m "$1" --user="$user"
}
+
make_git_commit() {
echo "$1" >> "$2" &&
git add "$2" &&
@@ -83,6 +84,7 @@ assert_git_author() {
fi
test "`git show -s --format='%an <%ae>' $ref`" = "$1"
}
+
assert_git_count() {
if test $# -eq 2 ; then
ref=$2
@@ -91,6 +93,7 @@ assert_git_count() {
fi
test `git rev-list $ref --count` -eq $1
}
+
assert_hg_count() {
if test $# -eq 2 ; then
rev=$2
@@ -100,10 +103,10 @@ assert_hg_count() {
test `hg log -q -r 0:$rev | wc -l` -eq $1

}
+
assert_git_notes() {
git notes --ref=hg merge $(basename $(ls .git/refs/notes/hg-*)) &&

git log --pretty="format:%N" --notes='hg' | grep -v '^$'
echo $1
test "`git log --pretty="format:%N" --notes='hg' | grep -v '^$'`" = "$1"
-
}

--
1.8.3.rc1.579.g184e698

[Felipe Contreras]

Signed-off-by: Felipe Contreras <felipe.c...@gmail.com>
---

test/test-lib.sh | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/test/test-lib.sh b/test/test-lib.sh
index 8c5fbc4..759f672 100644
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -8,13 +8,11 @@ export GIT_USER="$GIT_AUTHOR_NAME <$GIT_AUTHOR_EMAIL>"

export HG_USER="Hg User <hg....@example.com>"
export DEBUG_GITIFYHG=on
export GIT_PAGER=cat

-export NL='
-'
+export NL=$'\n'

make_hg_repo() {
- mkdir hg_repo &&
+ hg init hg_repo &&
cd hg_repo &&
- hg init &&
echo 'a\n' >> test_file &&
hg add test_file &&
hg commit --message="a" --user="$HG_USER"
--
1.8.3.rc1.579.g184e698

[Felipe Contreras]

Signed-off-by: Felipe Contreras <felipe.c...@gmail.com>
---

test/test-lib.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/test-lib.sh b/test/test-lib.sh
index 759f672..6429912 100644
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -6,7 +6,7 @@ export GIT_AUTHOR_EMAIL=git....@example.com
export GIT_AUTHOR_NAME='Git User'

export GIT_USER="$GIT_AUTHOR_NAME <$GIT_AUTHOR_EMAIL>"
export HG_USER="Hg User <hg....@example.com>"

-export DEBUG_GITIFYHG=on
+export DEBUG_GITIFYHG=$debug
export GIT_PAGER=cat
export NL=$'\n'

--
1.8.3.rc1.579.g184e698

[Felipe Contreras]

Signed-off-by: Felipe Contreras <felipe.c...@gmail.com>
---

test/test_push.t | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/test/test_push.t b/test/test_push.t
index 0133eca..a1320bd 100755
--- a/test/test_push.t
+++ b/test/test_push.t
@@ -30,7 +30,7 @@ test_expect_success 'simple push from master' '
cd ..
'

-test_expect_failure 'push not create bookmark' '
+test_expect_success 'push not create bookmark' '
test_when_finished "rm -rf hg_repo git_clone" &&
make_hg_repo &&
clone_repo &&
@@ -38,7 +38,7 @@ test_expect_failure 'push not create bookmark' '
git push &&
cd ../hg_repo &&

- test `hg bookmarks` = "no bookmarks set" &&
+ test "`hg bookmarks`" = "no bookmarks set" &&

cd ..
'
--
1.8.3.rc1.579.g184e698

[Jed Brown]

Felipe Contreras <felipe.c...@gmail.com> writes:

> Extensions can't interfere, because we need to enable them in our .hgrc.

Is the intent here to unsure that the test suite tests all variability
in user configuration (in contrast to being reproducible on its own)?
What happens when people enable extensions that change the behavior of
basic commands? Allowing this variability makes it more difficult to
reproduce reported problems because the bug report now needs to include
the versions of all extensions. Your commit effectively reverts Dusty's
below, so it would help to explain how his logic is flawed.

commit 468a4fe8c46f7fb6f571e1902b91b1ce66b831ea
Author: Dusty Phillips <du...@linux.ca>
Date: Sat Feb 9 15:25:18 2013 -0700

set HGRCPATH='' in both tests and remote.

This disallows loading of system or the user's personal hg config files, such as ~/.hgrc.
The main motivator for this is that extensions the user has loaded can cause dissatisfaction
for both scripts and tests. The script will complain bitterly, but will not communicate
clearly if it encounters an extension doing unexpected things -- unexpected by the script,
that is. We can assume the extension thinks it knows what it is doing.

All tests pass with this change, but I am marginally concerned about side effects in which
things like not having the user's credentials set could cause explosions.

diff --git a/gitifyhg.py b/gitifyhg.py
index 024119a..f47fccf 100644
--- a/gitifyhg.py
+++ b/gitifyhg.py
@@ -31,6 +31,9 @@ from time import strftime
# Enable "plain" mode to make us resilient against changes to the locale, as we
# rely on parsing certain messages produced by Mercurial. See issue #26.
os.environ['HGPLAIN'] = '1'
+# Disable loading of the user's $HOME/.hgrc as extensions can cause weird
+# interactions and it's better to run in a known state.
+os.environ['HGRCPATH'] = ''


from mercurial.ui import ui
diff --git a/test/conftest.py b/test/conftest.py
index 51b14c0..7eaedaa 100644
--- a/test/conftest.py
+++ b/test/conftest.py
@@ -21,6 +21,7 @@ def hg_repo(tmpdir):
an initialized hg repository with a single commit'''
os.environ['DEBUG_GITIFYHG'] = "on"
os.environ['GIT_PAGER'] = 'cat'
+ os.environ['HGRCPATH'] = '' # So extensions like pager don't interfere
tmpdir = p(tmpdir.strpath).abspath()
hg_base = tmpdir.joinpath('hg_base') # an hg repo to clone from
hg_base.mkdir()

[Felipe Contreras]

On Mon, May 13, 2013 at 7:53 AM, Jed Brown <j...@59a2.org> wrote:
> Felipe Contreras <felipe.c...@gmail.com> writes:
>
>> Extensions can't interfere, because we need to enable them in our .hgrc.
>
> Is the intent here to unsure that the test suite tests all variability
> in user configuration (in contrast to being reproducible on its own)?

I know what is the intent, but the intent is wrong.

> What happens when people enable extensions that change the behavior of
> basic commands?

Nothing would happen, I thought I made that clear; you need to enable
them in your .hgrc, and $HOME/.hgrc is empty.

> Allowing this variability makes it more difficult to
> reproduce reported problems because the bug report now needs to include
> the versions of all extensions. Your commit effectively reverts Dusty's
> below, so it would help to explain how his logic is flawed.
>
> commit 468a4fe8c46f7fb6f571e1902b91b1ce66b831ea
> Author: Dusty Phillips <du...@linux.ca>
> Date: Sat Feb 9 15:25:18 2013 -0700
>
> set HGRCPATH='' in both tests and remote.
>
> This disallows loading of system or the user's personal hg config files, such as ~/.hgrc.
> The main motivator for this is that extensions the user has loaded can cause dissatisfaction
> for both scripts and tests. The script will complain bitterly, but will not communicate
> clearly if it encounters an extension doing unexpected things -- unexpected by the script,
> that is. We can assume the extension thinks it knows what it is doing.
>
> All tests pass with this change, but I am marginally concerned about side effects in which
> things like not having the user's credentials set could cause explosions.
>
> diff --git a/gitifyhg.py b/gitifyhg.py

Here's the important part: it's for pytest, not for sharness. In
sharness everything works without that because it sets it's own $HOME,
and AFAIK all distributions ship with an empty system hgrc.

See:

% hg showconfig
extensions.hgext.schemes=
extensions.hgext.bookmarks=
extensions.hgext.purge=
extensions.hgk=
extensions.mq=
extensions.hggit=
extensions.graphlog=
system.test=true
ui.username=Felipe Contreras <felipe.c...@gmail.com>
web.cacerts=/etc/ssl/certs/ca-certificates.crt

---
#!/bin/sh

test_description='Test config'

. ./test-lib.sh

test_expect_success 'test config' 'hg showconfig'

(
echo "[ui]"
echo "username = H G Wells <we...@example.com>"
) >> "$HOME"/.hgrc

test_expect_success 'test config 2' 'hg showconfig'

export HGRCPATH="$HOME"/.hgrc

test_expect_success 'test config 3' 'hg showconfig'

export HGRCPATH=''

test_expect_success 'test config 4' 'hg showconfig'

test_done
---

% make test_config.t
*** test_config.t ***
expecting success: hg showconfig
system.test=true
ok 1 - test config

expecting success: hg showconfig
system.test=true
ui.username=H G Wells <we...@example.com>
ok 2 - test config 2

expecting success: hg showconfig

ui.username=H G Wells <we...@example.com>
ok 3 - test config 3

expecting success: hg showconfig
ok 4 - test config 4

# passed all 4 test(s)

I think it's pretty stupid to disable hgrc, when in fact the code does
rely on hgrc, but has it's own clunky parsing. Also, some extensions
are useful like 'schemes', to do for example "git clone
hg::bb:://felipec/test".

I for one would like to see a single extension that causes any problem.

Moreover, as user, I would expect expect "HGRCPATH=/tmp/myhgrc && git
command" to do the right thing, but with gitifyhg it won't: it will
*always* use the hard-coded value of ~/.hgrc, regardless of what the
user specifies. And for what gain?

Cheers.

--
Felipe Contreras

[Dusty Phillips]

Would you mind submitting and discussing these as github pull requests?
I don't have a pipeline in my inbox to organize what has been discussed
or addressed previously and it reduces cognitive dissonance for me as an
integrator.

I will try and review/integrate these ASAP, but the honest truth is I
don't see any time in my schedule until mid-June. I thought May would
free up, but some things have come up in my personal life.

Per the HGRC question, it *should* not be an issue either way because
gitifyhg explicitly disables hg extensions. However, it would be ideal
to test and assert that this is actually happening.

Cheers,

Dusty

[Jed Brown]

Felipe Contreras <felipe.c...@gmail.com> writes:

> On Mon, May 13, 2013 at 7:53 AM, Jed Brown <j...@59a2.org> wrote:
>
>> What happens when people enable extensions that change the behavior of
>> basic commands?
>
> Nothing would happen, I thought I made that clear; you need to enable
> them in your .hgrc, and $HOME/.hgrc is empty.

[...]

> Here's the important part: it's for pytest, not for sharness. In
> sharness everything works without that because it sets it's own $HOME,
> and AFAIK all distributions ship with an empty system hgrc.

So the claim is that it's unnecessary because sharness already goes
further by changing $HOME? If so, this would make a good commit
message. As is, all tests

Note that gitifyhg.py sets HGRCPATH too, but it is called without
sharness, so should it remain? From your statements below, maybe that
should also be changed?

> I think it's pretty stupid to disable hgrc, when in fact the code does
> rely on hgrc, but has it's own clunky parsing. Also, some extensions
> are useful like 'schemes', to do for example "git clone
> hg::bb:://felipec/test".

Indeed, this is a tangible benefit.

> I for one would like to see a single extension that causes any problem.

I recall that pager (and maybe color) were causing problems with
py.test. Presumably largefiles would cause real problems, but it's not
clear what the desired behavior would be in that case. Aliases can
modify standard commands, so this would break the test suite.

[alias]
log = log --limit 1

> Moreover, as user, I would expect expect "HGRCPATH=/tmp/myhgrc && git
> command" to do the right thing, but with gitifyhg it won't: it will
> *always* use the hard-coded value of ~/.hgrc, regardless of what the
> user specifies. And for what gain?

Reproducibility and reliability? I don't have a strong opinion either
way, but some explanation is warranted.

Your preamble said "clearly breaking Mercurial", but I haven't seen any
breakage, so perhaps you can be more specific?

[Felipe Contreras]

On Mon, May 13, 2013 at 9:43 AM, Jed Brown <j...@59a2.org> wrote:
> Felipe Contreras <felipe.c...@gmail.com> writes:
>
>> On Mon, May 13, 2013 at 7:53 AM, Jed Brown <j...@59a2.org> wrote:
>>
>>> What happens when people enable extensions that change the behavior of
>>> basic commands?
>>
>> Nothing would happen, I thought I made that clear; you need to enable
>> them in your .hgrc, and $HOME/.hgrc is empty.
>
> [...]
>
>> Here's the important part: it's for pytest, not for sharness. In
>> sharness everything works without that because it sets it's own $HOME,
>> and AFAIK all distributions ship with an empty system hgrc.
>
> So the claim is that it's unnecessary because sharness already goes
> further by changing $HOME? If so, this would make a good commit
> message. As is, all tests

Maybe, I just assumed everyone knew the basics of the test framework.

> Note that gitifyhg.py sets HGRCPATH too, but it is called without
> sharness, so should it remain? From your statements below, maybe that
> should also be changed?

Yes, I think it should, but I'm far less interested in gitifyhg.py.

>> I think it's pretty stupid to disable hgrc, when in fact the code does
>> rely on hgrc, but has it's own clunky parsing. Also, some extensions
>> are useful like 'schemes', to do for example "git clone
>> hg::bb:://felipec/test".
>
> Indeed, this is a tangible benefit.

And I just found another: mercurial's keyring extension, which now
works correctly in remote-hg.

>> I for one would like to see a single extension that causes any problem.
>
> I recall that pager (and maybe color) were causing problems with
> py.test. Presumably largefiles would cause real problems, but it's not
> clear what the desired behavior would be in that case. Aliases can
> modify standard commands, so this would break the test suite.

I just enabled the pager extension, I don't see any problem.

> [alias]
> log = log --limit 1

I'm not talking about the test-suite, the test suite has an empty .hgrc.

>> Moreover, as user, I would expect expect "HGRCPATH=/tmp/myhgrc && git
>> command" to do the right thing, but with gitifyhg it won't: it will
>> *always* use the hard-coded value of ~/.hgrc, regardless of what the
>> user specifies. And for what gain?
>
> Reproducibility and reliability?

Is that a question? Then the answer is 'no'; an empty HGRCPATH doesn't
bring any more reproducibility and reliability.

> Your preamble said "clearly breaking Mercurial", but I haven't seen any
> breakage, so perhaps you can be more specific?

Try the following script:

With gitifyhg:

To testgitifyhg::/tmp/test-hg
* [new tag] v0.1 -> v0.1
o changeset: 1:bc4a9f9cde7e
| tag: tip
| user: Felipe Contreras <felipe.c...@gmail.com>
| date: Tue May 14 16:28:37 2013 -0500
| summary: Added tag v0.1 for changeset 2991fcd3c1e2
|
@ changeset: 0:2991fcd3c1e2
tag: v0.1
user: Username <iw...@this.org>
date: Tue May 14 16:28:36 2013 -0500
summary: one

With remote-hg (in gitifyhg mode):

To hg::/tmp/test-hg
* [new tag] v0.1 -> v0.1
o changeset: 1:37af04248d3f
| tag: tip
| user: Username <iw...@this.irg>
| date: Tue May 14 16:27:06 2013 -0500
| summary: Added tag v0.1 for changeset 719913f27c0f
|
@ changeset: 0:719913f27c0f
tag: v0.1
user: Username <iw...@this.irg>
date: Tue May 14 16:27:05 2013 -0500
summary: one

---
(
echo "[ui]"
echo "username = Username <iw...@this.org>"
) >> /tmp/myhgrc

export HGRCPATH=/tmp/myhgrc

cd /tmp &&
rm -rf test-hg test-git &&
(
hg init test-hg &&
cd test-hg &&
echo one > content &&
hg add content &&
hg commit -m one
) &&
(
git clone testgitifyhg::/tmp/test-hg test-git &&
cd test-git &&
git tag v0.1 &&
git push origin v0.1
) &&
hg -R test-hg log --graph
---

--
Felipe Contreras

[Felipe Contreras]

On Mon, May 13, 2013 at 9:22 AM, Dusty Phillips <du...@buchuki.com> wrote:
> Would you mind submitting and discussing these as github pull requests? I
> don't have a pipeline in my inbox to organize what has been discussed or
> addressed previously and it reduces cognitive dissonance for me as an
> integrator.

If you don't have time to read a couple of trivial patches that arrive
right at your inbox, I don't have time for pull requests.

Once you review these patches and there are no changes needed, I can
send the the pull request, but I don't see what's the point, 'git am'
does the trick perfectly.

> Per the HGRC question, it *should* not be an issue either way because
> gitifyhg explicitly disables hg extensions. However, it would be ideal to
> test and assert that this is actually happening.

And how are you going to test that if the tests are creating a fake
environment? You should disable HGRCPATH right away in the tests if
you want to have any hope of catching these issues that would happen
anyway in a real environment.

--
Felipe Contreras

[Jed Brown]

Felipe Contreras <felipe.c...@gmail.com> writes:

> Maybe, I just assumed everyone knew the basics of the test framework.

There is a difference between knowing the basics of the test framework
and inferring the causal effect that you had in mind when writing the
terse commit message. And it seems that the issue extends beyond the
context of testing, so it should be explained regardless. Besides,
commit messages are largely to help out people who read the history
later, especially those less familiar with the component and unaware of
context that you have implicitly associated with the patch. It seems
like Junio has to point this out on every second patch you send to
git@vger.

>> Note that gitifyhg.py sets HGRCPATH too, but it is called without
>> sharness, so should it remain? From your statements below, maybe that
>> should also be changed?
>
> Yes, I think it should, but I'm far less interested in gitifyhg.py.

Then let's leave it as is until someone has time to support in both
places. The argument about what is proper behavior should be associated
with updates everywhere so that the project logic is consistent.
Inconsistency is worse than being consistently "wrong" because it's more
difficult to understand and more difficult to fix completely.

>> Reproducibility and reliability?
>
> Is that a question? Then the answer is 'no'; an empty HGRCPATH doesn't
> bring any more reproducibility and reliability.

Since the test environment is controlled anyway, I think your claim is
that you are not aware of any Mercurial extensions that affect the
behavior of the interfaces used by gitifyhg, except in positive ways
(keyring and schemes). That is a reasonable statement and others can
evaluate for themselves.

>> Your preamble said "clearly breaking Mercurial", but I haven't seen any
>> breakage, so perhaps you can be more specific?
>
> Try the following script:

Lightweight tags are not one-to-one regardless. Gitifyhg never stated
that HGRCPATH was a supported interface for influencing that inexact
mapping. Maybe it is a desired feature, but that is different from
"clearly breaking".

[Felipe Contreras]

On Tue, May 14, 2013 at 6:38 PM, Jed Brown <j...@59a2.org> wrote:
> Felipe Contreras <felipe.c...@gmail.com> writes:
>
>> Maybe, I just assumed everyone knew the basics of the test framework.
>
> There is a difference between knowing the basics of the test framework
> and inferring the causal effect that you had in mind when writing the
> terse commit message.

This is the explanation:

"Extensions can't interfere, because we need to enable them in our .hgrc."

Why don't you trust me? Because it's *you* the one that making the
assumption that the .hgrc would be the one from the $USER running the
test. And *you* make that assumption because you are not familiar with
the testing framework.

I'm not inferring anything, I'm telling you; extensions can't
interfere. This is a commit message, not a teaching session, if you
don't believe my claim on the basis that you are not familiar with the
testing framework, that's not a problem in the commit message.

> And it seems that the issue extends beyond the
> context of testing, so it should be explained regardless.

No, it does not. I don't see what makes you think so.

> Besides,
> commit messages are largely to help out people who read the history
> later, especially those less familiar with the component and unaware of
> context that you have implicitly associated with the patch.

The commit message would help those who read the history later, it's
not meant for people not familiar with the code.

> It seems
> like Junio has to point this out on every second patch you send to
> git@vger.

Please, take a look at the commit messages in *this* project. Dusty
doesn't even have his user configured properly and commits with "=
<du...@linux.ca>". Many don't even have any explanation at all.

>>> Note that gitifyhg.py sets HGRCPATH too, but it is called without
>>> sharness, so should it remain? From your statements below, maybe that
>>> should also be changed?
>>
>> Yes, I think it should, but I'm far less interested in gitifyhg.py.
>
> Then let's leave it as is until someone has time to support in both
> places.

Why? To test that it's working in gitifyhg.py you need to remove it
from the other side, otherwise you are not testing that code.

> The argument about what is proper behavior should be associated
> with updates everywhere so that the project logic is consistent.
> Inconsistency is worse than being consistently "wrong" because it's more
> difficult to understand and more difficult to fix completely.

What are you talking about?

>>> Reproducibility and reliability?
>>
>> Is that a question? Then the answer is 'no'; an empty HGRCPATH doesn't
>> bring any more reproducibility and reliability.
>
> Since the test environment is controlled anyway, I think your claim is
> that you are not aware of any Mercurial extensions that affect the
> behavior of the interfaces used by gitifyhg, except in positive ways
> (keyring and schemes). That is a reasonable statement and others can
> evaluate for themselves.

You are the one that attempted to make the claim that it's there for
reproducibility and reliability, but you have no evidence that is the
case. So no, we don't know it's there for that.

So my original claim remains; there is no gain in emptying HGRCPATH.

>>> Your preamble said "clearly breaking Mercurial", but I haven't seen any
>>> breakage, so perhaps you can be more specific?
>>
>> Try the following script:
>
> Lightweight tags are not one-to-one regardless. Gitifyhg never stated
> that HGRCPATH was a supported interface for influencing that inexact
> mapping.

So instead you are choosing to use the hard-coded value of "~/.hgrc".
Why ~/.hgrc? Why not ~/.gitifyhgrc?

It cannot be because that's what mercurial uses because that's not
true. Mercurial uses HGRCPATH, and in fact, it has a very specific
logic (HGUSER, EMAIL, askuser, etc.). If you are violating the whole
logic of how to determine the username, then what's stopping you from
going one step further and use ~/.gitifyhgrc?

That would be nonsense; you use ~/.hgrc because that's what Mercurial
uses, but you do it wrong, because Mercurial uses it depending on many
other factors that you ignore.

So much for caring about "consistency".

> Maybe it is a desired feature, but that is different from
> "clearly breaking".

A bug is unexpected behavior, and doing something different than what
Mercurial does is unexpected; it's is broken.

--
Felipe Contreras

[Jed Brown]

Felipe Contreras <felipe.c...@gmail.com> writes:

> "Extensions can't interfere, because we need to enable them in our .hgrc."
>
> Why don't you trust me? Because it's *you* the one that making the
> assumption that the .hgrc would be the one from the $USER running the
> test.

You said that the change _should_ be propagated through to gitifyhg.py
(but you didn't have time/interest to do so), in which case it will be
$HOME/.hgrc. That is what I was responding to, not the controlled
testing environment.

>> Besides,
>> commit messages are largely to help out people who read the history
>> later, especially those less familiar with the component and unaware of
>> context that you have implicitly associated with the patch.
>
> The commit message would help those who read the history later, it's
> not meant for people not familiar with the code.

Sure, but it's for people other than you, or for you after you have been
working on other things and have forgotten implications. One sentence
explaining the causal relationship saves a lot of discussion.

>> It seems
>> like Junio has to point this out on every second patch you send to
>> git@vger.
>
> Please, take a look at the commit messages in *this* project. Dusty
> doesn't even have his user configured properly and commits with "=
> <du...@linux.ca>". Many don't even have any explanation at all.

Fair enough, but it's not an excuse when you are asking someone else to
accept patches.

>> Then let's leave it as is until someone has time to support in both
>> places.
>
> Why? To test that it's working in gitifyhg.py you need to remove it
> from the other side, otherwise you are not testing that code.

Yes, it's natural to remove it from both places at once so that the
logical argument of why the change is correct doesn't get strung between
two disjoint commits.

[Felipe Contreras]

On Tue, May 14, 2013 at 7:35 PM, Jed Brown <j...@59a2.org> wrote:
> Felipe Contreras <felipe.c...@gmail.com> writes:
>
>> "Extensions can't interfere, because we need to enable them in our .hgrc."
>>
>> Why don't you trust me? Because it's *you* the one that making the
>> assumption that the .hgrc would be the one from the $USER running the
>> test.
>
> You said that the change _should_ be propagated through to gitifyhg.py
> (but you didn't have time/interest to do so), in which case it will be
> $HOME/.hgrc. That is what I was responding to, not the controlled
> testing environment.

You were not responding to the controlled testing environment? This is
the context that you just removed:

>>> Maybe, I just assumed everyone knew the basics of the test framework.
>>
>> There is a difference between knowing the basics of the test framework
>> and inferring the causal effect that you had in mind when writing the
>> terse commit message.

It's becoming really obvious that you are arguing for the sake of arguing again.

>>> Besides,
>>> commit messages are largely to help out people who read the history
>>> later, especially those less familiar with the component and unaware of
>>> context that you have implicitly associated with the patch.
>>
>> The commit message would help those who read the history later, it's
>> not meant for people not familiar with the code.
>
> Sure, but it's for people other than you, or for you after you have been
> working on other things and have forgotten implications. One sentence
> explaining the causal relationship saves a lot of discussion.

Anyone familiar with the code doesn't need an explanation of how the
testing framework works.

>>> It seems
>>> like Junio has to point this out on every second patch you send to
>>> git@vger.
>>
>> Please, take a look at the commit messages in *this* project. Dusty
>> doesn't even have his user configured properly and commits with "=
>> <du...@linux.ca>". Many don't even have any explanation at all.
>
> Fair enough, but it's not an excuse when you are asking someone else to
> accept patches.

I never put it as an excuse for anything.

>>> Then let's leave it as is until someone has time to support in both
>>> places.
>>
>> Why? To test that it's working in gitifyhg.py you need to remove it
>> from the other side, otherwise you are not testing that code.
>
> Yes, it's natural to remove it from both places at once so that the
> logical argument of why the change is correct doesn't get strung between
> two disjoint commits.

Not at all. They are two completely different changes, in fact, if
there was a problem it would make sense to first apply the patch to
the testing framework, then apply the fixes so the testing framework
works, and then remove it from gitifyhg.py. But I'm not interested in
the later stages.

It makes sense to apply this patch alone right now, if only to
discover issues that would be present in a real environment, which you
are not going to find with a fake testing environment where it's
impossible to find them. In fact, even if you never remove the empty
HGRCPATH from gitifyhg.py, it still makes sense to apply this patch,
for the previous reasons.

--
Felipe Contreras

[Jed Brown]

Felipe Contreras <felipe.c...@gmail.com> writes:
>
> You were not responding to the controlled testing environment?

There are two proposals:

1. Cleaning of HGRCPATH should be removed from testing because it hides
environment that would be present later, thus making testing less
representative. This is what your patch was for, though I think it is
flawed (see below).

2. Clearing of HGRCPATH should be removed from gitifyhg because it
prevents useful extensions like schemes and keyring (with the diversion
into lightweight tagging, which is moot due to issue #77). I think this
is a coherent argument.


Early in the thread, it was unclear whether part 1 was fixing a specific
problem or merely removing redundancy. More context in the commit
message would have made that clear, even if you think those implications
are "obvious" if one "knows the basics of the testing framework".

Aside: As someone who spends a lot of time reminding professors and PhD
scientists about "obvious" implications of things that they learned as
undergraduates, I submit that it's pretty much always worth writing the
sentence or two to make that connection explicit rather than assuming
that the reader will have made it themselves.

>> Yes, it's natural to remove it from both places at once so that the
>> logical argument of why the change is correct doesn't get strung between
>> two disjoint commits.
>
> Not at all. They are two completely different changes, in fact, if
> there was a problem it would make sense to first apply the patch to
> the testing framework, then apply the fixes so the testing framework
> works, and then remove it from gitifyhg.py. But I'm not interested in
> the later stages.
>
> It makes sense to apply this patch alone right now, if only to
> discover issues that would be present in a real environment, which you
> are not going to find with a fake testing environment where it's
> impossible to find them. In fact, even if you never remove the empty
> HGRCPATH from gitifyhg.py, it still makes sense to apply this patch,
> for the previous reasons.

Thanks, this is a more helpful description. The solution in your patch
is fragile, however:

$ cat > /tmp/hgrc-evil <<EOF
[alias]
log = log --graph
EOF
$ HGRCPATH=/tmp/hgrc-evil ./test_notes.t
... everything fails


Why is it "good" to let such aspects of the user's environment into the
test suite? I.e., what would the user be putting in their HGRCPATH that
positively affects the tests? What about this?

--- i/test/test-lib.sh
+++ w/test/test-lib.sh
@@ -10,6 +10,11 @@ export DEBUG_GITIFYHG=$debug

export GIT_PAGER=cat
export NL=$'\n'

+hgcommand=$(which hg)
+hg () {
+ HGRCPATH="$HOME/.hgrc" "$hgcommand" "$@"
+ }
+
make_hg_repo() {

hg init hg_repo &&
cd hg_repo &&

This allows the environment to affect gitifyhg (which we intend to be
immune to configuration---except for test_push_tags.t, pending issue
#77), but not direct invocation of hg for testing. Since
test_push_tags.t assumes that $HOME/.hgrc is read, those tests should
unset HGRCPATH. This ought to make it clear what is an intended
interaction and what is not intended.

[Jed Brown]

Felipe Contreras <felipe.c...@gmail.com> writes:

>>>> Your preamble said "clearly breaking Mercurial", but I haven't seen any
>>>> breakage, so perhaps you can be more specific?
>>>
>>> Try the following script:
>>
>> Lightweight tags are not one-to-one regardless. Gitifyhg never stated
>> that HGRCPATH was a supported interface for influencing that inexact
>> mapping.
>
> So instead you are choosing to use the hard-coded value of "~/.hgrc".
> Why ~/.hgrc? Why not ~/.gitifyhgrc?
>
> It cannot be because that's what mercurial uses because that's not
> true. Mercurial uses HGRCPATH, and in fact, it has a very specific
> logic (HGUSER, EMAIL, askuser, etc.). If you are violating the whole
> logic of how to determine the username, then what's stopping you from
> going one step further and use ~/.gitifyhgrc?

The claim in this issue is that the username should be obtained from
Git, not Hg.

https://github.com/buchuki/gitifyhg/issues/77

So either make a case for why the Hg user is better than the Git user or
drop that part of the argument. (You have pointed out other benefits to
reading the user's hgrc: keyring and schemes.)

[Felipe Contreras]

On Wed, May 15, 2013 at 9:14 PM, Jed Brown <j...@59a2.org> wrote:
> Felipe Contreras <felipe.c...@gmail.com> writes:
>>
>> You were not responding to the controlled testing environment?
>
> There are two proposals:

And in that context we were clearly talking about one of them, not the other.

> 1. Cleaning of HGRCPATH should be removed from testing because it hides
> environment that would be present later, thus making testing less
> representative. This is what your patch was for, though I think it is
> flawed (see below).
>
> 2. Clearing of HGRCPATH should be removed from gitifyhg because it
> prevents useful extensions like schemes and keyring (with the diversion
> into lightweight tagging, which is moot due to issue #77). I think this
> is a coherent argument.
>
>
> Early in the thread, it was unclear whether part 1 was fixing a specific
> problem or merely removing redundancy. More context in the commit
> message would have made that clear, even if you think those implications
> are "obvious" if one "knows the basics of the testing framework".

The patch begins with "test:" you don't need any more context to
realize this applies to to tests (1).

> Aside: As someone who spends a lot of time reminding professors and PhD
> scientists about "obvious" implications of things that they learned as
> undergraduates, I submit that it's pretty much always worth writing the
> sentence or two to make that connection explicit rather than assuming
> that the reader will have made it themselves.

You are the one that is assuming the reader has no familiarity with
the code. I'm not assuming anything, I'm writing this for the actual
users of the commit message: developers who understand the code.

>>> Yes, it's natural to remove it from both places at once so that the
>>> logical argument of why the change is correct doesn't get strung between
>>> two disjoint commits.
>>
>> Not at all. They are two completely different changes, in fact, if
>> there was a problem it would make sense to first apply the patch to
>> the testing framework, then apply the fixes so the testing framework
>> works, and then remove it from gitifyhg.py. But I'm not interested in
>> the later stages.
>>
>> It makes sense to apply this patch alone right now, if only to
>> discover issues that would be present in a real environment, which you
>> are not going to find with a fake testing environment where it's
>> impossible to find them. In fact, even if you never remove the empty
>> HGRCPATH from gitifyhg.py, it still makes sense to apply this patch,
>> for the previous reasons.
>
> Thanks, this is a more helpful description. The solution in your patch
> is fragile, however:
>
> $ cat > /tmp/hgrc-evil <<EOF
> [alias]
> log = log --graph
> EOF
> $ HGRCPATH=/tmp/hgrc-evil ./test_notes.t
> ... everything fails

Easy:

--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -8,7 +8,7 @@ export GIT_USER="$GIT_AUTHOR_NAME <$GIT_AUTHOR_EMAIL>"

export HG_USER="Hg User <hg....@example.com>"
export DEBUG_GITIFYHG=on
export GIT_PAGER=cat
-export HGRCPATH='' # So extensions like pager don't interfere

+export HGRCPATH="$HOME/.hgrc"
export NL='
'

--
Felipe Contreras

[Felipe Contreras]

On Wed, May 15, 2013 at 9:14 PM, Jed Brown <j...@59a2.org> wrote:
> Felipe Contreras <felipe.c...@gmail.com> writes:
>
>>>>> Your preamble said "clearly breaking Mercurial", but I haven't seen any
>>>>> breakage, so perhaps you can be more specific?
>>>>
>>>> Try the following script:
>>>
>>> Lightweight tags are not one-to-one regardless. Gitifyhg never stated
>>> that HGRCPATH was a supported interface for influencing that inexact
>>> mapping.
>>
>> So instead you are choosing to use the hard-coded value of "~/.hgrc".
>> Why ~/.hgrc? Why not ~/.gitifyhgrc?
>>
>> It cannot be because that's what mercurial uses because that's not
>> true. Mercurial uses HGRCPATH, and in fact, it has a very specific
>> logic (HGUSER, EMAIL, askuser, etc.). If you are violating the whole
>> logic of how to determine the username, then what's stopping you from
>> going one step further and use ~/.gitifyhgrc?
>
> The claim in this issue is that the username should be obtained from
> Git, not Hg.
>
> https://github.com/buchuki/gitifyhg/issues/77

I know. It works properly in remote-hg:

https://github.com/felipec/git/commit/501abe7a15ee64ecd81573e2b2d8d8d6cb466156

> So either make a case for why the Hg user is better than the Git user or
> drop that part of the argument.

You are making the flawed assumption that there's always a Git user configured.

As long as you read any mercurial configuration (hgrc), it should be
the done the same way mercurial does it (HGRCPATH).

--
Felipe Contreras

[Jed Brown]

Felipe Contreras <felipe.c...@gmail.com> writes:

>> Aside: As someone who spends a lot of time reminding professors and PhD
>> scientists about "obvious" implications of things that they learned as
>> undergraduates, I submit that it's pretty much always worth writing the
>> sentence or two to make that connection explicit rather than assuming
>> that the reader will have made it themselves.
>
> You are the one that is assuming the reader has no familiarity with
> the code. I'm not assuming anything, I'm writing this for the actual
> users of the commit message: developers who understand the code.

The intent is not to re-explain _what_ the code does, but to draw
attention to the interaction that produces a desired or undesired
effect. It is not productive to assume that the reader is following the
same train of thought as you are, even if they understand all the same
concepts.

>> Thanks, this is a more helpful description. The solution in your patch
>> is fragile, however:
>>
>> $ cat > /tmp/hgrc-evil <<EOF
>> [alias]
>> log = log --graph
>> EOF
>> $ HGRCPATH=/tmp/hgrc-evil ./test_notes.t
>> ... everything fails
>
> Easy:
>
> --- a/test/test-lib.sh
> +++ b/test/test-lib.sh
> @@ -8,7 +8,7 @@ export GIT_USER="$GIT_AUTHOR_NAME <$GIT_AUTHOR_EMAIL>"
> export HG_USER="Hg User <hg....@example.com>"
> export DEBUG_GITIFYHG=on
> export GIT_PAGER=cat
> -export HGRCPATH='' # So extensions like pager don't interfere
> +export HGRCPATH="$HOME/.hgrc"
> export NL='
> '

I considered this first, or rather 'unset HGRCPATH', however, quoting
from your last message:

It makes sense to apply this patch alone right now, if only to
discover issues that would be present in a real environment, which you
are not going to find with a fake testing environment where it's
impossible to find them. In fact, even if you never remove the empty
HGRCPATH from gitifyhg.py, it still makes sense to apply this patch,
for the previous reasons.

So which is it? You want the environment to interact with gitifyhg or
you want to shield the test from all aspects of the environment? This
sentence in the original commit message would have made the intent
clear:

Setting HGRCPATH='' prevents hg from reading $HOME/.hgrc, which we
intend to be read in order to obtain ui.username.

[Felipe Contreras]

I never said I wanted any of those things. I'm telling reasons why you
want to apply this patch. One way or the other setting up an empty
HGRCPATH doesn't make sense.

> This
> sentence in the original commit message would have made the intent
> clear:
>
> Setting HGRCPATH='' prevents hg from reading $HOME/.hgrc, which we
> intend to be read in order to obtain ui.username.

That is not the intention, and that description is not true. Nobody
prevents you from reading ~/.hgrc, in fact, you are doing so right
now.

--
Felipe Contreras

[Jed Brown]

Felipe Contreras <felipe.c...@gmail.com> writes:

>> So which is it? You want the environment to interact with gitifyhg or
>> you want to shield the test from all aspects of the environment?
>
> I never said I wanted any of those things. I'm telling reasons why you
> want to apply this patch. One way or the other setting up an empty
> HGRCPATH doesn't make sense.

Your preamble said:

I'm mainly interested in the first one, because it's clearly breaking
Mercurial because the .hgrc is not parsed.

The commit message for 1/6 did not didn't provide any more information.
When asked, you demonstrated how 'hg showconfig' within a test did not
parse $HOME/.hgrc, but no tests try to do this. Elsewhere in the
thread, you said that you think gitifyhg _should_ obtain configuration
using Hg semantics, which I agree with due to keyring and schemes.

The testing environment has no use for keyring or schemes, but your
argument was that the environment should still interact with gitifyhg,
if only to expose any presently-unknown bad influence on the behavior of
the interfaces that gitifyhg uses. I think that is also a cogent
argument, but the environment seen by the hg command must still be
controlled due to aliases (and maybe other) valid local configuration.

I suggested a way to isolote *only* the hg command, which is compatible
with your rationale for allowing the user's HGRCPATH to propagate into
the test suite.

Are you now saying that none of those things matter, and we should set
HGRCPATH=$HOME/.hgrc (or 'unset HGRCPATH')? In that case, would this
change not be purely cosmetic: $HOME/.hgrc is only intended to be read
by gitifyhg, which does not currently pay attention to HGRCPATH (you
think this should be changed), and make_hg_commit uses --user, so it
doesn't need $HOME/.hgrc either?


Note that if we isolate _only_ the hg command, issue #77 will have to be
resolved before gitifyhg clears HGRCPATH from its environment, and
test_push_tags should clear/set HGRCPATH when testing the fallback in
case of no Git configuration.

>> This
>> sentence in the original commit message would have made the intent
>> clear:
>>
>> Setting HGRCPATH='' prevents hg from reading $HOME/.hgrc, which we
>> intend to be read in order to obtain ui.username.
>
> That is not the intention, and that description is not true. Nobody
> prevents you from reading ~/.hgrc, in fact, you are doing so right
> now.

And you claim that gitifyhg reading it manually is a bug, but couldn't
be bothered to finish the job. It still seems to me that they should
come as a pair, because this commit has dubious value on its own. The
version that isolates only the hg executable would be more attractive on
its own, but would still be better paired with removal of HGRCPATH from
gitifyhg.

[Felipe Contreras]

On Thu, May 16, 2013 at 8:07 AM, Jed Brown <j...@59a2.org> wrote:
> Felipe Contreras <felipe.c...@gmail.com> writes:
>
>>> So which is it? You want the environment to interact with gitifyhg or
>>> you want to shield the test from all aspects of the environment?
>>
>> I never said I wanted any of those things. I'm telling reasons why you
>> want to apply this patch. One way or the other setting up an empty
>> HGRCPATH doesn't make sense.
>
> Your preamble said:
>
> I'm mainly interested in the first one, because it's clearly breaking
> Mercurial because the .hgrc is not parsed.

Let me explain to you the meaning of words.

When I say I'm *interested* in a patch I'm saying I'm interested in
getting the patch applied. When I say I never *wanted* any of those
things, I'm talking about the advantages this patch brings:

1) In the modified version where HGRCPATH="$HOME/.hgrc", it helps to
you guys so you can implement ui.username() properly.

2) In the original version where HGRCPATH is unset, it helps to you
guys to test that unsetting HGRCPATH does indeed work in gitifyhg.py.
In addition to 1)

I don't *want* any of those things, I don't care at all. All I care is
that the patch is applied. These are reasons for *you* to apply it,
and they are good reasons, whichever you pick, but which reason you
pick doesn't interest me in the least.

Do you understand? I don't see how I could be any clearer.

> The commit message for 1/6 did not didn't provide any more information.
> When asked, you demonstrated how 'hg showconfig' within a test did not
> parse $HOME/.hgrc, but no tests try to do this. Elsewhere in the
> thread, you said that you think gitifyhg _should_ obtain configuration
> using Hg semantics, which I agree with due to keyring and schemes.
>
> The testing environment has no use for keyring or schemes, but your
> argument was that the environment should still interact with gitifyhg,
> if only to expose any presently-unknown bad influence on the behavior of
> the interfaces that gitifyhg uses. I think that is also a cogent
> argument, but the environment seen by the hg command must still be
> controlled due to aliases (and maybe other) valid local configuration.
>
> I suggested a way to isolote *only* the hg command, which is compatible
> with your rationale for allowing the user's HGRCPATH to propagate into
> the test suite.

I gave you crystal clear commands and reasoning, not one, but two. And
if you can't see why they are good reason to apply this patch, I don't
know how else to say it, but you are an idiot.

> Are you now saying that none of those things matter, and we should set
> HGRCPATH=$HOME/.hgrc (or 'unset HGRCPATH')?

Where did I say none of those things matter? I say they don't matter
*TO ME*, because I don't care about gitifyhg. They should matter to
you.

> In that case, would this
> change not be purely cosmetic: $HOME/.hgrc is only intended to be read
> by gitifyhg, which does not currently pay attention to HGRCPATH (you
> think this should be changed), and make_hg_commit uses --user, so it
> doesn't need $HOME/.hgrc either?

make_hg_commit() is completely orthogonal to this patch. Stop
confounding it even more. You seem to have trouble following two
reasons, let's not add more.

> Note that if we isolate _only_ the hg command, issue #77 will have to be
> resolved before gitifyhg clears HGRCPATH from its environment,

We are not talking about that. The topic is "[PATCH 1/6] test: remove
empty HGRCPATH".

> and
> test_push_tags should clear/set HGRCPATH when testing the fallback in
> case of no Git configuration.

HGRCPATH should never be empty in the testing framework. To do
anything else is an exercise in futility.

>>> This
>>> sentence in the original commit message would have made the intent
>>> clear:
>>>
>>> Setting HGRCPATH='' prevents hg from reading $HOME/.hgrc, which we
>>> intend to be read in order to obtain ui.username.
>>
>> That is not the intention, and that description is not true. Nobody
>> prevents you from reading ~/.hgrc, in fact, you are doing so right
>> now.
>
> And you claim that gitifyhg reading it manually is a bug, but couldn't
> be bothered to finish the job.

I don't care about gitifyhg.

> It still seems to me that they should
> come as a pair, because this commit has dubious value on its own.

No. I already demonstrates beyond reasonable doubt the advantages this
patch brings to you. And the fact that you don't see it only shines a
light into your mental capacities.

> The
> version that isolates only the hg executable would be more attractive on
> its own, but would still be better paired with removal of HGRCPATH from
> gitifyhg.

It's a waste of time discussing with you. I have productive things to
do, I don't have time to discuss ad nauseam an obvious trivial fix
that can't possible affect negatively anybody in a project that
doesn't dedicate even 1% of this time to review 99% of the patches
that get applied, and is destined for oblivion.

Bye.

--
Felipe Contreras

[Jed Brown]

Felipe Contreras <felipe.c...@gmail.com> writes:

> On Thu, May 16, 2013 at 8:07 AM, Jed Brown <j...@59a2.org> wrote:
>> Your preamble said:
>>
>> I'm mainly interested in the first one, because it's clearly breaking
>> Mercurial because the .hgrc is not parsed.
>
> Let me explain to you the meaning of words.
>
> When I say I'm *interested* in a patch I'm saying I'm interested in
> getting the patch applied. When I say I never *wanted* any of those
> things, I'm talking about the advantages this patch brings:

Why are you interested in the patch being applied if you don't have any
interest in the benefits/consequences that the patch provides?

> 1) In the modified version where HGRCPATH="$HOME/.hgrc", it helps to
> you guys so you can implement ui.username() properly.
>
> 2) In the original version where HGRCPATH is unset,

It is not unset, it is _inherited_ from the environment, which can break
the test suite as I showed via the alias example.

> it helps to you guys to test that unsetting HGRCPATH does indeed work
> in gitifyhg.py. In addition to 1)
>
> I don't *want* any of those things, I don't care at all. All I care is
> that the patch is applied. These are reasons for *you* to apply it,
> and they are good reasons, whichever you pick, but which reason you
> pick doesn't interest me in the least.

Did you think my suggestion was an improvement? If not, why?

> Do you understand? I don't see how I could be any clearer.
>
>> The commit message for 1/6 did not didn't provide any more information.
>> When asked, you demonstrated how 'hg showconfig' within a test did not
>> parse $HOME/.hgrc, but no tests try to do this. Elsewhere in the
>> thread, you said that you think gitifyhg _should_ obtain configuration
>> using Hg semantics, which I agree with due to keyring and schemes.
>>
>> The testing environment has no use for keyring or schemes, but your
>> argument was that the environment should still interact with gitifyhg,
>> if only to expose any presently-unknown bad influence on the behavior of
>> the interfaces that gitifyhg uses. I think that is also a cogent
>> argument, but the environment seen by the hg command must still be
>> controlled due to aliases (and maybe other) valid local configuration.
>>
>> I suggested a way to isolote *only* the hg command, which is compatible
>> with your rationale for allowing the user's HGRCPATH to propagate into
>> the test suite.
>
> I gave you crystal clear commands and reasoning, not one, but two. And
> if you can't see why they are good reason to apply this patch, I don't
> know how else to say it, but you are an idiot.
>
>> Are you now saying that none of those things matter, and we should set
>> HGRCPATH=$HOME/.hgrc (or 'unset HGRCPATH')?
>
> Where did I say none of those things matter? I say they don't matter
> *TO ME*, because I don't care about gitifyhg. They should matter to
> you.

Normally when developing software, we talk about what matters to users.
I thought that was assumed, considering that neither of us are users.

>> In that case, would this
>> change not be purely cosmetic: $HOME/.hgrc is only intended to be read
>> by gitifyhg, which does not currently pay attention to HGRCPATH (you
>> think this should be changed), and make_hg_commit uses --user, so it
>> doesn't need $HOME/.hgrc either?
>
> make_hg_commit() is completely orthogonal to this patch. Stop
> confounding it even more. You seem to have trouble following two
> reasons, let's not add more.

Not at all. If make_hg_commit did not use --user explicitly, it would
read configuration from the usual place, respecting HGRCPATH if it was
set. So if the user had exported HGRCPATH=/home/user/day-job/.hgrc
before running the test suite, it would fail because commits would not
use ui.username from sharness' $HOME/.hgrc. A developer that has only
noticed that $HOME/.hgrc was setting ui.username would reasonably
anticipate that the hg executable was intended to read it. It is
surprising that we set $HOME/.hgrc, but intend for it to be irrelevant
whether the hg executable reads it.

For long-term maintainability and to allow new developers to quickly get
up to speed with a new project, it is important to make code locally
understandable.

>> Note that if we isolate _only_ the hg command, issue #77 will have to be
>> resolved before gitifyhg clears HGRCPATH from its environment,
>
> We are not talking about that. The topic is "[PATCH 1/6] test: remove
> empty HGRCPATH".
>
>> and
>> test_push_tags should clear/set HGRCPATH when testing the fallback in
>> case of no Git configuration.
>
> HGRCPATH should never be empty in the testing framework. To do
> anything else is an exercise in futility.

Yet your patch allows it to be arbitrary (including empty) from the
user's environment.

>>>> This
>>>> sentence in the original commit message would have made the intent
>>>> clear:
>>>>
>>>> Setting HGRCPATH='' prevents hg from reading $HOME/.hgrc, which we
>>>> intend to be read in order to obtain ui.username.
>>>
>>> That is not the intention, and that description is not true. Nobody
>>> prevents you from reading ~/.hgrc, in fact, you are doing so right
>>> now.
>>
>> And you claim that gitifyhg reading it manually is a bug, but couldn't
>> be bothered to finish the job.
>
> I don't care about gitifyhg.

Why submit patches at all?

>> It still seems to me that they should
>> come as a pair, because this commit has dubious value on its own.
>
> No. I already demonstrates beyond reasonable doubt the advantages this
> patch brings to you. And the fact that you don't see it only shines a
> light into your mental capacities.

You did not demonstrate any clear advantages _within_ the scope of
present tests, only indirectly through changes that you don't care to
make because you don't care about the project. I showed a way that your
patch allows the user's environment to break _present tests_. I'll
leave it to a third party to evaluate which is more important.


As a parting note, I propose that when confronted with the problem of
being chronically misunderstood, blaming it on the "mental capacities"
of everyone else is not the most productive reaction.

[Felipe Contreras]

On Thu, May 16, 2013 at 9:57 AM, Jed Brown <j...@59a2.org> wrote:
> Felipe Contreras <felipe.c...@gmail.com> writes:

>>> And you claim that gitifyhg reading it manually is a bug, but couldn't
>>> be bothered to finish the job.
>>
>> I don't care about gitifyhg.
>
> Why submit patches at all?

The test framework could be shared between the two projects. But
people like you make it impossible to move forward.

I'll keep gitifyhg fixes to myself.

--
Felipe Contreras

[Jed Brown]

Felipe Contreras <felipe.c...@gmail.com> writes:

> The test framework could be shared between the two projects. But
> people like you make it impossible to move forward.

Now this is useful justification, and would be great to have stated in
the original patch. I.e., "this does not affect gitifyhg, but matters
to remote-hg because it reads hgrc using the native mercurial methods".

Now, can you answer my earlier question? Which of these do you prefer:

1. The simple fix in test-libs.sh:

unset HGRCPATH

2. Allow arbitrary HGRCPATH to propagate from user's environment, but
control it in exactly those places that we intend to be affected.
Specifically, control the environment (HGRCPATH=$HOME/.hgrc) when
invoking the hg command (like 'hg log') and when using git-remote-* in a
way that is expected to consult $HOME/.hgrc (pushing lightweight tags)?

[Felipe Contreras]

On Thu, May 16, 2013 at 10:49 AM, Jed Brown <j...@59a2.org> wrote:
> Felipe Contreras <felipe.c...@gmail.com> writes:
>
>> The test framework could be shared between the two projects. But
>> people like you make it impossible to move forward.
>
> Now this is useful justification, and would be great to have stated in
> the original patch. I.e., "this does not affect gitifyhg, but matters
> to remote-hg because it reads hgrc using the native mercurial methods".

It doesn't matter. What is good for gitifyhg should be good for
remote-hg. The only reason why it's of no immediate need to gitifyhg
is because it's broken. And you shouldn't make assumptions about the
motivations behind people's patches.

> Now, can you answer my earlier question? Which of these do you prefer:

You didn't ask this question and I don't care.

--
Felipe Contreras

[Jed Brown]

Felipe Contreras <felipe.c...@gmail.com> writes:

> On Thu, May 16, 2013 at 10:49 AM, Jed Brown <j...@59a2.org> wrote:
>> Felipe Contreras <felipe.c...@gmail.com> writes:
>>
>>> The test framework could be shared between the two projects. But
>>> people like you make it impossible to move forward.
>>
>> Now this is useful justification, and would be great to have stated in
>> the original patch. I.e., "this does not affect gitifyhg, but matters
>> to remote-hg because it reads hgrc using the native mercurial methods".
>
> It doesn't matter. What is good for gitifyhg should be good for
> remote-hg. The only reason why it's of no immediate need to gitifyhg
> is because it's broken. And you shouldn't make assumptions about the
> motivations behind people's patches.

It's of no immediate value to gitifyhg because that environment variable
is not used by gitifyhg. So if you claim that a change is better, it
would help to justify. The primary justification seems to be that it's
needed by remote-hg and that gitifyhg should probably adopt the
remote-hg behavior in this regard.

>> Now, can you answer my earlier question? Which of these do you prefer:
>
> You didn't ask this question

Quoting:

So which is it? You want the environment to interact with gitifyhg or
you want to shield the test from all aspects of the environment?

> and I don't care.

Your original patch is broken due to

export HGRCPATH=$HOME/day-job/.hgrc
make test

so we would appreciate your opinion about the most satisfactory way to
make the test suite work with both remote helpers. This is a
philosophical question since both can be made "correct".

[Felipe Contreras]

On Thu, May 16, 2013 at 11:54 AM, Jed Brown <j...@59a2.org> wrote:
> Felipe Contreras <felipe.c...@gmail.com> writes:
>
>> On Thu, May 16, 2013 at 10:49 AM, Jed Brown <j...@59a2.org> wrote:
>>> Felipe Contreras <felipe.c...@gmail.com> writes:
>>>
>>>> The test framework could be shared between the two projects. But
>>>> people like you make it impossible to move forward.
>>>
>>> Now this is useful justification, and would be great to have stated in
>>> the original patch. I.e., "this does not affect gitifyhg, but matters
>>> to remote-hg because it reads hgrc using the native mercurial methods".
>>
>> It doesn't matter. What is good for gitifyhg should be good for
>> remote-hg. The only reason why it's of no immediate need to gitifyhg
>> is because it's broken. And you shouldn't make assumptions about the
>> motivations behind people's patches.
>
> It's of no immediate value to gitifyhg because that environment variable
> is not used by gitifyhg.

Because it's broken.

> So which is it? You want the environment to interact with gitifyhg or
> you want to shield the test from all aspects of the environment?

That's a different question. And I already answered I don't *want* anything.

> Your original patch is broken due to
>
> export HGRCPATH=$HOME/day-job/.hgrc
> make test

Good luck dreaming up hypothetical examples about issues nobody would
hit in a million years.

--
Felipe Contreras

[Jed Brown]

Felipe Contreras <felipe.c...@gmail.com> writes:

> Because it's broken.

In other words:

Please apply this patch. It doesn't affect your software. I think
your software is "broken", but you should really apply this patch
anyway.

>> So which is it? You want the environment to interact with gitifyhg or
>> you want to shield the test from all aspects of the environment?
>
> That's a different question. And I already answered I don't *want* anything.

I thought we went through this. Users and other developers are what
matters, so when we (as non-users) say "I want the software to behave as
follows", we mean "I think that it would be better for users and
developers if the software behaved as follows". I know you don't
personally care, but you care about users (or something) enough to write
patches.

>> Your original patch is broken due to
>>
>> export HGRCPATH=$HOME/day-job/.hgrc
>> make test
>
> Good luck dreaming up hypothetical examples about issues nobody would
> hit in a million years.

Are you suggesting that nobody exports HGRCPATH?

I'm inclined to 'unset HGRCPATH' because it's easy and likely to not
need revisiting.

[Felipe Contreras]

On Thu, May 16, 2013 at 12:18 PM, Jed Brown <j...@59a2.org> wrote:
> Felipe Contreras <felipe.c...@gmail.com> writes:
>
>> Because it's broken.
>
> In other words:
>
> Please apply this patch. It doesn't affect your software. I think
> your software is "broken", but you should really apply this patch
> anyway.

Yes. That's one of the reasons, but not the only one.

>>> So which is it? You want the environment to interact with gitifyhg or
>>> you want to shield the test from all aspects of the environment?
>>
>> That's a different question. And I already answered I don't *want* anything.
>
> I thought we went through this. Users and other developers are what
> matters, so when we (as non-users) say "I want the software to behave as
> follows", we mean "I think that it would be better for users and
> developers if the software behaved as follows". I know you don't
> personally care, but you care about users (or something) enough to write
> patches.

I care about *my* users, not yours. You should care about your users,
and I gave you reasons to apply this patch that would benefit your
users. If you don't understand that, it's not my problem.

--
Felipe Contreras

[Jed Brown]

Felipe Contreras <felipe.c...@gmail.com> writes:

>>> That's a different question. And I already answered I don't *want* anything.
>>
>> I thought we went through this. Users and other developers are what
>> matters, so when we (as non-users) say "I want the software to behave as
>> follows", we mean "I think that it would be better for users and
>> developers if the software behaved as follows". I know you don't
>> personally care, but you care about users (or something) enough to write
>> patches.
>
> I care about *my* users, not yours. You should care about your users,
> and I gave you reasons to apply this patch that would benefit your
> users. If you don't understand that, it's not my problem.

Then you could answer my question on behalf of your users? Do you want
the simple patch below or do you want HGRCPATH to propagate from the
environment (i.e., your patch, plus fixes for the things we need to
control)?

--- i/test/test-lib.sh
+++ w/test/test-lib.sh

@@ -8,7 +8,7 @@ export GIT_USER="$GIT_AUTHOR_NAME <$GIT_AUTHOR_EMAIL>"
export HG_USER="Hg User <hg....@example.com>"
export DEBUG_GITIFYHG=on
export GIT_PAGER=cat
-export HGRCPATH='' # So extensions like pager don't interfere

+unset HGRCPATH # Prevent user's environment from affecting hg command
export NL='
'

[Felipe Contreras]

On Thu, May 16, 2013 at 7:33 PM, Jed Brown <j...@59a2.org> wrote:
> Felipe Contreras <felipe.c...@gmail.com> writes:
>
>>>> That's a different question. And I already answered I don't *want* anything.
>>>
>>> I thought we went through this. Users and other developers are what
>>> matters, so when we (as non-users) say "I want the software to behave as
>>> follows", we mean "I think that it would be better for users and
>>> developers if the software behaved as follows". I know you don't
>>> personally care, but you care about users (or something) enough to write
>>> patches.
>>
>> I care about *my* users, not yours. You should care about your users,
>> and I gave you reasons to apply this patch that would benefit your
>> users. If you don't understand that, it's not my problem.
>
> Then you could answer my question on behalf of your users? Do you want
> the simple patch below or do you want HGRCPATH to propagate from the
> environment (i.e., your patch, plus fixes for the things we need to
> control)?

Anything that doesn't leave HGRCPATH empty: unset is fine, and so is
"$HOME/.hgrc".

--
Felipe Contreras

[Jed Brown]

Felipe Contreras <felipe.c...@gmail.com> writes:

> Anything that doesn't leave HGRCPATH empty: unset is fine, and so is
> "$HOME/.hgrc".

Thanks.

https://github.com/buchuki/gitifyhg/pull/85
https://github.com/buchuki/gitifyhg/pull/86

[Jed Brown]

Felipe Contreras <felipe.c...@gmail.com> writes:

> Signed-off-by: Felipe Contreras <felipe.c...@gmail.com>
> ---
> test/test-lib.sh | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/test/test-lib.sh b/test/test-lib.sh
> index 8c5fbc4..759f672 100644
> --- a/test/test-lib.sh
> +++ b/test/test-lib.sh
> @@ -8,13 +8,11 @@ export GIT_USER="$GIT_AUTHOR_NAME <$GIT_AUTHOR_EMAIL>"

> export HG_USER="Hg User <hg....@example.com>"
> export DEBUG_GITIFYHG=on
> export GIT_PAGER=cat

> -export NL='
> -'
> +export NL=$'\n'

This is not portable and broke the Travis tests, so I amended the commit.