[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: rev 1485 - trunk/subversion/libsvn_subr trunk/subversion/clients/cmdline trunk/subversion/clients/cmdline/man

From: Philip Martin <philip_at_codematters.co.uk>
Date: 2002-03-11 23:56:12 CET

bcollins@tigris.org writes:

> Modified: trunk/subversion/libsvn_subr/io.c
> ==============================================================================
> --- trunk/subversion/libsvn_subr/io.c (original)
> +++ trunk/subversion/libsvn_subr/io.c Mon Mar 11 13:47:34 2002
> @@ -614,6 +614,16 @@
> apr_status_t apr_err;
> apr_file_t *f = NULL;
>
> + /* If user passed '-', use stdin. We could use apr_file_open_stdin here, and
> + * in fact, it does work. Problem is that if the same command invokes the
> + * editor, stdin is crap, and the editor acts funny (if not die outright). I
> + * wanted to just disallow stdin reading and invoking the editor, but there's
> + * no easy callback for that right now. */
> + if (filename[0] == '-' && filename[1] == '\0')
> + return svn_error_create
> + (SVN_ERR_UNSUPPORTED_FEATURE, 0, NULL, pool,
> + "reading from stdin is currently broken, so disabled");
> +
> apr_err = apr_file_open (&f, filename, APR_READ, APR_OS_DEFAULT, pool);
> if (apr_err)
> return svn_error_createf (apr_err, 0, NULL, pool,
> @@ -637,11 +647,12 @@
> apr_pool_t *pool)
> {
> apr_size_t len;
> - apr_finfo_t finfo;
> apr_status_t apr_err;
> svn_stringbuf_t *res = svn_stringbuf_create("", pool);
> const char *fname;
> - char dummy;
> + char buf[BUFSIZ];
> +
> + /* XXX: We should check the incoming data for being of type binary. */
>
> apr_err = apr_file_name_get (&fname, file);
> if (!APR_STATUS_IS_SUCCESS(apr_err))
> @@ -649,32 +660,31 @@
> (apr_err, 0, NULL, pool,
> "svn_string_from_aprfile: failed to get filename");
>
> - apr_err = apr_stat (&finfo, fname, APR_FINFO_SIZE, pool);
> - if (!APR_STATUS_IS_SUCCESS(apr_err))
> - return svn_error_createf
> - (apr_err, 0, NULL, pool,
> - "svn_string_from_aprfile: failed to stat '%s'", fname);
> -
> - /* Reserve space for the data, ensuring that the stringbuf's pool is
> - used. */
> - svn_stringbuf_ensure (res, finfo.size + 1);
> - res->len = finfo.size;
> -
> - apr_err = apr_file_read_full (file, res->data, res->len, &len);
> - if (!APR_STATUS_IS_SUCCESS(apr_err))
> - return svn_error_createf
> - (apr_err, 0, NULL, pool,
> - "svn_string_from_aprfile: failed to read '%s'", fname);
> + /* If the apr_file_t was opened with apr_file_open_std{in,out,err}, then we
> + * wont get a filename for it. We assume that since we are reading, that in
> + * this case we would only ever be using stdin. */
> + if (NULL == fname)
> + fname = "stdin";
> +
> + /* apr_file_read will not return data and eof in the same call. So this loop
> + * is safe from missing read data. */
> + len = sizeof(buf);
> + apr_err = apr_file_read (file, buf, &len);
> + while (APR_STATUS_IS_SUCCESS(apr_err))
> + {
> + svn_stringbuf_appendbytes(res, buf, len);
> + len = sizeof(buf);
> + apr_err = apr_file_read (file, buf, &len);
> + }
>
> /* Having read all the data we *expect* EOF */
> - apr_err = apr_file_read_full (file, &dummy, 1, &len);
> if (!APR_STATUS_IS_EOF(apr_err))
> return svn_error_createf
> (apr_err, 0, NULL, pool,
> "svn_string_from_aprfile: EOF not seen for '%s'", fname);
> -
> - /* Null terminate the stringbuf. */
> - res->data[res->len] = 0;
> +
> + /* Null terminate the stringbuf. */
> + res->data[res->len] = 0;

I don't think the application code should be doing this, a) it relies
on svn_stringbuf_appendbytes allocating len+1 bytes, b) since we rely
on that we may as well rely on svn_stringbuf_appendbytes setting it to
zero.

>
> *result = res;
> return SVN_NO_ERROR;
> @@ -858,6 +868,7 @@
> /* Make sure we invoke cmd directly, not through a shell. */
> apr_err = apr_procattr_cmdtype_set (cmdproc_attr,
> inherit?APR_PROGRAM_PATH:APR_PROGRAM);
> +
> if (! APR_STATUS_IS_SUCCESS (apr_err))
> return svn_error_createf
> (apr_err, 0, NULL, pool,
>
[-]
> Modified: trunk/subversion/clients/cmdline/man/svn.1
> ==============================================================================
> --- trunk/subversion/clients/cmdline/man/svn.1 (original)
> +++ trunk/subversion/clients/cmdline/man/svn.1 Mon Mar 11 13:47:35 2002
> @@ -85,6 +85,11 @@
> \fB-L\fP or \fB --line-conversion GLOB\fP
> Do line-end conversion for files matching GLOB.
> .TP
> +\fB--targets\fP
> +Supply a file used as entry and URL args for a given command. You can also
> +supply '-' as the file to read from standard input. The file will be read
> +as one argument for each line, even if given on standard input.
> +.TP
> \fB--\fP
> End of option processing
> .TP
>
[-]
> Modified: trunk/subversion/clients/cmdline/main.c
> ==============================================================================
> --- trunk/subversion/clients/cmdline/main.c (original)
> +++ trunk/subversion/clients/cmdline/main.c Mon Mar 11 13:47:35 2002
> @@ -67,6 +67,7 @@
> {"username", svn_cl__auth_username_opt, 1, "specify a username ARG"},
> {"password", svn_cl__auth_password_opt, 1, "specify a password ARG"},
> {"extensions", 'x', 1, "pass \"ARG\" as bundled options to GNU diff"},
> + {"targets", 't', 1, "pass contents of file \"ARG\" as additional args"},

This allows both '--targets' and '-t' but the man page only mentions
the long form.

> {0, 0, 0}
> };
>
> @@ -117,7 +118,7 @@
> "Put files and directories under revision control, scheduling\n"
> "them for addition to repository. They will be added in next commit.\n"
> "usage: svn add [OPTIONS] [TARGETS]\n",
> - {svn_cl__recursive_opt} },
> + {'t', svn_cl__recursive_opt} },
>
> { "checkout", svn_cl__checkout, {"co"},
> "Check out a working copy from a repository.\n"
> @@ -137,7 +138,7 @@
> "usage: svn commit [TARGETS]\n\n"
> " Be sure to use one of -m or -F to send a log message;\n"
> " the -r switch is only for use with --xml-file.\n",
> - {'m', 'F', 'q',
> + {'m', 'F', 'q', 't',
> svn_cl__force_opt, svn_cl__auth_username_opt, svn_cl__auth_password_opt,
> svn_cl__xml_file_opt, 'r'} },
>
> @@ -158,7 +159,7 @@
> " upon next commit. (The working item itself will only be removed\n"
> " if --force is passed.) If run on URL, item is deleted from\n"
> " repository via an immediate commit.\n",
> - {svn_cl__force_opt, 'm', 'F',
> + {svn_cl__force_opt, 'm', 'F', 't',
> svn_cl__auth_username_opt, svn_cl__auth_password_opt} },
>
> { "diff", svn_cl__diff, {"di"},
> @@ -202,7 +203,7 @@
> " svn log http://www.example.com/repo/project/foo.c\n"
> "\n"
> " svn log http://www.example.com/repo/project foo.c bar.c\n",
> - {'r', 'D', 'v', svn_cl__auth_username_opt, svn_cl__auth_password_opt} },
> + {'r', 'D', 'v', 't', svn_cl__auth_username_opt, svn_cl__auth_password_opt} },
>
> { "merge", svn_cl__merge, {0},
> "Merge changes in the working copy. IMPLEMENTATION INCOMPLETE.\n"
> @@ -250,14 +251,14 @@
> "Set property PROPNAME to PROPVAL on files or directories.\n"
> "usage: propset PROPNAME PROPVAL [TARGETS]\n\n"
> " Use -F (instead of PROPVAL) to get the value from a file.\n",
> - {'F', 'q', svn_cl__recursive_opt} },
> + {'F', 'q', 't', svn_cl__recursive_opt} },
>
> { "revert", svn_cl__revert, {0},
> "Restore pristine working copy file (undo all local edits)\n"
> "usage: revert TARGET1 [TARGET2 [TARGET3 ... ]]\n\n"
> " Note: this routine does not require network access, and will\n"
> " remove any .rej produced when a file is in a state of conflict.\n",
> - {svn_cl__recursive_opt} },
> + {'t', svn_cl__recursive_opt} },
>
> { "status", svn_cl__status, {"stat", "st"},
> "Print the status of working copy files and directories.\n"
> @@ -347,7 +348,7 @@
> opts = apr_pstrcat (pool, opts, " arg", NULL);
>
> if (doc)
> - opts = apr_pstrcat (pool, opts, ": ", opt->description, NULL);
> + opts = apr_pstrcat (pool, opts, ":\t", opt->description, NULL);
>
> *string = opts;
> }
> @@ -901,6 +902,18 @@
> log_under_version_control = TRUE;
> }
> break;
> + case 't':
> + {
> + svn_stringbuf_t *buffer;
> + err = svn_string_from_file (&buffer, opt_arg, pool);
> + if (err)
> + {
> + svn_handle_error (err, stdout, FALSE);
> + svn_pool_destroy (pool);
> + return EXIT_FAILURE;
> + }
> + opt_state.targets = svn_cl__newlinelist_to_array(buffer, pool);
> + }

No break statement?

> case 'M':
> opt_state.modified = TRUE;
> break;
>

I notice you are using tabs for indentation, the HACKING file says
spaces should be used.

-- 
Philip
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Mon Mar 11 23:56:45 2002

This is an archived mail posted to the Subversion Dev mailing list.