Patchwork debuginstall: display warning when multiple installs are present (issue4141)

login
register
mail settings
Submitter Prasoon Shukla
Date Jan. 5, 2014, 11:17 a.m.
Message ID <CA+c5VvN1N=PLD793DYy1jDJOTzpKPm+ewm+OpQAgRAEe_ju-9A@mail.gmail.com>
Download mbox | patch
Permalink /patch/3260/
State Superseded
Headers show

Comments

Prasoon Shukla - Jan. 5, 2014, 11:17 a.m.
This is a patch for issue 4141 <http://bz.selenic.com/show_bug.cgi?id=4141>.
*I couldn't add appropriate tests* because the tests require matching
multiple lines with a regex, which isn't supported. I tried Chris' idea (
http://mercurial.808500.n3.nabble.com/Problem-writing-test-td4006240.html)
of temporarily changing path for the command like so:
$ PATH=`which hg | xargs -n1 dirname`:`which hg | xargs -n1 dirname` hg
debuginstall
but that didn't work either. If someone has any idea for adding tests,
please do so.

# HG changeset patch
# User Prasoon Shukla <prasoon92.iitr@gmail.com>
# Date 1388919253 -19800
#      Sun Jan 05 16:24:13 2014 +0530
# Node ID 43b34adb25531fe26daa54bc53dbca63780953ef
# Parent  082b2930fe2ca9a003b08439524384e097acaa0a
debuginstall: display warning when multiple installs are present (issue4141)

If multiple installations of hg are present, a warning is displayed along
with all the installs.
Matt Mackall - Jan. 5, 2014, 10:07 p.m.
On Sun, 2014-01-05 at 16:47 +0530, Prasoon Shukla wrote:
> This is a patch for issue 4141 <http://bz.selenic.com/show_bug.cgi?id=4141>.
> *I couldn't add appropriate tests* because the tests require matching
> multiple lines with a regex, which isn't supported. I tried Chris' idea (
> http://mercurial.808500.n3.nabble.com/Problem-writing-test-td4006240.html)
> of temporarily changing path for the command like so:
> $ PATH=`which hg | xargs -n1 dirname`:`which hg | xargs -n1 dirname` hg
> debuginstall
> but that didn't work either. If someone has any idea for adding tests,
> please do so.
> 
> # HG changeset patch
> # User Prasoon Shukla <prasoon92.iitr@gmail.com>
> # Date 1388919253 -19800
> #      Sun Jan 05 16:24:13 2014 +0530
> # Node ID 43b34adb25531fe26daa54bc53dbca63780953ef
> # Parent  082b2930fe2ca9a003b08439524384e097acaa0a
> debuginstall: display warning when multiple installs are present (issue4141)
> 
> If multiple installations of hg are present, a warning is displayed along
> with all the installs.
> 
> diff -r 082b2930fe2c -r 43b34adb2553 mercurial/commands.py
> --- a/mercurial/commands.py     Thu Jan 02 16:32:51 2014 -0600
> +++ b/mercurial/commands.py     Sun Jan 05 16:24:13 2014 +0530
> @@ -20,6 +20,7 @@
>  import random
>  import setdiscovery, treediscovery, dagutil, pvec, localrepo
>  import phases, obsolete
> +from distutils.spawn import find_executable

Please don't use this form of import. Not only does it pollute the
namespace, it's messier for our demandimport system and thus slows
down.. everything.

Use the style the rest of the imports use.

>  table = {}
> 
> @@ -2047,6 +2048,18 @@
> 
>      problems = 0
> 
> +    # installations
> +    installs = []
> +    pathlist = os.environ['PATH'].split(os.pathsep)
> +    for path in pathlist:
> +        exe = find_executable('hg', path)

What if the current executable is named something other than hg?

I personally have about 80 copies of hg in my path.. one for each
released version, with names like hg282. 

> +        if exe:
> +            installs.append(exe)
> +    ui.status(_("checking hg path (%s)...\n" % installs[0]))

What if the current running hg is NOT the first one in the path?

What if there's nothing named 'hg' in the path? 

> +    if len(installs) > 1:
> +        ui.warn(_("warning! multiple installs of hg detected:\n  %s...\n"
> +                  % '\n  '.join(installs)))
> +
>      # encoding
>      ui.status(_("checking encoding (%s)...\n") % encoding.encoding)
>      try:
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
Prasoon Shukla - Jan. 7, 2014, 5:24 a.m.
I'll try to fix things up as soon as I have some time. I'll probably have
to write a custom find_executable function.


On Mon, Jan 6, 2014 at 3:37 AM, Matt Mackall <mpm@selenic.com> wrote:

> On Sun, 2014-01-05 at 16:47 +0530, Prasoon Shukla wrote:
> > This is a patch for issue 4141 <
> http://bz.selenic.com/show_bug.cgi?id=4141>.
> > *I couldn't add appropriate tests* because the tests require matching
> > multiple lines with a regex, which isn't supported. I tried Chris' idea (
> >
> http://mercurial.808500.n3.nabble.com/Problem-writing-test-td4006240.html)
> > of temporarily changing path for the command like so:
> > $ PATH=`which hg | xargs -n1 dirname`:`which hg | xargs -n1 dirname` hg
> > debuginstall
> > but that didn't work either. If someone has any idea for adding tests,
> > please do so.
> >
> > # HG changeset patch
> > # User Prasoon Shukla <prasoon92.iitr@gmail.com>
> > # Date 1388919253 -19800
> > #      Sun Jan 05 16:24:13 2014 +0530
> > # Node ID 43b34adb25531fe26daa54bc53dbca63780953ef
> > # Parent  082b2930fe2ca9a003b08439524384e097acaa0a
> > debuginstall: display warning when multiple installs are present
> (issue4141)
> >
> > If multiple installations of hg are present, a warning is displayed along
> > with all the installs.
> >
> > diff -r 082b2930fe2c -r 43b34adb2553 mercurial/commands.py
> > --- a/mercurial/commands.py     Thu Jan 02 16:32:51 2014 -0600
> > +++ b/mercurial/commands.py     Sun Jan 05 16:24:13 2014 +0530
> > @@ -20,6 +20,7 @@
> >  import random
> >  import setdiscovery, treediscovery, dagutil, pvec, localrepo
> >  import phases, obsolete
> > +from distutils.spawn import find_executable
>
> Please don't use this form of import. Not only does it pollute the
> namespace, it's messier for our demandimport system and thus slows
> down.. everything.
>
> Use the style the rest of the imports use.
>
> >  table = {}
> >
> > @@ -2047,6 +2048,18 @@
> >
> >      problems = 0
> >
> > +    # installations
> > +    installs = []
> > +    pathlist = os.environ['PATH'].split(os.pathsep)
> > +    for path in pathlist:
> > +        exe = find_executable('hg', path)
>
> What if the current executable is named something other than hg?
>
> I personally have about 80 copies of hg in my path.. one for each
> released version, with names like hg282.
>
> > +        if exe:
> > +            installs.append(exe)
> > +    ui.status(_("checking hg path (%s)...\n" % installs[0]))
>
> What if the current running hg is NOT the first one in the path?
>
> What if there's nothing named 'hg' in the path?
>
> > +    if len(installs) > 1:
> > +        ui.warn(_("warning! multiple installs of hg detected:\n
>  %s...\n"
> > +                  % '\n  '.join(installs)))
> > +
> >      # encoding
> >      ui.status(_("checking encoding (%s)...\n") % encoding.encoding)
> >      try:
> > _______________________________________________
> > Mercurial-devel mailing list
> > Mercurial-devel@selenic.com
> > http://selenic.com/mailman/listinfo/mercurial-devel
>
> --
> Mathematics is the supreme nostalgia of our time.
>
>
>

Patch

diff -r 082b2930fe2c -r 43b34adb2553 mercurial/commands.py
--- a/mercurial/commands.py     Thu Jan 02 16:32:51 2014 -0600
+++ b/mercurial/commands.py     Sun Jan 05 16:24:13 2014 +0530
@@ -20,6 +20,7 @@ 
 import random
 import setdiscovery, treediscovery, dagutil, pvec, localrepo
 import phases, obsolete
+from distutils.spawn import find_executable

 table = {}

@@ -2047,6 +2048,18 @@ 

     problems = 0

+    # installations
+    installs = []
+    pathlist = os.environ['PATH'].split(os.pathsep)
+    for path in pathlist:
+        exe = find_executable('hg', path)
+        if exe:
+            installs.append(exe)
+    ui.status(_("checking hg path (%s)...\n" % installs[0]))
+    if len(installs) > 1:
+        ui.warn(_("warning! multiple installs of hg detected:\n  %s...\n"
+                  % '\n  '.join(installs)))
+
     # encoding
     ui.status(_("checking encoding (%s)...\n") % encoding.encoding)
     try: