Уведомления

Группа в Telegram: @pythonsu

#1 Июль 17, 2009 18:36:38

sober
От:
Зарегистрирован: 2009-07-17
Сообщения: 17
Репутация: +  0  -
Профиль   Отправить e-mail  

pysvn pre-commit hook

Всем привет. Я плохо знаком c python и pysvn в частности. Подскажите, пожалуйста, как реализовать следующее:
Появилась необходимость реализовать pre-commit hook, который будет выполнять checkstyle java файлов, и соответсвенно, фэйлить коммит, если checkstyle не проходит. Checkstyle проверка реализована выполнением ant таска, которому в качестве параметра необходимо передать директорию с файлами для проверки. Проблема в том, что я не могу скопировать те файлы, которые есть в транзакции в директорию svn репозитория, чтобы впоследствии иметь возможность натравить на эту директорию checkstyle.

Файлы пытаюсь получить так:

сlient = pysvn.Client()
t = pysvn.Transaction(repos, txn)
changes = t.changed()
for path in changes:
# src - лежит в папке hooks svn репозитория, там же где и pre-commit скрипт
# path - здесь ожидается, что path это полный путь к файлу, который будет коммититься, как получить этот путь?
client.copy(path, 'src')
извините, если сумбурно объяснил проблему;
буду благодарен за любой совет;



Отредактировано (Авг. 5, 2009 19:08:49)

Офлайн

#2 Июль 18, 2009 01:19:19

Ed
От:
Зарегистрирован: 2008-12-13
Сообщения: 1032
Репутация: +  13  -
Профиль   Отправить e-mail  

pysvn pre-commit hook

Так не получится достать файл. Он еще не закомичен, а вы его пытаетесь клиентом достать. Вы достанете не то, что комитится, а то, что уже в репозитории. Нужно из транзакции. Где-то так:

#!/usr/bin/env python

import sys
import pysvn

trans = pysvn.Transaction(*sys.argv[1:])
for path in trans.changed():
sys.stderr.write('content: %s' % trans.cat(path))
sys.exit(1)
Это работающий код. Он выводит содержимое файла, который комитят и безусловно блокирует коммит - возвращает 1.
В результате на клиенте при коммите вы сможете увидеть нечто типа этого:
$ svn commit -m ‘test hook’
Sending x.txt
Transmitting file data .svn: Commit failed (details follow):
svn: Commit blocked by pre-commit hook (exit code 1) with output:
content: bla
, где ‘bla’ - содержимое моего файла x.txt, который я пытаюсь закоммитить.



Отредактировано (Июль 18, 2009 01:20:27)

Офлайн

#3 Июль 27, 2009 17:49:24

sober
От:
Зарегистрирован: 2009-07-17
Сообщения: 17
Репутация: +  0  -
Профиль   Отправить e-mail  

pysvn pre-commit hook

Спасибо. :) Появилась другая проблема, при экспорте файлов репозитория метод экспорта, почему-то выполняется рекурсивно.

основной скрипт

#!c:\python26\python.exe

import os, sys
import pysvn
import re
import utils
import logging

CURRENT_DIR = os.path.dirname(os.path.realpath(__file__))
CHECK_DIR = os.path.join(CURRENT_DIR, 'src')

CHECKSTYLE_PATTERN = r'checkstyle\s*\=\s*(\w+)'

# setup logger
logging.basicConfig
logger = logging.getLogger(__name__)
logger.setLevel(logging.DEBUG)

formatter = logging.Formatter('%(asctime)s - %(filename)s - %(levelname)s - %(message)s')
handler = logging.FileHandler(os.path.join(CURRENT_DIR, 'pre-commit.log'), mode='w')
handler.setFormatter(formatter)
logger.addHandler(handler)

def main (repos, txn):

trans = pysvn.Transaction(repos, txn)

helper = utils.Helper(CURRENT_DIR, CHECK_DIR)

# checking value of 'skip.checkstyle' option
# if it has been set in log message
logMsg = trans.revpropget('svn:log')
s = re.compile(CHECKSTYLE_PATTERN).search(logMsg)

if s != None and s.group(1) == 'false':
logger.info('skipping checkstyle')
sys.exit(0)

changes = trans.changed()

modified = []
added = []
all_changes = []

for entry in changes.keys():
if entry.endswith('.java'):
if changes.get(entry)[0] == 'R':
modified.append(entry)
all_changes.append(entry)

elif changes.get(entry)[0] == 'A':
added.append(entry)
all_changes.append(entry)

logger.info('%s files modified' % len(modified))
logger.info('%s files added' % len(added))

# preparing check dir
if not os.path.exists(CHECK_DIR):
os.makedirs(CHECK_DIR)
helper.cleanDir(CHECK_DIR)

# recieving contents of files
for path in all_changes:
if (path.endswith('.java')):
content = trans.cat(path)

