From c978b2b65a227d00e6e8bd512b44f35797fbab9d Mon Sep 17 00:00:00 2001 From: Paul Beckingham Date: Sun, 3 Mar 2013 09:34:54 -0500 Subject: [PATCH] Transport Errors - The 'push' and 'pull' commands now properly distinguish between a missing transport utility and other errors (thanks to Russell Steicke). --- AUTHORS | 1 + ChangeLog | 2 + NEWS | 1 + src/Transport.cpp | 3 +- src/TransportCurl.cpp | 2 +- src/TransportRSYNC.cpp | 78 ++++++++++++++++++------------------ src/TransportSSH.cpp | 90 +++++++++++++++++++++--------------------- src/en-US.h | 2 +- src/es-ES.h | 6 +-- src/util.cpp | 2 +- test/bug.668.t | 4 +- 11 files changed, 98 insertions(+), 93 deletions(-) diff --git a/AUTHORS b/AUTHORS index f50c9da04..668f377b5 100644 --- a/AUTHORS +++ b/AUTHORS @@ -82,6 +82,7 @@ The following submitted code, packages or analysis, and deserve special thanks: Vincent Petithory Rainer Müller Jakub Wilk + Russell Steicke Thanks to the following, who submitted detailed bug reports and excellent suggestions: diff --git a/ChangeLog b/ChangeLog index 72b2a90f0..11cde4463 100644 --- a/ChangeLog +++ b/ChangeLog @@ -35,6 +35,8 @@ Features + Fixed the mechanism used for selecting translations (thanks to Fidel Mato). + Added new export script: export-tsv.pl. + Added the configuration variable 'print.empty.columns'. + + The 'push' and 'pull' commands now properly distinguish between a missing + transport utility and other errors (thanks to Russell Steicke). Bugs + Fixed bug #642, so that the default 'data.location=~/.task' preserves the diff --git a/NEWS b/NEWS index ad34e0e20..1ce7be678 100644 --- a/NEWS +++ b/NEWS @@ -11,6 +11,7 @@ New Features in taskwarrior 2.2.0 - The 'columns' command now supports search term for the column name. - New date shortcuts, 'socm' and 'eocm', meaning start and end of current month. + - Improved error messages for 'push' and 'pull' transport problems. New commands in taskwarrior 2.2.0 diff --git a/src/Transport.cpp b/src/Transport.cpp index 37f5efd56..86b10921c 100644 --- a/src/Transport.cpp +++ b/src/Transport.cpp @@ -93,7 +93,8 @@ int Transport::execute() } int result = ::execute (_executable, _arguments); int err; - switch (result) { + switch (result) + { case 127: throw format (STRING_TRANSPORT_NORUN, _executable); case -1: diff --git a/src/TransportCurl.cpp b/src/TransportCurl.cpp index b436a3b18..bd1e9fe47 100644 --- a/src/TransportCurl.cpp +++ b/src/TransportCurl.cpp @@ -35,7 +35,7 @@ //////////////////////////////////////////////////////////////////////////////// TransportCurl::TransportCurl(const Uri& uri) : Transport(uri) { - _executable = "curl"; + _executable = "curl"; } //////////////////////////////////////////////////////////////////////////////// diff --git a/src/TransportRSYNC.cpp b/src/TransportRSYNC.cpp index 70908bcd2..c6c852ef5 100644 --- a/src/TransportRSYNC.cpp +++ b/src/TransportRSYNC.cpp @@ -34,68 +34,68 @@ //////////////////////////////////////////////////////////////////////////////// TransportRSYNC::TransportRSYNC(const Uri& uri) : Transport(uri) { - _executable = "rsync"; + _executable = "rsync"; } //////////////////////////////////////////////////////////////////////////////// void TransportRSYNC::send(const std::string& source) { - if (_uri._host == "") - throw std::string (STRING_TRANSPORT_RSYNC_URI); + if (_uri._host == "") + throw std::string (STRING_TRANSPORT_RSYNC_URI); - // Is there more than one file to transfer? - // Then path has to end with a '/' - if (is_filelist(source) && !_uri.is_directory()) + // Is there more than one file to transfer? + // Then path has to end with a '/' + if (is_filelist(source) && !_uri.is_directory()) throw format (STRING_TRANSPORT_URI_NODIR, _uri._path); - // cmd line is: rsync [--port=PORT] source [user@]host::path - if (_uri._port != "") - { - _arguments.push_back ("--port=" + _uri._port); - } + // cmd line is: rsync [--port=PORT] source [user@]host::path + if (_uri._port != "") + { + _arguments.push_back ("--port=" + _uri._port); + } - _arguments.push_back (source); + _arguments.push_back (source); - if (_uri._user != "") - { - _arguments.push_back (_uri._user + "@" + _uri._host + "::" + _uri._path); - } - else - { - _arguments.push_back (_uri._host + "::" + _uri._path); - } + if (_uri._user != "") + { + _arguments.push_back (_uri._user + "@" + _uri._host + "::" + _uri._path); + } + else + { + _arguments.push_back (_uri._host + "::" + _uri._path); + } - if (execute ()) + if (execute ()) throw std::string (STRING_TRANSPORT_RSYNC_FAIL); } //////////////////////////////////////////////////////////////////////////////// void TransportRSYNC::recv(std::string target) { - if (_uri._host == "") - throw std::string (STRING_TRANSPORT_RSYNC_URI); + if (_uri._host == "") + throw std::string (STRING_TRANSPORT_RSYNC_URI); - // Is there more than one file to transfer? - // Then target has to end with a '/' - if (is_filelist(_uri._path) && !is_directory(target)) + // Is there more than one file to transfer? + // Then target has to end with a '/' + if (is_filelist(_uri._path) && !is_directory(target)) throw format (STRING_TRANSPORT_URI_NODIR, target); - // cmd line is: rsync [--port=PORT] [user@]host::path target - if (_uri._port != "") - _arguments.push_back ("--port=" + _uri._port); + // cmd line is: rsync [--port=PORT] [user@]host::path target + if (_uri._port != "") + _arguments.push_back ("--port=" + _uri._port); - if (_uri._user != "") - { - _arguments.push_back (_uri._user + "@" + _uri._host + "::" + _uri._path); - } - else - { - _arguments.push_back (_uri._host + "::" + _uri._path); - } + if (_uri._user != "") + { + _arguments.push_back (_uri._user + "@" + _uri._host + "::" + _uri._path); + } + else + { + _arguments.push_back (_uri._host + "::" + _uri._path); + } - _arguments.push_back (target); + _arguments.push_back (target); - if (execute ()) + if (execute ()) throw std::string (STRING_TRANSPORT_RSYNC_FAIL); } diff --git a/src/TransportSSH.cpp b/src/TransportSSH.cpp index c42cbaa39..b9eacaf7c 100644 --- a/src/TransportSSH.cpp +++ b/src/TransportSSH.cpp @@ -35,73 +35,73 @@ //////////////////////////////////////////////////////////////////////////////// TransportSSH::TransportSSH(const Uri& uri) : Transport(uri) { - _executable = "scp"; + _executable = "scp"; } //////////////////////////////////////////////////////////////////////////////// void TransportSSH::send(const std::string& source) { - if (_uri._host == "") - throw std::string (STRING_TRANSPORT_SSH_URI); + if (_uri._host == "") + throw std::string (STRING_TRANSPORT_SSH_URI); - // Is there more than one file to transfer? - // Then path has to end with a '/' - if (is_filelist(source) && !_uri.is_directory()) + // Is there more than one file to transfer? + // Then path has to end with a '/' + if (is_filelist(source) && !_uri.is_directory()) throw format (STRING_TRANSPORT_URI_NODIR, _uri._path); - // cmd line is: scp [-p port] [user@]host:path - if (_uri._port != "") - { - _arguments.push_back ("-P"); - _arguments.push_back (_uri._port); - } + // cmd line is: scp [-p port] [user@]host:path + if (_uri._port != "") + { + _arguments.push_back ("-P"); + _arguments.push_back (_uri._port); + } - _arguments.push_back (source); + _arguments.push_back (source); - if (_uri._user != "") - { - _arguments.push_back (_uri._user + "@" + _uri._host + ":" + escape (_uri._path, ' ')); - } - else - { - _arguments.push_back (_uri._host + ":" + escape (_uri._path, ' ')); - } + if (_uri._user != "") + { + _arguments.push_back (_uri._user + "@" + _uri._host + ":" + escape (_uri._path, ' ')); + } + else + { + _arguments.push_back (_uri._host + ":" + escape (_uri._path, ' ')); + } - if (execute ()) - throw std::string (STRING_TRANSPORT_SSH_FAIL); + if (execute ()) + throw std::string (STRING_TRANSPORT_SSH_FAIL); } //////////////////////////////////////////////////////////////////////////////// void TransportSSH::recv(std::string target) { - if (_uri._host == "") - throw std::string (STRING_TRANSPORT_SSH_URI); + if (_uri._host == "") + throw std::string (STRING_TRANSPORT_SSH_URI); - // Is there more than one file to transfer? - // Then target has to end with a '/' - if (is_filelist(_uri._path) && !is_directory(target)) + // Is there more than one file to transfer? + // Then target has to end with a '/' + if (is_filelist(_uri._path) && !is_directory(target)) throw format (STRING_TRANSPORT_URI_NODIR, target); - // cmd line is: scp [-p port] [user@]host:path - if (_uri._port != "") - { - _arguments.push_back ("-P"); - _arguments.push_back (_uri._port); - } + // cmd line is: scp [-p port] [user@]host:path + if (_uri._port != "") + { + _arguments.push_back ("-P"); + _arguments.push_back (_uri._port); + } - if (_uri._user != "") - { - _arguments.push_back (_uri._user + "@" + _uri._host + ":" + escape (_uri._path, ' ')); - } - else - { - _arguments.push_back (_uri._host + ":" + escape (_uri._path, ' ')); - } + if (_uri._user != "") + { + _arguments.push_back (_uri._user + "@" + _uri._host + ":" + escape (_uri._path, ' ')); + } + else + { + _arguments.push_back (_uri._host + ":" + escape (_uri._path, ' ')); + } - _arguments.push_back (target); + _arguments.push_back (target); - if (execute ()) - throw std::string (STRING_TRANSPORT_SSH_FAIL); + if (execute ()) + throw std::string (STRING_TRANSPORT_SSH_FAIL); } //////////////////////////////////////////////////////////////////////////////// diff --git a/src/en-US.h b/src/en-US.h index 9eccb4954..e8e37f894 100644 --- a/src/en-US.h +++ b/src/en-US.h @@ -838,7 +838,7 @@ // Transport #define STRING_TRANSPORT_NORUN "Could not run '{1}'. Is it installed, and available in $PATH?" -#define STRING_TRANSPORT_NOFORK "Could not run '{1}': {2}. Are you out of system resources?" +#define STRING_TRANSPORT_NOFORK "Could not run '{1}': {2}. Are you out of system resources?" #define STRING_TRANSPORT_URI_NODIR "The uri '{1}' does not appear to be a directory." #define STRING_TRANSPORT_CURL_URI "When using the 'curl' protocol, the uri must contain a hostname." #define STRING_TRANSPORT_CURL_WILDCD "When using the 'curl' protocol, wildcards are not supported." diff --git a/src/es-ES.h b/src/es-ES.h index 89ad6db74..eeef59491 100644 --- a/src/es-ES.h +++ b/src/es-ES.h @@ -853,16 +853,16 @@ #define STRING_TEXT_AMBIGUOUS "Ambiguo {1} '{2}' - puede ser " // Transport -#define STRING_TRANSPORT_NORUN "Could not run '{1}'. Is it installed, and available in $PATH?" +#define STRING_TRANSPORT_NORUN "No se pudo lanzar '{1}'. ¿Está instalado y disponible en $PATH?" #define STRING_TRANSPORT_NOFORK "Could not run '{1}': {2}. Are you out of system resources?" #define STRING_TRANSPORT_URI_NODIR "El uri '{1}' no parece ser un directorio." #define STRING_TRANSPORT_CURL_URI "Cuando se usa el protocolo 'curl' el uri debe contener un nombre de máquina." #define STRING_TRANSPORT_CURL_WILDCD "Cuando se usa el protocolo 'curl' no están soportados los comodines." #define STRING_TRANSPORT_CURL_FAIL "Curl falló, consulte los mensajes precedentes." #define STRING_TRANSPORT_RSYNC_URI "Cuando se usa el protocolo 'rsync' el uri debe contener un nombre de máquina." -#define STRING_TRANSPORT_RSYNC_FAIL "rsync failed, see output above." +#define STRING_TRANSPORT_RSYNC_FAIL "Rsync falló, consulte los mensajes precedentes." #define STRING_TRANSPORT_SSH_URI "Cuando se usa el protocolo 'ssh' el uri debe contener un nombre de máquina." -#define STRING_TRANSPORT_SSH_FAIL "ssh failed, see output above." +#define STRING_TRANSPORT_SSH_FAIL "Ssh falló, consulte los mensajes precedentes." // Uri #define STRING_URI_QUOTES "No se pudo interpretar el uri '{1}', uso erróneo de comillas simples." diff --git a/src/util.cpp b/src/util.cpp index 98a399ece..17bdb83d5 100644 --- a/src/util.cpp +++ b/src/util.cpp @@ -427,7 +427,7 @@ int execute(const std::string& executable, std::vector arguments) char** argv = new char*[4]; argv[0] = shell; // sh argv[1] = opt; // -c - argv[2] = (char*)cmdline.c_str(); // e.g. scp undo.data user@host:.task/ + argv[2] = (char*)cmdline.c_str(); // e.g. scp undo.data user@host:.task/ argv[3] = NULL; // required by execv int ret = execvp(shell, argv); diff --git a/test/bug.668.t b/test/bug.668.t index fff0d1f16..92ff8d33b 100755 --- a/test/bug.668.t +++ b/test/bug.668.t @@ -41,12 +41,12 @@ if (open my $fh, '>', 'bug.rc') # Bug 668: URL should allow users with dot character my $output = qx{../src/task rc:bug.rc merge user.name\@taskwarrior.org:undo.data 2>&1}; -like ($output, qr/^Could not run ssh. Is it installed, and available in \$PATH\?$/m, 'ssh does not connect'); +like ($output, qr/ssh failed/, 'ssh does not connect'); unlike ($output, qr/not a valid modifier/, 'scp syntax with dots'); unlike ($output, qr/not in the expected format/, 'scp syntax with dots'); $output = qx{../src/task rc:bug.rc merge ssh://user.name\@taskwarrior.org/undo.data 2>&1}; -like ($output, qr/^Could not run ssh. Is it installed, and available in \$PATH\?$/m, 'ssh does not connect'); +like ($output, qr/ssh failed/, 'ssh does not connect'); unlike ($output, qr/not a valid modifier/, 'standard syntax with dots'); unlike ($output, qr/not in the expected format/, 'standard syntax with dots');