Patrick Steinhardt wrote:
On Wed, Nov 16, 2016 at 04:30:55PM +0000, Julian Foad wrote:
On 07/11/16, Patrick Steinhardt wrote:
attached is a patch that fixes a test for Git mirrors. The error
results from the fact that Git does not track empty directories,
which one test relies upon.

As we're already doing some path processing for the missing
directories and as we're also creating directories in the other
import tests, I think this is the most sensible place to fix the
issue. For Subversion working copies it becomes a no-op anyway as
the directories already exist.

I just took a quick look at this. Your patch is of a kind which would
have to be repeated for each new test that uses the same approach. It
also seems a bit redundant: that small extra bit of code is almost
enough code to create the whole tree from scratch.

It seems to me that storing what is supposed to be a plain unversioned
disk tree within our Subversion versioned tree isn't the right approach.
Instead I think it would be better to store the import data tree as a
Python data structure within the test and create it at run time. If
there isn't already a handy function for doing this, there should be.

Something like this:

[[[
Index: subversion/tests/cmdline/import_tests.py
===================================================================
--- subversion/tests/cmdline/import_tests.py    (revision 1767744)
+++ subversion/tests/cmdline/import_tests.py    (working copy)
@@ -446,8 +446,8 @@ def import_inherited_ignores(sbox):
    #   DIR7
    #     file7.foo
    #     DIR8.noo
-  import_tree_dir = os.path.join(os.path.dirname(sys.argv[0]),
-                                 'import_tests_data', 'import_tree')
+  import_tree_path = 'import_tree'
+  import_tree_dir = os.path.join(sbox.wc_dir, import_tree_path)

    # Relative WC paths of the imported tree.
    dir1_path  = os.path.join('DIR1.noo')
@@ -466,6 +466,31 @@ def import_inherited_ignores(sbox):
    file7_path = os.path.join('DIR6', 'DIR7', 'file7.foo')
    dir8_path  = os.path.join('DIR6', 'DIR7', 'DIR8.noo')

+  import_dirs = [os.path.join(import_tree_path, p) for p in [
+    dir1_path,
+    dir2_path,
+    dir3_path,
+    dir4_path,
+    dir5_path,
+    dir6_path,
+    dir7_path,
+    dir8_path,
+    ]]
+  import_files = [os.path.join(import_tree_path, p) for p in [
+    file1_path,
+    file2_path,
+    file3_path,
+    file4_path,
+    file5_path,
+    file6_path,
+    file7_path,
+    ]]
+
+  sbox.simple_mkdir(import_tree_path)
+  sbox.simple_mkdir(*[svntest.wc.to_relpath(p) for p in import_dirs])
+  sbox.simple_add_text('A file',
+                       *[svntest.wc.to_relpath(p) for p in import_files])
+
    # Import the tree to ^/A/B/E.
    # We should not see any *.noo paths because those are blocked at the
    # root of the repository by the svn:global-ignores property.  Likewise
]]]

and:

    svn rm subversion/tests/cmdline/import_tests_data

Does that work for you?

It certainly would, yes. I opted for the other path as Git
mirrors are not really something Subversion developers care
about, so I wanted to keep the patch as small as possible. If
your approach is preferred, though, I don't mind to take that one
instead.

Would anyone else care to comment, and review my patch if you think it looks good in principle, please?

- Julian

Reply via email to