filePath = helper.getExpandedWorkingCopyPath(path, root=CHECK_DIR)
helper.prepareDir(filePath)
f = file(filePath, 'w')
f.write(content)
f.close()

logger.info('running checkstyle over %s' % len(all_changes))
cur_errors = helper.runCheckstyle(CURRENT_DIR, CHECK_DIR)
if (cur_errors > 0):
helper.cleanDir(CHECK_DIR)
helper.exportRepositoryFiles(modified)
logger.info('running checkstyle over %s' % len(modified))
prev_errors = helper.runCheckstyle(CURRENT_DIR, CHECK_DIR)

if cur_errors > prev_errors:
sys.stderr.write('\nCURRENT amount of checkstyle violations:\t- %s' % cur_errors)
sys.stderr.write('\nPREVIOUS amount of checkstyle violations:\t- %s' % prev_errors)

sys.exit(1)

if __name__ == '__main__':
if len(sys.argv) < 3:
sys.stdout.write("\nUsage: %s REPOS TXN\n" % (sys.argv[0]))
else:
sys.exit(main(sys.argv[1], sys.argv[2]))
import errno
import logging
import os
import pysvn
import re
import shutil
import subprocess as subp
import sys

WARNING_PATTERN = r':\swarning:\s|:\serror:\s'

class Helper:

def __init__(self, curDir, checkDir):
self.curDir = curDir
self.checkDir = checkDir
self.svn_client = pysvn.Client()

logger = logging.getLogger(__name__)
logger.setLevel(logging.DEBUG)
formatter = logging.Formatter('%(asctime)s - %(filename)s - %(levelname)s - %(message)s')
handler = logging.FileHandler(os.path.join(curDir, 'pre-commit.log'), mode='a')
handler.setFormatter(formatter)
logger.addHandler(handler)
self._logger = logger

def prepareDir(self, filename):
dir = os.path.dirname(filename)
if not os.path.exists(dir):
os.makedirs(dir)

def cleanDir(self, dir):
""" Removes all content of the specified directory """
self._logger.info('cleaning dir \'%s\'' % dir)
if os.path.exists(dir):
for root, dirs, files in os.walk(dir):
for f in files:
os.unlink(os.path.join(root, f))
for d in dirs:
shutil.rmtree(os.path.join(root, d))
else:
raise OSError(errno.ENOENT, 'The system cannot find the file specified: \'%s\'' % dir)

def formatDirPath(self, path):
"""Appends trailing separator to non-empty path if it is missing.

Args:
path: path string, which may be with or without a trailing separator,
or even empty or None

Returns:
path unchanged if path evaluates to False or already ends with a trailing
separator; otherwise, a / separator is appended
"""
if path and not path.endswith('/'):
path = ''.join([path, '/'])
return path

def useLocalOsSep(self, path):
"""Return path with all / characters replaced with os.sep, to be OS-agnostic.

Args:
path: an SVN path (either working copy path or relative path, but not a
full repository URL) that uses the canonical / separators
"""
return path.replace('/', os.sep)

def getExpandedWorkingCopyPath(self, path, root=None):
"""Returns expanded, local, native filesystem working copy path.

Args:
path: path to expand and convert to local filesystem directory separators
root: if present, prepended to path first
"""
path = self.useLocalOsSep(path)

if root:
# prepend (Windows-compatible) working copy root if one was supplied
path = os.path.join(self.useLocalOsSep(root), path)

path = self.getExpandedPath(path)

return path

def getExpandedPath(self, path):
"""Returns an expanded, normalized, absolute path.

Args:
path: path (possibly relative, possibly containing environment variables,
etc.) to be expanded, normalized and made absolute

Returns:
absolute path, after expanding any environment variables and "~", then
removing excess . and .. path elements
"""
return os.path.abspath(
os.path.normpath(
os.path.expanduser(
os.path.expandvars(path))))

def exportFiles(self, files):
self._logger.info('exporting files...')
for src in files.keys():
dest = files[src]
self.prepareDir(dest)
self.svn_client.export(src, dest)

def runCheckstyle(self, wd, path):
""" Returns amount of checkstyle violations
"""
self._logger.info('running checkstyle')
cmd = 'ant -Dcheck.dir=%s' % path
p = subp.Popen(cmd, shell = True, stdout = subp.PIPE, stderr = subp.STDOUT, cwd=wd)
m = re.compile(WARNING_PATTERN)
errors = len(m.findall(p.communicate()[0]))
return errors

