Submitter | anatoly techtonik |
---|---|
Date | June 10, 2014, 11:57 a.m. |
Message ID | <917c6c9356d47f3d3a97.1402401431@BlackBox> |
Download | mbox | patch |
Permalink | /patch/4963/ |
State | Superseded |
Headers | show |
Comments
On 10 June 2014, anatoly techtonik said: > +def moduleversion(module): > + """gets version information from the module > + > + This code was inspired by Django Debug Toolbar's VersionDebugPanel. > + """ > + extension = module > + if hasattr(extension, 'get_version'): > + get_version = extension.get_version > + if callable(get_version): > + version = get_version() > + else: > + version = get_version > + elif hasattr(extension, 'VERSION'): > + version = extension.VERSION > + elif hasattr(extension, '__version__'): > + version = extension.__version__ > + else: > + version = '' > + if isinstance(version, (list, tuple)): > + version = '.'.join(str(o) for o in version) > + return version 1) If this code needs to exist, IMHO it belongs in mercurial.extensions 2) Too complex! IMHO we should just support __version__, and we should document that in http://mercurial.selenic.com/wiki/PublishingExtensions. "There should one, and preferably only one, way to do it." Greg
On Tue, Jun 10, 2014 at 7:57 AM, anatoly techtonik <techtonik@gmail.com> wrote: > # HG changeset patch > # User anatoly techtonik <techtonik@gmail.com> > # Date 1402401369 -10800 > # Tue Jun 10 14:56:09 2014 +0300 > # Branch stable > # Node ID 917c6c9356d47f3d3a97ca45967a1f91813c771a > # Parent bd3150aeb5949c8ca2304bf52c6206730b73a8f9 > version: add extensions version info (issue4209) > > Query extensions for version info and format nicely into columns > > diff -r bd3150aeb594 -r 917c6c9356d4 mercurial/commands.py > --- a/mercurial/commands.py Tue Jun 10 13:44:37 2014 +0300 > +++ b/mercurial/commands.py Tue Jun 10 14:56:09 2014 +0300 > @@ -14,6 +14,7 @@ > import patch, help, encoding, templatekw, discovery > import archival, changegroup, cmdutil, hbisect > import sshserver, hgweb, commandserver > +from extensions import extensions I'm a /little/ worried this might lead to import cycles with extensions, but I think I've talked myself out of that because of how extension loading happens. > from hgweb import server as hgweb_server > import merge as mergemod > import minirst, revset, fileset > @@ -5908,6 +5909,29 @@ > """ > return hg.verify(repo) > > + > +def moduleversion(module): > + """gets version information from the module > + > + This code was inspired by Django Debug Toolbar's VersionDebugPanel. > + """ > + extension = module > + if hasattr(extension, 'get_version'): > + get_version = extension.get_version > + if callable(get_version): > + version = get_version() > + else: > + version = get_version > + elif hasattr(extension, 'VERSION'): > Use mercurial.util.safehasattr. See http://doc.bazaar.canonical.com/developers/code-style.html#hasattr-and-getattr for why. Also, make sure to run 'make test-check-code-hg.t' which should have warned you about this. > + version = extension.VERSION > + elif hasattr(extension, '__version__'): > + version = extension.__version__ > + else: > + version = '' > + if isinstance(version, (list, tuple)): > + version = '.'.join(str(o) for o in version) > + return version > + > @command('version', []) > def version_(ui): > """output version and copyright information""" > @@ -5930,8 +5954,14 @@ > # by Markus Zapke-Gruendemann <info@keimlink.de> > ui.note(_("\nEnabled extensions:\n\n")) > if ui.verbose: > - for name, module in ui.configitems('extensions'): > - ui.write(" %s\n" % name) > + names = [] > + vers = [] > + for name, module in extensions(): > + names.append(name) > + vers.append(moduleversion(module)) > + maxnamelen = max(len(n) for n in names) > + for i,n in enumerate(names): > + ui.write(" %s %s\n" % (n.ljust(maxnamelen), vers[i])) > Not sure how others would feel, but you could probably make a rst table and then render it with minirst to avoid some of this hand formatting. > > > norepo = ("clone init version help debugcommands debugcomplete" > _______________________________________________ > Mercurial-devel mailing list > Mercurial-devel@selenic.com > http://selenic.com/mailman/listinfo/mercurial-devel >
On Tue, Jun 10, 2014 at 4:38 PM, Greg Ward <greg@gerg.ca> wrote: > On 10 June 2014, anatoly techtonik said: > > +def moduleversion(module): > > + """gets version information from the module > > + > > + This code was inspired by Django Debug Toolbar's VersionDebugPanel. > > + """ > > + extension = module > > + if hasattr(extension, 'get_version'): > > + get_version = extension.get_version > > + if callable(get_version): > > + version = get_version() > > + else: > > + version = get_version > > + elif hasattr(extension, 'VERSION'): > > + version = extension.VERSION > > + elif hasattr(extension, '__version__'): > > + version = extension.__version__ > > + else: > > + version = '' > > + if isinstance(version, (list, tuple)): > > + version = '.'.join(str(o) for o in version) > > + return version > > 1) If this code needs to exist, IMHO it belongs in mercurial.extensions > 2) Too complex! IMHO we should just support __version__, and we should > document that in http://mercurial.selenic.com/wiki/PublishingExtensions > . > I find it useful to support at least get_version(). get_version() may return human readable string in free format that can also contain critical libraries that extension uses. For example, see the output of hg-git patched with: https://bitbucket.org/durin42/hg-git/pull-request/32/add-version-info-to-hggit-itself-to-query/diff D:\>hg version -v ... Enabled extensions: churn color convert hggit 0.6.0 (Dulwich 0.9.6) strip ...
On Tue, Jun 10, 2014 at 4:57 PM, Augie Fackler <lists@durin42.com> wrote: > > +from extensions import extensions > > > I'm a /little/ worried this might lead to import cycles with extensions, > but I think I've talked myself out of that because of how extension loading > happens. > Is it acceptable to make this import local to version command? > from hgweb import server as hgweb_server >> import merge as mergemod >> import minirst, revset, fileset >> @@ -5908,6 +5909,29 @@ >> """ >> return hg.verify(repo) >> >> + >> +def moduleversion(module): >> + """gets version information from the module >> + >> + This code was inspired by Django Debug Toolbar's VersionDebugPanel. >> + """ >> + extension = module >> + if hasattr(extension, 'get_version'): >> + get_version = extension.get_version >> + if callable(get_version): >> + version = get_version() >> + else: >> + version = get_version >> + elif hasattr(extension, 'VERSION'): >> > > Use mercurial.util.safehasattr. See > http://doc.bazaar.canonical.com/developers/code-style.html#hasattr-and-getattr > for why. > > Also, make sure to run 'make test-check-code-hg.t' which should have > warned you about this. > I am on Windoze with no make, so it is just too much to setup. > + version = extension.VERSION >> + elif hasattr(extension, '__version__'): >> + version = extension.__version__ >> + else: >> + version = '' >> + if isinstance(version, (list, tuple)): >> + version = '.'.join(str(o) for o in version) >> + return version >> + >> @command('version', []) >> def version_(ui): >> """output version and copyright information""" >> @@ -5930,8 +5954,14 @@ >> # by Markus Zapke-Gruendemann <info@keimlink.de> >> ui.note(_("\nEnabled extensions:\n\n")) >> if ui.verbose: >> - for name, module in ui.configitems('extensions'): >> - ui.write(" %s\n" % name) >> + names = [] >> + vers = [] >> + for name, module in extensions(): >> + names.append(name) >> + vers.append(moduleversion(module)) >> + maxnamelen = max(len(n) for n in names) >> + for i,n in enumerate(names): >> + ui.write(" %s %s\n" % (n.ljust(maxnamelen), vers[i])) >> > > Not sure how others would feel, but you could probably make a rst table > and then render it with minirst to avoid some of this hand formatting. > But for that you need to create rst table first, which is the same problem. I'd appreciate if python stdlib included such function somewhere along textwrap utils. So, how to send updates to this patch - edit commits and resend to mailing list with the same subject?
On Jun 10, 2014, at 6:30 PM, anatoly techtonik <techtonik@gmail.com> wrote: > On Tue, Jun 10, 2014 at 4:57 PM, Augie Fackler <lists@durin42.com> wrote: > >> >> +from extensions import extensions >> >> >> I'm a /little/ worried this might lead to import cycles with extensions, >> but I think I've talked myself out of that because of how extension loading >> happens. >> > > Is it acceptable to make this import local to version command? I'd rather not - leave it at top level and we'll move it later if it ends up being a problem. > > >> from hgweb import server as hgweb_server >>> import merge as mergemod >>> import minirst, revset, fileset >>> @@ -5908,6 +5909,29 @@ >>> """ >>> return hg.verify(repo) >>> >>> + >>> +def moduleversion(module): >>> + """gets version information from the module >>> + >>> + This code was inspired by Django Debug Toolbar's VersionDebugPanel. >>> + """ >>> + extension = module >>> + if hasattr(extension, 'get_version'): >>> + get_version = extension.get_version >>> + if callable(get_version): >>> + version = get_version() >>> + else: >>> + version = get_version >>> + elif hasattr(extension, 'VERSION'): >>> >> >> Use mercurial.util.safehasattr. See >> http://doc.bazaar.canonical.com/developers/code-style.html#hasattr-and-getattr >> for why. >> >> Also, make sure to run 'make test-check-code-hg.t' which should have >> warned you about this. >> > > I am on Windoze with no make, so it is just too much to setup. > > >> + version = extension.VERSION >>> + elif hasattr(extension, '__version__'): >>> + version = extension.__version__ >>> + else: >>> + version = '' >>> + if isinstance(version, (list, tuple)): >>> + version = '.'.join(str(o) for o in version) >>> + return version >>> + >>> @command('version', []) >>> def version_(ui): >>> """output version and copyright information""" >>> @@ -5930,8 +5954,14 @@ >>> # by Markus Zapke-Gruendemann <info@keimlink.de> >>> ui.note(_("\nEnabled extensions:\n\n")) >>> if ui.verbose: >>> - for name, module in ui.configitems('extensions'): >>> - ui.write(" %s\n" % name) >>> + names = [] >>> + vers = [] >>> + for name, module in extensions(): >>> + names.append(name) >>> + vers.append(moduleversion(module)) >>> + maxnamelen = max(len(n) for n in names) >>> + for i,n in enumerate(names): >>> + ui.write(" %s %s\n" % (n.ljust(maxnamelen), vers[i])) >>> >> >> Not sure how others would feel, but you could probably make a rst table >> and then render it with minirst to avoid some of this hand formatting. >> > > But for that you need to create rst table first, which is the same problem. > I'd appreciate if python stdlib included such function somewhere along > textwrap utils. > > > So, how to send updates to this patch - edit commits and resend to mailing > list with the same subject? Yes, edit the commits (probably with commit --amend), and then use 'hg email' with '--flag v2' so it's easy for us to spot the resend.
On 11 June 2014, anatoly techtonik said: > > Use mercurial.util.safehasattr. See > > http://doc.bazaar.canonical.com/developers/code-style.html#hasattr-and-getattr > > for why. > > > > Also, make sure to run 'make test-check-code-hg.t' which should have > > warned you about this. > > > > I am on Windoze with no make, so it is just too much to setup. The Makefile actually does very little, so it's easy to run tests manually. E.g. to run the whole test suite: cd tests ./run-tests.py -l and to run just a single test: ./run-tests.py -l test-check-code.t BTW, it's best to run the whole test suite before you submit a patch. You never know what test you might have accidentally broken! Greg
On Tue, Jun 10, 2014 at 5:40 PM, anatoly techtonik <techtonik@gmail.com> wrote: > 2) Too complex! IMHO we should just support __version__, and we should >> document that in >> http://mercurial.selenic.com/wiki/PublishingExtensions. >> > > I find it useful to support at least get_version(). get_version() may > return human > readable string in free format that can also contain critical libraries > that extension > uses. > He's got a great point: for hgsubversion we generally need to know both the version of hgsubversion and the bindings that are in use.
On Wed, Jun 11, 2014 at 2:50 PM, Greg Ward <greg@gerg.ca> wrote: > On 11 June 2014, anatoly techtonik said: > > > Use mercurial.util.safehasattr. See > > > > http://doc.bazaar.canonical.com/developers/code-style.html#hasattr-and-getattr > > > for why. > > > > > > Also, make sure to run 'make test-check-code-hg.t' which should have > > > warned you about this. > > > > > > > I am on Windoze with no make, so it is just too much to setup. > > The Makefile actually does very little, so it's easy to run tests > manually. E.g. to run the whole test suite: > > cd tests > ./run-tests.py -l > > and to run just a single test: > > ./run-tests.py -l test-check-code.t > > BTW, it's best to run the whole test suite before you submit a patch. > You never know what test you might have accidentally broken! No luck. >py run-tests.py -l ... Failed test-issue672.t: output changed and returned error code 1 INTERRUPTED: test-mq-qfold.t (after 0 seconds) # Ran 246 tests, 0 skipped, 0 warned, 246 failed. python hash seed: 115336818 >py run-tests.py -l test-check-code.t .. ERROR: test-check-code.t output changed and returned error code 1 ! Failed test-check-code.t: output changed and returned error code 1 # Ran 1 tests, 0 skipped, 0 warned, 1 failed. python hash seed: 832726526
On Wed, Jun 11, 2014 at 4:50 AM, Greg Ward <greg@gerg.ca> wrote: > On 11 June 2014, anatoly techtonik said: > [Greg] > > > Also, make sure to run 'make test-check-code-hg.t' which should have > > > warned you about this. > > > > > > > I am on Windoze with no make, so it is just too much to setup. > > The Makefile actually does very little, so it's easy to run tests > manually. E.g. to run the whole test suite: > > cd tests > ./run-tests.py -l > > and to run just a single test: > > ./run-tests.py -l test-check-code.t > > BTW, it's best to run the whole test suite before you submit a patch. > You never know what test you might have accidentally broken! > Setting up a virtual machine is easy enough on Windows, with VmWare Player and Oracle Virtual Box, and maybe MS Virtual PC. I'm not sure if there is open source equivalent (on Linux, there's Xen, isn't there?). i'm pretty comfortable with a Fedora install, but Ubuntu and Mint are more popular, especially for people just getting their feet wet with Linux. /dps
On Wed, Jun 18, 2014 at 2:42 AM, Dave S <snidely.too@gmail.com> wrote: > On Wed, Jun 11, 2014 at 4:50 AM, Greg Ward <greg@gerg.ca> wrote: > >> On 11 June 2014, anatoly techtonik said: >> > > [Greg] > >> > > Also, make sure to run 'make test-check-code-hg.t' which should have >> > > warned you about this. >> > > >> > >> > I am on Windoze with no make, so it is just too much to setup. >> >> The Makefile actually does very little, so it's easy to run tests >> manually. E.g. to run the whole test suite: >> >> cd tests >> ./run-tests.py -l >> >> and to run just a single test: >> >> ./run-tests.py -l test-check-code.t >> >> BTW, it's best to run the whole test suite before you submit a patch. >> You never know what test you might have accidentally broken! >> > > Setting up a virtual machine is easy enough on Windows, with VmWare Player > and Oracle Virtual Box, and maybe MS Virtual PC. I'm not sure if there is > open source equivalent (on Linux, there's Xen, isn't there?). > > i'm pretty comfortable with a Fedora install, but Ubuntu and Mint are more > popular, especially for people just getting their feet wet with Linux. > Given that I started all of this just to download NumPy repository from GitHub and avoid learn Git, I am not sure it was a wise choice given my goal. =) I'll see what I can do.
On Wed, Jun 18, 2014 at 2:42 AM, Dave S <snidely.too@gmail.com> wrote: > On Wed, Jun 11, 2014 at 4:50 AM, Greg Ward <greg@gerg.ca> wrote: > >> On 11 June 2014, anatoly techtonik said: >> > > [Greg] > >> > > Also, make sure to run 'make test-check-code-hg.t' which should have >> > > warned you about this. >> > > >> > >> > I am on Windoze with no make, so it is just too much to setup. >> >> The Makefile actually does very little, so it's easy to run tests >> manually. E.g. to run the whole test suite: >> >> cd tests >> ./run-tests.py -l >> >> and to run just a single test: >> >> ./run-tests.py -l test-check-code.t >> >> BTW, it's best to run the whole test suite before you submit a patch. >> You never know what test you might have accidentally broken! >> > > Setting up a virtual machine is easy enough on Windows, with VmWare Player > and Oracle Virtual Box, and maybe MS Virtual PC. I'm not sure if there is > open source equivalent (on Linux, there's Xen, isn't there?). > I've setup drone.io to run tests, but they seem to fail. What's wrong? https://drone.io/bitbucket.org/techtonik/hg/2 I also expected instruction about running tests in http://selenic.com/hg/file/c00822e0b8ea/README as a sanity check after checkout. > i'm pretty comfortable with a Fedora install, but Ubuntu and Mint are more > popular, especially for people just getting their feet wet with Linux. > I've got very comfortable and speedy gumboots from http://farmanager.com/ and while mc looks like a substitute, it makes my feet bleed and long days with wet foot in bare Linux console get me a cold.
On 18 June 2014, anatoly techtonik said: > I've setup drone.io to run tests Oh nice! I didn't know about drone.io. Glad to see someone has done a more generalized version of travis. > but they seem to fail. What's wrong? Well, reading the output is instructive. $ hg ci -m updatesub - created new head + abort: error getting current working directory: No such file or directory + [255] Mercurial's test output is presented as a diff: expected output versus actual output. If there is no diff, the test succeeds. In this case, the text expected created new head but what actually happened was abort: error getting current working directory: No such file or directory with an exit status of 255 (which is what you always get when hg aborts). IOW: there is something weird about the runtime environment where the tests are running. Maybe drone.io is broken, or maybe you set it up wrong. You can find out where precisely Mercurial is failing with a quick grep: $ grep -n 'error getting current working directory' `hg locate -I "**.py"` mercurial/dispatch.py:613: raise util.Abort(_("error getting current working directory: %s") % From the surrounding code: try: wd = os.getcwd() except OSError, e: raise util.Abort(_("error getting current working directory: %s") % e.strerror) Huh. That's not good. Any environment where you can't call os.getcwd() is not a working environment. Greg
On Thu, Jun 19, 2014 at 4:37 AM, Greg Ward <greg@gerg.ca> wrote: > On 18 June 2014, anatoly techtonik said: > > I've setup drone.io to run tests > > Oh nice! I didn't know about drone.io. Glad to see someone has done a > more generalized version of travis. And they've made it a 3-click setup for open source repositories. > $ grep -n 'error getting current working directory' `hg locate -I > "**.py"` > mercurial/dispatch.py:613: raise util.Abort(_("error getting > current working directory: %s") % > > From the surrounding code: > > try: > wd = os.getcwd() > except OSError, e: > raise util.Abort(_("error getting current working directory: %s") % > e.strerror) > > Huh. That's not good. Any environment where you can't call os.getcwd() > is not a working environment. The environment is ok: $ python -c 'import os; print os.getcwd()' /home/ubuntu/src/bitbucket.org/techtonik/hg From https://drone.io/bitbucket.org/techtonik/hg/7 So it is not a problem in os.getcwd()
On 20/06/2014 23:12, anatoly techtonik wrote: > On Thu, Jun 19, 2014 at 4:37 AM, Greg Ward <greg@gerg.ca > <mailto:greg@gerg.ca>> wrote: > > On 18 June 2014, anatoly techtonik said: > > I've setup drone.io <http://drone.io> to run tests > > Oh nice! I didn't know about drone.io <http://drone.io>. Glad to see > someone has done a > more generalized version of travis. > > > And they've made it a 3-click setup for open source repositories. > > > $ grep -n 'error getting current working directory' `hg locate -I > "**.py"` > mercurial/dispatch.py:613: raise util.Abort(_("error > getting current working directory: %s") % > > From the surrounding code: > > try: > wd = os.getcwd() > except OSError, e: > raise util.Abort(_("error getting current working directory: > %s") % > e.strerror) > > Huh. That's not good. Any environment where you can't call os.getcwd() > is not a working environment. > > > The environment is ok: > > $ python -c 'import os; print os.getcwd()' > /home/ubuntu/src/bitbucket.org/techtonik/hg > <http://bitbucket.org/techtonik/hg> > > From https://drone.io/bitbucket.org/techtonik/hg/7 > So it is not a problem in os.getcwd() It must be a bit subtle. I spent a couple of hours tracking this a few months ago and to me it looks like drone.io has a problem. It does fail deterministically, but not all instances of os.getcwd() fail. I did send a mail to the technical contact, but didn't get anything in return. Regards, Aurélien.
Patch
diff -r bd3150aeb594 -r 917c6c9356d4 mercurial/commands.py --- a/mercurial/commands.py Tue Jun 10 13:44:37 2014 +0300 +++ b/mercurial/commands.py Tue Jun 10 14:56:09 2014 +0300 @@ -14,6 +14,7 @@ import patch, help, encoding, templatekw, discovery import archival, changegroup, cmdutil, hbisect import sshserver, hgweb, commandserver +from extensions import extensions from hgweb import server as hgweb_server import merge as mergemod import minirst, revset, fileset @@ -5908,6 +5909,29 @@ """ return hg.verify(repo) + +def moduleversion(module): + """gets version information from the module + + This code was inspired by Django Debug Toolbar's VersionDebugPanel. + """ + extension = module + if hasattr(extension, 'get_version'): + get_version = extension.get_version + if callable(get_version): + version = get_version() + else: + version = get_version + elif hasattr(extension, 'VERSION'): + version = extension.VERSION + elif hasattr(extension, '__version__'): + version = extension.__version__ + else: + version = '' + if isinstance(version, (list, tuple)): + version = '.'.join(str(o) for o in version) + return version + @command('version', []) def version_(ui): """output version and copyright information""" @@ -5930,8 +5954,14 @@ # by Markus Zapke-Gruendemann <info@keimlink.de> ui.note(_("\nEnabled extensions:\n\n")) if ui.verbose: - for name, module in ui.configitems('extensions'): - ui.write(" %s\n" % name) + names = [] + vers = [] + for name, module in extensions(): + names.append(name) + vers.append(moduleversion(module)) + maxnamelen = max(len(n) for n in names) + for i,n in enumerate(names): + ui.write(" %s %s\n" % (n.ljust(maxnamelen), vers[i])) norepo = ("clone init version help debugcommands debugcomplete"