Bert Huijben wrote:
> Use svn c...@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 d...@s.a.o:

Message-ID: <4b524246.5060...@elego.de>
Date: Sat, 16 Jan 2010 23:48:38 +0100
From: Neels J Hofmeyr <ne...@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 <ne...@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));
+    }

   if (text_base == NULL)
     {
]]]

~Neels


> 
> Bert (mobile phone)
> 
> ----- Oorspronkelijk bericht -----
> Van: Neels J Hofmeyr <ne...@elego.de>
> Verzonden: zaterdag 16 januari 2010 23:48
> Aan: dev@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
> 


Attachment: signature.asc
Description: OpenPGP digital signature

Reply via email to