def exportRepositoryFiles(self, changes):
self._logger.info(self.svn_client)
exportList = {}
for entry in changes:
src =''.join(['svn://localhost/', entry])
dest = self.getExpandedWorkingCopyPath(entry, root=self.checkDir)
exportList[src] = dest
self.exportFiles(exportList)
в лог пишеться:
2009-07-27 17:13:57,868 - pre-commit.py - INFO - 90 files modified
2009-07-27 17:13:57,977 - pre-commit.py - INFO - 8 files added
2009-07-27 17:13:58,961 - pre-commit.py - INFO - running checkstyle over 98
\hooks\src'
2009-07-27 17:13:58,961 - utils.py - INFO - running checkstyle
2009-07-27 17:14:07,275 - utils.py - INFO - cleaning dir ‘D:\projects\SVNHook\hooks\src’
2009-07-27 17:14:07,322 - utils.py - INFO - <Client object at 0x00B2C9EC>
2009-07-27 17:14:07,322 - utils.py - INFO - exporting files…
2009-07-27 17:14:07,354 - utils.py - INFO - exporting files…
2009-07-27 17:14:07,384 - utils.py - INFO - exporting files…
2009-07-27 17:14:07,431 - utils.py - INFO - exporting files…
2009-07-27 17:14:07,479 - utils.py - INFO - exporting files…
2009-07-27 17:14:07,540 - utils.py - INFO - exporting files…
2009-07-27 17:14:07,618 - utils.py - INFO - exporting files…
2009-07-27 17:14:07,729 - utils.py - INFO - exporting files…
2009-07-27 17:14:07,838 - utils.py - INFO - exporting files…
и так до тех пор пока скрипт не умрет с ошибкой:
Commit failed (details follow):
Commit blocked by pre-commit hook (exit code 1) with output:
Traceback (most recent call last):
File “D:\projects\SVNHook\hooks\pre-commit.py”, line 102, in <module>
sys.exit(main(sys.argv, sys.argv))
File “D:\projects\SVNHook\hooks\pre-commit.py”, line 88, in main
helper.exportRepositoryFiles(modified)
File “D:\projects\SVNHook\hooks\utils.py”, line 126, in exportRepositoryFiles
self.exportFiles(exportList)
File “D:\projects\SVNHook\hooks\utils.py”, line 107, in exportFiles
self.svn_client.export(src, dest)
pysvn._pysvn_2_6.ClientError: Can't connect to host ‘localhost’: Only one usage
of each socket address (protocol/network address/port) is normally permitted.
Буду также очень признателен за любые замечания касательно убогости кода. К сожалению, это фактически мой первый скрипт на python.



Отредактировано (Авг. 5, 2009 19:09:35)

Офлайн

#4 Июль 27, 2009 19:18:40

Ferroman
От:
Зарегистрирован: 2006-11-16
Сообщения: 2759
Репутация: +  1  -
Профиль   Отправить e-mail  

pysvn pre-commit hook

prepareDir, exportFiles, exportRepositoryFiles - не хватает докстринга :)
prepareDir, useLocalOsSep - я бы сделал _prepareDir, _useLocalOsSep и т. п. То есть - закрыл бы вызовы этих методов вне методов класса.
А так, вообще, весьма неплохо, особенно как на первый скрипт.

Офлайн

#5 Июль 27, 2009 22:49:01

Ed
От:
Зарегистрирован: 2008-12-13
Сообщения: 1032
Репутация: +  13  -
Профиль   Отправить e-mail  

pysvn pre-commit hook

Насколько я вижу вы немного ошиблись в методе exportRepositoryFiles, зовете self.exportFiles(exportList) внутри цикла. А нужно в цикле тольк осделать этот самый exportList, а потом уже на готовый позвать self.ExportFiles.



Офлайн

#6 Июль 27, 2009 23:20:53

Ed
От:
Зарегистрирован: 2008-12-13
Сообщения: 1032
Репутация: +  13  -
Профиль   Отправить e-mail  

pysvn pre-commit hook

По поводу кода замечаний много.
Вообще советую посмотреть на это: http://www.python.org/dev/peps/pep-0008/ и стараться его придерживаться. Без фанатизма, но стараться. В этом вам поможет pylint, pychecker и прочие тулзы для проверки стиля кода. Настоятельно рекомендую попользоваться. Я лично пользуюсь pylint чаще, но это дело вкуса.

