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

Sorry for the delay in responding. I was a few days away from home..

> -----Original Message-----
> From: Karl Fogel [mailto:kfogel_at_red-bean.com]
> Sent: donderdag 1 mei 2008 20:03
> To: Bert Huijben
> Cc: 'Mark Phippard'; 'Bert Huijben'; dev_at_subversion.tigris.org
> Subject: Re: [PATCH] RE: Commit strips common prefix instead of
current
> directory (Issue #3168)
>
> "Bert Huijben (TCG)" <b.huijben_at_competence.biz> writes:
> > So both functions work correctly, following their own definition..
> But the
> > result of those two combined is commit notifications use a not-
> relative and
> > a not-absolute path. (Which breaks using the notifications to keep
an
> in
> > memory-file-state-cache up to date).
> >
> > The real problem is: How can we fix this?
> >
> > A user does not expect to see absolute paths when committing, unless
> he
> > passed absolute paths to 'svn ci' and he does not expect to see
> relative
> > paths if he passed only absolute paths.
>
> Hmmm, yeah, choosing a good behavior here is hard. The primary goal
is
> predictability, I would think. One possibility, off the top of my
> head:
>
> 1. If any path was absolute, print all paths as absolute.

This should be easy to implement and would fix my usage pattern. (I
always provide absolute paths as I can't change the working directory
without side effects)

>
> 2. Otherwise, print all paths as relative to the top ("root") of
the
> working copy, OR:
>
> Print them relative to cwd iff cwd is a (grand, etc)parent of
all
> the committed paths, else default to relative-to-root-of-wc.

The notify method is documented to be called with absolute paths, or
relative from the current working directory (no other values allowed).

To 'print' paths relative from another directory than cwd in the
svn-cli, the extra information should be provided to the cli notify
handler which could then print them in any other format.
(Not sure if we should calculate the working copy root; but providing
the action root should be very easy and it would give the same cli
output as the current version)

The easiest solution to integrate 1 and 2 is to move the path splitting
code to the cli notification handler and pass absolute paths to the
notification handler. The attached patch does exactly this.

So this patch does not fix the issue for the cli, but it will for
library users. And it moves the code to a place where it can be changed
per client. (I don't think the CLI output is documented to be a valid
path or the issue would have been noticed a long time ago)

        Bert

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

* subversion/include/svn_wc.h
  (svn_wc_notify_t) Added path_prefix containing an absolute prefix
which
  can be subtracted from the absolute path passed. Noted versions in
which
  new fields where added.

* subversion/libsvn_wc/util.c
  (svn_wc_create_notify, svn_wc_dup_notify) Added path_prefix handling

* subversion/libsvn_client/client.h
  (svn_client__do_commit) Updated documentation to document the new
handling
  of notify_path_prefix

* subversion/libsvn_client/commit_util.c
  (svn_client__do_commit, do_item_commit) Removed local handling of
  notify_path_prefix. Instead pass the prefix path via path_prefix to
the
  notification handler.
  
* subversion/libsvn_client/commit.c
  (svn_client_commit4) Document the calculation of the notify prefix by
renaming
  and adding a local variable.

* 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.

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

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org