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