Re: svn commit: r26396 - in trunk/subversion: include libsvn_wc

[Eric Gillespie <ep...@google.com>]

dionisos@tigris.org writes:

> * subversion/include/svn_wc.h
> * subversion/libsvn_wc/adm_crawler.c
>   (svn_wc_transmit_prop_deltas): No longer leave a temp file behind.
>     Also implement in terms of svn_wc_get_prop_diffs().

I'm a little concerned about this.  svn_client_copy depends on
having these "tempfiles" (in fact, future prop- and text- bases)
returned so it can remove them.  This is to avoid this:

1. edit foo.c
2. svn cp . $URL/snapshot (leaving behind future base)
3. edit foo.c again
4. svn ci (installs the base from 2)

This, of course, leaves you with a corrupt base file.  I know
about this because gvn (which effectively does svn cp . $URL to
snapshot a changebranch) was *not* removing the future base for a
long time.  It took me a long time to figure out why we some
times ended up with corrupt text-bases.

Anyway, I still haven't tested prop-bases.  As far as I can tell,
we'll have the exact same problem with them after this change.
Looking at client/commit_util.c:do_item_commit and
copy.c:wc_to_repos_copy, this seems to be the case: the future
prop base is saved in the tempfiles hash by the former, and the
latter removes all files in the hash.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit: r26396 - in trunk/subversion: include libsvn_wc

[Eric Gillespie <ep...@pretzelnet.org>]

"Erik Huelsmann" <eh...@gmail.com> writes:

> Right. With this change, there is no 'future base' (for props - ever),
> so it can't install it....

Oh, I see.  Nevermind then :).

> I can imagine :-) What I'd like to achieve is that this can't happen
> anymore: I'd like libsvn_wc to be able to manage its own tempfiles
> without bothering client-authors about it at all. I think this should
> be possible, probably by using pool-cleanup functions.

That would be nice.

Thanks.

--  
Eric Gillespie <*> epg@pretzelnet.org

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit: r26396 - in trunk/subversion: include libsvn_wc

[Erik Huelsmann <eh...@gmail.com>]

On 8/31/07, Eric Gillespie <ep...@google.com> wrote:
> dionisos@tigris.org writes:
>
> > * subversion/include/svn_wc.h
> > * subversion/libsvn_wc/adm_crawler.c
> >   (svn_wc_transmit_prop_deltas): No longer leave a temp file behind.
> >     Also implement in terms of svn_wc_get_prop_diffs().
>
> I'm a little concerned about this.  svn_client_copy depends on
> having these "tempfiles" (in fact, future prop- and text- bases)
> returned so it can remove them.

I see it uses remove_tmpfiles() to do so. In remove_tmpfiles, it
*only* tries to remove any temp files returned (and doesn't remove any
when none are returned). It doesn't actually use the files themselves,
nor did the api ever guarantee any files would be returned at all.

>  This is to avoid this:
>
> 1. edit foo.c
> 2. svn cp . $URL/snapshot (leaving behind future base)
> 3. edit foo.c again
> 4. svn ci (installs the base from 2)

Right. With this change, there is no 'future base' (for props - ever),
so it can't install it....

> This, of course, leaves you with a corrupt base file.  I know
> about this because gvn (which effectively does svn cp . $URL to
> snapshot a changebranch) was *not* removing the future base for a
> long time.

Ok. But the functions removing the temp baseprops won't error out when
there is no temp baseprop: the temp files are only added to the hash
of tempfiles when a temp file name is returned by
svn_wc_transmit_prop_deltas()

>  It took me a long time to figure out why we some
> times ended up with corrupt text-bases.

I can imagine :-) What I'd like to achieve is that this can't happen
anymore: I'd like libsvn_wc to be able to manage its own tempfiles
without bothering client-authors about it at all. I think this should
be possible, probably by using pool-cleanup functions.

> Anyway, I still haven't tested prop-bases.  As far as I can tell,
> we'll have the exact same problem with them after this change.

I thought the reverse: since no temp files will be created at all,
there's no possibility of them laying around too long.

> Looking at client/commit_util.c:do_item_commit and
> copy.c:wc_to_repos_copy, this seems to be the case: the future
> prop base is saved in the tempfiles hash by the former, and the
> latter removes all files in the hash.

The names of the tempfiles are only added to the hash when there's a
name to add: the interface always documented it was allowed not to
return a temp file. The only difference is that now it won't ever
return one at all.

The change introduces even more efficiency than I thought: presumably,
when committing local changes (svn ci), the temp files will be
installed as the future base, but when copying the working copy to a
tag (svn cp . <URL>) they will just be thrown away. Seems like
complete waste to me. I'll consider changing the copy code not to ask
for tempfiles at all.


bye,


Erik.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org