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

login
register
mail settings
Submitter Prasoon Shukla
Date Jan. 11, 2014, 9:24 a.m.
Message ID <CA+c5VvOunupe_wJVD_dHz0HiN7DksLNNp3iV-HE9g0hit5Os-Q@mail.gmail.com>
Download mbox | patch
Permalink /patch/3292/
State Changes Requested
Headers show

Comments

Prasoon Shukla - Jan. 11, 2014, 9:24 a.m.
Version 2 of patch for issue 4141<http://bz.selenic.com/show_bug.cgi?id=4141>.
Tests have not been added because there is not support for multiline
matching. The problems mentioned by Matt have been fixed. Please see this
thread for details:
http://mercurial.808500.n3.nabble.com/PATCH-debuginstall-display-warning-when-multiple-installs-are-present-issue4141-td4006273.html

# HG changeset patch
# User Prasoon Shukla <prasoon92.iitr@gmail.com>
# Date 1389431301 -19800
# Node ID 5d9942f43bee3e6c04c24cba24b340be5d7ab6cc
# Parent  d2704c48f4176d8cd6f21d33500820d44763585c
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.

+    return matches
Giovanni Gherdovich - Feb. 18, 2014, 7:35 a.m.
Hello Prasoon,

Sorry for the late reply to your patch.

On Sat, Jan 11, 2014 at 10:24 AM, Prasoon Shukla <prasoon92.iitr@gmail.com>
wrote:
>
> Version 2 of patch for issue 4141. Tests have not been added because
> there is not support for multiline matching. The problems mentioned
> by Matt have been fixed. Please see this thread for details:
>
http://mercurial.808500.n3.nabble.com/PATCH-debuginstall-display-warning-when-multiple-installs-are-present-issue4141-td4006273.html
>

Uhm, you added some text on the top of the patch.
Here we usually send patches in the body of emails,
but we don't compose them by hand: we use the patchbomb extension
to send the emails automatically.
Are you using patchbomb too?
If not, I encourage you to, it's the safest way:
http://mercurial.selenic.com/wiki/ContributingChanges#Emailing_patches

I guess that adding text not related to the patch also complicates
the job of the mantainers who apply them to their repo.

