threads / patch / 33401

patchremote-helpers: remove --graph in hg_log()

Subject: [PATCH] remote-helpers: remove --graph in hg_log()

## tl;dr

6 messages between Apr 6, 2013 and Apr 7, 2013. Diffs are folded; open one to read it.

replies: 5people: 3as markdown or json

Antoine Pelisse· Apr 6, 2013, 16:00 UTC · lore

The hg_log() test helper uses the "--graph" parameter that is implemented by the GraphLog extension. If the extension is not activated by the user, the parameter is not available. Do not use the option that is unnecessary.

Also changes the way we grep the output in hg_log(). The pipe operator can hide the return code of hg command. As a matter of fact, if log fails because it doesn't know about "--graph", it doesn't report any failure and let's you think everything worked.

Signed-off-by: Antoine Pelisse <apelisse@gmail.com>
---
Hey Felipe,
I'm not so confident that --graph is useless to the test. If it's really
necessary, it would be nice either to activate it in setup() or to use
it just for the command through: "--config extensions.graphlog=".

Cheers,

 contrib/remote-helpers/test-hg-bidi.sh |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Show changes to contrib/remote-helpers/test-hg-bidi.sh +2 −2

diff --git a/contrib/remote-helpers/test-hg-bidi.sh b/contrib/remote-helpers/test-hg-bidi.sh
index 1d61982..96cd5a7 100755
--- a/contrib/remote-helpers/test-hg-bidi.sh
+++ b/contrib/remote-helpers/test-hg-bidi.sh
@@ -50,7 +50,8 @@ hg_push () {
 }

 hg_log () {
-	hg -R $1 log --graph --debug | grep -v 'tag: *default/'
+	hg -R $1 log --debug >log &&
+	grep -v 'tag: *default/' log
 }

 setup () {
--
1.7.9.5

Felipe Contreras· Apr 6, 2013, 16:06 UTC · re: Antoine Pelisse · lore

Re: [PATCH] remote-helpers: remove --graph in hg_log()

On Sat, Apr 6, 2013 at 10:00 AM, Antoine Pelisse <apelisse@gmail.com> wrote:

> The hg_log() test helper uses the "--graph" parameter that is
> implemented by the GraphLog extension. If the extension is not activated
> by the user, the parameter is not available. Do not use the option that
> is unnecessary.

> I'm not so confident that --graph is useless to the test. If it's really
> necessary, it would be nice either to activate it in setup() or to use
> it just for the command through: "--config extensions.graphlog=".

I think it should be activated in the setup, it comes packaged with mercurial, and it's likely that many users have it enabled.

> Also changes the way we grep the output in hg_log(). The pipe operator
> can hide the return code of hg command. As a matter of fact, if log
> fails because it doesn't know about "--graph", it doesn't report any
> failure and let's you think everything worked.

Yeap, that is good to have.

Cheers.

-- 
Felipe Contreras

Antoine Pelisse· Apr 6, 2013, 16:12 UTC · re: Felipe Contreras · lore

Re: [PATCH] remote-helpers: remove --graph in hg_log()

On Sat, Apr 6, 2013 at 6:06 PM, Felipe Contreras <felipe.contreras@gmail.com> wrote:

Show 7 quoted lines

> On Sat, Apr 6, 2013 at 10:00 AM, Antoine Pelisse <apelisse@gmail.com> wrote:
>> I'm not so confident that --graph is useless to the test. If it's really
>> necessary, it would be nice either to activate it in setup() or to use
>> it just for the command through: "--config extensions.graphlog=".
>
> I think it should be activated in the setup, it comes packaged with
> mercurial, and it's likely that many users have it enabled.

But is it relevant to the tests ? I have the feeling that it's not strictly necessary to both add an extension to hgrc and a command line option. (and indeed, the tests still work for me, but maybe I'm missing something).

Felipe Contreras· Apr 6, 2013, 16:32 UTC · re: Antoine Pelisse · lore

Re: [PATCH] remote-helpers: remove --graph in hg_log()

On Sat, Apr 6, 2013 at 10:12 AM, Antoine Pelisse <apelisse@gmail.com> wrote:

Show 14 quoted lines

> On Sat, Apr 6, 2013 at 6:06 PM, Felipe Contreras
> <felipe.contreras@gmail.com> wrote:
>> On Sat, Apr 6, 2013 at 10:00 AM, Antoine Pelisse <apelisse@gmail.com> wrote:
>>> I'm not so confident that --graph is useless to the test. If it's really
>>> necessary, it would be nice either to activate it in setup() or to use
>>> it just for the command through: "--config extensions.graphlog=".
>>
>> I think it should be activated in the setup, it comes packaged with
>> mercurial, and it's likely that many users have it enabled.
>
> But is it relevant to the tests ? I have the feeling that it's not
> strictly necessary to both add an extension to hgrc and a command line
> option. (and indeed, the tests still work for me, but maybe I'm
> missing something).

It's possible that the order of the commits make them look the same, but the topology is different. At least I saw a couple of cases when I was working on them, but of course, that was on test-hg-hg-git.sh, which should also have a patch. I think adding it in hgrc is the best option.

-- 
Felipe Contreras

Antoine Pelisse· Apr 6, 2013, 16:50 UTC · re: Felipe Contreras · lore

[PATCH] remote-hg: activate graplog extension for hg_log()

Signed-off-by: Antoine Pelisse <apelisse@gmail.com>
---
- Updated the title to use remote-hg instead of remote-helpers
- Activate the extension instead of removing the option
- Apply the same to test-hg-hg-git

 contrib/remote-helpers/test-hg-bidi.sh   |    5 ++++-
 contrib/remote-helpers/test-hg-hg-git.sh |    4 +++-
 2 files changed, 7 insertions(+), 2 deletions(-)

Show changes to 2 files +7 −3

contrib/remote-helpers/test-hg-bidi.sh, contrib/remote-helpers/test-hg-hg-git.sh

diff --git a/contrib/remote-helpers/test-hg-bidi.sh b/contrib/remote-helpers/test-hg-bidi.sh
index 1d61982..84b9228 100755
--- a/contrib/remote-helpers/test-hg-bidi.sh
+++ b/contrib/remote-helpers/test-hg-bidi.sh
@@ -50,7 +50,8 @@ hg_push () {
 }

 hg_log () {
-	hg -R $1 log --graph --debug | grep -v 'tag: *default/'
+	hg -R $1 log --graph --debug >log &&
+	grep -v 'tag: *default/' log
 }

 setup () {
@@ -62,6 +63,8 @@ setup () {
 	echo "commit = -d \"0 0\""
 	echo "debugrawcommit = -d \"0 0\""
 	echo "tag = -d \"0 0\""
+	echo "[extensions]"
+	echo "graphlog ="
 	) >> "$HOME"/.hgrc &&
 	git config --global remote-hg.hg-git-compat true

diff --git a/contrib/remote-helpers/test-hg-hg-git.sh b/contrib/remote-helpers/test-hg-hg-git.sh
index 7e3967f..17a074e 100755
--- a/contrib/remote-helpers/test-hg-hg-git.sh
+++ b/contrib/remote-helpers/test-hg-hg-git.sh
@@ -78,7 +78,8 @@ hg_push_hg () {
 }

 hg_log () {
-	hg -R $1 log --graph --debug | grep -v 'tag: *default/'
+	hg -R $1 log --graph --debug >log &&
+	grep -v 'tag: *default/' log
 }

 git_log () {
@@ -97,6 +98,7 @@ setup () {
 	echo "[extensions]"
 	echo "hgext.bookmarks ="
 	echo "hggit ="
+	echo "graphlog ="
 	) >> "$HOME"/.hgrc &&
 	git config --global receive.denycurrentbranch warn
 	git config --global remote-hg.hg-git-compat true
--
1.7.9.5

Eric Sunshine· Apr 7, 2013, 01:25 UTC · re: Antoine Pelisse · lore

Re: [PATCH] remote-hg: activate graplog extension for hg_log()

On Sat, Apr 6, 2013 at 12:50 PM, Antoine Pelisse <apelisse@gmail.com> wrote:

> remote-hg: activate graplog extension for hg_log()

s/graplog/graphlog/

Show 11 quoted lines

> The hg_log() test helper uses the "--graph" parameter that is
> implemented by the GraphLog extension. If the extension is not activated
> by the user, the parameter is not available. Activate the extension in
> setup().
>
> Also changes the way we grep the output in hg_log(). The pipe operator
> can hide the return code of hg command. As a matter of fact, if log
> fails because it doesn't know about "--graph", it doesn't report any
> failure and let's you think everything worked.
>
> Signed-off-by: Antoine Pelisse <apelisse@gmail.com>