Общие замечания при беглом взгляде на код:
Helper:
- не советую делать logger глобальной переменной. Они вообще зло, а в данном конкретном случае еще и неоправданное. Передайте его в функцию как параметр - будет гораздо красивше.
- WARNING_PATTERN используется только в классе Helper, так и внесите его туда, зачем делать это глобальной переменной, сделайте переменной класса.И не нужно его компилировать каждый раз, когда зовете runCheckstyle, он от этого лучше не станет. Либо зовите findall просто, без компиляции, либо скомпилируйте где-нибудь отдельно и потом юзайте.
- Зачем в Helper свой логгер? Передайте тот, который у вас уже есть ему в __init__. Даже если ему нужен будет другой, то такое решение гораздо лучше, чем дублировать один и тот же код в 2х местах.
- Вместо cleanDir попользуйте shutils.rmtree
- Не вижу где у вас используется formatDirPath и мне оно не нравится. Посмотрите на os.path.normath, его можно использовать. Он не добавит, а уберет последний слэш, но вам же просто отнормализовать надо, без особой разницы как.
- Вы используете соединение по сети через localhost. По-моему можно обойтись file://, у вас же репозиторий локальный, скрипт же на сервере запускается.
main:
- то же, что и раньше: s = re.compile(CHECKSTYLE_PATTERN).search(logMsg) не нужно. Компиляция нужна для ускорения работы при многократном использовании, чтобы не компилить каждый раз. В данном случае s = re.search(CHECKSTYLE_PATTERN, logMsg) абсолютно равнозначно, но выглядит короче и понятнее.
- if s != None выглядит не pythonic. Лучше написать if not s
- sys.exit(0) внутри функции main юзать нехорошо. Верните 0, наверху и произойдет sys.exit с ним
- if not os.path.exists(CHECK_DIR):
os.makedirs(CHECK_DIR)
helper.cleanDir(CHECK_DIR)
Лучше записать так:
if not os.path.exists(CHECK_DIR):
os.makedirs(CHECK_DIR)
else:
helper.cleanDir(CHECK_DIR)
Нет лишнего вызова, да и понятнее.
- content = trans.cat(path) - переменная content используется один раз. Ну и напишите f.write(trans.cat(path)), зачем это лишнее присваивание?
- манипуляции со списками modifiled, added и all_changes я не осознал. added не используется нигде, кроме как в подсчете количества элементов в нем. Вместо списка вполне хватит переменной-счетчика. А вот с modified происходит вторая проверка, уже после проверки всех all_changes и это непонятно. Там бы не помешал объясняющий логику программы коментарий.

Ну, наверное хватит пока. Исправите - приходите еще :)



И напоследок. Это конечно дело вкуса, но такие длинные имена функций по-моему захламляют код. Лучше сделать короткое имя и написать толковый docstring..
-



Офлайн

#7 Авг. 4, 2009 18:50:32

sober
От:
Зарегистрирован: 2009-07-17
Сообщения: 17
Репутация: +  0  -
Профиль   Отправить e-mail  

pysvn pre-commit hook

Спасибо большое за комментарии. Я постарался учесть замечания. Пришел к следующему:

pre_commit.py

"""
A subversion pre-commit hook that validates the commited files for
checkstyle violations.
"""

__authors__ = [
# alphabetical order by last name, please
'sober',
]

import os, sys
import pysvn
import re
import utils
import logging
#import timeit

# magic keyword which allows to skip checkstyle
CHECKSTYLE_PATTERN = '.*skipcheckstyle.*'
# directory in which this script is located
CURRENT_DIR = os.path.dirname(os.path.realpath(__file__))
# directory which should contain files to be checked
CHECK_DIR = os.path.join(CURRENT_DIR, 'src\\')
# path to the checkstyle report
ERRORS_REPORT_PATH = os.path.join(CURRENT_DIR, 'checkstyle\\checkstyle_errors.xml')

# setup LOGGER
LOGGER = logging.getLogger(__name__)
LOGGER.setLevel(logging.DEBUG)

FORMATTER = logging.Formatter('%(asctime)s - %(filename)s - %(levelname)s - %(message)s')
HANDLER = logging.FileHandler(os.path.join(CURRENT_DIR, 'pre-commit.log'), mode='w')
HANDLER.setFormatter(FORMATTER)
LOGGER.addHandler(HANDLER)

def main (repos, txn):
""" Hook entry point """
trans = pysvn.Transaction(repos, txn)
helper = utils.Helper(CURRENT_DIR)

# checking the value of 'skip.checkstyle' option (set in the commit log)
log_msg = trans.revpropget('svn:log')
match = re.search(CHECKSTYLE_PATTERN, log_msg)

if match is not None:
LOGGER.info('skipping checkstyle')
sys.exit(0)

changes = trans.changed()
all_changes = []
modified = []

for path in changes.keys():
if path.endswith('.java'):
if changes.get(path)[0] is not 'D':
all_changes.append(path)

if changes.get(path)[0] is 'R':
modified.append(path)

if len(all_changes) > 0:

# preparing check dir
if not os.path.exists(CHECK_DIR):
os.makedirs(CHECK_DIR)
else:
helper.clean_dir(CHECK_DIR)

# recieving contents of the files
for path in all_changes:
file_path = \
helper.get_expanded_working_path(re.search(r'.*trunk/(.*)', path).group(1),root=CHECK_DIR)
helper.prepare_dir(file_path)
f = file(file_path, 'w')
f.write(trans.cat(path))
f.close()

