Comment 7 for bug 264275

Revision history for this message
Declan McGrath (declanmg) wrote :

Update
* Firstly, thanks for the feedback Andrew
* I have refined the code and added appropriate tests to test_add.py under the blackbox tests
* I chose to leave my changes at the builtins.py level (thus making the changes subject to blackbox tests rather than unit tests) as, with my current knowledge, they fit best under the tree_files_for_add() method rather than somewhere deeper in the core of bazaar. Please advise where to move code to if this is incorrect.
* I would now like to know is there anything else I need to do? Or can this changeset be formally submitted and a start made on getting this bug resolved?

What does this fix do?
* The changes on my branch prevent the add command from adding anything (files, sym-links or directories) that lives under a sym-linked folder whose target is outside the working tree.

What code changes are there?
* When adding something via the add command, we ensure that the realpath of its parent directory lives under the basedir of the repository. If not then addition is not allowed and a user-friendly error message displayed.

Some areas to focus on when reviewing these changes
* Is there anything in there that could have a detrimental impact on performance?
* Is this the correct layer of the code to target the fix (or should it be pushed deeper)?
* Are there any extra conditions I could add to correctly cater for OSes that don't support sym-links?
* Are there any other obvious test cases not covered by the blackbox tests?

What tests are included
* 3 blackbox tests have been added
  - Adding a file under a symlinked dir inside the working copy
  - Adding a file under a symlinked dir outside the working copy
  - Adding a symlink under a symlinked dir outside the working copy