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

dionisostigris.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-unsubscribesubversion.tigris.org
For additional commands, e-mail: dev-helpsubversion.tigris.org

On 8/31/07, Eric Gillespie <epggoogle.com> wrote:
> dionisostigris.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-unsubscribesubversion.tigris.org
For additional commands, e-mail: dev-helpsubversion.tigris.org