Submitter:	Christian Hammond	Reviewers
Branch:	trunk	Groups:	reviewboard
Bugs:	362	People:
Change Number:	None	Repository:	Review Board SVN

Description:

Fixed issues with SVN keyword expansion. If a file had $Id$ or $Date$ or something and the keyword was defined in that file's svn:keywords property, svn cat would return the file with the keyword expanded. This broke patching for us and essentially meant that any patches modifying anything near a keyword expansion would break.

We now collapse the keyword by turning $Keyword:.*$ into $Keyword$. We only do this for keywords actually defined in that file's svn:keywords, in order to prevent collapsing things that aren't actually defined keywords but look like them.

This also supports known keyword aliases.

Testing Done:

Unit tests succeed! Debug output shows we're collapsing correctly.

Diff revision 4 (Latest)

Jump to revision:	1 2 4
Changes between r4 and:	1 2 4

Files Changed:

/trunk/reviewboard/scmtools/svn.py: 5 changes [ 1 2 3 4 5 ]
/trunk/reviewboard/scmtools/tests.py: 4 changes [ 1 2 3 4 ]
/trunk/reviewboard/scmtools/testdata/svn_repo/db/current: 1 change [ ]
/trunk/reviewboard/scmtools/testdata/svn_repo/db/revprops/4: 1 change [ new content ]
/trunk/reviewboard/scmtools/testdata/svn_repo/db/revs/4: binary file

Expand changes

 import re
 try:
     from pysvn import ClientError, Revision, opt_revision_kind
 except ImportError:
     pass
 from reviewboard.diffviewer.parser import DiffParser
 from reviewboard.scmtools.core import \
     SCMError, FileNotFoundError, SCMTool, HEAD, PRE_CREATION, UNKNOWN
 class SVNTool(SCMTool):
     # Mapping of keywords to known aliases
     keywords = {
         # Standard keywords
         'Date':                ['Date', 'LastChangedDate'],
         'Revision':            ['Revision', 'LastChangedRevision', 'Rev'],
         'Author':              ['Author', 'LastChangedBy'],
         'HeadURL':             ['HeadURL', 'URL'],
         # Aliases
         'LastChangedDate':     ['LastChangedDate', 'Date'],
         'LastChangedRevision': ['LastChangedRevision', 'Rev', 'Revision'],
         'LastChangedBy':       ['LastChangedBy', 'Author'],
         'URL':                 ['URL', 'HeadURL'],
+    }
     def __init__(self, repository):
         self.repopath = repository.path
         if self.repopath[-1] == '/':
             self.repopath = self.repopath[:-1]
     def get_file(self, path, revision=HEAD):
         if not path:
             raise FileNotFoundError(path, revision)
         try:
-            return self.client.cat(self.__normalize_path(path),
+            normpath = self.__normalize_path(path)
-                                   self.__normalize_revision(revision))
+            normrev  = self.__normalize_revision(revision)
             data = self.client.cat(normpath, normrev)
             # Find out if this file has any keyword expansion set.
             # If it does, collapse these keywords. This is because SVN
             # will return the file expanded to us, which would break patching.
             keywords = self.client.propget("svn:keywords", normpath, normrev,
                                            recurse=True)
             if normpath in keywords:
                 data = self.collapse_keywords(data, keywords[normpath])
             return data
         except ClientError, e:
             stre = str(e)
             if 'File not found' in stre:
                 raise FileNotFoundError(path, revision, str(e))
             elif 'callback_ssl_server_trust_prompt required' in stre:
                 raise SCMError(
                     'HTTPS certificate not accepted.  Please ensure that ' +
                     'the proper certificate exists in ~/.subversion/auth ' +
                     'for the user that reviewboard is running as.')
             else:
                 raise SCMError(e)
     def collapse_keywords(self, data, keyword_str):
         """
         Collapse SVN keywords in string.
         SVN allows for several keywords (such as $Id$ and $Revision$) to
         be expanded, though these keywords are limited to a fixed set
         (and associated aliases) and must be enabled per-file.
         Keywords can take two forms: $Keyword$ and $Keyword::     $
         The latter allows the field to take a fixed size when expanded.
         When we cat a file on SVN, the keywords come back expanded, which
         isn't good for us as we need to diff against the collapsed version.
         This function makes that transformation.
         """
         def repl(m):
             if m.group(2):
                 return "$%s::%s$" % (m.group(1), " " * len(m.group(3)))
             return "$%s$" % m.group(1)
         # Get any aliased keywords
         keywords = [keyword
                     for name in keyword_str.split(" ")
                     for keyword in self.keywords.get(name, [])]
         return re.sub(r"\$(%s):(:?)([^\$]+)\$" % '|'.join(keywords),
                       repl, data)
     def parse_diff_revision(self, file_str, revision_str):
         if revision_str == "(working copy)":
             return file_str, HEAD

     def testKeywordDiff(self):
         """Testing parsing SVN diff with keywords"""
         # 'svn cat' will expand special variables in svn:keywords,
         # but 'svn diff' doesn't expand anything.  This causes the
         # patch to fail if those variables appear in the patch context.
-        diff = "Index: Makefile\n==========================================" + \
+        diff = "Index: Makefile\n" \
-               "=========================\n--- Makefile    (revision 3)\n++" + \
+               "===========================================================" \
-               "+ Makefile    (working copy)\n@@ -1,4 +1,5 @@\n # $Id$\n+# " + \
+               "========\n" \
-               "foo\n include ../tools/Makefile.base-vars\n NAME = misc-doc" + \
+               "--- Makefile    (revision 4)\n" \
-               "s\n OUTNAME = svn-misc-docs\n"
+               "+++ Makefile    (working copy)\n" \
                "@@ -1,6 +1,7 @@\n" \
                " # $Id$\n" \
                " # $Rev$\n" \
                " # $Revision::     $\n" \
                "+# foo\n" \
                " include ../tools/Makefile.base-vars\n" \
                " NAME = misc-docs\n" \
                " OUTNAME = svn-misc-docs\n"
         filename = 'trunk/doc/misc-docs/Makefile'
-        rev = Revision('3')
+        rev = Revision('4')
         file = self.tool.get_file(filename, rev)
-        newfile = patch(diff, file, filename) # Throws an exception!
+        patch(diff, file, filename)
 class PerforceTests(unittest.TestCase):
     """Unit tests for perforce.

/trunk/reviewboard/scmtools/testdata/svn_repo/db/current
Revision 1190		New Change
1	3 5 1	1	4 5 1

 K 10
 svn:author
 V 7
 chipx86
 K 8
 svn:date
 V 27
 -02-24T23:04:51.477951Z
 K 7
 svn:log
 V 66
 Add a couple keyword tests, and add Revision to the keyword list.
 END

/trunk/reviewboard/scmtools/testdata/svn_repo/db/revs/4
Revision UNKNOWN		New Change
This is a binary file. The content cannot be displayed.

/trunk/reviewboard/scmtools/svn.py: 5 changes [ 1 2 3 4 5 ]
/trunk/reviewboard/scmtools/tests.py: 4 changes [ 1 2 3 4 ]
/trunk/reviewboard/scmtools/testdata/svn_repo/db/current: 1 change [ ]
/trunk/reviewboard/scmtools/testdata/svn_repo/db/revprops/4: 1 change [ new content ]
/trunk/reviewboard/scmtools/testdata/svn_repo/db/revs/4: binary file

Review Board

beta

This change has been marked as submitted.

Fix keyword expansion in files from SVN repositories

Diff revision 4 (Latest)

Other reviews

Your comment