gmyers

3776: Problems with excluding files for diff/post with SVN

gmyers
brennie
 brennie
March 6, 2015 
What version are you running?
RBTools 0.7.1

What's the URL of the page containing the problem?
n/a

What steps will reproduce the problem?
1. See below
2.
3.

What is the expected output? What do you see instead?


What operating system are you using? What browser?
CentOS 7.0, Python 2.7.5, Subversion 1.7.14

Please provide any additional information below.


I've run into a couple of problems with excluding files with svn.

*Issue 1*
Exclude patterns and/or filename pattern matching do not appear to work correctly when the working copy does not come from the root of the repository.  Consider the following:

[gmyers@centos7 wc_exclude]$ svn info
Path: .
Working Copy Root Path: /home/gmyers/wc_exclude
URL: file:///home/gmyers/svn_repo/myrepo/trunk
Repository Root: file:///home/gmyers/svn_repo/myrepo
Repository UUID: 72c2860e-d8ea-4a48-91a1-97be758f8aea
<snip>

So I have checked out the trunk folder, which is one level below the repo root, into the wc_exclude working copy.  I have a single local file mod:

[gmyers@centos7 wc_exclude]$ svn status

M       a.txt

Running 'rbt diff' nominally and then also while attempting to exclude a.txt produce unexpectedly identical results:

[gmyers@centos7 wc_exclude]$ rbt diff
Index: /trunk/a.txt
===================================================================
--- /trunk/a.txt        (revision 4)
+++ /trunk/a.txt        (working copy)
@@ -1 +1,2 @@
 This is a file.
+edit

[gmyers@centos7 wc_exclude]$ rbt diff -X a.txt
Index: /trunk/a.txt
===================================================================
--- /trunk/a.txt        (revision 4)
+++ /trunk/a.txt        (working copy)
@@ -1 +1,2 @@
 This is a file.
+edit


The problem appears to be in the call to filter_diff() and the subsequent call to filename_match_any_patterns().  The base_dir param to filter_diff() is taken from the 'Working Copy Root Path' in 'svn info' and is effectively appended with the filename from the diff to produce:
    /home/gmyers/wc_exclude/trunk/a.txt
This is compared against the normalized exclude pattern which is:
    /home/gmyers/wc_exclude/a.txt
No match is found here due to the extra 'trunk' in the path.

Performing the same process in a working copy which is checked out from the repo root produced the expected behavior where a.txt is excluded and an empty diff results.

*Issue 2*
Following from issue 1, using 'Working Copy Root Path' from svn info is limiting as this information did not appear in the info output prior to SVN version 1.7 (see https://groups.google.com/forum/#!topic/reviewboard-dev/eozb9m9b7fA).  I don't have 1.6 available right now to produce an example output, but here is a different example where this was problematic in some older code: https://groups.google.com/d/msg/reviewboard/WYgpNZXWn2o/ZtCJOOtAQYQJ

*Issue 3*
Again following from issue 1, suppose we attempt to generate a diff from outside of the working copy with the --repository-url option:

[gmyers@centos7 ~]$ rbt diff --repository-url file:///home/gmyers/svn_repo/myrepo -X /trunk/a.txt 2:1
Failed to execute command: ['svn', 'info', '.']
["svn: E155007: '/home/gmyers' is not a working copy\n"]

This fails because when calling 'svn info' to set the base_dir param for filter_diff(), '.' (e.g. the current working directory) is used as the path -- base_dir=self.svn_info('.')['Working Copy Root Path'].  But, in this case we are not in a working copy, so 'svn info' fails.


I think the primary challenge currently in applying the exclusion logic is the conversion between local filesystem paths and repository paths.  I wonder if a straightforward solution is just to move the exclude pattern processing logic immediately prior to the call to convert_to_absolute_paths() in order to keep all paths in terms of the local filesystem.  Maybe this would only work for the working copy case and still be problematic for the --repository-url case?
brennie
#1 brennie 
Hi Griffin, I've started working on this as https://reviews.reviewboard.org/r/7000.

I'm trying to reproduce your first issue, but cannot seem to get SVN to checkout a subdirectory of my repository. Running the command:

    svn co file:///repos/my_repo/trunk/ trunk_checkout

Results in the following error:

    svn: E170000: URL 'file:///repos/my_repo/trunk' doesn't exist

If you could point out how to reproduce your setup, that would be very helpful.

Regards,
Barret
  • +Started
  • +Milestone-RBTools-Release0.7.x
    +SCM-Subversion
  • +brennie
gmyers
#2 gmyers 
Barret,

Here is a simple bash script that should illustrate what you need:

#######################################################
#!/bin/bash
set -x

svnadmin create /tmp/testrepo
mkdir foo
cd foo
mkdir trunk branches tags
echo "test" > trunk/a.txt
svn import . file:///tmp/testrepo -m "initial commit"
cd ..
svn co file:///tmp/testrepo/trunk foo_trunk_wc
cd foo_trunk_wc
echo "stuff" >> a.txt
rbt diff --server 127.0.0.1:8080 -X a.txt
cd ..
svn co file:///tmp/testrepo foo_root_wc
cd foo_root_wc
echo "stuff" >> trunk/a.txt
rbt diff --server 127.0.0.1:8080 -X trunk/a.txt
cd trunk
rbt diff --server 127.0.0.1:8080 -X a.txt
######################################################


This will create a repo with trunk, branches, and tags directories in the root.  Then it will checkout trunk and perform an `rbt diff` with exclusion.  For completeness it also checks out the root and performs the diff again, and then a third time in the trunk subdirectory from the root checkout.  With respect to the first issue, the initial`rbt diff` execution is where the exclusion fails while the other instances work correctly.

Incidentally, when I test this against rev 5 of review request 7000 everything works as expected.  I'll be adding feedback there soon.
brennie
#3 brennie 
The fix for this has landed on both release-0.7.x and master as commit 202154a.
  • -Started
    +Fixed