[PATCH] issue #2740: "out-of-date" error text varies depending on access method

Hello,

To get my feet wet with subversion issues I've looked into
issue #2740 and here is a trivial patch to fix it:

Index: subversion/libsvn_repos/commit.c
===================================================================
--- subversion/libsvn_repos/commit.c (revision 25496)
+++ subversion/libsvn_repos/commit.c (working copy)
@@ -122,7 +122,8 @@
 out_of_date(const char *path, const char *txn_name)
 {
   return svn_error_createf(SVN_ERR_FS_TXN_OUT_OF_DATE, NULL,
- _("Out of date: '%s' in transaction '%s'"),
+ _("Your file or directory '%s' is probably "
+ "out-of-date in transaction '%s'"),
                            path, txn_name);
 }
 
I've also attached a recipe I used to reproduce the bug.

Here's an example log message as per guidelines:

[[[
Fix issue #2740: "out-of-date" error text varies depending on access method

* subversion/libsvn_repos/commit.c
  (out_of_date): Use similar wording as in libsvn_ra_neon/commit.c
  for out-of-date error message.
]]]

I'm not sure how to update the locales for the message.

In spite of the issue submitter stating

        "that first message is a bit user-hostile: we poor users
        don't know what transactions are!"

I decided to keep the "in transaction '%s'" bit in for now.
I'm not sure whether it's better to just remove it from the file://
case or not. I'm not sure whether an equivalent of txn_name can be
retrieved from a commit_ctx_t object as used in libsvn_ra_neon/commit.c
so we could add the transaction name to the http case error message.

One issue that remains is that the path printed in the error
message is still different for each type of access method.

The following examples are based on a simple project that only
contains one directory and one file:

        dir/
        dir/file

This is what is printed by the file access method without my patch,
using subversion-1.4.3_2 from FreeBSD ports:

[stsp@ted ~/elego/issues/2740/ws2-file]$ svn commit -m ""
Sending dir/file
svn: Commit failed (details follow):
svn: Out of date: '/dir/file' in transaction '2-1'

And this is what is printed with the http access method:

[stsp@ted ~/elego/issues/2740/ws2-http]$ svn commit -m ""
Sending dir/file
svn: Commit failed (details follow):
svn: Your file or directory 'file' is probably out-of-date
svn: The version resource does not correspond to the resource within the transaction. Either the requested version resource is out of date (needs to be updated), or the requested version resource is newer than the transaction root (restart the commit).

With current svn as of yesterday + my patch I now get:

[stsp@ted ~/elego/issues/2740/ws2-file]$ ../../../prefix/bin/svn commit -m ""
Sending dir/file
svn: Commit failed (details follow):
svn: Your file or directory '/dir/file' is probably out-of-date in transaction 'ted-stsp-lan-75017-1182527566183574-1'

And with http:

[stsp@ted ~/elego/issues/2740/ws2-http]$ ../../../prefix/bin/svn commit -m ""
Sending dir/file
svn: Commit failed (details follow):
svn: Your file or directory 'file' is probably out-of-date
svn: The version resource does not correspond to the resource within the transaction. Either the requested version resource is out of date (needs to be updated), or the requested version resource is newer than the transaction root (restart the commit).

So we get '/dir/file' with file:// and just 'file' with http://

I've tried to make both messages print '/dir/file', but it seems
that the full path is not yet available when the error is thrown
in libsvn_ra_neon.

Code snippets below are from libsvn_ra_neon/commit.c.

It seems that rsrc->wr_url is supposed to contain the file
path relative to the working copy:
 ** WR_URL refers to a working resource for this resource

But it also says:
 ** WR_URL can be NULL if the resource has not (yet) been checked out.

And indeed at the point the error is thrown, rsrc->wr_url is still NULL.
So currently the error message prints rsrc->local_path instead,
which is just "file" in my example, even without piping it through
svn_path_local_style() before printing:

 if (err->apr_err == SVN_ERR_FS_CONFLICT)
        return svn_error_createf
          (err->apr_err, err,
           _("Your file or directory '%s' is probably out-of-date"),
           svn_path_local_style(rsrc->local_path, pool));

The code that initialises rsrc->wr_url is a couple of lines later,
after the checkout has completed successfully.

Is there a nice way to normalise the path consistently in both cases?

Or do you think that the current patch makes the error messages
similar enough to satisfy #2740? Users see an additional error
message thrown by the neon layer anyway already when using http,
so the client output will never be exactly the same...

Sorry for ranting so long about a trivial issue :)
I will possibly try to work on more subversion issues in the
future so this is just to get me started.

Thanks,

-- 
Stefan Sperling <stsp@elego.de>                 Software Developer
elego Software Solutions GmbH                            HRB 77719
Ohmstrasse 9                              Tel:  +49 30 40 04 19 29
10179 Berlin                              Fax:  +49 30 23 45 86 95
http://www.elego.de                 Geschaeftsfuehrer: Olaf Wagner