On Sunday 10 April 2005 18:21, Philip Martin wrote:
> jszakmeister@tigris.org writes:
> > Author: jszakmeister
> > Date: Sun Apr 10 13:21:56 2005
> > New Revision: 14081
> >
> > Modified:
> > trunk/subversion/libsvn_client/cat.c
> > Log:
> > Fix Issue #1361: svn cat -rBASE contacts the repository. This also
> > prepares us for supporting running 'svn cat' at revision WORKING, if
> > we can come to a consensus that it should be the default. :-)
> > * subversion/libsvn_client/cat.c
> > (cat_local_file): New.
> >
> > (svn_client_cat2): Stop contacting the repository to cat a file in
> > the working copy.
> >
> >
> >
> > Modified: trunk/subversion/libsvn_client/cat.c
> > Url:
> > http://svn.collab.net/viewcvs/svn/trunk/subversion/libsvn_client/cat.
> >c?view=diff&rev=14081&p1=trunk/subversion/libsvn_client/cat.c&r1=14080
> >&p2=trunk/subversion/libsvn_client/cat.c&r2=14081
> > =====================================================================
> >========= --- trunk/subversion/libsvn_client/cat.c (original)
> > +++ trunk/subversion/libsvn_client/cat.c Sun Apr 10 13:21:56 2005
> > @@ -37,6 +37,106 @@
> >
> > /*** Code. ***/
>
> Please document new functions.
Will do.
> > +static svn_error_t *
> > +cat_local_file (const char *path,
> > + svn_stream_t *output,
> > + svn_wc_adm_access_t *adm_access,
> > + const svn_opt_revision_t *revision,
> > + apr_pool_t *pool)
> > +{
> > + const svn_wc_entry_t *entry;
> > + svn_subst_keywords_t kw = { 0 };
> > + svn_subst_eol_style_t style;
> > + apr_hash_t *props;
> > + const char *base;
> > + svn_string_t *eol_style, *keywords, *special;
> > + const char *eol = NULL;
> > + svn_boolean_t local_mod = FALSE;
> > + apr_time_t tm;
> > + apr_file_t *input_file;
> > + svn_stream_t *input;
> > +
> > + SVN_ERR (svn_wc_entry (&entry, path, adm_access, FALSE, pool));
> > +
> > + if (entry->kind != svn_node_file)
>
> entry could be NULL
Yep, Julian caught that. I'm actually putting together some cat tests now
so this doesn't slip by again. I somehow missed it from a patch I made a
little while ago.
> > + return svn_error_createf(SVN_ERR_CLIENT_IS_DIRECTORY, NULL,
> > + _("'%s' refers to a directory"), path);
> > +
> > + if (revision->kind != svn_opt_revision_working)
> > + {
> > + SVN_ERR (svn_wc_get_pristine_copy_path (path, &base, pool));
> > + SVN_ERR (svn_wc_get_prop_diffs (NULL, &props, path,
> > adm_access, pool)); + }
> > + else
> > + {
> > + svn_wc_status2_t *status;
> > +
> > + base = path;
> > + SVN_ERR (svn_wc_prop_list (&props, path, adm_access, pool));
> > + SVN_ERR (svn_wc_status2 (&status, path, adm_access, pool));
> > + if (status->text_status != svn_wc_status_normal)
> > + local_mod = TRUE;
> > + }
>
> Should cat_local_file follow svn:special symbolic links?
Good question... I'll look into it.
> > +
> > + eol_style = apr_hash_get (props, SVN_PROP_EOL_STYLE,
> > + APR_HASH_KEY_STRING);
> > + keywords = apr_hash_get (props, SVN_PROP_KEYWORDS,
> > + APR_HASH_KEY_STRING);
> > + special = apr_hash_get (props, SVN_PROP_SPECIAL,
> > + APR_HASH_KEY_STRING);
> > +
> > + if (eol_style)
> > + svn_subst_eol_style_from_value (&style, &eol, eol_style->data);
> > +
> > + if (local_mod && (! special))
> > + {
> > + /* Use the modified time from the working copy if
> > + the file */
> > + SVN_ERR (svn_io_file_affected_time (&tm, path, pool));
> > + }
> > + else
> > + {
> > + tm = entry->cmt_date;
> > + }
> > +
> > + if (keywords)
> > + {
> > + const char *fmt;
> > + const char *author;
> > +
> > + if (local_mod)
> > + {
> > + /* For locally modified files, we'll append an 'M'
> > + to the revision number, and set the author to
> > + "(local)" since we can't always determine the
> > + current user's username */
> > + fmt = "%ldM";
> > + author = _("(local)");
>
> I think you are inventing stuff that will never appear in a committed
> file, and it means that cat_local_file output won't match the working
> file. Is that a good idea?
Well, we have a bit of an unresolved issue... mainly, that we've never
tackled issue #1191 and decided whether or not we should support 'svn
cat' for rWORKING, as well as several other commands for that matter. If
you're uncomfortable with it there, I can remove it and only support the
BASE revision... as that's really the only revision allowed to get
through right now anyways (rWORKING is unsupported).
All of that said, I believe this should match the export working copy
case, and that is what we decided to do there.
[snip]
> > +
> > + SVN_ERR (svn_wc_adm_probe_open3 (&adm_access, NULL,
> > path_or_url, FALSE, + 0,
> > ctx->cancel_func, ctx->cancel_baton, +
> > pool));
>
> I'd prefer svn_wc_adm_open3 on the svn_path_dirname rather than
> svn_wc_adm_probe_open3 on the path. It seems silly to allow a
> directory to be opened when a file is required.
Will fix.
> > +
> > + SVN_ERR (cat_local_file (path_or_url, out, adm_access,
> > revision, pool)); +
> > + SVN_ERR (svn_wc_adm_close (adm_access));
> > +
> > + return SVN_NO_ERROR;
> > + }
> >
> > /* Get an RA plugin for this filesystem object. */
> > SVN_ERR (svn_client__ra_session_from_path (&ra_session, &rev,
Thanks for the review Philip.
-John
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Mon Apr 11 03:08:47 2005