Search for missing CVS files in the Attic
Review Request #555 — Created Sept. 17, 2008 and discarded
Whenever a CVS file is not found on the server an error occurs. Now it is possible that the file was moved out of or into the Attic. In this case, the patched code looks for the file also in the Attic (if the file was not found outside the Attic) of outside the Attic (if the file could not be found within the Attic) As I do not have write access to the reviewboard repository, the contents of the review cannot be commited by me. A member of the project has to merge the changeset into the file. Thanks.
I tested both ways within our development project
TF
-
I did some changes but I will wait until these changes are testet in our installation before I will publish the next diff version.
-
I did not want to duplicate code. My code extension is only necessary if no file was found. Then, the cvs access has to be done again, this time with the Attic-path-element added or removed, depending on the previous way it failed. I added a second method as I had to add a parameter to prevent endless recursion. The Attic-swap may only be done once as it could cycle endless (try with Attic, then without, then with, then without, ...) If there is a possibility in python to set a parameter to a default value, then this would be better (e.g. cat_file(self, filename, revision, tryAttic = 1). I don't know this as I am "only" a java and .net developer with only little experience in python.
-
-
-
not so good as this would imply that the CVS repository resides on the same os as Review Board does, what normally must not be true. I made the implementation better as I checked before, what kind of path separator is used within the current cvs filename and adept the Attic-part to that.
-
Sorry for taking so long to get to this. I'm unsure about your changes here. I have more details in the comments below, but it looks like the core problem is that we strip out the "/Attic/" early. I would imagine what we really want to do is save the non-stripped version and check both. What tests did you run? By the way, to reply to this, comment on the review request page, not in the diff viewer. You can't reply to existing comments there, and it really messes with the e-mails and flow of the discussion.
-
We already do this above (see the line above that does a filename.rsplit('/Attic/', 1)). So this probably isn't necessary. I imagine we'll just need to try putting the "/Attic/" on. -
The rest of this file makes the assumption that paths are always "/". I believe CVS on both Windows and Linux represents paths at this point as "/", since it's dealing in server paths. Given the assumption, we probably don't need the Windows-style checks due to this, as they just won't work anyway. If the assumption is wrong on Windows, then we'll have to do a lot more work in this function and should just have some path normalization function at the end that goes from "/" to "\". So we should really only need the "/" checks for now.