Éric Araujo | a0e92a8 | 2011-07-26 18:01:08 +0200 | [diff] [blame] | 1 | #!/usr/bin/env python3 |
Brett Cannon | 57ee0c8 | 2017-06-24 17:59:49 -0700 | [diff] [blame] | 2 | """Check proposed changes for common issues.""" |
Georg Brandl | a9afb68 | 2010-10-21 12:49:28 +0000 | [diff] [blame] | 3 | import re |
| 4 | import sys |
| 5 | import shutil |
Christian Heimes | ada8c3b | 2008-03-18 18:26:33 +0000 | [diff] [blame] | 6 | import os.path |
| 7 | import subprocess |
Éric Araujo | a3e072b | 2011-07-30 21:34:04 +0200 | [diff] [blame] | 8 | import sysconfig |
Christian Heimes | ada8c3b | 2008-03-18 18:26:33 +0000 | [diff] [blame] | 9 | |
| 10 | import reindent |
Georg Brandl | a9afb68 | 2010-10-21 12:49:28 +0000 | [diff] [blame] | 11 | import untabify |
Christian Heimes | ada8c3b | 2008-03-18 18:26:33 +0000 | [diff] [blame] | 12 | |
| 13 | |
Victor Stinner | d23b1c4 | 2017-08-17 16:53:27 +0200 | [diff] [blame] | 14 | # Excluded directories which are copies of external libraries: |
| 15 | # don't check their coding style |
| 16 | EXCLUDE_DIRS = [os.path.join('Modules', '_ctypes', 'libffi'), |
| 17 | os.path.join('Modules', '_ctypes', 'libffi_osx'), |
| 18 | os.path.join('Modules', '_ctypes', 'libffi_msvc'), |
| 19 | os.path.join('Modules', '_decimal', 'libmpdec'), |
| 20 | os.path.join('Modules', 'expat'), |
| 21 | os.path.join('Modules', 'zlib')] |
Éric Araujo | a3e072b | 2011-07-30 21:34:04 +0200 | [diff] [blame] | 22 | SRCDIR = sysconfig.get_config_var('srcdir') |
| 23 | |
Victor Stinner | d23b1c4 | 2017-08-17 16:53:27 +0200 | [diff] [blame] | 24 | |
Brett Cannon | 058173e | 2010-07-04 22:05:34 +0000 | [diff] [blame] | 25 | def n_files_str(count): |
| 26 | """Return 'N file(s)' with the proper plurality on 'file'.""" |
| 27 | return "{} file{}".format(count, "s" if count != 1 else "") |
| 28 | |
Florent Xicluna | e4a3380 | 2010-08-09 12:24:20 +0000 | [diff] [blame] | 29 | |
Christian Heimes | ada8c3b | 2008-03-18 18:26:33 +0000 | [diff] [blame] | 30 | def status(message, modal=False, info=None): |
| 31 | """Decorator to output status info to stdout.""" |
| 32 | def decorated_fxn(fxn): |
| 33 | def call_fxn(*args, **kwargs): |
| 34 | sys.stdout.write(message + ' ... ') |
| 35 | sys.stdout.flush() |
| 36 | result = fxn(*args, **kwargs) |
| 37 | if not modal and not info: |
| 38 | print("done") |
| 39 | elif info: |
| 40 | print(info(result)) |
| 41 | else: |
Florent Xicluna | e4a3380 | 2010-08-09 12:24:20 +0000 | [diff] [blame] | 42 | print("yes" if result else "NO") |
Christian Heimes | ada8c3b | 2008-03-18 18:26:33 +0000 | [diff] [blame] | 43 | return result |
| 44 | return call_fxn |
| 45 | return decorated_fxn |
| 46 | |
Florent Xicluna | e4a3380 | 2010-08-09 12:24:20 +0000 | [diff] [blame] | 47 | |
Nadeem Vawda | 9f64f73 | 2012-02-22 11:46:41 +0200 | [diff] [blame] | 48 | def mq_patches_applied(): |
| 49 | """Check if there are any applied MQ patches.""" |
| 50 | cmd = 'hg qapplied' |
| 51 | with subprocess.Popen(cmd.split(), |
| 52 | stdout=subprocess.PIPE, |
| 53 | stderr=subprocess.PIPE) as st: |
| 54 | bstdout, _ = st.communicate() |
| 55 | return st.returncode == 0 and bstdout |
| 56 | |
| 57 | |
Nick Coghlan | 2f38625 | 2017-03-12 16:17:46 +1000 | [diff] [blame] | 58 | def get_git_branch(): |
| 59 | """Get the symbolic name for the current git branch""" |
| 60 | cmd = "git rev-parse --abbrev-ref HEAD".split() |
| 61 | try: |
| 62 | return subprocess.check_output(cmd, stderr=subprocess.DEVNULL) |
| 63 | except subprocess.CalledProcessError: |
| 64 | return None |
| 65 | |
| 66 | |
| 67 | def get_git_upstream_remote(): |
| 68 | """Get the remote name to use for upstream branches |
| 69 | |
| 70 | Uses "upstream" if it exists, "origin" otherwise |
| 71 | """ |
| 72 | cmd = "git remote get-url upstream".split() |
| 73 | try: |
| 74 | subprocess.check_output(cmd, stderr=subprocess.DEVNULL) |
| 75 | except subprocess.CalledProcessError: |
| 76 | return "origin" |
| 77 | return "upstream" |
| 78 | |
| 79 | |
| 80 | @status("Getting base branch for PR", |
| 81 | info=lambda x: x if x is not None else "not a PR branch") |
| 82 | def get_base_branch(): |
Nick Coghlan | 4c116cb | 2017-04-09 19:22:36 +1000 | [diff] [blame] | 83 | if not os.path.exists(os.path.join(SRCDIR, '.git')): |
Nick Coghlan | 2f38625 | 2017-03-12 16:17:46 +1000 | [diff] [blame] | 84 | # Not a git checkout, so there's no base branch |
| 85 | return None |
| 86 | version = sys.version_info |
| 87 | if version.releaselevel == 'alpha': |
| 88 | base_branch = "master" |
| 89 | else: |
| 90 | base_branch = "{0.major}.{0.minor}".format(version) |
| 91 | this_branch = get_git_branch() |
| 92 | if this_branch is None or this_branch == base_branch: |
| 93 | # Not on a git PR branch, so there's no base branch |
| 94 | return None |
| 95 | upstream_remote = get_git_upstream_remote() |
| 96 | return upstream_remote + "/" + base_branch |
| 97 | |
| 98 | |
Christian Heimes | ada8c3b | 2008-03-18 18:26:33 +0000 | [diff] [blame] | 99 | @status("Getting the list of files that have been added/changed", |
Georg Brandl | a9afb68 | 2010-10-21 12:49:28 +0000 | [diff] [blame] | 100 | info=lambda x: n_files_str(len(x))) |
Nick Coghlan | 2f38625 | 2017-03-12 16:17:46 +1000 | [diff] [blame] | 101 | def changed_files(base_branch=None): |
Christian Heimes | d98c677 | 2015-04-23 11:24:14 +0200 | [diff] [blame] | 102 | """Get the list of changed or added files from Mercurial or git.""" |
| 103 | if os.path.isdir(os.path.join(SRCDIR, '.hg')): |
Nick Coghlan | 2f38625 | 2017-03-12 16:17:46 +1000 | [diff] [blame] | 104 | if base_branch is not None: |
| 105 | sys.exit('need a git checkout to check PR status') |
Christian Heimes | d98c677 | 2015-04-23 11:24:14 +0200 | [diff] [blame] | 106 | cmd = 'hg status --added --modified --no-status' |
| 107 | if mq_patches_applied(): |
| 108 | cmd += ' --rev qparent' |
| 109 | with subprocess.Popen(cmd.split(), stdout=subprocess.PIPE) as st: |
Victor Stinner | d23b1c4 | 2017-08-17 16:53:27 +0200 | [diff] [blame] | 110 | filenames = [x.decode().rstrip() for x in st.stdout] |
Nick Coghlan | 61a82a5 | 2017-03-12 20:00:20 +1000 | [diff] [blame] | 111 | elif os.path.exists(os.path.join(SRCDIR, '.git')): |
| 112 | # We just use an existence check here as: |
| 113 | # directory = normal git checkout/clone |
| 114 | # file = git worktree directory |
Nick Coghlan | 2f38625 | 2017-03-12 16:17:46 +1000 | [diff] [blame] | 115 | if base_branch: |
| 116 | cmd = 'git diff --name-status ' + base_branch |
| 117 | else: |
| 118 | cmd = 'git status --porcelain' |
Christian Heimes | d98c677 | 2015-04-23 11:24:14 +0200 | [diff] [blame] | 119 | filenames = [] |
| 120 | with subprocess.Popen(cmd.split(), stdout=subprocess.PIPE) as st: |
| 121 | for line in st.stdout: |
| 122 | line = line.decode().rstrip() |
Nick Coghlan | 2f38625 | 2017-03-12 16:17:46 +1000 | [diff] [blame] | 123 | status_text, filename = line.split(maxsplit=1) |
| 124 | status = set(status_text) |
Christian Heimes | d98c677 | 2015-04-23 11:24:14 +0200 | [diff] [blame] | 125 | # modified, added or unmerged files |
| 126 | if not status.intersection('MAU'): |
| 127 | continue |
Christian Heimes | d98c677 | 2015-04-23 11:24:14 +0200 | [diff] [blame] | 128 | if ' -> ' in filename: |
| 129 | # file is renamed |
| 130 | filename = filename.split(' -> ', 2)[1].strip() |
| 131 | filenames.append(filename) |
Christian Heimes | d98c677 | 2015-04-23 11:24:14 +0200 | [diff] [blame] | 132 | else: |
| 133 | sys.exit('need a Mercurial or git checkout to get modified files') |
Florent Xicluna | e4a3380 | 2010-08-09 12:24:20 +0000 | [diff] [blame] | 134 | |
Victor Stinner | d23b1c4 | 2017-08-17 16:53:27 +0200 | [diff] [blame] | 135 | filenames2 = [] |
| 136 | for filename in filenames: |
| 137 | # Normalize the path to be able to match using .startswith() |
| 138 | filename = os.path.normpath(filename) |
| 139 | if any(filename.startswith(path) for path in EXCLUDE_DIRS): |
| 140 | # Exclude the file |
| 141 | continue |
| 142 | filenames2.append(filename) |
| 143 | |
| 144 | return filenames2 |
| 145 | |
Christian Heimes | ada8c3b | 2008-03-18 18:26:33 +0000 | [diff] [blame] | 146 | |
Brett Cannon | 058173e | 2010-07-04 22:05:34 +0000 | [diff] [blame] | 147 | def report_modified_files(file_paths): |
| 148 | count = len(file_paths) |
| 149 | if count == 0: |
| 150 | return n_files_str(count) |
| 151 | else: |
| 152 | lines = ["{}:".format(n_files_str(count))] |
| 153 | for path in file_paths: |
| 154 | lines.append(" {}".format(path)) |
| 155 | return "\n".join(lines) |
| 156 | |
Florent Xicluna | e4a3380 | 2010-08-09 12:24:20 +0000 | [diff] [blame] | 157 | |
Brett Cannon | 57ee0c8 | 2017-06-24 17:59:49 -0700 | [diff] [blame] | 158 | @status("Fixing Python file whitespace", info=report_modified_files) |
Christian Heimes | ada8c3b | 2008-03-18 18:26:33 +0000 | [diff] [blame] | 159 | def normalize_whitespace(file_paths): |
| 160 | """Make sure that the whitespace for .py files have been normalized.""" |
| 161 | reindent.makebackup = False # No need to create backups. |
Benjamin Peterson | 4177eff | 2011-06-27 18:25:06 -0500 | [diff] [blame] | 162 | fixed = [path for path in file_paths if path.endswith('.py') and |
Éric Araujo | ad548b8 | 2011-07-31 18:33:00 +0200 | [diff] [blame] | 163 | reindent.check(os.path.join(SRCDIR, path))] |
Brett Cannon | 058173e | 2010-07-04 22:05:34 +0000 | [diff] [blame] | 164 | return fixed |
Christian Heimes | ada8c3b | 2008-03-18 18:26:33 +0000 | [diff] [blame] | 165 | |
Florent Xicluna | e4a3380 | 2010-08-09 12:24:20 +0000 | [diff] [blame] | 166 | |
Georg Brandl | a9afb68 | 2010-10-21 12:49:28 +0000 | [diff] [blame] | 167 | @status("Fixing C file whitespace", info=report_modified_files) |
| 168 | def normalize_c_whitespace(file_paths): |
| 169 | """Report if any C files """ |
| 170 | fixed = [] |
| 171 | for path in file_paths: |
Éric Araujo | a3e072b | 2011-07-30 21:34:04 +0200 | [diff] [blame] | 172 | abspath = os.path.join(SRCDIR, path) |
| 173 | with open(abspath, 'r') as f: |
Georg Brandl | a9afb68 | 2010-10-21 12:49:28 +0000 | [diff] [blame] | 174 | if '\t' not in f.read(): |
| 175 | continue |
Éric Araujo | a3e072b | 2011-07-30 21:34:04 +0200 | [diff] [blame] | 176 | untabify.process(abspath, 8, verbose=False) |
Georg Brandl | a9afb68 | 2010-10-21 12:49:28 +0000 | [diff] [blame] | 177 | fixed.append(path) |
| 178 | return fixed |
| 179 | |
| 180 | |
| 181 | ws_re = re.compile(br'\s+(\r?\n)$') |
| 182 | |
| 183 | @status("Fixing docs whitespace", info=report_modified_files) |
| 184 | def normalize_docs_whitespace(file_paths): |
| 185 | fixed = [] |
| 186 | for path in file_paths: |
Éric Araujo | a3e072b | 2011-07-30 21:34:04 +0200 | [diff] [blame] | 187 | abspath = os.path.join(SRCDIR, path) |
Georg Brandl | a9afb68 | 2010-10-21 12:49:28 +0000 | [diff] [blame] | 188 | try: |
Éric Araujo | a3e072b | 2011-07-30 21:34:04 +0200 | [diff] [blame] | 189 | with open(abspath, 'rb') as f: |
Georg Brandl | a9afb68 | 2010-10-21 12:49:28 +0000 | [diff] [blame] | 190 | lines = f.readlines() |
| 191 | new_lines = [ws_re.sub(br'\1', line) for line in lines] |
| 192 | if new_lines != lines: |
Éric Araujo | a3e072b | 2011-07-30 21:34:04 +0200 | [diff] [blame] | 193 | shutil.copyfile(abspath, abspath + '.bak') |
| 194 | with open(abspath, 'wb') as f: |
Georg Brandl | a9afb68 | 2010-10-21 12:49:28 +0000 | [diff] [blame] | 195 | f.writelines(new_lines) |
| 196 | fixed.append(path) |
| 197 | except Exception as err: |
| 198 | print('Cannot fix %s: %s' % (path, err)) |
| 199 | return fixed |
| 200 | |
| 201 | |
Christian Heimes | ada8c3b | 2008-03-18 18:26:33 +0000 | [diff] [blame] | 202 | @status("Docs modified", modal=True) |
| 203 | def docs_modified(file_paths): |
Brett Cannon | 058173e | 2010-07-04 22:05:34 +0000 | [diff] [blame] | 204 | """Report if any file in the Doc directory has been changed.""" |
| 205 | return bool(file_paths) |
Christian Heimes | ada8c3b | 2008-03-18 18:26:33 +0000 | [diff] [blame] | 206 | |
Florent Xicluna | e4a3380 | 2010-08-09 12:24:20 +0000 | [diff] [blame] | 207 | |
Christian Heimes | ada8c3b | 2008-03-18 18:26:33 +0000 | [diff] [blame] | 208 | @status("Misc/ACKS updated", modal=True) |
| 209 | def credit_given(file_paths): |
| 210 | """Check if Misc/ACKS has been changed.""" |
Terry Jan Reedy | 6e2711b | 2013-07-21 20:57:44 -0400 | [diff] [blame] | 211 | return os.path.join('Misc', 'ACKS') in file_paths |
Christian Heimes | ada8c3b | 2008-03-18 18:26:33 +0000 | [diff] [blame] | 212 | |
Florent Xicluna | e4a3380 | 2010-08-09 12:24:20 +0000 | [diff] [blame] | 213 | |
Miss Islington (bot) | e00e1b8 | 2017-09-06 17:38:51 -0700 | [diff] [blame^] | 214 | @status("Misc/NEWS.d updated with `blurb`", modal=True) |
Christian Heimes | ada8c3b | 2008-03-18 18:26:33 +0000 | [diff] [blame] | 215 | def reported_news(file_paths): |
Miss Islington (bot) | e00e1b8 | 2017-09-06 17:38:51 -0700 | [diff] [blame^] | 216 | """Check if Misc/NEWS.d has been changed.""" |
| 217 | return any(p.startswith(os.path.join('Misc', 'NEWS.d', 'next')) |
| 218 | for p in file_paths) |
Christian Heimes | ada8c3b | 2008-03-18 18:26:33 +0000 | [diff] [blame] | 219 | |
Ross Lagerwall | 6c52c57 | 2012-03-11 19:21:07 +0200 | [diff] [blame] | 220 | @status("configure regenerated", modal=True, info=str) |
| 221 | def regenerated_configure(file_paths): |
| 222 | """Check if configure has been regenerated.""" |
Matthias Klose | 5ce31cc | 2012-03-14 23:17:31 +0100 | [diff] [blame] | 223 | if 'configure.ac' in file_paths: |
Ross Lagerwall | 6c52c57 | 2012-03-11 19:21:07 +0200 | [diff] [blame] | 224 | return "yes" if 'configure' in file_paths else "no" |
| 225 | else: |
| 226 | return "not needed" |
| 227 | |
| 228 | @status("pyconfig.h.in regenerated", modal=True, info=str) |
| 229 | def regenerated_pyconfig_h_in(file_paths): |
| 230 | """Check if pyconfig.h.in has been regenerated.""" |
Matthias Klose | 5ce31cc | 2012-03-14 23:17:31 +0100 | [diff] [blame] | 231 | if 'configure.ac' in file_paths: |
Ross Lagerwall | 6c52c57 | 2012-03-11 19:21:07 +0200 | [diff] [blame] | 232 | return "yes" if 'pyconfig.h.in' in file_paths else "no" |
| 233 | else: |
| 234 | return "not needed" |
Christian Heimes | ada8c3b | 2008-03-18 18:26:33 +0000 | [diff] [blame] | 235 | |
Brett Cannon | 57ee0c8 | 2017-06-24 17:59:49 -0700 | [diff] [blame] | 236 | def travis(pull_request): |
| 237 | if pull_request == 'false': |
| 238 | print('Not a pull request; skipping') |
| 239 | return |
| 240 | base_branch = get_base_branch() |
| 241 | file_paths = changed_files(base_branch) |
| 242 | python_files = [fn for fn in file_paths if fn.endswith('.py')] |
| 243 | c_files = [fn for fn in file_paths if fn.endswith(('.c', '.h'))] |
| 244 | doc_files = [fn for fn in file_paths if fn.startswith('Doc') and |
| 245 | fn.endswith(('.rst', '.inc'))] |
| 246 | fixed = [] |
| 247 | fixed.extend(normalize_whitespace(python_files)) |
| 248 | fixed.extend(normalize_c_whitespace(c_files)) |
| 249 | fixed.extend(normalize_docs_whitespace(doc_files)) |
| 250 | if not fixed: |
| 251 | print('No whitespace issues found') |
| 252 | else: |
| 253 | print(f'Please fix the {len(fixed)} file(s) with whitespace issues') |
| 254 | print('(on UNIX you can run `make patchcheck` to make the fixes)') |
| 255 | sys.exit(1) |
| 256 | |
Christian Heimes | ada8c3b | 2008-03-18 18:26:33 +0000 | [diff] [blame] | 257 | def main(): |
Nick Coghlan | 2f38625 | 2017-03-12 16:17:46 +1000 | [diff] [blame] | 258 | base_branch = get_base_branch() |
| 259 | file_paths = changed_files(base_branch) |
Brett Cannon | 058173e | 2010-07-04 22:05:34 +0000 | [diff] [blame] | 260 | python_files = [fn for fn in file_paths if fn.endswith('.py')] |
| 261 | c_files = [fn for fn in file_paths if fn.endswith(('.c', '.h'))] |
Georg Brandl | 24f0717 | 2014-10-19 11:54:08 +0200 | [diff] [blame] | 262 | doc_files = [fn for fn in file_paths if fn.startswith('Doc') and |
| 263 | fn.endswith(('.rst', '.inc'))] |
Miss Islington (bot) | e00e1b8 | 2017-09-06 17:38:51 -0700 | [diff] [blame^] | 264 | misc_files = {p for p in file_paths if p.startswith('Misc')} |
Brett Cannon | 058173e | 2010-07-04 22:05:34 +0000 | [diff] [blame] | 265 | # PEP 8 whitespace rules enforcement. |
| 266 | normalize_whitespace(python_files) |
Georg Brandl | a9afb68 | 2010-10-21 12:49:28 +0000 | [diff] [blame] | 267 | # C rules enforcement. |
| 268 | normalize_c_whitespace(c_files) |
| 269 | # Doc whitespace enforcement. |
| 270 | normalize_docs_whitespace(doc_files) |
Christian Heimes | ada8c3b | 2008-03-18 18:26:33 +0000 | [diff] [blame] | 271 | # Docs updated. |
Georg Brandl | a9afb68 | 2010-10-21 12:49:28 +0000 | [diff] [blame] | 272 | docs_modified(doc_files) |
Christian Heimes | ada8c3b | 2008-03-18 18:26:33 +0000 | [diff] [blame] | 273 | # Misc/ACKS changed. |
Terry Jan Reedy | 6e2711b | 2013-07-21 20:57:44 -0400 | [diff] [blame] | 274 | credit_given(misc_files) |
Christian Heimes | ada8c3b | 2008-03-18 18:26:33 +0000 | [diff] [blame] | 275 | # Misc/NEWS changed. |
Terry Jan Reedy | 6e2711b | 2013-07-21 20:57:44 -0400 | [diff] [blame] | 276 | reported_news(misc_files) |
Ross Lagerwall | 6c52c57 | 2012-03-11 19:21:07 +0200 | [diff] [blame] | 277 | # Regenerated configure, if necessary. |
| 278 | regenerated_configure(file_paths) |
| 279 | # Regenerated pyconfig.h.in, if necessary. |
| 280 | regenerated_pyconfig_h_in(file_paths) |
Christian Heimes | ada8c3b | 2008-03-18 18:26:33 +0000 | [diff] [blame] | 281 | |
| 282 | # Test suite run and passed. |
Éric Araujo | fbc5ff6 | 2011-08-12 17:50:08 +0200 | [diff] [blame] | 283 | if python_files or c_files: |
Ezio Melotti | 5e12bb7 | 2013-01-11 14:07:47 +0200 | [diff] [blame] | 284 | end = " and check for refleaks?" if c_files else "?" |
Éric Araujo | fbc5ff6 | 2011-08-12 17:50:08 +0200 | [diff] [blame] | 285 | print() |
Ezio Melotti | 5e12bb7 | 2013-01-11 14:07:47 +0200 | [diff] [blame] | 286 | print("Did you run the test suite" + end) |
Christian Heimes | ada8c3b | 2008-03-18 18:26:33 +0000 | [diff] [blame] | 287 | |
| 288 | |
| 289 | if __name__ == '__main__': |
Brett Cannon | 57ee0c8 | 2017-06-24 17:59:49 -0700 | [diff] [blame] | 290 | import argparse |
| 291 | parser = argparse.ArgumentParser(description=__doc__) |
| 292 | parser.add_argument('--travis', |
| 293 | help='Perform pass/fail checks') |
| 294 | args = parser.parse_args() |
| 295 | if args.travis: |
| 296 | travis(args.travis) |
| 297 | else: |
| 298 | main() |