LOGGER.info('Executing checkstyle over committed files ...')
helper.run_checkstyle(CHECK_DIR)

cur_report = helper.get_checkstyle_report(ERRORS_REPORT_PATH)
helper.clean_dir(CHECK_DIR)

helper.export_repos_files(modified, CHECK_DIR)
LOGGER.info('Executing checkstyle over repository versions of commited files ...')
helper.run_checkstyle(CHECK_DIR)
prev_report = helper.get_checkstyle_report(ERRORS_REPORT_PATH)

error_msg = ''
for path in cur_report.keys():
msg_pattern = '\nAmount of checkstyle violations in file \'%s\' has increased from %s to %s'

if path not in prev_report.keys() and cur_report[path] > 0:
error_msg += (msg_pattern % (path[len(CHECK_DIR):], '0', cur_report[path]))

elif prev_report[path] < cur_report[path]:
error_msg += (msg_pattern % (path[len(CHECK_DIR):], prev_report[path], cur_report[path]))

LOGGER.info(error_msg)

if len(error_msg) > 0:
sys.stderr.write(error_msg)
return 1

if __name__ == '__main__':
if len(sys.argv) < 3:
sys.stdout.write("\nUsage: %s REPOS TXN\n" % (sys.argv[0]))
else:
sys.exit(main(sys.argv[1], sys.argv[2]))
# this code is for testing purpose
#t = timeit.Timer('main(\'%s\', \'%s\')' % (sys.argv[1], sys.argv[2]), 'from __main__ import main')
#aver = t.timeit(number=1)
#LOGGER.info('Hook execution time is %s sec' % aver)
utils.py
""" Common utils used for svn pre-commit hook """

import os
import pysvn
import shutil
import subprocess as subp
from xml.dom import minidom

class Helper:
""" help class which aggregates utils used for svn pre-commit hook """

def __init__(self, cur_dir):
self.cur_dir = cur_dir
self.svn_client = pysvn.Client()

def prepare_dir(self, filename):
""" Creates directory tree if not exists """
dir_path = os.path.dirname(filename)
if not os.path.exists(dir_path):
os.makedirs(dir_path)

def clean_dir(self, dir):
""" Removes all content of the specified directory """
#self._logger.info('cleaning dir \'%s\'' % dir)
if os.path.exists(dir):
for root, dirs, files in os.walk(dir):
for file_path in files:
os.unlink(os.path.join(root, file_path))
for dir_path in dirs:
shutil.rmtree(os.path.join(root, dir_path))
else:
raise OSError('The system cannot find the file specified: \'%s\'' % dir)

def use_local_os_sep(self, path):
"""Return path with all / characters replaced with os.sep, to be OS-agnostic.

Args:
path: an SVN path (either working copy path or relative path, but not a
full repository URL) that uses the canonical / separators
"""
return path.replace('/', os.sep)

def get_expanded_working_path(self, path, root=None):
"""Returns expanded, local, native filesystem working copy path.

Args:
path: path to expand and convert to local filesystem directory separators
root: if present, prepended to path first
"""
path = self.use_local_os_sep(path)

if root:
# prepend (Windows-compatible) working copy root if one was supplied
path = os.path.join(self.use_local_os_sep(root), path)

path = self.get_expanded_path(path)

return path

def get_expanded_path(self, path):
"""Returns an expanded, normalized, absolute path.

Args:
path - path (possibly relative, possibly containing environment variables,
etc.) to be expanded, normalized and made absolute

Returns:
absolute path, after expanding any environment variables and "~", then
removing excess . and .. path elements
"""
return os.path.abspath(
os.path.normpath(
os.path.expanduser(
os.path.expandvars(path))))

def export_files(self, files):
""" Export list of files specified as the dictionary keys
to the path specified as dictionary values """
#self._logger.info('exporting files...')
for src in files.keys():
dest = files[src]
self.prepare_dir(dest)
self.svn_client.export(src, dest)

def run_checkstyle(self, path):
""" Executes checkstyle validation as the separate process
which executes the Ant checkstyle task

Args:
path - path that should be checked
"""
#self._logger.info('running checkstyle')
cmd = 'ant -Dcheck.dir=%s' % path
proc = subp.Popen(cmd, shell = True, stdout = subp.PIPE, stderr = subp.STDOUT, cwd=self.cur_dir)
proc.communicate()

def export_repos_files(self, changes, path):
""" Exports committed files from transaction to the specified path
Args:
changes - list of changed files
path - root destination of the files being exported
"""
#self._logger.info('Exporting %s files from repository' % len(changes))
export_list = {}
for entry in changes:
src = ''.join(['svn://localhost/', entry])
dest = self.get_expanded_working_path(entry, root=path)
export_list[src] = dest
self.export_files(export_list)

