Edgewall Software
Modify

Opened 8 years ago

Closed 3 years ago

#10316 closed defect (fixed)

trac-admin deploy makes recursive copy of directories

Reported by: paulcarlisle@… Owned by: Ryan J Ollos
Priority: normal Milestone: 1.2.1
Component: admin/console Version:
Severity: normal Keywords:
Cc: Thijs Triemstra Branch:
Release Notes:

Fixed recursive directory copies with the trac-admin deploy command. The target directory cannot be equal to or below any of the source directories (e.g. $env/htdocs is not allowed as a target).

API Changes:

Description (last modified by Thijs Triemstra)

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.

Attachments (0)

Change History (20)

comment:1 by Remy Blank, 8 years ago

Keywords: needinfo added

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

comment:2 by anonymous, 8 years ago

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

Last edited 4 years ago by Jun Omae (previous) (diff)

comment:3 by Remy Blank, 8 years ago

Keywords: needinfo removed
Milestone: next-minor-0.12.x

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

comment:4 by Thijs Triemstra, 8 years ago

Cc: Thijs Triemstra added

comment:5 by Thijs Triemstra, 7 years ago

Description: modified (diff)

in reply to:  5 ; comment:6 by Christian Boos, 7 years ago

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 :)

in reply to:  6 comment:7 by Christian Boos, 7 years ago

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.

comment:8 by Christian Boos, 7 years ago

Resolution: worksforme
Status: newclosed

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

comment:9 by Peter Suter, 7 years ago

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

in reply to:  9 comment:10 by Christian Boos, 7 years ago

Resolution: worksforme
Status: closedreopened

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!

comment:11 by Ryan J Ollos, 5 years ago

Milestone: next-minor-0.12.xnext-stable-1.0.x

comment:12 by Ryan J Ollos, 5 years ago

Status: reopenednew

comment:13 by Ryan J Ollos, 3 years ago

Milestone: next-stable-1.0.xnext-stable-1.2.x

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.

comment:14 by Peter Suter, 3 years ago

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

comment:15 by Ryan J Ollos, 3 years ago

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.

comment:16 by Ryan J Ollos, 3 years ago

Milestone: next-stable-1.2.x1.2.1
Owner: set to Ryan J Ollos
Status: newassigned

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

comment:17 by Peter Suter, 3 years ago

Looks good to me.

comment:18 by Jun Omae, 3 years ago

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

  • trac/util/__init__.py

    diff --git a/trac/util/__init__.py b/trac/util/__init__.py
    index 7f2d58365..a24ef15a0 100644
    a b def is_path_below(path, parent):  
    494494    """Return True iff `path` is equal to parent or is located below `parent`
    495495    at any level.
    496496    """
    497     path = os.path.abspath(path)
    498     parent = os.path.abspath(parent)
     497    def normalize(path):
     498        if os.name == 'nt' and not isinstance(path, unicode):
     499            path = path.decode('mbcs')
     500        return os.path.normcase(os.path.abspath(path))
     501    path = normalize(path)
     502    parent = normalize(parent)
    499503    return path == parent or path.startswith(parent + os.sep)
    500504
    501505

comment:19 by Ryan J Ollos, 3 years ago

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

comment:20 by Ryan J Ollos, 3 years ago

Release Notes: modified (diff)
Resolution: fixed
Status: assignedclosed

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

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Ryan J Ollos.
The resolution will be deleted. Next status will be 'reopened'.
to as closed The owner will be changed from Ryan J Ollos to the specified user.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.