You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Greg Stein <gs...@lyra.org> on 2001/11/15 21:21:04 UTC
Re: svn commit: rev 461 - trunk/subversion/libsvn_wc
Looks good overall, although a lot of extra work is being done rather than
using features of the existing functions.
Also: it appears that the wcprops did not get the extension added to them. I
haven't looked into the code, but I still have ".svn/wcprops/foo.c" in my
working copy. So they still appear in a "find" :-)
And I apologize for not reviewing the patch beforehand. I would have seen
the comments below:
On Thu, Nov 15, 2001 at 12:26:46PM -0600, kevin@tigris.org wrote:
>...
> --- OLD/trunk/subversion/libsvn_wc/props.c Thu Nov 15 12:26:44 2001
> +++ NEW/trunk/subversion/libsvn_wc/props.c Thu Nov 15 12:26:44 2001
>...
> @@ -625,12 +625,14 @@
> SVN_WC__ADM_PROP_BASE,
> name->data,
> NULL);
> + svn_stringbuf_appendcstr(tmp_prop_base, SVN_WC__BASE_EXT);
> real_prop_base = svn_wc__adm_path (svn_stringbuf_create ("", pool),
> 0, /* no tmp */
> pool,
> SVN_WC__ADM_PROP_BASE,
> name->data,
> NULL);
> + svn_stringbuf_appendcstr(real_prop_base, SVN_WC__BASE_EXT);
The above two chunks of code would be simplified by simply placing
SVN_WC__BASE_EXT directly into the svn_wc__adm_path() call. No need for a
second function call to append the extension.
>...
> --- OLD/trunk/subversion/libsvn_wc/adm_files.c Thu Nov 15 12:26:45 2001
> +++ NEW/trunk/subversion/libsvn_wc/adm_files.c Thu Nov 15 12:26:46 2001
> @@ -339,6 +339,7 @@
> {
> svn_stringbuf_t *newpath, *basename;
> svn_path_split (path, &newpath, &basename, svn_path_local_style, pool);
> + svn_stringbuf_appendcstr (basename, SVN_WC__BASE_EXT);
> return sync_adm_file (newpath,
> pool,
> SVN_WC__ADM_TEXT_BASE,
Same here.
>...
> +svn_stringbuf_t *
> +svn_wc__text_base_path (const svn_stringbuf_t *path,
> + svn_boolean_t tmp,
> + apr_pool_t *pool)
> {
> svn_stringbuf_t *newpath, *basename;
> svn_path_split (path, &newpath, &basename, svn_path_local_style, pool);
> -
> + svn_stringbuf_appendcstr (basename, SVN_WC__BASE_EXT);
> extend_with_adm_name (newpath,
> 0,
> pool,
> tmp ? SVN_WC__ADM_TMP : "",
> - thing,
> + SVN_WC__ADM_TEXT_BASE,
> basename->data,
> NULL);
And here.
>...
> @@ -714,6 +707,7 @@
> {
> svn_stringbuf_t *newpath, *basename;
> svn_path_split (path, &newpath, &basename, svn_path_local_style, pool);
> + svn_stringbuf_appendcstr (basename, SVN_WC__BASE_EXT);
> return open_adm_file (handle, newpath, flags, pool,
> SVN_WC__ADM_TEXT_BASE, basename->data, NULL);
And here.
Notice a pattern? :-)
(sorry that I didn't get the patch reviewed beforehand :-( )
> }
> @@ -727,6 +721,7 @@
> {
> svn_stringbuf_t *newpath, *basename;
> svn_path_split (path, &newpath, &basename, svn_path_local_style, pool);
> + svn_stringbuf_appendcstr (basename, SVN_WC__BASE_EXT);
> return close_adm_file (fp, newpath, write, pool,
> SVN_WC__ADM_TEXT_BASE, basename->data, NULL);
And here.
> }
> @@ -790,8 +785,11 @@
> return open_adm_file (handle, parent_dir, flags, pool,
> SVN_WC__ADM_DIR_PROP_BASE, NULL);
> else
> - return open_adm_file (handle, parent_dir, flags, pool,
> - SVN_WC__ADM_PROP_BASE, basename->data, NULL);
> + {
> + svn_stringbuf_appendcstr (basename, SVN_WC__BASE_EXT);
> + return open_adm_file (handle, parent_dir, flags, pool,
> + SVN_WC__ADM_PROP_BASE, basename->data, NULL);
> + }
Again.
> }
> else if (wcprops)
> {
> @@ -850,8 +848,11 @@
> return close_adm_file (fp, parent_dir, sync, pool,
> SVN_WC__ADM_DIR_PROP_BASE, NULL);
> else
> - return close_adm_file (fp, parent_dir, sync, pool,
> - SVN_WC__ADM_PROP_BASE, basename->data, NULL);
> + {
> + svn_stringbuf_appendcstr (basename, SVN_WC__BASE_EXT);
> + return close_adm_file (fp, parent_dir, sync, pool,
> + SVN_WC__ADM_PROP_BASE, basename->data, NULL);
> + }
Another
> }
> else if (wcprops)
> {
> @@ -909,8 +910,11 @@
> return sync_adm_file (parent_dir, pool,
> SVN_WC__ADM_DIR_PROP_BASE, NULL);
> else
> - return sync_adm_file (parent_dir, pool,
> - SVN_WC__ADM_PROP_BASE, basename->data, NULL);
> + {
> + svn_stringbuf_appendcstr (basename, SVN_WC__BASE_EXT);
> + return sync_adm_file (parent_dir, pool,
> + SVN_WC__ADM_PROP_BASE, basename->data, NULL);
> + }
and yet another...
Cheers,
-g
--
Greg Stein, http://www.lyra.org/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: rev 461 - trunk/subversion/libsvn_wc
Posted by Kevin Pilch-Bisson <ke...@pilch-bisson.net>.
On Thu, Nov 15, 2001 at 09:22:56PM -0600, Ben Collins-Sussman wrote:
> Kevin Pilch-Bisson <ke...@pilch-bisson.net> writes:
>
> > On Thu, Nov 15, 2001 at 01:21:04PM -0800, Greg Stein wrote:
> >
> > > Also: it appears that the wcprops did not get the extension added
> > > to them. I haven't looked into the code, but I still have
> > > ".svn/wcprops/foo.c" in my working copy. So they still appear in a
> > > "find" :-)
>
> I don't understand; what's the problem? wcprops have no "base",
> because we're never interested in looking at their diffs. They're
> just stored and retrieved as opaque tokens by the ra-layer.
>
We want to change the extension of all the files in the .svn dir so that
someone can do a "find . -name '*.svn'" in their working copy and not come
up with anything in the .svn dir.
Note that I would still _recommend_ using
"find . -name .svn -prune -o -name '*.c'" for performance reasons, but this
would protect someone who didn't think to do that.
--
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Kevin Pilch-Bisson http://www.pilch-bisson.net
"Historically speaking, the presences of wheels in Unix
has never precluded their reinvention." - Larry Wall
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Re: svn commit: rev 461 - trunk/subversion/libsvn_wc
Posted by Ben Collins-Sussman <su...@collab.net>.
Kevin Pilch-Bisson <ke...@pilch-bisson.net> writes:
> On Thu, Nov 15, 2001 at 01:21:04PM -0800, Greg Stein wrote:
>
> > Also: it appears that the wcprops did not get the extension added
> > to them. I haven't looked into the code, but I still have
> > ".svn/wcprops/foo.c" in my working copy. So they still appear in a
> > "find" :-)
I don't understand; what's the problem? wcprops have no "base",
because we're never interested in looking at their diffs. They're
just stored and retrieved as opaque tokens by the ra-layer.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: rev 461 - trunk/subversion/libsvn_wc
Posted by Kevin Pilch-Bisson <ke...@pilch-bisson.net>.
On Thu, Nov 15, 2001 at 01:21:04PM -0800, Greg Stein wrote:
> Looks good overall, although a lot of extra work is being done rather than
> using features of the existing functions.
>
> Also: it appears that the wcprops did not get the extension added to them. I
> haven't looked into the code, but I still have ".svn/wcprops/foo.c" in my
> working copy. So they still appear in a "find" :-)
Well I only did the base-props, so if there was a file with regular props on
it, it would still show up in the props directory. I guess that will be
another wc busting commit. :)
>
> And I apologize for not reviewing the patch beforehand. I would have seen
> the comments below:
>
> On Thu, Nov 15, 2001 at 12:26:46PM -0600, kevin@tigris.org wrote:
> >...
> > --- OLD/trunk/subversion/libsvn_wc/props.c Thu Nov 15 12:26:44 2001
> > +++ NEW/trunk/subversion/libsvn_wc/props.c Thu Nov 15 12:26:44 2001
> >...
> > @@ -625,12 +625,14 @@
> > SVN_WC__ADM_PROP_BASE,
> > name->data,
> > NULL);
> > + svn_stringbuf_appendcstr(tmp_prop_base, SVN_WC__BASE_EXT);
> > real_prop_base = svn_wc__adm_path (svn_stringbuf_create ("", pool),
> > 0, /* no tmp */
> > pool,
> > SVN_WC__ADM_PROP_BASE,
> > name->data,
> > NULL);
> > + svn_stringbuf_appendcstr(real_prop_base, SVN_WC__BASE_EXT);
>
> The above two chunks of code would be simplified by simply placing
> SVN_WC__BASE_EXT directly into the svn_wc__adm_path() call. No need for a
> second function call to append the extension.
Not quite. The problem is that it makes a string of the form:
.svn/[tmp/]va_arg1/va_arg2/.../path
using svn_path_add_component, and always with the args before the given path.
There is no way to add something to the end of the path (with the existing
extend_with_adm_name, sync_adm_file, etc). That was what I did first, then
I went to test it out, and was getting messed up paths. Maybe what I should
have done was make the functions take an 'is_base' are, similar to 'tmp' arg,
and paste it on if the arg was true. Wouldn't be that hard to do if people
think it is better. In fact, it might be if we want to add an extension to
non-base props files as well (which we should).
--
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Kevin Pilch-Bisson http://www.pilch-bisson.net
"Historically speaking, the presences of wheels in Unix
has never precluded their reinvention." - Larry Wall
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~