def get_checkstyle_report(self, path):
""" Returns checkstyle report as a dictionary which contains amount
of checkstyle violations for every file
Args:
path - path to the checkstyle report
Returns:
dictionary which contains amount of checkstyle violations for every file
"""
report = {}
xmldoc = minidom.parse(path)
file_tags = xmldoc.getElementsByTagName('file')
for tag in file_tags:
key = tag.getAttribute('name')
value = len(tag.getElementsByTagName('error'))
if key in report:
report[key] += value
else:
report[key] = value

return report
код старался форматировать в соответствии с замечаниями pylint'a, но некоторые замечания я не понял:
R: 16:Helper.prepare_dir: Method could be a function
W: 22:Helper.clean_dir: Redefining built-in ‘dir’
появилась необходимость сравнивать количество чекстайл ошибок для каждого файла, для чего я стал выводить результат чекстайл проверки в лог; к сожалению, в этой реализации хука, столкнулся с проблемой, которая выражается в том, что svn фэйлит коммит при попытке закоммитить больше N файлов, при чем это N варьируется (стабильно файлит при попытке закоммитить больше чем 60 файлов); :(

еще раз спасибо всем за помощь; :)



Отредактировано (Авг. 4, 2009 19:02:23)

Офлайн

#8 Авг. 5, 2009 00:20:06

Ed
От:
Зарегистрирован: 2008-12-13
Сообщения: 1032
Репутация: +  13  -
Профиль   Отправить e-mail  

pysvn pre-commit hook

код старался форматировать в соответствии с замечаниями pylint'a, но некоторые замечания я не понял:
R: 16:Helper.prepare_dir: Method could be a function
W: 22:Helper.clean_dir: Redefining built-in ‘dir’
Первое значит, что у вас в методе грубо говоря не используется сам объект, нет обращения к self. Pylint совершенно правильно говорит, что такой метод может быть простой функцией. Вообще на мой взгляд ваш класс Helper выглядит несколько сумбурно и ненужно. Вы в него включили все, что нужно и не нужно. Лучше сделайте несколько функций и этого будет достаточно. Либо передизайньте так, чтобы использование объектов было оправданным.
Второе - у вас там есть параметр с названием ‘dir’, а в Python есть функция с таким же именем. То есть вы переопределили питоновый dir, что не есть хорошо. Переименуйте параметр и pylint перестанет ругаться.

появилась необходимость сравнивать количество чекстайл ошибок для каждого файла, для чего я стал выводить результат чекстайл проверки в лог; к сожалению, в этой реализации хука, столкнулся с проблемой, которая выражается в том, что svn фэйлит коммит при попытке закоммитить больше N файлов, при чем это N варьируется (стабильно файлит при попытке закоммитить больше чем 60 файлов); :(
Это не может быть следствием некоего таймаута? Если нет, то я могу у себя попробовать воспроизвести. Только скажите с какой ошибкой отваливается коммит, есть там какая-нибудь диагностика?

Теперь немного о том, что мне все еще не нравится в вашем коде:
- много глобальных переменных. Если они у вас используются только в main, то почему бы не сделать их локальными там?
- if match is not None: и подобное выглядит не pythonic. В Питоне обычно проверка на нечто пустое (строка, None, 0, False, пустой список, пустой словарь и т.д.) пишется просто if not match:, этого достаточно.
- changes.get(path) зовется 2 раза. Лучше присвоить результат переменной и ее использовать
- if len(all_changes) > 0: - то же, что и раньше. Список можно проверить на непустоту проще: if all_changes:
- использование .keys() для словарей не нужно. Проверка ключа делается просто так: if key in dictionary: А итерация по ключам: for key in dictionary:
- Мне не совсем понятен смысл многочисленных функций по работе с путями, но общее впечатление такое, что это можно сделать проще и использовать родные питоновые функции. Вообще необходимость этого Helper весьма сомнительна.



Отредактировано (Авг. 5, 2009 00:22:32)

Офлайн

#9 Авг. 5, 2009 19:01:55

sober
От:
Зарегистрирован: 2009-07-17
Сообщения: 17
Репутация: +  0  -
Профиль   Отправить e-mail  

pysvn pre-commit hook

Ed, очень признателен Вам за комментарии. :)
Я перенес весь необходимый функционал из helper.py в pre-commit.py, как обычно, старался учесть замечания:

"""
A subversion pre-commit hook that validates the commited files for
checkstyle violations.
"""

__authors__ = [
# alphabetical order by last name, please
'sober',
]

import logging
import os, sys
import pysvn
import re
import shutil
import subprocess as subp
from xml.dom import minidom
#import timeit

# directory in which this script is located
CURRENT_DIR = os.path.dirname(os.path.realpath(__file__))
SVN_CLIENT = pysvn.Client()

