> -----Original Message-----
> From: Neels J Hofmeyr [mailto:neels_at_elego.de]
> Sent: zondag 17 januari 2010 22:47
> To: dev_at_subversion.apache.org
> Cc: Bert Huijben
> Subject: Re: libsvn_wc bug: pristine contents for locally replaced file
>
> Bert Huijben wrote:
> > Use svn cat_at_BASE to get the file.
>
> Same problem. 'svn cat' seems to select BASE by default.
> My point is that that fails with an error that shouldn't happen.
>
> > File is moved to revertbase to allow replacement with history.
>
> But thanks, I understand now: the original base contents are, in this
> case,
> in the revert-base.
>
> The point remains that svn_wc__get_pristine_contents() acts buggy when
> it's
> called on a locally replaced file.
>
> The following patch solves the problem and passes `make check'. But I'm
> not
> sure if simply judging the state from the existence of revert- and
> text-base
> files is "Good". It makes sense logically, but is it the right method?
>
> [[[
> Fix error when WC tries to get the pristine contents of a locally
> replaced
> file. To reproduce, see this mail to dev_at_s.a.o:
>
> Message-ID: <4B524246.5060203_at_elego.de>
> Date: Sat, 16 Jan 2010 23:48:38 +0100
> From: Neels J Hofmeyr <neels_at_elego.de>
> Subject: libsvn_wc bug: pristine contents for locally replaced file
>
> * subversion/libsvn_wc/adm_ops.c
> (svn_wc__get_pristine_contents): Prefer a revert-base if present.
>
> Patch by: Neels J. Hofmeyr <neels_at_elego.de>
>
> --This line, and those below, will be ignored--
> Index: subversion/libsvn_wc/adm_ops.c
> ===================================================================
> --- subversion/libsvn_wc/adm_ops.c (revision 900159)
> +++ subversion/libsvn_wc/adm_ops.c (working copy)
> @@ -2337,9 +2337,16 @@ svn_wc__get_pristine_contents(svn_stream
> apr_pool_t *scratch_pool)
> {
> const char *text_base;
> + svn_node_kind_t kind;
>
> - SVN_ERR(svn_wc__text_base_path(&text_base, db, local_abspath, FALSE,
> - scratch_pool));
> + SVN_ERR(svn_wc__text_revert_path(&text_base, db, local_abspath,
> + scratch_pool));
> + SVN_ERR(svn_io_check_path(text_base, &kind, scratch_pool));
> + if (kind == svn_node_none)
> + {
> + SVN_ERR(svn_wc__text_base_path(&text_base, db, local_abspath,
> FALSE,
> + scratch_pool));
> + }
This breaks svn cat file_at_BASE for replacement with history, which normally
shows the pristine/historic version of the added file. I'm surprised that
this doesn't fail any tests, as it would at least break diff support in
AnkhSVN and TortoiseSVN on these files. (These tools use a visual diff
between svn cat file_at_BASE and the working file.
If you want to fix this behavior, you would have to check the exact status
information to determine that you are exactly in the replacement without
history case.
This is one of the hardest statuses to detect with WC-NG (which was easy as
entries), but luckily we don't need it any more for the processing in the
three tree storage model with a SHA-1 based pristine store. There we store a
hash in BASE and in WORKING.
While building the in-db property code I found out that relying on files to
exist in our working copy area, doesn't work out correctly. In some cases we
just accept them as if the user sets them if we forgot to delete them
earlier. (I fixed a very old issue in svn_wc_add4() that added properties on
a brand new file in a specific series of events in our test suite).
Bert
>
> if (text_base == NULL)
> {
> ]]]
>
> ~Neels
>
>
> >
> > Bert (mobile phone)
> >
> > ----- Oorspronkelijk bericht -----
> > Van: Neels J Hofmeyr <neels_at_elego.de>
> > Verzonden: zaterdag 16 januari 2010 23:48
> > Aan: dev_at_subversion.apache.org
> > Onderwerp: libsvn_wc bug: pristine contents for locally replaced file
> >
> > Hi,
> >
> > is there a good reason why a local replace has to get rid of the
> pristine
> > base file? Because, if the file was kept, the problems described
> below would
> > be resolved.
> >
> >
> > <tell-mode>
> > I stumbled over an error using 'svn cat <wc_path>' on a locally
> replaced
> > file. (Not a common use case, but read on.) 'svn cat <wc_path>'
> appears to
> > want to output the pristine base content (which is not documented).
> >
> > But when a file is locally replaced (not committed), it currently has
> no
> > pristine base file, apparently;
> > 'svn cat wc/locally_replaced_file'
> > calls
> > svn_wc__get_pristine_contents()
> > which errors with:
> >
> > [[[
> > $ svn cat file
> > subversion/svn/cat-cmd.c:81: (apr_err=2)
> > subversion/svn/util.c:960: (apr_err=2)
> > subversion/libsvn_client/cat.c:88: (apr_err=2)
> > subversion/libsvn_client/cat.c:88: (apr_err=2)
> > subversion/libsvn_subr/stream.c:774: (apr_err=2)
> > subversion/libsvn_subr/stream.c:774: (apr_err=2)
> > subversion/libsvn_subr/io.c:2711: (apr_err=2)
> > svn: Can't open file '/tmp/wc/.svn/text-base/file.svn-base': No such
> file or
> > directory
> > ]]]
> > (reproduction script attached)
> >
> > 'svn cat' is just an example of how to hit this. This same function
> is used
> > in many other places. A quick impact grep study suggests at least
> export,
> > copy, update, diff, and probably others.
> > (Todo: investigation on whether current callers can hit a locally
> replaced
> > file and whether they work around it.)
> > </tell-mode>
> >
> > <bug-hunting>
> > I guess svn_wc__get_pristine_contents() wants to return the contents
> of the
> > file that were committed in revision <BASE>. But the implementation
> expects
> > a file to exist which isn't there:
> >
> > [[[
> > svn_error_t *
> > svn_wc__get_pristine_contents(svn_stream_t **contents,
> > svn_wc__db_t *db,
> > const char *local_abspath,
> > apr_pool_t *result_pool,
> > apr_pool_t *scratch_pool)
> > {
> > const char *text_base;
> >
> > SVN_ERR(svn_wc__text_base_path(&text_base, db, local_abspath,
> FALSE,
> > scratch_pool));
> >
> > if (text_base == NULL)
> > {
> > *contents = NULL;
> > return SVN_NO_ERROR;
> > }
> >
> > return svn_stream_open_readonly(contents, text_base, result_pool,
> > scratch_pool);
> > // ^^^^^ hits error here, file *text_base does not exist.
> > }
> > ]]]
> >
> >
> > I see two ill things:
> >
> > (1) Looking at the function's intention, it should return an empty
> stream if
> > there is no base file. But svn_wc__text_base_path() returns a path
> that
> > doesn't exist.
> >
> > (2) When the file is locally replaced, it theoretically *does* have a
> > pristine base, i.e. the file's content committed at revision <BASE>.
> The
> > function fails to return that content.
> > </bug-hunting>
> >
> >
> > So, back to the question: is there a good reason why a local delete
> followed
> > by a local add has to get rid of the pristine base file?
> >
> > Thanks,
> > ~Neels
> >
>
Received on 2010-01-17 23:15:39 CET