You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Karl Fogel <kf...@red-bean.com> on 2018/02/12 21:04:38 UTC
[PATCH] Better error message for svn bindings import in mailer.py.
Hi all. I think one of the import error messages in tools/hook-scripts/mailer/mailer.py is misleading (I discovered this while debugging the problem that Troy Curtis Jr. solved in r1823802). Any objections if I install this patch?
[[[
Give correct error message on failure to import Subversion Python bindings.
* tools/hook-scripts/mailer/mailer.py: If the Subversion Python bindings
could not be found for import at all, have the error message state
that clearly, instead of duplicating the error message used for
importing too old a version of the bindings.
]]]
Index: tools/hook-scripts/mailer/mailer.py
===================================================================
--- tools/hook-scripts/mailer/mailer.py (revision 1823993)
+++ tools/hook-scripts/mailer/mailer.py (working copy)
@@ -70,23 +70,21 @@
_MIN_SVN_VERSION = [1, 5, 0]
# Import the Subversion Python bindings, making sure they meet our
# minimum version requirements.
try:
import svn.fs
import svn.delta
import svn.repos
import svn.core
except ImportError:
- sys.stderr.write(
- "You need version %s or better of the Subversion Python bindings.\n" \
- % ".".join([str(x) for x in _MIN_SVN_VERSION]))
+ sys.stderr.write("Unable to import Subversion Python bindings.\n")
sys.exit(1)
if _MIN_SVN_VERSION > [svn.core.SVN_VER_MAJOR,
svn.core.SVN_VER_MINOR,
svn.core.SVN_VER_PATCH]:
sys.stderr.write(
"You need version %s or better of the Subversion Python bindings.\n" \
% ".".join([str(x) for x in _MIN_SVN_VERSION]))
sys.exit(1)
Re: [PATCH] Better error message for svn bindings import in
mailer.py.
Posted by Kenneth Porter <sh...@sewingwitch.com>.
On 2/12/2018 1:04 PM, Karl Fogel wrote:
> * tools/hook-scripts/mailer/mailer.py: If the Subversion Python bindings
> could not be found for import at all, have the error message state
> that clearly, instead of duplicating the error message used for
> importing too old a version of the bindings.
I was also bitten by this when testing on a secondary system that lacked
the bindings sub-package. I had to examine the source to understand what
was really wrong. So +1.
Re: [PATCH] Better error message for svn bindings import in
mailer.py.
Posted by "C. Michael Pilato" <cm...@collab.net>.
On 02/12/2018 04:04 PM, Karl Fogel wrote:
> Hi all. I think one of the import error messages in tools/hook-scripts/mailer/mailer.py is misleading (I discovered this while debugging the problem that Troy Curtis Jr. solved in r1823802). Any objections if I install this patch?
>
> [[[
> Give correct error message on failure to import Subversion Python bindings.
>
> * tools/hook-scripts/mailer/mailer.py: If the Subversion Python bindings
> could not be found for import at all, have the error message state
> that clearly, instead of duplicating the error message used for
> importing too old a version of the bindings.
> ]]]
>
> Index: tools/hook-scripts/mailer/mailer.py
> ===================================================================
> --- tools/hook-scripts/mailer/mailer.py (revision 1823993)
> +++ tools/hook-scripts/mailer/mailer.py (working copy)
> @@ -70,23 +70,21 @@
> _MIN_SVN_VERSION = [1, 5, 0]
>
> # Import the Subversion Python bindings, making sure they meet our
> # minimum version requirements.
> try:
> import svn.fs
> import svn.delta
> import svn.repos
> import svn.core
> except ImportError:
> - sys.stderr.write(
> - "You need version %s or better of the Subversion Python bindings.\n" \
> - % ".".join([str(x) for x in _MIN_SVN_VERSION]))
> + sys.stderr.write("Unable to import Subversion Python bindings.\n")
> sys.exit(1)
> if _MIN_SVN_VERSION > [svn.core.SVN_VER_MAJOR,
> svn.core.SVN_VER_MINOR,
> svn.core.SVN_VER_PATCH]:
> sys.stderr.write(
> "You need version %s or better of the Subversion Python bindings.\n" \
> % ".".join([str(x) for x in _MIN_SVN_VERSION]))
> sys.exit(1)
>
>
+1
Re: [PATCH] Better error message for svn bindings import in
mailer.py.
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Karl Fogel wrote on Sat, Feb 17, 2018 at 16:52:39 -0600:
> Daniel Shahaf <d....@daniel.shahaf.name> writes:
> >It would be useful to also print the caught exception in case it contains more
> >specific information about the cause of the problem…
> >
> >… but in that case, why not remove the try/except entirely?
>
> That's a good point. I already committed my original proposed change in
> r1824080. Would you like to follow up with a removal of the try/except?
Done: r1824690.
Re: [PATCH] Better error message for svn bindings import in mailer.py.
Posted by Karl Fogel <kf...@red-bean.com>.
Daniel Shahaf <d....@daniel.shahaf.name> writes:
>It would be useful to also print the caught exception in case it contains more
>specific information about the cause of the problem…
>
>… but in that case, why not remove the try/except entirely?
That's a good point. I already committed my original proposed change in r1824080. Would you like to follow up with a removal of the try/except?
Re: [PATCH] Better error message for svn bindings import in mailer.py.
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Karl Fogel wrote on Mon, 12 Feb 2018 15:04 -0600:
> +++ tools/hook-scripts/mailer/mailer.py (working copy)
> @@ -70,23 +70,21 @@
> _MIN_SVN_VERSION = [1, 5, 0]
>
> # Import the Subversion Python bindings, making sure they meet our
> # minimum version requirements.
> try:
> import svn.fs
> import svn.delta
> import svn.repos
> import svn.core
> except ImportError:
> - sys.stderr.write(
> - "You need version %s or better of the Subversion Python bindings.\n" \
> - % ".".join([str(x) for x in _MIN_SVN_VERSION]))
> + sys.stderr.write("Unable to import Subversion Python bindings.\n")
> sys.exit(1)
It would be useful to also print the caught exception in case it contains more
specific information about the cause of the problem…
… but in that case, why not remove the try/except entirely?
> if _MIN_SVN_VERSION > [svn.core.SVN_VER_MAJOR,
> svn.core.SVN_VER_MINOR,
> svn.core.SVN_VER_PATCH]:
> sys.stderr.write(
> "You need version %s or better of the Subversion Python bindings.\n" \
> % ".".join([str(x) for x in _MIN_SVN_VERSION]))
> sys.exit(1)