[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [PATCH] RE: Commit strips common prefix instead of current directory (Issue #3168) (2/3)

From: Karl Fogel <kfogel_at_red-bean.com>
Date: Wed, 14 May 2008 16:19:05 -0400

"Bert Huijben" <bert_at_qqmail.nl> writes:
> [[[
> Move the creation of final display paths to the CLI notification
> handler, to make sure notifications always receive a valid path in the
> path field.
>
> [...]

This patch causes virtually every command-line regression test to fail.

Did you run 'make check' before posting? :-)

You probably know why already: output paths are apparently now absolute
instead of relative. For example, here's the output of a simple script
to commit one file (see http://paste.lisp.org/display/60776 for the
script, if you want). This is with your patch applied:

   $ ./doit.sh
   ### Making a Greek Tree for import...
   ### Done.
   
   ### Importing it...
   ### Done.
   
   ### Making a change to A/D/G/pi...
   ### Done. Now committing it...
   
   Sending /home/kfogel/src/subversion/sandbox/wc/A/D/G/pi
   Transmitting file data .
   Committed revision 2.
   
   ### Done.
   $
   
(I've slightly reformatted this output to save some vertical space, by
the way, but not in a way that affects the results.) Now here's a run
of the same script without your patch:

   $ ./doit.sh
   ### Making a Greek Tree for import...
   ### Done.
   
   ### Importing it...
   ### Done.
   
   ### Making a change to A/D/G/pi...
   ### Done. Now committing it...
   
   Sending A/D/G/pi
   Transmitting file data .
   Committed revision 2.
   
   ### Done.
   $

If you regexp search for "^FAIL:" in tests.log after a 'make check' run,
you'll see the same problem:

   EXPECTED OUTPUT TREE:
   ROOT
     +-- svn-test-work
           +-- working_copies
                 +-- basic_tests-3
                       +-- A
                             |-- mu
                             +-- D
                                   +-- G
                                         +-- rho
   ACTUAL OUTPUT TREE:
   ROOT
     +--
           +-- home
                 +-- kfogel
                       +-- src
                             +-- subversion
                                   +-- subversion
                                         +-- tests
                                               +-- cmdline
                                                     +-- svn-test-work
                                                           +-- [...]

(There's more, but I think you get the idea.)

I assume that it was not your goal to always produce absolute paths in
svn output? If it was, we'd better discuss some more; I think in most
circumstances that is not what users want. I'm trying to locate the
mail where you described a proposed new behavior, actually...

Below is the exact patch I tested with. It's the same as the patch you
posted, just with a couple of documentation tweaks.

Best,
-Karl

[[[
Move the creation of final display paths to the CLI notification
handler, to make sure notifications always receive a valid path in the
path field.

Patch by: Bert Huijben <b.huijben_at_competence.biz>

* subversion/include/svn_wc.h
  (svn_wc_notify_t): New field path_prefix.

* subversion/libsvn_wc/util.c
  (svn_wc_create_notify, svn_wc_dup_notify): Handle new path_prefix field.

* subversion/libsvn_client/client.h
  (svn_client__do_commit): Document the new handling of notify_path_prefix.

* subversion/libsvn_client/commit_util.c
  (svn_client__do_commit, do_item_commit) Remove local handling of
    notify_path_prefix. Instead pass path_prefix to the notification
    handler.
  
* subversion/svn/notify.c
  (notify): Use the new path_prefix to split the first part of the path
    if it matches the specified prefix. Don't bother testing for urls
    as the path is documented to be a local path.
]]]

Index: subversion/include/svn_wc.h
===================================================================
--- subversion/include/svn_wc.h (revision 31174)
+++ subversion/include/svn_wc.h (working copy)
@@ -909,6 +909,11 @@
    * left and right sides of the merge are from the same URL. In all
    * other cases, it is @c NULL. @since New in 1.5 */
   svn_merge_range_t *merge_range;
+ /** If non-NULL, specifies an absolute path prefix that can be subtracted
+ * from the start of the absolute path in @c path. Its purpose is to
+ * allow notification to remove a common prefix from all the paths
+ * displayed for an operation. @since New in 1.6 */
+ const char *path_prefix;
   /* NOTE: Add new fields at the end to preserve binary compatibility.
      Also, if you add fields here, you have to update svn_wc_create_notify
      and svn_wc_dup_notify. */
Index: subversion/libsvn_wc/util.c
===================================================================
--- subversion/libsvn_wc/util.c (revision 31174)
+++ subversion/libsvn_wc/util.c (working copy)
@@ -128,6 +128,7 @@
   ret->revision = SVN_INVALID_REVNUM;
   ret->changelist_name = NULL;
   ret->merge_range = NULL;
+ ret->path_prefix = NULL;
 
   return ret;
 }
@@ -164,6 +165,8 @@
     ret->changelist_name = apr_pstrdup(pool, ret->changelist_name);
   if (ret->merge_range)
     ret->merge_range = svn_merge_range_dup(ret->merge_range, pool);
+ if (ret->path_prefix)
+ ret->path_prefix = apr_pstrdup(pool, ret->path_prefix);
 
   return ret;
 }
Index: subversion/libsvn_client/client.h
===================================================================
--- subversion/libsvn_client/client.h (revision 31174)
+++ subversion/libsvn_client/client.h (working copy)
@@ -915,9 +915,8 @@
    CTX->NOTIFY_FUNC/CTX->BATON will be called as the commit progresses, as
    a way of describing actions to the application layer (if non NULL).
 
- NOTIFY_PATH_PREFIX is used to send shorter, relative paths to the
- notify_func (it's a prefix that will be subtracted from the front
- of the paths.)
+ NOTIFY_PATH_PREFIX will be passed to CTX->notify_func2() as the
+ common absolute path prefix of the committed paths. It can be NULL.
 
    If the caller wants to keep track of any outstanding temporary
    files left after the transmission of text and property mods,
Index: subversion/libsvn_client/commit_util.c
===================================================================
--- subversion/libsvn_client/commit_util.c (revision 31174)
+++ subversion/libsvn_client/commit_util.c (working copy)
@@ -1056,7 +1056,8 @@
   void *edit_baton; /* commit editor's baton */
   apr_hash_t *file_mods; /* hash: path->file_mod_t */
   apr_hash_t *tempfiles; /* hash of tempfiles created */
- const char *notify_path_prefix; /* notification path prefix */
+ const char *notify_path_prefix; /* notification path prefix
+ (NULL is okay, else abs path) */
   svn_client_ctx_t *ctx; /* client context baton */
   apr_hash_t *commit_items; /* the committables */
 };
@@ -1080,7 +1081,6 @@
   svn_wc_adm_access_t *adm_access = cb_baton->adm_access;
   const svn_delta_editor_t *editor = cb_baton->editor;
   apr_hash_t *file_mods = cb_baton->file_mods;
- const char *notify_path_prefix = cb_baton->notify_path_prefix;
   svn_client_ctx_t *ctx = cb_baton->ctx;
 
   /* Do some initializations. */
@@ -1121,20 +1121,9 @@
      describe what we're about to do to this item. */
   if (ctx->notify_func2)
     {
- /* Convert an absolute path into a relative one (if possible.) */
- const char *npath = NULL;
+ const char *npath = item->path;
       svn_wc_notify_t *notify;
 
- if (notify_path_prefix)
- {
- if (strcmp(notify_path_prefix, item->path))
- npath = svn_path_is_child(notify_path_prefix, item->path, pool);
- else
- npath = ".";
- }
- if (! npath)
- npath = item->path; /* Otherwise just use full path */
-
       if ((item->state_flags & SVN_CLIENT_COMMIT_ITEM_DELETE)
           && (item->state_flags & SVN_CLIENT_COMMIT_ITEM_ADD))
         {
@@ -1183,6 +1172,7 @@
       if (notify)
         {
           notify->kind = item->kind;
+ notify->path_prefix = cb_baton->notify_path_prefix;
           (*ctx->notify_func2)(ctx->notify_baton2, notify, pool);
         }
     }
@@ -1434,22 +1424,11 @@
       if (ctx->notify_func2)
         {
           svn_wc_notify_t *notify;
- const char *npath = NULL;
-
- if (notify_path_prefix)
- {
- if (strcmp(notify_path_prefix, item->path) != 0)
- npath = svn_path_is_child(notify_path_prefix, item->path,
- subpool);
- else
- npath = ".";
- }
- if (! npath)
- npath = item->path;
- notify = svn_wc_create_notify(npath,
+ notify = svn_wc_create_notify(item->path,
                                         svn_wc_notify_commit_postfix_txdelta,
                                         subpool);
           notify->kind = svn_node_file;
+ notify->path_prefix = notify_path_prefix;
           (*ctx->notify_func2)(ctx->notify_baton2, notify, subpool);
         }
 

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-05-14 22:19:19 CEST

This is an archived mail posted to the Subversion Dev mailing list.