[PATCH] blackbox test for revert in subdir

[John Arbash Meinel]

Aaron Bentley wrote:
> John Arbash Meinel wrote:
> 
> 
>>>+1 from me, does this mean we have +2? I'll merge into jam-integration
>>>if we do.
> 
> 
> Go ahead and merge that bad boy.
> 

Do you have it as a branch, or just the patch?

> 
>>>>>>It seems that it should look at WorkingTree.open_containing, and fix
>>>>>>relpath appropriately.
>>>>
>>>>
>>>>I don't know what you mean.
> 
> 
>>>I was thinking about (psuedocode):
>>>
>>>tree, relpath = WorkingTree.open_containing(list[0])
>>>for fname in list:
>>>  fullpath = relpath + fname
>>>  do stuff here
> 
> 
> What you've entered there is broken.  Say you do 'bzr revert foo' in
> your project root.  "foo" will be the result of
> WorkingTree.open_containing(list[0])[1].
> 
> So fullpath will be "foofoo".

Well, that's why I said "pseudocode". We would have to handle when it is
a directory (so we want the full path), versus when it is a file (when
we need to take the dirname()). And besides, we would always use
pathjoin() anyway,.

> 
> Or are you saying that we should change open_containing to return the
> absolute path of the working tree root?
> 
> I believe the way it is currently handled internal_tree_files is
> correct, because we reevaluate each argument using tree.relpath.  Note
> that cwd-relative paths are completely acceptable input to tree.relpath,
> despite the documentation.
> 
> 
>>>But since WorkingTree.relpath() calls osutils.relpath() which uses
>>>abspath() on the filename we should be okay. I'm a little concerned that
>>>relpath uses abspath() since we might change directories from the time
>>>'bzr foo' is called, to the time WorkingTree.relpath() is evaluated. We
>>>don't yet, it just seems like we might.
> 
> 
> Yes, it would probably be best to accept a cwd argument in
> WorkingTree.relpath, but tree_files is meant to be called very early,
> before any cwd fun takes place.
> 

Yes, tree_files is. But is WorkingTree.relpath()? Right now it *has* to
be called early. Unless you supply an absolute path. But that is a
dangerous distinction in my mind.
I would rather have 'relpath' always take absolute paths, and we just
make all callers ensure that they have an absolute path ahead of time.

But this doesn't effect your changes, I will merge them.

> 
>>>But for now, I won't worry about it. We don't use 'os.chdir' very often
>>>inside the code, and we shouldn't anyway.
> 
> 
> Indeed.
> 
> Aaron

John
=:->
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 256 bytes
Desc: OpenPGP digital signature
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20060124/1dc25105/attachment.pgp