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

Commit strips common prefix instead of current directory (Was: RE: Commit notifies invalid paths (Issue #3168))

From: Bert Huijben <bert_at_vmoo.com>
Date: Thu, 10 Apr 2008 12:02:42 +0200

        Hi,

After further investigation, this seems to be a problem on all supported
operating systems. (not just Windows)
(Thanks lgo!)

svn_client_commit4() calls svn_client__do_commit() with as
notify_path_prefix the shortest common part
of the calculated basepath of the commit and the current directory.

e.g.
Current path: /folder1/current/
Commit path: /folder1/my/commit/location

The path placed in display_dir will be '/folder1'
(This part is pretty obscured by several translation functions using the
same variable as input and output and doing several path validations)
Variable display_dir is first filled with the current directory (which would
be a valid value) and then the last part is cut off which makes it invalid

This is then passed to svn_client__do_commit() which gets the display_path
as notify_path prefix.

This argument is documented as:
---- client.h:
   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.)
-------------

When the call finally reaches do_item_commit() in commit_util.c it does what
it said in client.h

It verifies if the commit item starts with the prefix, and if it does strips
of the prefix.

For my example it would check if '/folder1/my/commit/location/my.txt' would
start with '/folder1' and would then call notify with jusr
'my/commit/location/my.txt' which is neither a valid absolute path, nor a
valid relative path from the current path.

This example is not commonly touched from the cli, as you normally just move
your current directory to a more logical location; but GUI clients will see
this issue quite frequently as they would not change the current directory
very often.

Possible solutions:
* Only pass notify_path_prefix if the prefix matches the current directory..

(Or just pass current directory all the time. Though this might give strange
results if your current directory lies beneath your commit base)
* Add an extra parameter to svn_client__do_commit() which contains a prefix
to be applied if the path is stripped of. In the example case '..' should be
set in this value.
* Some combination of the above which calculates whether a relative prefix
would be shorter than the absolute one and chooses based on that
information.

More thoughts:
* A global search revealed there is only one caller of
svn_client__do_commit() which actually sets the notify_path_prefix argument.
All other callers (just 1) just pass NULL. (See issue #3168).
* It was quite easy to apply a new global test on invalid paths in the wc
notify handler in the SharpSvn test framework. It was only triggered by this
exact issue. All other notifies (of the current tests) received at least a
valid path.

I would like to send a patch to resolve this issue for 1.5, but I need some
more input on which solution to take.

        Bert Huijben

> -----Original Message-----
> From: Bert Huijben [mailto:B.Huijben_at_competence.biz]
> Sent: woensdag 9 april 2008 12:05
> To: dev_at_subversion.tigris.org
> Subject: Commit notifies invalid paths on windows (Issue #3168)
>
> Hi,
>
> As I noted on irc yesterday I created the following issue
> (I did some more research on the bug to find out whether other commands
> had the same problem.. but they have not)
>
> --
> The definition of svn_wc_notify_func2_t function uses svn_wc_notify_t
> which
> declares
>
> typedef struct svn_wc_notify_t {
> /** Path, either absolute or relative to the current working
> directory
> * (i.e., not relative to an anchor). */
> const char *path;
>
> But calling (at the client level in pseudo code)
> svn.Commit("E:\tmp\wc",....)
>
> when your current directory is "E:\dev\sharpsvn\src\SharpSvn.Tests"
>
> Gives you notifies where the path is "tmp\wc\testfile.txt" which is
> neither a
> valid relative path nor a valid absolute path.
>
> lgo mentioned on irc this bug is related to issue #1711.
>
> Further testing in the SharpSvn testsuite showed the issue is triggered
> on all
> notifies from Commit() but is /not/ triggered from most import calls
> (did not
> test all variants), or any of the other commands run from the SharpSvn
> testsuite (extended version of AnkhSVN's NSvn tests).
>
> I really would like to have this issue fixed for (at least) absolute
> paths in
> 1.5, as this breaks using the notify for updating the in memory cache
> in
> the
> new AnkhSVN. (More on this in issue #3147)
>
> --
>
> Looking through the code seems to tell this issue has been in
> subversion
> since far before 1.0, but I really would like to have some solution in
> 1.5 (or at least the 1.5.x branch, so I can merge it in the subversion
> build from the SharpSvn environment)
>
> Bert Huijben
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
> For additional commands, e-mail: dev-help_at_subversion.tigris.org

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-04-10 12:03:07 CEST

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

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.