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 >
signature.asc
Description: OpenPGP digital signature