Thanks for sending text/plain patches!

> +++ tools/server-side/svn-backup-dumps.py     (working copy)
> +    def get_head_rev_for_url(self):
> +        extra_param = self.get_extra_param()
> +
> +        # use 'svn yougest' to get the HEAD revision of URL
> +        # 'svn yougest' is supported on subversion 1.9 or laster.
> +        cmd = [ self.__svn_path, "youngest", self.__repospath ]
> +        cmd[2:2] = extra_param
> +        r = self.exec_cmd(cmd)
> +        if r[0] == 0 and len(r[2]) == 0:
> +            return int(r[1].strip())
> +
> +        # use 'svn log' to get the latest revision of URL
> +        # it may be different from the HEAD revision. 
> +        cmd = [ self.__svn_path, "log", "-l 1", "-q", self.__repospath ]
> +        cmd[2:2] = extra_param
> +        r = self.exec_cmd(cmd)
> +        if r[0] == 0 and len(r[2]) == 0:
> +            revison_regex = re.compile("^r(\d+)")

Typo, "revision_regex"

> +            cmd = [ self.__svnadmin_path, "dump",
> +                    "--incremental", "-r", revparam, self.__repospath ]

That doesn't look right.  If self.__repospath can be a local path to a
repository root, you shouldn't pass it as an argument to 'svn' a few lines
above.

Also, it'd be good practice to pass "--" in front of self.__repospath, but that
appears to be a preexisting problem in the code (i.e., not introduced by your
patch).

> +        else:
> +            cmd = [ self.__svnrdump_path, "dump",
> +                    "--incremental", "-r", revparam, self.__repospath ]

Cheers,

Daniel
(haven't read the whole patch; these two issues just jumped out on a quick scan)

Reply via email to