> # HG changeset patch
> # User Prasoon Shukla <prasoon92.iitr@gmail.com>
> # Date 1389431301 -19800
> # Node ID 5d9942f43bee3e6c04c24cba24b340be5d7ab6cc
> # Parent  d2704c48f4176d8cd6f21d33500820d44763585c
> 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 d2704c48f417 -r 5d9942f43bee mercurial/commands.py
> --- a/mercurial/commands.py Fri Nov 15 23:28:43 2013 -0500
> +++ b/mercurial/commands.py Sat Jan 11 14:38:21 2014 +0530
> @@ -8,7 +8,7 @@
>  from node import hex, bin, nullid, nullrev, short
>  from lock import release
>  from i18n import _
> -import os, re, difflib, time, tempfile, errno
> +import os, sys, re, difflib, time, tempfile, errno
>  import hg, scmutil, util, revlog, copies, error, bookmarks
>  import patch, help, encoding, templatekw, discovery
>  import archival, changegroup, cmdutil, hbisect
> @@ -2047,6 +2047,16 @@
>
>      problems = 0
>
> +    # installations
> +    installs = util.findhgexe()
> +    if not installs:
> +        ui.warn(_("warning! no file named hg found in path"))
> +    else:
> +        ui.status(_("checking hg path (%s)...\n" % sys.argv[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:
> diff -r d2704c48f417 -r 5d9942f43bee mercurial/util.py
> --- a/mercurial/util.py Fri Nov 15 23:28:43 2013 -0500
> +++ b/mercurial/util.py Sat Jan 11 14:38:21 2014 +0530
> @@ -14,7 +14,7 @@
>  """
>
>  from i18n import _
> -import error, osutil, encoding
> +import error, osutil, encoding, glob
>  import errno, re, shutil, sys, tempfile, traceback
>  import os, time, datetime, calendar, textwrap, signal, collections
>  import imp, socket, urllib
> @@ -1983,3 +1983,27 @@
>          self._hooks.sort(key=lambda x: x[0])
>          for source, hook in self._hooks:
>              hook(*args)
> +
> +def findhgexe(path=None):
> +    '''Find all executables containing `hg` in `path`.'''
> +
> +    matches = []
> +
> +    if (sys.platform == 'win32' or os.name == 'os2'):
> +        exe = 'hg*.exe'
> +    else:
> +        exe = 'hg*'

First off, note that I am totally incompetent on this matter.
But I have seen this patch sadly rottening here in the archives,
so I asked on IRC if somebody could comment.

Here I report verbatim what I got from Henrik Stuart.
If you want to discuss with him, look for hstuart on #mercurial (freenode).

--------------------------
hg can easily be started as hg.bat or hg.cmd
or any other variation of it; Windows contains
an environment variable, PATHEXT, that controls what
possible suffixes you can use
(honestly, I can't recall what OS/2 uses),
but that patch, as is, is not good enough [...].

also, usually, we move these os-specific things
into the respective files, which he has failed to do.

and if it's to detect multiple installs of hg, I don't see
why he searches for hg*
(I think he took away the wrong message from Matt;
it's not that he should search for hg*, it's that he should
use for a command with the same name as the one that was started)
--------------------------

This is it.
Can you please ponder these suggestions?

Cheers,
GGhh

> +
> +    if path is None:
> +        path = os.environ['PATH']
> +    paths = path.split(os.pathsep)
> +
> +    for p in paths:
> +        matchstr = p + '/' + exe
> +        allexes = [f for f in glob.glob(matchstr) if
> +                   all(['ssh' not in f, 'web' not in f, 'editor' not in
f])]
> +        for f in allexes:
> +            if os.path.isfile(f):
> +                matches.append(f)
> +
> +    return matches
>
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
>
Pierre-Yves David - April 14, 2014, 4:12 a.m.
V3 of this patches seems to have never existed.

Shall we hope for an updated version?

On 02/18/2014 02:35 AM, Giovanni Gherdovich wrote:
> Hello Prasoon,
>
> Sorry for the late reply to your patch.
>
> On Sat, Jan 11, 2014 at 10:24 AM, Prasoon Shukla
> <prasoon92.iitr@gmail.com <mailto:prasoon92.iitr@gmail.com>> wrote:
>  >
>  > Version 2 of patch for issue 4141. Tests have not been added because
>  > there is not support for multiline matching. The problems mentioned
>  > by Matt have been fixed. Please see this thread for details:
>  >
> http://mercurial.808500.n3.nabble.com/PATCH-debuginstall-display-warning-when-multiple-installs-are-present-issue4141-td4006273.html
>  >
>
> Uhm, you added some text on the top of the patch.
> Here we usually send patches in the body of emails,
> but we don't compose them by hand: we use the patchbomb extension
> to send the emails automatically.
> Are you using patchbomb too?
> If not, I encourage you to, it's the safest way:
> http://mercurial.selenic.com/wiki/ContributingChanges#Emailing_patches
>
> I guess that adding text not related to the patch also complicates
> the job of the mantainers who apply them to their repo.
>
>  > # HG changeset patch
>  > # User Prasoon Shukla <prasoon92.iitr@gmail.com
> <mailto:prasoon92.iitr@gmail.com>>
>  > # Date 1389431301 -19800
>  > # Node ID 5d9942f43bee3e6c04c24cba24b340be5d7ab6cc
>  > # Parent  d2704c48f4176d8cd6f21d33500820d44763585c
>  > 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 d2704c48f417 -r 5d9942f43bee mercurial/commands.py
>  > --- a/mercurial/commands.py Fri Nov 15 23:28:43 2013 -0500
>  > +++ b/mercurial/commands.py Sat Jan 11 14:38:21 2014 +0530
>  > @@ -8,7 +8,7 @@
>  >  from node import hex, bin, nullid, nullrev, short
>  >  from lock import release
>  >  from i18n import _
>  > -import os, re, difflib, time, tempfile, errno
>  > +import os, sys, re, difflib, time, tempfile, errno
>  >  import hg, scmutil, util, revlog, copies, error, bookmarks
>  >  import patch, help, encoding, templatekw, discovery
>  >  import archival, changegroup, cmdutil, hbisect
>  > @@ -2047,6 +2047,16 @@
>  >
>  >      problems = 0
>  >
>  > +    # installations
>  > +    installs = util.findhgexe()
>  > +    if not installs:
>  > +        ui.warn(_("warning! no file named hg found in path"))
>  > +    else:
>  > +        ui.status(_("checking hg path (%s)...\n" % sys.argv[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:
>  > diff -r d2704c48f417 -r 5d9942f43bee mercurial/util.py
>  > --- a/mercurial/util.py Fri Nov 15 23:28:43 2013 -0500
>  > +++ b/mercurial/util.py Sat Jan 11 14:38:21 2014 +0530
>  > @@ -14,7 +14,7 @@
>  >  """
>  >
>  >  from i18n import _
>  > -import error, osutil, encoding
>  > +import error, osutil, encoding, glob
>  >  import errno, re, shutil, sys, tempfile, traceback
>  >  import os, time, datetime, calendar, textwrap, signal, collections
>  >  import imp, socket, urllib
>  > @@ -1983,3 +1983,27 @@
>  >          self._hooks.sort(key=lambda x: x[0])
>  >          for source, hook in self._hooks:
>  >              hook(*args)
>  > +
>  > +def findhgexe(path=None):
>  > +    '''Find all executables containing `hg` in `path`.'''
>  > +
>  > +    matches = []
>  > +
>  > +    if (sys.platform == 'win32' or os.name <http://os.name> == 'os2'):
>  > +        exe = 'hg*.exe'
>  > +    else:
>  > +        exe = 'hg*'
>
> First off, note that I am totally incompetent on this matter.
> But I have seen this patch sadly rottening here in the archives,
> so I asked on IRC if somebody could comment.
>
> Here I report verbatim what I got from Henrik Stuart.
> If you want to discuss with him, look for hstuart on #mercurial (freenode).
>
> --------------------------
> hg can easily be started as hg.bat or hg.cmd
> or any other variation of it; Windows contains
> an environment variable, PATHEXT, that controls what
> possible suffixes you can use
> (honestly, I can't recall what OS/2 uses),
> but that patch, as is, is not good enough [...].
>
> also, usually, we move these os-specific things
> into the respective files, which he has failed to do.
>
> and if it's to detect multiple installs of hg, I don't see
> why he searches for hg*
> (I think he took away the wrong message from Matt;
> it's not that he should search for hg*, it's that he should
> use for a command with the same name as the one that was started)
> --------------------------
>
> This is it.
> Can you please ponder these suggestions?
>
> Cheers,
> GGhh
>
>  > +
>  > +    if path is None:
>  > +        path = os.environ['PATH']
>  > +    paths = path.split(os.pathsep)
>  > +
>  > +    for p in paths:
>  > +        matchstr = p + '/' + exe
>  > +        allexes = [f for f in glob.glob(matchstr) if
>  > +                   all(['ssh' not in f, 'web' not in f, 'editor' not
> in f])]
>  > +        for f in allexes:
>  > +            if os.path.isfile(f):
>  > +                matches.append(f)
>  > +
>  > +    return matches
>  >
>  > _______________________________________________
>  > Mercurial-devel mailing list
>  > Mercurial-devel@selenic.com <mailto:Mercurial-devel@selenic.com>
>  > http://selenic.com/mailman/listinfo/mercurial-devel
>  >
>
>
>
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
>

Patch

diff -r d2704c48f417 -r 5d9942f43bee mercurial/commands.py
--- a/mercurial/commands.py Fri Nov 15 23:28:43 2013 -0500
+++ b/mercurial/commands.py Sat Jan 11 14:38:21 2014 +0530
@@ -8,7 +8,7 @@ 
 from node import hex, bin, nullid, nullrev, short
 from lock import release
 from i18n import _
-import os, re, difflib, time, tempfile, errno
+import os, sys, re, difflib, time, tempfile, errno
 import hg, scmutil, util, revlog, copies, error, bookmarks
 import patch, help, encoding, templatekw, discovery
 import archival, changegroup, cmdutil, hbisect
@@ -2047,6 +2047,16 @@ 

     problems = 0

+    # installations
+    installs = util.findhgexe()
+    if not installs:
+        ui.warn(_("warning! no file named hg found in path"))
+    else:
+        ui.status(_("checking hg path (%s)...\n" % sys.argv[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:
diff -r d2704c48f417 -r 5d9942f43bee mercurial/util.py
--- a/mercurial/util.py Fri Nov 15 23:28:43 2013 -0500
+++ b/mercurial/util.py Sat Jan 11 14:38:21 2014 +0530
@@ -14,7 +14,7 @@ 
 """

 from i18n import _
-import error, osutil, encoding
+import error, osutil, encoding, glob
 import errno, re, shutil, sys, tempfile, traceback
 import os, time, datetime, calendar, textwrap, signal, collections
 import imp, socket, urllib
@@ -1983,3 +1983,27 @@ 
         self._hooks.sort(key=lambda x: x[0])
         for source, hook in self._hooks:
             hook(*args)
+
+def findhgexe(path=None):
+    '''Find all executables containing `hg` in `path`.'''
+
+    matches = []
+
+    if (sys.platform == 'win32' or os.name == 'os2'):
+        exe = 'hg*.exe'
+    else:
+        exe = 'hg*'
+
+    if path is None:
+        path = os.environ['PATH']
+    paths = path.split(os.pathsep)
+
+    for p in paths:
+        matchstr = p + '/' + exe
+        allexes = [f for f in glob.glob(matchstr) if
+                   all(['ssh' not in f, 'web' not in f, 'editor' not in
f])]
+        for f in allexes:
+            if os.path.isfile(f):
+                matches.append(f)
+