trac-admin deploy makes recursive copy of directories

[paulcarlisle@…]

When I run trac-admin [project] deploy [output_dir], output_dir contains:

• output_dir
  • acct_mgr/
  • common/
  • site/
    • acct_mgr/
    • common/
    • site/

Subdirectory contents are also duplicated. The recursion stops at one level, as shown.

There is no documentation on what these output directories should look like, so it's possible that this is correct. If so, more attention needs to be paid to the documentation to provide a clear description of what should be there.

[Remy Blank]

I don't understand what you see below output_dir. Could you please clarify (or fix the WikiFormatting)?

[anonymous]

*output_dir

*acct_mgr *common *site

*acct_mgr *common *site

Both the contents of the duplicated subdirs are complete copies. As noted, the recursion stops at this level.

[Remy Blank]

Ok, that's weird. We need to check that.

[Christian Boos]

Replying to thijstriemstra:

Does this indicate you're able to reproduce this somehow? Otherwise I think we can "worksforme" this one.

There used to be a bug that might have explained that behavior, taking "." as a source… Rémy probably remembers this better than I do :)

[Christian Boos]

Replying to cboos:

There used to be a bug that might have explained that behavior, taking "." as a source… Rémy probably remembers this better than I do :)

Ok, the changeset I was thinking about was actually done by me (r10406). I'm not sure the glitch mentioned there could have explain the present issue, but it's indeed similar.

[Christian Boos]

… and closing, since without more detailed reproduction steps, we probably won't figure out what happened.

[Peter Suter]

I think I once by mistake called trac-admin <project> deploy <project>/htdocs, which recursively copied htdocs below htdocs/site.

[Christian Boos]

Replying to psuter:

I think I once by mistake called trac-admin <project> deploy <project>/htdocs, which recursively copied htdocs below htdocs/site.

Ah yes, this is a good reproduction recipe, thanks!

[Ryan J Ollos]

Moved ticket assigned to next-stable-1.0.x since maintenance of 1.0.x is coming to a close. Please move the ticket back if it's critical to fix on 1.0.x.

[Peter Suter]

Maybe we should abort if is_path_below(dest, source)​ before calling copytree(source, dest)​?

[Ryan J Ollos]

That sounds like a good idea. In that case we would iterate through all the template_providers with is_path_below, before iterating again to do the copytree operations?: tags/trac-1.2/trac/env.py@:984,997#L974​.

[Ryan J Ollos]

Proposed changes in log:rjollos.git:t10316_avoid_recursion_in_deploy. I'll add tests before committing.

[Peter Suter]

Looks good to me.

[Jun Omae]

I think is_path_below() should use os.path.normcase() for case-insensitive filesystem on Windows.

trac/util/__init__.py

@@ -494 +494 @@
     """Return True iff `path` is equal to parent or is located below `parent`
     at any level.
     """
-    path = os.path.abspath(path)
-    parent = os.path.abspath(parent)
+    def normalize(path):
+        if os.name == 'nt' and not isinstance(path, unicode):
+            path = path.decode('mbcs')
+        return os.path.normcase(os.path.abspath(path))
+    path = normalize(path)
+    parent = normalize(parent)
     return path == parent or path.startswith(parent + os.sep)
 
 

[Ryan J Ollos]

Thanks, I'll add comment:18 changes, test on Windows and commit with tests.

[Ryan J Ollos]

Fixed on 1.2-stable in r15680, merged to trunk in r15681.