# setup logger
logger = logging.getLogger(__name__)
logger.setLevel(logging.DEBUG)
formatter = logging.Formatter('%(asctime)s - %(filename)s - %(levelname)s - %(message)s')
handler = logging.FileHandler(os.path.join(CURRENT_DIR, 'pre-commit.log'), mode='w')
handler.setFormatter(formatter)
logger.addHandler(handler)

def prepare_dir(filename):
""" Creates directory tree if not exists """
dir_path = os.path.dirname(filename)
if not os.path.exists(dir_path):
os.makedirs(dir_path)

def clean_dir(path):
""" Removes all content of the specified directory """
#logger.info('cleaning dir \'%s\'' % dir)
if os.path.exists(path):
for root, dirs, files in os.walk(path):
for file_path in files:
os.unlink(os.path.join(root, file_path))
for dir_path in dirs:
shutil.rmtree(os.path.join(root, dir_path))
else:
raise OSError('The system cannot find the file specified: \'%s\'' % path)

def use_local_os_sep(path):
"""Return path with all / characters replaced with os.sep, to be OS-agnostic.

Args:
path: an SVN path (either working copy path or relative path, but not a
full repository URL) that uses the canonical / separators
"""
return path.replace('/', os.sep)

def get_expanded_working_path(path, root=None):
"""Returns expanded, local, native filesystem working copy path.

Args:
path: path to expand and convert to local filesystem directory separators
root: if present, prepended to path first
"""
path = use_local_os_sep(path)

if root:
# prepend (Windows-compatible) working copy root if one was supplied
path = os.path.join(use_local_os_sep(root), path)

path = get_expanded_path(path)

return path

def get_expanded_path(path):
"""Returns an expanded, normalized, absolute path.

Args:
path - path (possibly relative, possibly containing environment variables,
etc.) to be expanded, normalized and made absolute

Returns:
absolute path, after expanding any environment variables and "~", then
removing excess . and .. path elements
"""
return os.path.abspath(
os.path.normpath(
os.path.expanduser(
os.path.expandvars(path))))

def export_files(files):
""" Export list of files specified as the dictionary keys
to the path specified as dictionary values """
#logger.info('exporting files...')
for src in files:
dest = files[src]
prepare_dir(dest)
SVN_CLIENT.export(src, dest)

def run_checkstyle(path):
""" Executes checkstyle validation as the separate process
which executes the Ant checkstyle task

Args:
path - path that should be checked
"""
#logger.info('running checkstyle')
cmd = 'ant -Dcheck.dir=%s' % path
proc = subp.Popen(cmd, shell = True, stdout = subp.PIPE, stderr = subp.STDOUT, cwd=CURRENT_DIR)
proc.communicate()

def export_repos_files(changes, path):
""" Exports committed files from transaction to the specified path
Args:
changes - list of changed files
path - root destination of the files being exported
"""
#logger.info('Exporting %s files from repository' % len(changes))
export_list = {}
for entry in changes:
src = ''.join(['svn://localhost/', entry])
dest = get_expanded_working_path(entry, root=path)
export_list[src] = dest
export_files(export_list)

def get_checkstyle_report(path):
""" Returns checkstyle report as a dictionary which contains amount
of checkstyle violations for every file
Args:
path - path to the checkstyle report
Returns:
dictionary which contains amount of checkstyle violations for every file
"""
report = {}
xmldoc = minidom.parse(path)
file_tags = xmldoc.getElementsByTagName('file')
for tag in file_tags:
key = tag.getAttribute('name')
value = len(tag.getElementsByTagName('error'))
if key in report:
report[key] += value
else:
report[key] = value

return report

def main (repos, txn):
""" Hook entry point """

# magic keyword which allows to skip checkstyle
checkstyle_pattern = '.*skipcheckstyle.*'

# directory which should contain files to be checked
check_dir = os.path.join(CURRENT_DIR, 'src\\')
# path to the checkstyle report
errors_report_path = os.path.join(CURRENT_DIR, 'checkstyle\\checkstyle_errors.xml')

trans = pysvn.Transaction(repos, txn)

# checking the value of 'skip.checkstyle' option (set in the commit log)
log_msg = trans.revpropget('svn:log')
if re.search(checkstyle_pattern, log_msg):
logger.info('skipping checkstyle')
sys.exit(0)

changes = trans.changed()
all_changes = []
modified = []

for path in changes:
if path.endswith('.java'):
status = changes.get(path)[0]
if status is not 'D':
all_changes.append(path)
if status is 'R':
modified.append(path)

if all_changes:

# preparing check dir
if not os.path.exists(check_dir):
os.makedirs(check_dir)
else:
clean_dir(check_dir)

# recieving contents of the files
for path in all_changes:
file_path = \
get_expanded_working_path(re.search(r'.*trunk/(.*)', path).group(1),root=check_dir)
prepare_dir(file_path)
f = file(file_path, 'w')
f.write(trans.cat(path))
f.close()

