On 31 Jan 2002, Karl Fogel wrote:
> Can you repost the whole patch each time? (Makes successive reviews
> easier...)
Re-posting the patch with the svn_boolean_t corrected, this time it is
complete with my suggested new "client_errors.h" header file too. (The
comments from before regarding the functionality still apply.)
--- a Fri Feb 1 07:56:13 2002
+++ client_errors.h Wed Jan 30 23:44:46 2002
@@ -0,0 +1,77 @@
+/*
+ * client_errors.h: error codes this command line client features
+ *
+ * ====================================================================
+ * Copyright (c) 2000-2002 CollabNet. All rights reserved.
+ *
+ * This software is licensed as described in the file COPYING, which
+ * you should have received as part of this distribution. The terms
+ * are also available at http://subversion.tigris.org/license-1.html.
+ * If newer versions of this license are posted there, you may use a
+ * newer version instead, at your option.
+ *
+ * This software consists of voluntary contributions made by many
+ * individuals. For exact contribution history, see the revision
+ * history and logs, available at http://subversion.tigris.org/.
+ * ====================================================================
+ */
+
+/* ==================================================================== */
+
+
+
+#ifndef SVN_CLIENT_ERRORS_H
+#define SVN_CLIENT_ERRORS_H
+
+#ifdef __cplusplus
+extern "C" {
+#endif /* __cplusplus */
+
+#define SVN_ERROR_START \
+ typedef enum svn_client_errno_t { \
+ SVN_ERR_CDMLINE__WARNING = SVN_ERR_LAST + 1,
+#define SVN_ERRDEF(n, s) n,
+#define SVN_ERROR_END SVN_ERR_CMDLINE__ERR_LAST } svn_client_errno_t;
+
+#define SVN_ERROR_ENUM_DEFINED
+
+/* Define custom command line client error numbers */
+
+SVN_ERROR_START
+
+ /* BEGIN Client errors */
+
+ SVN_ERRDEF (SVN_ERR_CMDLINE__FAILED_WRITING_TO_TEMPORARY_FILE,
+ "Failed writing to temporary file.")
+
+ SVN_ERRDEF (SVN_ERR_CMDLINE__FAILED_STAT_ON_TEMPORARY_FILE,
+ "Failed getting info about temporary file.")
+
+ SVN_ERRDEF (SVN_ERR_CMDLINE__FAILED_OPENING_TEMPORARY_FILE,
+ "Failed opening temporary file.")
+
+ SVN_ERRDEF (SVN_ERR_CMDLINE__FAILED_READING_TEMPORARY_FILE,
+ "Failed reading temporary file.")
+
+ /* END Client errors */
+
+
+SVN_ERROR_END
+
+#undef SVN_ERROR_START
+#undef SVN_ERRDEF
+#undef SVN_ERROR_END
+
+#ifdef __cplusplus
+}
+#endif /* __cplusplus */
+
+
+
+#endif /* SVN_CLIENT_ERRORS_H */
+
+/* ----------------------------------------------------------------
+ * local variables:
+ * eval: (load-file "../../svn-dev.el")
+ * end:
+ */
Index: ./commit-cmd.c
===================================================================
--- ./commit-cmd.c
+++ ./commit-cmd.c Thu Jan 31 00:29:27 2002
@@ -35,23 +35,32 @@
#include "svn_sorts.h"
#include "cl.h"
+#include "client_errors.h"
+
+
/*** Code. ***/
/*
* Prints a single status line to a given file about the given entry.
+ * Return failure, that means TRUE if something went wrong.
*/
-static void
-print_short_format (apr_file_t *file,
- const char *path,
- svn_wc_status_t *status)
+static svn_boolean_t
+print_single_file_status (apr_file_t *file,
+ const char *path,
+ svn_wc_status_t *status)
{
char str_status[5];
- char array[128];
+ char array[256]; /* the larger this is, the longer path names we allow in
+ the commit message editor file */
apr_size_t size;
+ apr_size_t written;
+ svn_boolean_t failure = FALSE;
+ apr_status_t rc;
if (! status)
- return;
+ /* this shouldn't happen */
+ return TRUE;
/* Create local-mod status code block. */
if (status->text_status != svn_wc_status_unversioned)
@@ -65,26 +74,36 @@
status->locked,
status->copied);
- apr_snprintf(array, 128, "SVN: %s %s\n", str_status, path);
+ apr_snprintf (array, sizeof (array),
+ "SVN: %s %s\n", str_status, path);
+
+ size = strlen (array);
- size = strlen(array);
+ rc = apr_file_write_full (file, array, size, &written);
- apr_file_write(file, array, &size);
+ if ((size != written) || (! APR_STATUS_IS_SUCCESS (rc)))
+ {
+ /* we didn't write the complete line, return an error code */
+ failure = TRUE;
+ }
}
+
+ return failure;
}
/*
* Walks throught the 'nelts' of the given hash and calls the status-
* print function for each.
*/
-void static
-print_status (apr_file_t *file,
- apr_hash_t *statushash,
- apr_pool_t *pool)
+static svn_boolean_t
+print_hash_status (apr_file_t *file,
+ apr_hash_t *statushash,
+ apr_pool_t *pool)
{
int i;
apr_array_header_t *statusarray;
svn_wc_status_t *status = NULL;
+ svn_boolean_t failure=FALSE;
/* Convert the unordered hash to an ordered, sorted array */
statusarray = apr_hash_sorted_keys (statushash,
@@ -96,11 +115,17 @@
{
const svn_item_t *item;
- item = &APR_ARRAY_IDX(statusarray, i, const svn_item_t);
+ item = &APR_ARRAY_IDX (statusarray, i, const svn_item_t);
status = item->value;
- print_short_format (file, item->key, status);
+ if (print_single_file_status (file, item->key, status))
+ {
+ failure = TRUE;
+ break;
+ }
}
+
+ return failure;
}
/*
@@ -108,11 +133,11 @@
* possibly edited in your favourite $EDITOR.
*/
static svn_error_t *
-write_status_to_file(apr_pool_t *pool,
- apr_file_t *file,
- svn_client_auth_baton_t *auth_baton,
- svn_cl__opt_state_t *opt_state,
- apr_array_header_t *targets)
+write_status_to_file (apr_pool_t *pool,
+ apr_file_t *file,
+ svn_client_auth_baton_t *auth_baton,
+ svn_cl__opt_state_t *opt_state,
+ apr_array_header_t *targets)
{
apr_hash_t *statushash;
svn_revnum_t youngest = SVN_INVALID_REVNUM;
@@ -129,43 +154,135 @@
switches (-n, -u, -[vV]) : */
SVN_ERR (svn_client_status (&statushash, &youngest, target, auth_baton,
- opt_state->nonrecursive ? 0 : 1,
+ opt_state->nonrecursive ? FALSE : TRUE,
opt_state->verbose,
FALSE, /* no update */
pool));
- /* Now print the structures to the screen.
- The flag we pass indicates whether to use the 'detailed'
- output format or not. */
- print_status (file,
- statushash,
- pool);
+ /* Now print the structures to a file. */
+ if (print_hash_status (file, statushash, pool))
+ {
+ return svn_error_create
+ (SVN_ERR_CMDLINE__FAILED_WRITING_TO_TEMPORARY_FILE, 0, NULL,
+ pool,
+ "Failed to write status information to temporary file: %s");
+ }
}
return SVN_NO_ERROR;
}
+static svn_stringbuf_t *
+strip_svn_from_buffer (svn_stringbuf_t *buffer,
+ apr_pool_t *pool)
+{
+ char *ptr;
+ char *prefix;
+
+ /* cut off all the SVN: lines, but nothing else */
+ ptr = buffer->data;
+
+ do
+ {
+ prefix=strstr (ptr, "SVN:");
+
+ if (prefix)
+ {
+ /* substring found */
+
+ if ( (prefix == buffer->data) ||
+ ((prefix[-1] == '\n') || (prefix[-1] == '\r')))
+ {
+ /* it is on the start of a line */
+
+ /* Figure out the end of the line. This needs a
+ little better intelligence since a LF is not the
+ end-of-line on every imaginable system .*/
+ char *eol= strchr (prefix, '\n');
+ size_t linelen;
+
+ if(NULL == eol)
+ {
+ /* last line, we just make the buffer shorter,
+ no need to move around any data */
+ linelen = strlen (prefix);
+
+ buffer->len -= linelen;
+
+ /* set a new zero terminator */
+ buffer->data [ buffer->len] = 0;
+
+ break;
+ }
+
+ eol++; /* eol now points to the first character
+ beyond */
+
+ /* this line that is about to get cut off measures
+ from 'prefix' to 'eol' */
+ linelen = eol-prefix;
+
+ /* move the rest of the chunk over this line that
+ shouldn't be a part of the final message */
+ memmove (prefix, eol,
+ buffer->len -
+ (prefix-buffer->data) -
+ linelen);
+
+ /* decrease total message size */
+ buffer->len -= linelen;
+
+ /* set a new zero terminator */
+ buffer->data [ buffer->len] = 0;
+
+ /* continue searching from here */
+ ptr = prefix;
+ }
+ else
+ {
+ /* substring found but not on the first column of
+ a line, just continue from here */
+ ptr = prefix+1;
+ }
+ }
+ }
+ while (prefix);
+
+ return buffer;
+}
+
/*
* Invoke $EDITOR to get a commit message.
*/
static svn_error_t *
-message_from_editor(apr_pool_t *pool,
- apr_array_header_t *targets,
- svn_client_auth_baton_t *auth_baton,
- svn_stringbuf_t *path,
- svn_cl__opt_state_t *opt_state,
- svn_stringbuf_t **messagep )
+message_from_editor (apr_pool_t *pool,
+ apr_array_header_t *targets,
+ svn_client_auth_baton_t *auth_baton,
+ svn_stringbuf_t *path,
+ svn_cl__opt_state_t *opt_state,
+ svn_stringbuf_t **messagep,
+ const char *default_msg, /* text to include in editor */
+ svn_boolean_t include_status_output)
{
char const *editor;
- char *command;
- size_t editorlen;
apr_file_t *tempfile;
apr_status_t rc;
const char *fullfile;
+ apr_finfo_t finfo_before;
+ apr_finfo_t finfo_after;
+ apr_size_t size;
+ apr_size_t written;
+ svn_error_t *error;
/* default is no returned message */
*messagep = NULL;
+ /* ### FIXME: editor-choice:
+ 1. the command line
+ 2. config file.
+ 3. environment variable
+ 4. configure-default */
+
/* try to get an editor to use */
editor = getenv ("SVN_EDITOR");
if(NULL == editor)
@@ -176,6 +293,7 @@
if(NULL == editor)
{
/* no custom editor, use built-in defaults */
+ /* ### FIXME: the path should be discovered in configure */
#ifdef SVN_WIN32
editor = "notepad.exe";
#else
@@ -190,208 +308,170 @@
pool));
/* we need to know the name of the temporary file */
- apr_file_name_get (&fullfile, tempfile);
+ rc = apr_file_name_get (&fullfile, tempfile);
+ if (APR_SUCCESS != rc)
+ {
+ /* close temp file first, but we can't remove it since we can't get
+ its name! */
+ apr_file_close (tempfile);
- editorlen = strlen (editor);
+ /* could not get name we can't continue */
+ return svn_error_create
+ (SVN_ERR_CMDLINE__FAILED_WRITING_TO_TEMPORARY_FILE, 0, NULL,
+ pool,
+ "Failed to write status information to temporary file.");
+ }
- command = (char *)malloc (editorlen + strlen(fullfile) + 2);
+ size = strlen (default_msg);
+ rc = apr_file_write_full (tempfile, default_msg, size, &written);
+
+ if ((APR_SUCCESS != rc) || written != size)
+ {
+ error = svn_error_create
+ (SVN_ERR_CMDLINE__FAILED_WRITING_TO_TEMPORARY_FILE, 0, NULL,
+ pool,
+ "Failed to write stauts information to temporary file.");
+ }
+ else if (include_status_output)
+ {
+ error = write_status_to_file (pool, tempfile, auth_baton,
+ opt_state, targets);
+ }
-#define DEFAULT_MSG \
-"\nSVN: ----------------------------------------------------------------------\n" \
-"SVN: Enter Log. Lines beginning with 'SVN:' are removed automatically\n" \
-"SVN: \n" \
-"SVN: Current status of the target files and directories:\n"\
-"SVN: \n"
+ apr_file_close (tempfile); /* we don't check return code here, since
+ we have no way to deal with errors on
+ file close */
+
+ if (error)
+ {
+ /* we didn't manage to write the complete file, we can't fulfill
+ what we're set out to do, get out */
+
+ return error;
+ }
- if (command)
+ /* Get information about the temporary file before the user has
+ been allowed to edit any message */
+ rc = apr_stat (&finfo_before, fullfile,
+ APR_FINFO_MTIME|APR_FINFO_SIZE, pool);
+
+ if (APR_SUCCESS != rc)
{
- apr_finfo_t finfo_before;
- apr_finfo_t finfo_after;
- apr_size_t size, written;
+ /* remove file */
+ apr_file_remove (fullfile, pool);
- size = strlen (DEFAULT_MSG);
+ return svn_error_create
+ (SVN_ERR_CMDLINE__FAILED_STAT_ON_TEMPORARY_FILE, 0, NULL,
+ pool,
+ "Failed getting info about temporary file.");
+ }
- rc = apr_file_write_full (tempfile, DEFAULT_MSG, size, &written);
+#if 0
+ {
+ /* This is what I think is almost proper code to run the editor command
+ line. We should somehow tell the svn_io_run_cmd() that we want it
+ to search the PATH and inherit our environment. */
+ int exitcode;
+ apr_exit_why_e exitwhy;
+ const char *cmdargs[3];
+
+ /* setup the editor command line */
+ cmdargs[0] = editor;
+ cmdargs[1] = fullfile;
+ cmdargs[2] = NULL;
+
+ /* run the editor and come back when done */
+ error = svn_io_run_cmd (".",
+ editor,
+ cmdargs,
+ &exitcode,
+ &exitwhy,
+ NULL, NULL, NULL, /* no in, out or err files */
+ pool);
+ }
+#else
+ {
+ /* ### FIXME:
+ this is just a work-around as the above doesn't seem to work!
+ Remove as soon as possible.
+ */
+ char *command=(char *)malloc(strlen(editor)+strlen(fullfile)+2);
+ sprintf(command, "%s %s", editor, fullfile);
+ system(command);
+ free(command);
+ }
+#endif
- write_status_to_file (pool, tempfile, auth_baton, opt_state, targets);
+ if (error)
+ {
+ /* remove file */
+ apr_file_remove (fullfile, pool);
- apr_file_close (tempfile);
+ return error;
+ }
- /* we didn't manage to write the complete file, we can't fulfill
- what we're set out to do, get out */
- /* ### FIXME: The documentation for apr_file_full_write()
- doesn't explicitly promise that if size != written, then
- there *must* be an error returned, so below we handle the two
- cases separately. But a glance at apr_file_full_write's
- implementation, on Unix at least, shows that it could
- document that promise. Maybe we should fix the doc in APR,
- and just check rc below? */
- if (! APR_STATUS_IS_SUCCESS (rc))
- {
- return svn_error_createf
- (rc, 0, NULL, pool, "Trouble writing `%s'", fullfile);
- }
- else if (written != size)
- {
- /* ### FIXME: this error code may change when there is a
- general need to revamp the client's error code system. */
- return svn_error_createf
- (SVN_ERR_INCOMPLETE_DATA,
- 0, NULL, pool, "Failed to completely write `%s'", fullfile);
- }
+ /* Get information about the message file after the assumed editing. */
+ rc = apr_stat (&finfo_after, fullfile,
+ APR_FINFO_MTIME|APR_FINFO_SIZE, pool);
+
+ if (APR_SUCCESS != rc)
+ {
+ /* remove file */
+ apr_file_remove (fullfile, pool);
- /* Get information about the temporary file before the user has
- been allowed to edit any message */
- apr_stat (&finfo_before, fullfile,
- APR_FINFO_MTIME|APR_FINFO_SIZE, pool);
+ return svn_error_create
+ (SVN_ERR_CMDLINE__FAILED_STAT_ON_TEMPORARY_FILE, 0, NULL,
+ pool,
+ "Failed getting info about temporary file.");
+ }
- /* create the command line */
- apr_snprintf (command, editorlen + strlen(fullfile) + 2,
- "%s %s", editor, fullfile);
- /* run the editor command line */
- system (command);
-
- /* Get information about the message file after the assumed editing. */
- apr_stat (&finfo_after, fullfile,
- APR_FINFO_MTIME|APR_FINFO_SIZE, pool);
-
- /* Check if there seems to be any changes in the file */
- if((finfo_before.mtime == finfo_after.mtime) &&
- (finfo_before.size == finfo_after.size))
+ /* Check if there seems to be any changes in the file */
+ if ((finfo_before.mtime != finfo_after.mtime) ||
+ (finfo_before.size != finfo_after.size))
+ {
+ /* The file seem to have been modified, load it and strip it */
+ apr_file_t *read_file;
+
+ /* we have a commit message in a temporary file, get it */
+ rc = apr_file_open (&read_file, fullfile,
+ APR_READ, APR_UREAD, pool);
+
+ if (APR_SUCCESS != rc) /* open failed */
{
- /* The file doesn't seem to have been modified, no
- need to load it and strip it and such */
+ /* This is an annoying situation, as the file seems to have
+ been edited but we can't read it! */
+
+ /* remove file */
+ apr_file_remove (fullfile, pool);
+
+ return svn_error_create
+ (SVN_ERR_CMDLINE__FAILED_OPENING_TEMPORARY_FILE, 0, NULL,
+ pool,
+ "Failed opening temporary file.");
}
- else {
- apr_file_t *read_file;
+ else
+ {
+ svn_stringbuf_t *entirefile;
- /* we have a commit message in a temporary file, get it */
- rc = apr_file_open (&read_file, fullfile,
- APR_READ, APR_UREAD, pool);
-
- if(APR_SUCCESS != rc) /* open failed */
- {
- /* This is an annoying situation, as the file seems to have
- been edited but we can't read it! */
- }
- else
- {
- /* read the entire file into one chunk of memory */
- char readbuffer[1024];
- svn_stringbuf_t *entirefile;
- char *ptr;
- char *prefix;
-
- /* create a buffer */
- entirefile = svn_stringbuf_ncreate ("", 0, pool);
+ /* read the entire file into one chunk of memory */
+ SVN_ERR (svn_string_from_aprfile (&entirefile, read_file, pool));
- do
- {
- size = sizeof (readbuffer);
- apr_file_read (read_file, readbuffer, &size);
-
- /* append chunk to the entirefile string */
- svn_stringbuf_appendbytes (entirefile, readbuffer, size);
-
- if( size != sizeof (readbuffer))
- {
- /* partly filled buffer, this is the end */
- break; /* out of loop */
- }
- }
- while(1);
-
- /* close the file */
- apr_file_close (read_file);
-
- /* a full chunk was read, now strip all the SVN: lines, but
- nothing else */
- ptr = entirefile->data;
-
- do
- {
- prefix=strstr (ptr, "SVN:");
-
- if(prefix)
- {
- /* substring found */
-
- if( (prefix == entirefile->data) ||
- ((prefix[-1] == '\n') || (prefix[-1] == '\r')))
- {
- /* it is on the start of a line */
-
- /* Figure out the end of the line. This needs a little
- better intelligence since a LF is not the
- end-of-line on every imaginable system .*/
- char *eol= strchr(prefix, '\n');
- size_t linelen;
-
- if(NULL == eol)
- {
- /* last line, we just make the buffer shorter,
- no need to move around any data */
- linelen = strlen(prefix);
-
- entirefile->len -= linelen;
-
- /* set a new zero terminator */
- entirefile->data [ entirefile->len] = 0;
-
- break;
- }
-
- eol++; /* eol now points to the first character
- beyond */
-
- /* this line that is about to get cut off measures
- from 'prefix' to 'eol' */
- linelen = eol-prefix;
-
- /* move the rest of the chunk over this line that
- shouldn't be a part of the final message */
- memmove (prefix, eol,
- entirefile->len - (prefix-entirefile->data) -
- linelen);
-
- /* decrease total message size */
- entirefile->len -= linelen;
-
- /* set a new zero terminator */
- entirefile->data [ entirefile->len] = 0;
-
- /* continue searching from here */
- ptr = prefix;
- }
- else
- {
- /* substring found but not on the first column of
- a line, just continue from here */
- ptr = prefix+1;
- }
- }
- }
- while(prefix);
-
- /* set the return-message to the entire-file buffer */
- *messagep = entirefile;
- }
-
- }
+ /* close the file */
+ apr_file_close (read_file);
- /* free the memory allocated for the command line here */
- free (command);
- }
- else
- {
- /* major memory problem */
+ /* strip SVN: lines */
+ entirefile = strip_svn_from_buffer(entirefile, pool);
+
+ /* set the return-message to the entire-file buffer */
+ *messagep = entirefile;
+ }
+
}
/* remove the temporary file */
apr_file_remove (fullfile, pool);
return SVN_NO_ERROR;
-
}
/*
@@ -399,14 +479,15 @@
* This function also outputs this fact to stdout for users to see.
*/
static svn_error_t *
-store_message(svn_stringbuf_t *message,
- svn_stringbuf_t *path,
- apr_pool_t *pool)
+store_message (svn_stringbuf_t *message,
+ svn_stringbuf_t *path,
+ apr_pool_t *pool)
{
/* Store the message in a temporary file name and display the
file name to the user */
apr_file_t *tempfile;
apr_size_t size;
+ apr_size_t written;
apr_status_t rc;
const char *fullfile;
@@ -416,24 +497,30 @@
pool));
/* we need to know the name of the temporary file */
- apr_file_name_get (&fullfile, tempfile);
+ rc = apr_file_name_get (&fullfile, tempfile);
- size = message->len;
- rc = apr_file_write (tempfile, message->data, &size);
+ if (APR_SUCCESS == rc)
+ {
+ size = message->len;
+ rc = apr_file_write_full (tempfile, message->data, size, &written);
+ }
- apr_file_close(tempfile);
+ apr_file_close (tempfile);
- if (APR_SUCCESS == rc)
+ if ((APR_SUCCESS == rc) && (size == written))
{
- printf("The commit message has been stored in this location:\n%s\n",
- fullfile);
+ printf ("The commit message has been stored in this location:\n%s\n",
+ fullfile);
}
else
{
- /* FIX! return a proper error message here */
+ return svn_error_createf
+ (SVN_ERR_CMDLINE__FAILED_WRITING_TO_TEMPORARY_FILE, 0, NULL,
+ pool,
+ "Failed writing to temporary file %s.", fullfile);
}
- return NULL;
+ return SVN_NO_ERROR;
}
svn_error_t *
@@ -506,18 +593,27 @@
/* There was no commit message given anywhere in the command line,
fire up our favourite editor to get one instead! */
+ /* this default message might need to be configurable somehow */
+ const char *default_msg=
+ "\nSVN: ----------------------------------------------------------------------\n"
+ "SVN: Enter Log. Lines beginning with 'SVN:' are removed automatically\n" \
+ "SVN: \n"
+ "SVN: Current status of the target files and directories:\n"
+ "SVN: \n";
+
/* no message given, try getting one from $EDITOR */
message_from_editor (pool, targets, auth_baton,
- base_dir, opt_state, &messagep);
+ base_dir, opt_state, &messagep,
+ default_msg, TRUE);
- if(messagep)
+ if (messagep)
{
/* We did get message, now check if it is anything more than just
white space as we will consider white space only as empty */
int len;
for (len=messagep->len; len>=0; len--)
- if(!apr_isspace (messagep->data[len]))
+ if (!apr_isspace (messagep->data[len]))
break;
if (len >= 0)
/* there was something else besides space */
@@ -526,7 +622,7 @@
/* message can still be NULL here if none was entered or if an
error occurred */
- if(NULL == message)
+ if (NULL == message)
{
char *reply;
svn_cl__prompt_user (&reply,
--
Daniel Stenberg - http://daniel.haxx.se - +46-705-44 31 77
ech`echo xiun|tr nu oc|sed 'sx\([sx]\)\([xoi]\)xo un\2\1 is xg'`ol
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sat Oct 21 14:37:02 2006