logger.info('Executing checkstyle over committed files ...')
run_checkstyle(check_dir)

cur_report = get_checkstyle_report(errors_report_path)
clean_dir(check_dir)

export_repos_files(modified, check_dir)
logger.info('Executing checkstyle over repository versions of commited files ...')
run_checkstyle(check_dir)
prev_report = get_checkstyle_report(errors_report_path)

error_msg = ''
for path in cur_report:
msg_pattern = '\nAmount of checkstyle violations in file \'%s\' has increased from %s to %s'

if path not in prev_report and cur_report[path] > 0:
error_msg += (msg_pattern % (path[len(check_dir):], '0', cur_report[path]))

elif prev_report[path] < cur_report[path]:
error_msg += (msg_pattern % (path[len(check_dir):], prev_report[path], cur_report[path]))

logger.info(error_msg)

if error_msg:
sys.stderr.write(error_msg)
return 1

if __name__ == '__main__':
if len(sys.argv) < 3:
sys.stdout.write("\nUsage: %s REPOS TXN\n" % (sys.argv[0]))
else:
sys.exit(main(sys.argv[1], sys.argv[2]))
# this code is for testing purpose
#t = timeit.Timer('main(\'%s\', \'%s\')' % (sys.argv[1], sys.argv[2]), 'from __main__ import main')
#aver = t.timeit(number=1)
#logger.info('Hook execution time is %s sec' % aver)
Не могу понять, почему методы в helper по работе с путями не нужны? В этих методах как раз используются методы из стандартной библиотеки python.

Что касается проблемы с закрытием коннекшена, я все еще ломаю голову…
Использую svn - 1.6.3, python 2.6.2, pysvn for svn 1.6.0

Запускаю svn демон следующим образом:
svnserve –log-file=D:\Projects\SVNHook\svnserve.log -d -r d:\Projects\SVNHook
При попытке закоммитить большое количество файлов, больше 60, например, svn клиент завершает работу с ошибкой
svn: Network connection closed unexpectedly
При этом в логe svnserve нет записей связанных с ошибкой. Действительно похоже, что конекшн закрывается по таймауту, информации как установить timeout для для коммита я не нашел. Если кто-то захочет попробовать потестировать хук, могу выложить архив со всем необходимым. :)



Отредактировано (Авг. 5, 2009 19:11:32)

Офлайн

#10 Авг. 5, 2009 20:12:25

Ed
От:
Зарегистрирован: 2008-12-13
Сообщения: 1032
Репутация: +  13  -
Профиль   Отправить e-mail  

pysvn pre-commit hook

sober
Не могу понять, почему методы в helper по работе с путями не нужны? В этих методах как раз используются методы из стандартной библиотеки python.
Может они и нужны, просто выглядят как-то громоздко. Вот смотрите:
get_expanded_path - зовется один раз. Не уверен, что имеет смысл выносить это в функцию. По крайней мере пока.
use_local_os_sep - это path.replace('/', os.sep). Не проще ли так и написать, а не оформлять все это в функцию? Это улучшит читабельность на мой взгляд. path.replace('/', os.sep) достаточно информативно само по себе, зачем тратить время и место на 6 строк docstring?
clean_dir - по-моему os.walk здесь - это overkill, хватит 2х строчек: shutil.rmtree(path, True); os.makedirs(path). Да и вообще не уверен, что нужно это делать функцией опять же. Проще эти 2 строчки написать там, где они нужны, заменив вот эту конструкцию:
# preparing check dir
if not os.path.exists(check_dir):
os.makedirs(check_dir)
else:
clean_dir(check_dir)
export_files - тоже самое. По-моему проще эти 4 строчки положить туда, где оно единожды вызывается.

Ну и так далее.

Питон сам по себе высокоуровневый и достаточно ясный язык, не нужно наворачивать нечто сверху, если в этом нет необходимости. Во многих случаях это будет только затруднять понимание кода.

Кстати, pylint пускайте чаще - вырабатывает правильный стиль. Поскольку вы его нечасто юзаете, то у вас появились новые стилистические ошибки в коде.

Что касается проблемы с закрытием коннекшена, я все еще ломаю голову…
Какой клиент используете? Может дело в нем?
Давайте свой репозиторий, попробую у себя воспроизвести. Только из-за ваших виндовых штучек типа этого: ‘src\\’, ‘checkstyle\\checkstyle_errors.xml’ и подобных мне нужно будет править код, чтобы оно хоть как-то заработало. Так что если можно, то сделайте это платформо-независымым, плз.

Да, я уже писал об этом, но вы видимо пропустили:
svn://localhost/ - не лучший метод для работы локально, по-моему. Для этого есть file://



Офлайн

Board footer

Модераторировать

Powered by DjangoBB

Lo-Fi Version